subprocess.check_output(['echo', 'test'], text=True, input=None) fails

[ThiefMaster]

BPO	42388
Nosy	@gpshead, @ThiefMaster, @izbyshev, @miss-islington
PRs	bpo-42388: Fix subprocess.check_output input=None when text=True #23467 [3.9] bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467) #23934 [3.8] bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467) #23935

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = None
created_at = <Date 2020-11-17.13:56:29.591>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = "subprocess.check_output(['echo', 'test'], text=True, input=None) fails"
updated_at = <Date 2020-12-25.05:18:40.656>
user = 'https://github.com/ThiefMaster'

bugs.python.org fields:

activity = <Date 2020-12-25.05:18:40.656>
actor = 'miss-islington'
assignee = 'gregory.p.smith'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-11-17.13:56:29.591>
creator = 'ThiefMaster'
dependencies = []
files = []
hgrepos = []
issue_num = 42388
keywords = ['patch']
message_count = 8.0
messages = ['381234', '381622', '381623', '381628', '383714', '383716', '383718', '383719']
nosy_count = 4.0
nosy_names = ['gregory.p.smith', 'ThiefMaster', 'izbyshev', 'miss-islington']
pr_nums = ['23467', '23934', '23935']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue42388'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[ThiefMaster]

subprocess.check_output(['echo', 'test'], text=True, input=None) fails with AttributeError: 'bytes' object has no attribute 'encode' due to the function only checking for universal_newlines but not text:

cpython/Lib/subprocess.py

Line 423 in 2ffba2a

kwargs['input'] = '' if kwargs.get('universal_newlines', False) else b''

This is inconsistent with the docs, which state that "text was added as a more readable alias for universal_newlines.".

[izbyshev]

It seems that allowing input=None to mean "redirect stdin to a pipe and send an empty string there" in subprocess.check_output was an accident(?), and this behavior is inconsistent with subprocess.run and communicate, where input=None has the same effect as not specifying it at all.

The docs for subprocess.check_output say:

The arguments shown above are merely some common ones. The full function signature is largely the same as that of 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.

They don't make it clear the effect of passing input=None though. I also couldn't find any tests that would check that effect.

Since we can't just forbid input=None for check_output at this point (probably can't even limit that to the case when text is used, since it was added in 3.7), I think that we need to extend the check pointed to by ThiefMaster to cover text, clarify the docs and add a test.

[izbyshev]

(probably can't even limit that to the case when text is used, since it was added in 3.7)

Well, actually, we can, since we probably don't need to preserve compatibility with the AttributeError currently caused by text=True with input=None. However, it seems to me that treating input=None differently depending on the other arguments would be confusing.

[gpshead]

It was a mere oversight that this didn't handle text= the same as universal_newlines=. I made a PR to keep their behavior consistent and match the documentation.

[gpshead]

New changeset 64abf37 by Gregory P. Smith in branch 'master':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
64abf37

[gpshead]

Meta issue behind this one: The input= behavior on check_output is yet another unfortunate wart in the subprocess collection of APIs.

PR to the main branch is in, 3.9 and 3.8 will automerge after CI runs.

[miss-islington]

New changeset d5aadb2 by Miss Islington (bot) in branch '3.8':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
d5aadb2

[miss-islington]

New changeset 7acfe41 by Miss Islington (bot) in branch '3.9':
bpo-42388: Fix subprocess.check_output input=None when text=True (GH-23467)
7acfe41

[gpshead]

this looks complete.