From b9ae8e279038c65e4a81867e518bcb35719f30b6 Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Wed, 15 Jan 2025 22:32:22 +0800 Subject: [PATCH 1/6] Fix address and flags for send_fds/recv_fds --- Lib/socket.py | 5 ++- Lib/test/test_socket.py | 84 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/Lib/socket.py b/Lib/socket.py index be37c24d6174a2..4b8aa575b74594 100644 --- a/Lib/socket.py +++ b/Lib/socket.py @@ -563,7 +563,8 @@ def send_fds(sock, buffers, fds, flags=0, address=None): import array return sock.sendmsg(buffers, [(_socket.SOL_SOCKET, - _socket.SCM_RIGHTS, array.array("i", fds))]) + _socket.SCM_RIGHTS, array.array("i", fds))], + flags, address) __all__.append("send_fds") if hasattr(_socket.socket, "recvmsg"): @@ -579,7 +580,7 @@ def recv_fds(sock, bufsize, maxfds, flags=0): # Array of ints fds = array.array("i") msg, ancdata, flags, addr = sock.recvmsg(bufsize, - _socket.CMSG_LEN(maxfds * fds.itemsize)) + _socket.CMSG_LEN(maxfds * fds.itemsize), flags) for cmsg_level, cmsg_type, cmsg_data in ancdata: if (cmsg_level == _socket.SOL_SOCKET and cmsg_type == _socket.SCM_RIGHTS): fds.frombytes(cmsg_data[: diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index faf326d9164e1b..ce65f90c5f4bb1 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7037,6 +7037,12 @@ def test_dual_stack_client_v6(self): @requireAttrs(socket, "recv_fds") @requireAttrs(socket, "AF_UNIX") class SendRecvFdsTests(unittest.TestCase): + def _test_pipe(self, rfd, wfd, msg): + assert len(msg) < 512 + os.write(wfd, msg) + data = os.read(rfd, 512) + self.assertEqual(data, msg) + def testSendAndRecvFds(self): def close_pipes(pipes): for fd1, fd2 in pipes: @@ -7066,13 +7072,79 @@ def close_fds(fds): # don't test addr # test that file descriptors are connected - for index, fds in enumerate(pipes): - rfd, wfd = fds - os.write(wfd, str(index).encode()) + for index, ((_, wfd), rfd) in enumerate(zip(pipes, fds2)): + self._test_pipe(rfd, wfd, str(index).encode()) + + def test_send_recv_fds_with_addrs(self): + sock1 = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM) + sock2 = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM) + rfd, wfd = os.pipe() + self.addCleanup(os.close, rfd) + self.addCleanup(os.close, wfd) + + with tempfile.TemporaryDirectory() as tmpdir, sock1, sock2: + sock1_addr = os.path.join(tmpdir, "sock1") + sock2_addr = os.path.join(tmpdir, "sock2") + sock1.bind(sock1_addr) + sock2.bind(sock2_addr) + sock2.setblocking(False) + + socket.send_fds(sock1, [MSG], [rfd], address=sock2_addr) + msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) + new_rfd = fds[0] + self.addCleanup(os.close, new_rfd) + + self.assertEqual(msg, MSG) + self.assertEqual(len(fds), 1) + self.assertEqual(addr, sock1_addr) + + self._test_pipe(new_rfd, wfd, MSG) + + @requireAttrs(socket, "MSG_PEEK") + def test_recv_fds_peek(self): + rfd, wfd = os.pipe() + self.addCleanup(os.close, rfd) + self.addCleanup(os.close, wfd) - for index, rfd in enumerate(fds2): - data = os.read(rfd, 100) - self.assertEqual(data, str(index).encode()) + sock1, sock2 = socket.socketpair(socket.AF_UNIX, socket.SOCK_DGRAM) + with sock1, sock2: + socket.send_fds(sock1, [MSG], [rfd]) + sock2.setblocking(False) + + # peek message on sock2 + peek_len = len(MSG) // 2 + msg, fds, flags, addr = socket.recv_fds(sock2, peek_len, 1, + socket.MSG_PEEK) + self.addCleanup(os.close, fds[0]) + self.assertEqual(len(msg), peek_len) + self.assertEqual(msg, MSG[:peek_len]) + self.assertEqual(flags & socket.MSG_TRUNC, socket.MSG_TRUNC) + self._test_pipe(fds[0], wfd, MSG) + + # will raise BlockingIOError if MSG_PEEK didn't work + msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) + self.addCleanup(os.close, fds[0]) + self.assertEqual(msg, MSG) + self._test_pipe(fds[0], wfd, MSG) + + @requireAttrs(socket, "MSG_DONTWAIT") + def test_send_fds_dontwait(self): + rfd, wfd = os.pipe() + self.addCleanup(os.close, rfd) + self.addCleanup(os.close, wfd) + + sock1, sock2 = socket.socketpair(socket.AF_UNIX, socket.SOCK_DGRAM) + with sock1, sock2: + sock1.setblocking(True) + with self.assertRaises(BlockingIOError): + for _ in range(64 * 1024): + socket.send_fds(sock1, [MSG], [rfd], socket.MSG_DONTWAIT) + + msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) + self.addCleanup(os.close, fds[0]) + + self.assertEqual(msg, MSG) + self._test_pipe(fds[0], wfd, MSG) class FreeThreadingTests(unittest.TestCase): From 45b28f67b5fb40ce905974716ab21340a6bcfd5d Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Wed, 15 Jan 2025 22:51:04 +0800 Subject: [PATCH 2/6] Add NEWS --- .../next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst diff --git a/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst b/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst new file mode 100644 index 00000000000000..4c259b8a26f03b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst @@ -0,0 +1,2 @@ +Fix ``flags`` and ``address`` parameters which were ignored in +``socket.send_fds`` and ``socket.recv_fds``. From fbb26680d698fb897384caf89281b0f7572070ce Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Wed, 15 Jan 2025 23:01:33 +0800 Subject: [PATCH 3/6] Only test MSG_DONTWAIT on Linux --- Lib/test/test_socket.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index ce65f90c5f4bb1..2d73b1d0cfd3e6 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7128,6 +7128,7 @@ def test_recv_fds_peek(self): self._test_pipe(fds[0], wfd, MSG) @requireAttrs(socket, "MSG_DONTWAIT") + @unittest.skipUnless(sys.platform in ('linux', 'android'), 'Linux specific test') def test_send_fds_dontwait(self): rfd, wfd = os.pipe() self.addCleanup(os.close, rfd) From c58d2de6d27ae5bb1a0ed405aa10776de8d40cca Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Fri, 17 Jan 2025 00:30:53 +0800 Subject: [PATCH 4/6] Fix cleanup fds --- Lib/test/test_socket.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 2d73b1d0cfd3e6..b33252689ea848 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7037,6 +7037,12 @@ def test_dual_stack_client_v6(self): @requireAttrs(socket, "recv_fds") @requireAttrs(socket, "AF_UNIX") class SendRecvFdsTests(unittest.TestCase): + def _cleanup_fds(self, fds): + def close_fds(fds): + for fd in fds: + os.close(fd) + self.addCleanup(close_fds, fds) + def _test_pipe(self, rfd, wfd, msg): assert len(msg) < 512 os.write(wfd, msg) @@ -7044,18 +7050,9 @@ def _test_pipe(self, rfd, wfd, msg): self.assertEqual(data, msg) def testSendAndRecvFds(self): - def close_pipes(pipes): - for fd1, fd2 in pipes: - os.close(fd1) - os.close(fd2) - - def close_fds(fds): - for fd in fds: - os.close(fd) - # send 10 file descriptors pipes = [os.pipe() for _ in range(10)] - self.addCleanup(close_pipes, pipes) + self._cleanup_fds(fd for pair in pipes for fd in pair) fds = [rfd for rfd, wfd in pipes] # use a UNIX socket pair to exchange file descriptors locally @@ -7064,7 +7061,7 @@ def close_fds(fds): socket.send_fds(sock1, [MSG], fds) # request more data and file descriptors than expected msg, fds2, flags, addr = socket.recv_fds(sock2, len(MSG) * 2, len(fds) * 2) - self.addCleanup(close_fds, fds2) + self._cleanup_fds(fds2) self.assertEqual(msg, MSG) self.assertEqual(len(fds2), len(fds)) @@ -7091,14 +7088,13 @@ def test_send_recv_fds_with_addrs(self): socket.send_fds(sock1, [MSG], [rfd], address=sock2_addr) msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) - new_rfd = fds[0] - self.addCleanup(os.close, new_rfd) + self._cleanup_fds(fds) self.assertEqual(msg, MSG) self.assertEqual(len(fds), 1) self.assertEqual(addr, sock1_addr) - self._test_pipe(new_rfd, wfd, MSG) + self._test_pipe(fds[0], wfd, MSG) @requireAttrs(socket, "MSG_PEEK") def test_recv_fds_peek(self): @@ -7115,16 +7111,20 @@ def test_recv_fds_peek(self): peek_len = len(MSG) // 2 msg, fds, flags, addr = socket.recv_fds(sock2, peek_len, 1, socket.MSG_PEEK) - self.addCleanup(os.close, fds[0]) + self._cleanup_fds(fds) + self.assertEqual(len(msg), peek_len) self.assertEqual(msg, MSG[:peek_len]) self.assertEqual(flags & socket.MSG_TRUNC, socket.MSG_TRUNC) + self.assertEqual(len(fds), 1) self._test_pipe(fds[0], wfd, MSG) # will raise BlockingIOError if MSG_PEEK didn't work msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) - self.addCleanup(os.close, fds[0]) + self._cleanup_fds(fds) + self.assertEqual(msg, MSG) + self.assertEqual(len(fds), 1) self._test_pipe(fds[0], wfd, MSG) @requireAttrs(socket, "MSG_DONTWAIT") @@ -7142,9 +7142,10 @@ def test_send_fds_dontwait(self): socket.send_fds(sock1, [MSG], [rfd], socket.MSG_DONTWAIT) msg, fds, flags, addr = socket.recv_fds(sock2, len(MSG), 1) - self.addCleanup(os.close, fds[0]) + self._cleanup_fds(fds) self.assertEqual(msg, MSG) + self.assertEqual(len(fds), 1) self._test_pipe(fds[0], wfd, MSG) From aefccca1d15cfeb235d9dc791efd1d97f85e0c6a Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Fri, 17 Jan 2025 00:39:00 +0800 Subject: [PATCH 5/6] Skip platform-dependent tests --- Lib/test/test_socket.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index b33252689ea848..d80e45a75e6aa9 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7072,6 +7072,8 @@ def testSendAndRecvFds(self): for index, ((_, wfd), rfd) in enumerate(zip(pipes, fds2)): self._test_pipe(rfd, wfd, str(index).encode()) + @unittest.skipUnless(sys.platform in ("linux", "android", "darwin"), + "works on Linux and macOS") def test_send_recv_fds_with_addrs(self): sock1 = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM) sock2 = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM) @@ -7097,6 +7099,7 @@ def test_send_recv_fds_with_addrs(self): self._test_pipe(fds[0], wfd, MSG) @requireAttrs(socket, "MSG_PEEK") + @unittest.skipUnless(sys.platform in ("linux", "android"), "works on Linux") def test_recv_fds_peek(self): rfd, wfd = os.pipe() self.addCleanup(os.close, rfd) @@ -7128,7 +7131,7 @@ def test_recv_fds_peek(self): self._test_pipe(fds[0], wfd, MSG) @requireAttrs(socket, "MSG_DONTWAIT") - @unittest.skipUnless(sys.platform in ('linux', 'android'), 'Linux specific test') + @unittest.skipUnless(sys.platform in ("linux", "android"), "Linux specific test") def test_send_fds_dontwait(self): rfd, wfd = os.pipe() self.addCleanup(os.close, rfd) From 6b58f4b6f74341f6eae18ee21edb9842a5c623a6 Mon Sep 17 00:00:00 2001 From: GalaxySnail Date: Sat, 18 Jan 2025 17:31:38 +0800 Subject: [PATCH 6/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Lib/test/test_socket.py | 2 +- .../next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index d80e45a75e6aa9..6d3864f0d5791f 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -7113,7 +7113,7 @@ def test_recv_fds_peek(self): # peek message on sock2 peek_len = len(MSG) // 2 msg, fds, flags, addr = socket.recv_fds(sock2, peek_len, 1, - socket.MSG_PEEK) + flags=socket.MSG_PEEK) self._cleanup_fds(fds) self.assertEqual(len(msg), peek_len) diff --git a/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst b/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst index 4c259b8a26f03b..7c2a874e048d59 100644 --- a/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst +++ b/Misc/NEWS.d/next/Library/2025-01-15-22-50-41.gh-issue-128881.JBL_9E.rst @@ -1,2 +1,2 @@ Fix ``flags`` and ``address`` parameters which were ignored in -``socket.send_fds`` and ``socket.recv_fds``. +:func:`socket.send_fds` and :func:`socket.recv_fds`.