bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)

When the modern text= spelling of the universal_newlines= parameter was added
for Python 3.7, check_output's special case around input=None was overlooked.
So it behaved differently with universal_newlines=True vs text=True.  This
reconciles the behavior to be consistent and adds a test to guarantee it.

Also clarifies the existing check_output documentation.

Co-authored-by: Alexey Izbyshev <izbyshev@ispras.ru>
This commit is contained in:
Gregory P. Smith 2020-12-24 20:57:21 -08:00 committed by GitHub
parent 8badadec53
commit 64abf37344
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 3 deletions

View File

@ -1187,8 +1187,9 @@ calls these functions.
The arguments shown above are merely some common ones.
The full function signature is largely the same as that of :func:`run` -
most arguments are passed directly through to that interface.
However, explicitly passing ``input=None`` to inherit the parent's
standard input file handle is not supported.
One API deviation from :func:`run` behavior exists: passing ``input=None``
will behave the same as ``input=b''`` (or ``input=''``, depending on other
arguments) rather than using the parent's standard input file handle.
By default, this function will return the data as encoded bytes. The actual
encoding of the output data may depend on the command being invoked, so the

View File

@ -420,7 +420,11 @@ def check_output(*popenargs, timeout=None, **kwargs):
if 'input' in kwargs and kwargs['input'] is None:
# Explicitly passing input=None was previously equivalent to passing an
# empty string. That is maintained here for backwards compatibility.
kwargs['input'] = '' if kwargs.get('universal_newlines', False) else b''
if kwargs.get('universal_newlines') or kwargs.get('text'):
empty = ''
else:
empty = b''
kwargs['input'] = empty
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
**kwargs).stdout

View File

@ -204,6 +204,28 @@ class ProcessTestCase(BaseTestCase):
input=b'pear')
self.assertIn(b'PEAR', output)
def test_check_output_input_none(self):
"""input=None has a legacy meaning of input='' on check_output."""
output = subprocess.check_output(
[sys.executable, "-c",
"import sys; print('XX' if sys.stdin.read() else '')"],
input=None)
self.assertNotIn(b'XX', output)
def test_check_output_input_none_text(self):
output = subprocess.check_output(
[sys.executable, "-c",
"import sys; print('XX' if sys.stdin.read() else '')"],
input=None, text=True)
self.assertNotIn('XX', output)
def test_check_output_input_none_universal_newlines(self):
output = subprocess.check_output(
[sys.executable, "-c",
"import sys; print('XX' if sys.stdin.read() else '')"],
input=None, universal_newlines=True)
self.assertNotIn('XX', output)
def test_check_output_stdout_arg(self):
# check_output() refuses to accept 'stdout' argument
with self.assertRaises(ValueError) as c:

View File

@ -0,0 +1,2 @@
Fix subprocess.check_output(..., input=None) behavior when text=True to be
consistent with that of the documentation and universal_newlines=True.