Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
ThiefMaster mannequin opened this issue Nov 17, 2020 · 9 comments
Closed

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

ThiefMaster mannequin opened this issue Nov 17, 2020 · 9 comments
Assignees
@gpshead
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ThiefMaster
Copy link
Mannequin

ThiefMaster mannequin commented Nov 17, 2020

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
    Copy link
    Mannequin Author

    ThiefMaster mannequin commented Nov 17, 2020

    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.".

    @ThiefMaster ThiefMaster mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 17, 2020
    @izbyshev
    Copy link
    Mannequin

    izbyshev mannequin commented Nov 22, 2020

    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 izbyshev mannequin added 3.8 only security fixes 3.10 only security fixes labels Nov 22, 2020
    @izbyshev
    Copy link
    Mannequin

    izbyshev mannequin commented Nov 22, 2020

    (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
    Copy link
    Member

    gpshead commented Nov 22, 2020

    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 gpshead self-assigned this Nov 22, 2020
    @gpshead
    Copy link
    Member

    gpshead commented Dec 25, 2020

    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
    Copy link
    Member

    gpshead commented Dec 25, 2020

    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
    Copy link
    Contributor

    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
    Copy link
    Contributor

    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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @gpshead
    Copy link
    Member

    gpshead commented Jun 17, 2022

    this looks complete.

    @gpshead gpshead closed this as completed Jun 17, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @gpshead gpshead

    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @gpshead @miss-islington