Skip to content

Commit dfd804f

Browse files
DemiMarieDemiMarie
committed
Fix flaky qrexec agent tests
Various tests assumed that qrexec would deliver stdout in a single message. Qrexec does not make this guarantee: calls to write(2), send(2), sendmsg(2), etc are not guaranteed to correspond 1-to-1 to MSG_DATA_STDOUT messages on the vchan. This caused https://gitlab.com/QubesOS/qubes-core-qrexec/-/jobs/6616564043 to wrongly fail, even thouggh the code is correct. Fix this problem by concatenating the payloads of all stdout messages into a single bytes object before comparing with the expected stdout value. Also add a utility function for this, saving a lot of code in tests.
1 parent edc80de commit dfd804f

File tree

1 file changed

+29
-129
lines changed

1 file changed

+29
-129
lines changed

‎qrexec/tests/socket/agent.py

+29-129
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,21 @@ def check_dom0(self, dom0):
5656
),
5757
)
5858

59+
def assertExpectedStdout(self, target, expected_stdout: bytes, *, exit_code=0):
60+
messages = util.sort_messages(target.recv_all_messages())
61+
self.assertListEqual(messages[-3:],
62+
[
63+
(qrexec.MSG_DATA_STDOUT, b""),
64+
(qrexec.MSG_DATA_STDERR, b""),
65+
(qrexec.MSG_DATA_EXIT_CODE, struct.pack("<L", exit_code))
66+
])
67+
stdout_entries = []
68+
for msg_type, msg_body in messages[:-3]:
69+
# messages before last are not empty, hence truthy
70+
self.assertTrue(msg_body)
71+
self.assertEqual(msg_type, qrexec.MSG_DATA_STDOUT)
72+
stdout_entries.append(msg_body)
73+
5974
def setUp(self):
6075
self.tempdir = tempfile.mkdtemp()
6176
os.mkdir(os.path.join(self.tempdir, "local-rpc"))
@@ -188,16 +203,7 @@ def test_exec_cmdline(self):
188203

189204
target.send_message(qrexec.MSG_DATA_STDIN, b"")
190205

191-
messages = target.recv_all_messages()
192-
self.assertListEqual(
193-
util.sort_messages(messages),
194-
[
195-
(qrexec.MSG_DATA_STDOUT, b"Hello world\n"),
196-
(qrexec.MSG_DATA_STDOUT, b""),
197-
(qrexec.MSG_DATA_STDERR, b""),
198-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
199-
],
200-
)
206+
self.assertExpectedStdout(target, b"Hello world\n")
201207
self.check_dom0(dom0)
202208

203209
def test_trigger_service(self):
@@ -314,16 +320,7 @@ def test_exec_service(self):
314320
)
315321
target, dom0 = self.execute_qubesrpc("qubes.Service+arg", "domX")
316322
target.send_message(qrexec.MSG_DATA_STDIN, b"")
317-
messages = target.recv_all_messages()
318-
self.assertListEqual(
319-
util.sort_messages(messages),
320-
[
321-
(qrexec.MSG_DATA_STDOUT, b"arg: arg, remote domain: domX\n"),
322-
(qrexec.MSG_DATA_STDOUT, b""),
323-
(qrexec.MSG_DATA_STDERR, b""),
324-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
325-
],
326-
)
323+
self.assertExpectedStdout(target, b"arg: arg, remote domain: domX\n")
327324
self.check_dom0(dom0)
328325

329326
def test_exec_service_keyword(self):
@@ -342,20 +339,11 @@ def test_exec_service_keyword(self):
342339
)
343340
target, dom0 = self.execute_qubesrpc("qubes.Service", "domX")
344341
target.send_message(qrexec.MSG_DATA_STDIN, b"")
345-
messages = target.recv_all_messages()
346-
self.assertListEqual(
347-
util.sort_messages(messages),
348-
[
349-
(qrexec.MSG_DATA_STDOUT, b"""arg: , remote domain: domX
342+
self.assertExpectedStdout(target, b"""arg: , remote domain: domX
350343
target name: NONAME
351344
target keyword: NOKEYWORD
352345
target type: ''
353-
"""),
354-
(qrexec.MSG_DATA_STDOUT, b""),
355-
(qrexec.MSG_DATA_STDERR, b""),
356-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
357-
],
358-
)
346+
""")
359347
self.check_dom0(dom0)
360348

361349
def test_exec_service_with_config(self):
@@ -376,16 +364,7 @@ def test_exec_service_with_config(self):
376364
""")
377365
target, dom0 = self.execute_qubesrpc("qubes.Service+arg", "domX")
378366
target.send_message(qrexec.MSG_DATA_STDIN, b"")
379-
messages = target.recv_all_messages()
380-
self.assertListEqual(
381-
util.sort_messages(messages),
382-
[
383-
(qrexec.MSG_DATA_STDOUT, b"arg: arg, remote domain: domX\n"),
384-
(qrexec.MSG_DATA_STDOUT, b""),
385-
(qrexec.MSG_DATA_STDERR, b""),
386-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
387-
],
388-
)
367+
self.assertExpectedStdout(target, b"arg: arg, remote domain: domX\n")
389368
self.check_dom0(dom0)
390369

391370
def test_wait_for_session(self):
@@ -449,20 +428,9 @@ def _test_wait_for_session(self, config_name, service_name="qubes.Service", argu
449428
# Do not send EOF. Shell read doesn't need it, and this checks that
450429
# qrexec does not wait for EOF on stdin before sending the exit code
451430
# from the remote process.
452-
messages = target.recv_all_messages()
453-
self.assertListEqual(
454-
util.sort_messages(messages),
455-
[
456-
(
457-
qrexec.MSG_DATA_STDOUT,
458-
b"arg: " + argument.encode("ascii", "strict")
459-
+ b", remote domain: domX, input: stdin data\n",
460-
),
461-
(qrexec.MSG_DATA_STDOUT, b""),
462-
(qrexec.MSG_DATA_STDERR, b""),
463-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
464-
],
465-
)
431+
expected_stdout = (b"arg: " + argument.encode("ascii", "strict")
432+
+ b", remote domain: domX, input: stdin data\n")
433+
self.assertExpectedStdout(target, expected_stdout)
466434
self.check_dom0(dom0)
467435

468436
def test_exec_service_fail(self):
@@ -599,16 +567,7 @@ def test_exec_null_argument_finds_service_for_empty_argument(self):
599567
)
600568
target, dom0 = self.execute_qubesrpc("qubes.Service", "domX")
601569
target.send_message(qrexec.MSG_DATA_STDIN, b"")
602-
messages = target.recv_all_messages()
603-
self.assertListEqual(
604-
util.sort_messages(messages),
605-
[
606-
(qrexec.MSG_DATA_STDOUT, b"specific service: qubes.Service\n"),
607-
(qrexec.MSG_DATA_STDOUT, b""),
608-
(qrexec.MSG_DATA_STDERR, b""),
609-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
610-
],
611-
)
570+
self.assertExpectedStdout(target, b"specific service: qubes.Service\n")
612571
self.check_dom0(dom0)
613572

614573
def test_socket_null_argument_finds_service_for_empty_argument(self):
@@ -806,16 +765,7 @@ def test_pass_stdin(self):
806765
)
807766

808767
target.send_message(qrexec.MSG_DATA_STDIN, b"")
809-
messages = target.recv_all_messages()
810-
self.assertListEqual(
811-
util.sort_messages(messages),
812-
[
813-
(qrexec.MSG_DATA_STDOUT, b""),
814-
(qrexec.MSG_DATA_STDERR, b""),
815-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
816-
],
817-
)
818-
self.check_dom0(dom0)
768+
self.assertExpectedStdout(target, b"")
819769

820770
def test_close_stdin_early(self):
821771
# Make sure that we cover the error on writing stdin into living
@@ -835,15 +785,7 @@ def test_close_stdin_early(self):
835785
target.send_message(qrexec.MSG_DATA_STDIN, b"data 2\n")
836786
target.send_message(qrexec.MSG_DATA_STDIN, b"")
837787

838-
messages = target.recv_all_messages()
839-
self.assertListEqual(
840-
util.sort_messages(messages),
841-
[
842-
(qrexec.MSG_DATA_STDOUT, b""),
843-
(qrexec.MSG_DATA_STDERR, b""),
844-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
845-
],
846-
)
788+
self.assertExpectedStdout(target, b"")
847789
self.check_dom0(dom0)
848790

849791
def test_buffer_stdin(self):
@@ -871,29 +813,7 @@ def test_buffer_stdin(self):
871813
with open(fifo, "a") as f:
872814
f.write("end\n")
873815
f.flush()
874-
875-
messages = []
876-
received_data = b""
877-
while len(received_data) < data_size:
878-
message_type, message = target.recv_message()
879-
if message_type != qrexec.MSG_DATA_STDOUT:
880-
messages.append((message_type, message))
881-
else:
882-
self.assertEqual(message_type, qrexec.MSG_DATA_STDOUT)
883-
received_data += message
884-
885-
self.assertEqual(len(received_data), data_size)
886-
self.assertEqual(received_data, data)
887-
888-
messages += target.recv_all_messages()
889-
self.assertListEqual(
890-
util.sort_messages(messages),
891-
[
892-
(qrexec.MSG_DATA_STDOUT, b""),
893-
(qrexec.MSG_DATA_STDERR, b""),
894-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
895-
],
896-
)
816+
self.assertExpectedStdout(target, data)
897817
self.check_dom0(dom0)
898818

899819
def test_close_stdout_stderr_early(self):
@@ -948,16 +868,7 @@ def test_stdio_socket(self):
948868
target.send_message(qrexec.MSG_DATA_STDIN, b"stdin\n")
949869
target.send_message(qrexec.MSG_DATA_STDIN, b"")
950870

951-
messages = target.recv_all_messages()
952-
self.assertListEqual(
953-
util.sort_messages(messages),
954-
[
955-
(qrexec.MSG_DATA_STDOUT, b"received: stdin\n"),
956-
(qrexec.MSG_DATA_STDOUT, b""),
957-
(qrexec.MSG_DATA_STDERR, b""),
958-
(qrexec.MSG_DATA_EXIT_CODE, b"\0\0\0\0"),
959-
],
960-
)
871+
self.assertExpectedStdout(target, b"received: stdin\n")
961872
self.check_dom0(dom0)
962873

963874
def test_exit_before_closing_streams(self):
@@ -995,18 +906,7 @@ def test_exit_before_closing_streams(self):
995906
with open(fifo, "a") as f:
996907
f.write("end\n")
997908
f.flush()
998-
self.assertEqual(
999-
target.recv_message(), (qrexec.MSG_DATA_STDOUT, b"child exiting\n")
1000-
)
1001-
messages = target.recv_all_messages()
1002-
self.assertListEqual(
1003-
util.sort_messages(messages),
1004-
[
1005-
(qrexec.MSG_DATA_STDOUT, b""),
1006-
(qrexec.MSG_DATA_STDERR, b""),
1007-
(qrexec.MSG_DATA_EXIT_CODE, struct.pack("<L", 42)),
1008-
],
1009-
)
909+
self.assertExpectedStdout(target, b"child exiting\n", exit_code=42)
1010910
self.check_dom0(dom0)
1011911

1012912

0 commit comments

Comments
 (0)