From c780ce054de3e619a022f927abf22d5dba69d74f Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 30 May 2025 18:30:35 +0000 Subject: [PATCH 1/4] GH-134453: Fix subprocess memoryview input handling on POSIX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix inconsistent subprocess.Popen.communicate() behavior between Windows and POSIX when using memoryview objects with non-byte elements as input. On POSIX systems, the code was incorrectly comparing bytes written against element count instead of byte count, causing data truncation for large inputs with non-byte element types. Changes: - Cast memoryview inputs to byte view when input is already a memoryview - Fix progress tracking to use len(input_view) instead of len(self._input) - Add comprehensive test coverage for memoryview inputs 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude pre-commit-whitespace-fixup --- Lib/subprocess.py | 7 +++++-- Lib/test/test_subprocess.py | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 54c2eb515b60da..c1116358a39ce1 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -2103,7 +2103,10 @@ def _communicate(self, input, endtime, orig_timeout): self._save_input(input) if self._input: - input_view = memoryview(self._input) + if not isinstance(self._input, memoryview): + input_view = memoryview(self._input) + else: + input_view = self._input.cast("b") # byte input required with _PopenSelector() as selector: if self.stdin and input: @@ -2139,7 +2142,7 @@ def _communicate(self, input, endtime, orig_timeout): selector.unregister(key.fileobj) key.fileobj.close() else: - if self._input_offset >= len(self._input): + if self._input_offset >= len(input_view): selector.unregister(key.fileobj) key.fileobj.close() elif key.fileobj in (self.stdout, self.stderr): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f0e350c71f60ea..20f1f43a82a35d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -957,6 +957,47 @@ def test_communicate(self): self.assertEqual(stdout, b"banana") self.assertEqual(stderr, b"pineapple") + def test_communicate_memoryview_input(self): + # Test memoryview input with byte elements + test_data = b"Hello, memoryview!" + mv = memoryview(test_data) + p = subprocess.Popen([sys.executable, "-c", + 'import sys; sys.stdout.write(sys.stdin.read())'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) + self.addCleanup(p.stdout.close) + self.addCleanup(p.stdin.close) + (stdout, stderr) = p.communicate(mv) + self.assertEqual(stdout, test_data) + self.assertEqual(stderr, None) + + def test_communicate_memoryview_input_nonbyte(self): + # Test memoryview input with non-byte elements (e.g., int32) + # This tests the fix for gh-134453 where non-byte memoryviews + # had incorrect length tracking on POSIX + import array + # Create an array of 32-bit integers that's large enough to trigger + # the chunked writing behavior (> PIPE_BUF) + pipe_buf = getattr(select, 'PIPE_BUF', 512) + # Each 'i' element is 4 bytes, so we need more than pipe_buf/4 elements + # Add some extra to ensure we exceed the buffer size + num_elements = (pipe_buf // 4) + 100 + test_array = array.array('i', range(num_elements)) # 'i' = signed int (4 bytes each) + expected_bytes = test_array.tobytes() + mv = memoryview(test_array) + + p = subprocess.Popen([sys.executable, "-c", + 'import sys; ' + 'data = sys.stdin.buffer.read(); ' + 'sys.stdout.buffer.write(data)'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) + self.addCleanup(p.stdout.close) + self.addCleanup(p.stdin.close) + (stdout, stderr) = p.communicate(mv) + self.assertEqual(stdout, expected_bytes) + self.assertEqual(stderr, None) + def test_communicate_timeout(self): p = subprocess.Popen([sys.executable, "-c", 'import sys,os,time;' From 7e1e75cc78b0fd13c55193a3ebe74419de0e0dd9 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 30 May 2025 18:37:46 +0000 Subject: [PATCH 2/4] NEWS entry --- .../Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst diff --git a/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst new file mode 100644 index 00000000000000..a1fb813ec24878 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst @@ -0,0 +1,4 @@ +Fixed :mod:`subprocess.communicate` ``input=`` handling of :class:`memoryview` +instances that were non-byte shaped on POSIX platforms. Those are now properly +cast to a byte shaped view instead of truncating the input. Windows platforms +did not have this bug. From a975c91555a916a0c6e0d4020d57d051a96a562f Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 30 May 2025 19:00:28 +0000 Subject: [PATCH 3/4] old-man-yells-at-ReST --- .../next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst index a1fb813ec24878..a4ce93db89f473 100644 --- a/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst +++ b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst @@ -1,4 +1,4 @@ -Fixed :mod:`subprocess.communicate` ``input=`` handling of :class:`memoryview` +Fixed :func:`subprocess.communicate` ``input=`` handling of :class:`memoryview` instances that were non-byte shaped on POSIX platforms. Those are now properly cast to a byte shaped view instead of truncating the input. Windows platforms did not have this bug. From 5849552d58d449f7535be739cc78959cafed8224 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 30 May 2025 14:07:33 -0700 Subject: [PATCH 4/4] Update 2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst --- .../next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst index a4ce93db89f473..fee975ea70230c 100644 --- a/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst +++ b/Misc/NEWS.d/next/Library/2025-05-30-18-37-44.gh-issue-134453.kxkA-o.rst @@ -1,4 +1,4 @@ -Fixed :func:`subprocess.communicate` ``input=`` handling of :class:`memoryview` +Fixed :func:`subprocess.Popen.communicate` ``input=`` handling of :class:`memoryview` instances that were non-byte shaped on POSIX platforms. Those are now properly cast to a byte shaped view instead of truncating the input. Windows platforms did not have this bug.