From fa8d19c54aebb9ac4486ee7dcae9efba4b9ef51f Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Tue, 2 Aug 2022 19:05:13 +0100 Subject: [PATCH 1/8] Implement handling of unicode decode error from incomplete code bytes --- Lib/subprocess.py | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 7ae8df154b481f9..4475972ee2e8613 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1245,11 +1245,39 @@ def _check_timeout(self, endtime, orig_timeout, stdout_seq, stderr_seq, """Convenience for checking if a timeout has expired.""" if endtime is None: return + + def translate_newlines_partial_output(data, encoding, errors): + # Handle decoding the data considering it may be truncated + # mid-codepoint (ignore the trailing partial codepoint). + # See https://github.com/python/cpython/issues/87597 + try: + output = self._translate_newlines(data, encoding, errors) + except UnicodeDecodeError as exc: + if exc.end == len(data): + output = self._translate_newlines(data[:exc.start], + encoding, + errors) + else: + raise + return output + if skip_check_and_raise or _time() > endtime: + if stdout_seq: + stdout = b''.join(stdout_seq) + if self.text_mode: + stdout = translate_newlines_partial_output( + stdout, self.stdout.encoding, self.stdout.errors) + else: + stdout = None + if stderr_seq: + stderr = b''.join(stderr_seq) + if self.text_mode: + stderr = translate_newlines_partial_output( + stderr, self.stderr.encoding, self.stderr.errors) + else: + stderr = None raise TimeoutExpired( - self.args, orig_timeout, - output=b''.join(stdout_seq) if stdout_seq else None, - stderr=b''.join(stderr_seq) if stderr_seq else None) + self.args, orig_timeout, output=stdout, stderr=stderr) def wait(self, timeout=None): From aecc55ba28b98705c9052019a37fe04451b4c0b4 Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Tue, 2 Aug 2022 19:05:39 +0100 Subject: [PATCH 2/8] Add testcase --- Lib/test/test_subprocess.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f6854922a5b8782..f10d8e9bd2b7f09 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1129,6 +1129,27 @@ def test_universal_newlines_communicate_encodings(self): stdout, stderr = popen.communicate(input='') self.assertEqual(stdout, '1\n2\n3\n4') + def test_universal_newlines_timeout(self): + with self.assertRaises(subprocess.TimeoutExpired) as c: + p = subprocess.run( + [ + sys.executable, "-c", + "import sys, time;" + r"sys.stderr.buffer.write(b'foo \xc2\xa4 bar');" + "sys.stderr.buffer.flush();" + r"sys.stdout.buffer.write(b'foo \xc2');" + "sys.stdout.buffer.flush();" + "time.sleep(0.1);" + r"sys.stdout.buffer.write(b'\xa4 bar');" + "sys.stdout.buffer.flush();" + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + timeout=0.05) + self.assertEqual(c.exception.stdout, "foo ") + self.assertEqual(c.exception.stderr, "foo ¤ bar") + def test_communicate_errors(self): for errors, expected in [ ('ignore', ''), From 4b4b7eb9b0c83fa057707cd84986465ab9f00b56 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 2 Aug 2022 18:12:36 +0000 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst b/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst new file mode 100644 index 000000000000000..4c1228bd3972f08 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst @@ -0,0 +1 @@ +Store decoded output from `subprocess.run()` on `TimeoutExpired` exception when using `text=True` mode. From c4b10121a7fa97e3d10bf7efea2e31147bca41a2 Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Tue, 2 Aug 2022 21:23:28 +0100 Subject: [PATCH 4/8] Update Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst --- .../2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst b/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst index 4c1228bd3972f08..ff0fdb7e04f3b3e 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-08-02-18-12-34.gh-issue-87597.UHFR0H.rst @@ -1 +1 @@ -Store decoded output from `subprocess.run()` on `TimeoutExpired` exception when using `text=True` mode. +Store decoded output from ``subprocess.run()`` on ``TimeoutExpired`` exception when using ``text=True`` mode. From a2c7e068326e16d163f94862579b2f569681c2be Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Tue, 2 Aug 2022 21:40:49 +0100 Subject: [PATCH 5/8] Skip testcase on Windows - after timeout the threads reading output are left running --- Lib/test/test_subprocess.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index f10d8e9bd2b7f09..220c8d543a140f1 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1129,6 +1129,7 @@ def test_universal_newlines_communicate_encodings(self): stdout, stderr = popen.communicate(input='') self.assertEqual(stdout, '1\n2\n3\n4') + @unittest.skipIf(mswindows, "behavior currently not supported on Windows") def test_universal_newlines_timeout(self): with self.assertRaises(subprocess.TimeoutExpired) as c: p = subprocess.run( From 9d2168ea21096829a10a0b7e77f748a2b3fef166 Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Wed, 3 Aug 2022 23:22:51 +0100 Subject: [PATCH 6/8] Test markups --- Lib/test/test_subprocess.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 220c8d543a140f1..491adb63596fc51 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1140,14 +1140,12 @@ def test_universal_newlines_timeout(self): "sys.stderr.buffer.flush();" r"sys.stdout.buffer.write(b'foo \xc2');" "sys.stdout.buffer.flush();" - "time.sleep(0.1);" - r"sys.stdout.buffer.write(b'\xa4 bar');" - "sys.stdout.buffer.flush();" + "time.sleep(10);" ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True, - timeout=0.05) + timeout=3) self.assertEqual(c.exception.stdout, "foo ") self.assertEqual(c.exception.stderr, "foo ¤ bar") From 98bff39a8c85169f7da6c78b2d01791b83a4d414 Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Sun, 7 Aug 2022 13:27:09 +0100 Subject: [PATCH 7/8] If no output before timeout is hit ensure empty string/bytes is stored rather than None --- Lib/subprocess.py | 4 ++-- Lib/test/test_subprocess.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 4475972ee2e8613..75fe912c330e957 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1262,14 +1262,14 @@ def translate_newlines_partial_output(data, encoding, errors): return output if skip_check_and_raise or _time() > endtime: - if stdout_seq: + if stdout_seq is not None: stdout = b''.join(stdout_seq) if self.text_mode: stdout = translate_newlines_partial_output( stdout, self.stdout.encoding, self.stdout.errors) else: stdout = None - if stderr_seq: + if stderr_seq is not None: stderr = b''.join(stderr_seq) if self.text_mode: stderr = translate_newlines_partial_output( diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 491adb63596fc51..59d3b9ee9721a93 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1149,6 +1149,17 @@ def test_universal_newlines_timeout(self): self.assertEqual(c.exception.stdout, "foo ") self.assertEqual(c.exception.stderr, "foo ¤ bar") + @unittest.skipIf(mswindows, "behavior currently not supported on Windows") + def test_no_output_timeout(self): + with self.assertRaises(subprocess.TimeoutExpired) as c: + p = subprocess.run( + [sys.executable, "-c", "import time; time.sleep(10)"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=3) + self.assertEqual(c.exception.stdout, b"") + self.assertEqual(c.exception.stderr, b"") + def test_communicate_errors(self): for errors, expected in [ ('ignore', ''), From 39393a8deee3328908b9c8db1bf31cf05d91fb34 Mon Sep 17 00:00:00 2001 From: Lewis Gaul Date: Sun, 7 Aug 2022 13:27:50 +0100 Subject: [PATCH 8/8] Reduce timeout when not waiting for any output --- Lib/test/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 59d3b9ee9721a93..413f63a3c9aa319 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1156,7 +1156,7 @@ def test_no_output_timeout(self): [sys.executable, "-c", "import time; time.sleep(10)"], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - timeout=3) + timeout=0.1) self.assertEqual(c.exception.stdout, b"") self.assertEqual(c.exception.stderr, b"")