Repository navigation
argparse.FileType for '-' doesn't work for a mode of 'rb' #58364
Description
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 = None
closed_at = <Date 2022-03-18.15:03:40.764>
created_at = <Date 2012-02-29.11:01:10.726>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = "argparse.FileType for '-' doesn't work for a mode of 'rb'"
updated_at = <Date 2022-03-18.15:03:40.762>
user = 'https://github.com/anacrolix'bugs.python.org fields:
activity = <Date 2022-03-18.15:03:40.762>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-18.15:03:40.764>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2012-02-29.11:01:10.726>
creator = 'anacrolix'
dependencies = []
files = ['24868', '26484', '42072']
hgrepos = []
issue_num = 14156
keywords = ['patch', 'needs review']
message_count = 33.0
messages = ['154612', '154641', '155761', '155923', '162342', '166170', '215299', '215410', '221271', '221456', '221457', '261218', '282851', '282922', '282948', '283210', '283213', '283216', '283463', '293095', '294544', '297997', '298237', '324859', '324892', '325011', '341767', '341796', '373025', '414479', '414614', '414615', '415501']
nosy_count = 17.0
nosy_names = ['bethard', 'naufraghi', 'r.david.murray', 'eli.bendersky', 'paul.j3', 'serhiy.storchaka', 'moritz', 'markgrandi', 'josh.r', 'lekensteyn', 'palaviv', 'evan_', 'sedrubal', 'Marcel H2', 'miss-islington', 'Leo Singer', 'yossarian']
pr_nums = ['9124', '13165', '31706', '31979']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue14156'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
Activity
anacrolix commentedon Feb 29, 2012
If an argument of '-' is handled by argparse.FileType, it defaults to sys.stdin. However a mode of 'rb' is ignored, the returned file object does not work with raw bytes.
bethard commentedon Feb 29, 2012
Yes, the problem is in FileType.__call__ - the handling of '-' is pretty simplistic. Patches welcome.
anacrolix commentedon Mar 14, 2012
Roger that. I'll start on a patch for this in a month or two if all goes well.
anacrolix commentedon Mar 15, 2012
Steven, patch attached. I lost steam in the unittests with all the meta, suffice it that the names match the file descriptors of the stream sources. i.e. FileType('rb') would give a file with name=0, and so forth. My chosen method also allows other mode flags as well as custom bufsizes.
moritz commentedon Jun 5, 2012
I don't know how if this is the perfect solution but it keeps the program from crashing.
1154c1154,1157
< return _sys.stdin
---
1156c1159,1162
< return _sys.stdout
---
bethard commentedon Jul 22, 2012
The fix looks right, but we definitely need a test. I tried to write one, but I'm not sure how to do this properly given how test_argparse redirects standard input and output (so that fileno() doesn't work anymore). I've attached my current (failing) attempt to test this.
paulj3 commentedon Apr 1, 2014
A related issue http://bugs.python.org/issue13824
"argparse.FileType opens a file and never closes it"
I proposed a FileContext class that returns a `partial(open, filename,...)' context object. The issues of how to deal with stdin/out are similar.
paulj3 commentedon Apr 3, 2014
There are a couple of complications to using 'fileno'.
We probably don't want to close 'sys.stdin' or 'sys.stdout' (not even if they are redirected to other files?). That means using:
'closefd', on the other hand, has to be True for string file specifications.
But in 'test_argparse.py', 'sys.stdout' is redirected to an 'io.StringIO'. This has many of the same features as an open file, but 'fileno' is not implemented. So the TypeFile probably needs to make an exception for this case. I don't how this will play with a 'BytesIO' for 'wb' cases.
elibendersky commentedon Jun 22, 2014
Nosy-ing myself since I just ran into it. Annoying issue that precludes from using argparse's builtin '-' recognition for reading binary data.
I'll try to carve some time later to look at the patches.
elibendersky commentedon Jun 24, 2014
The patch looks reasonable? Is the only remaining problem with crafting the test?
elibendersky commentedon Jun 24, 2014
[sorry, the first question mark shouldn't be - the patch indeed looks reasonable to me]
Steven - how about launching a subprocess for stdin tests to avoid weird issues?
18 remaining items
serhiy-storchaka commentedon Sep 11, 2018
The solution with fileno() is clever, but as was mentioned before, it doesn't work if stdin or stdout are not real files, but something like StringIO. It is not that in common use of argparse for parsing arguments in scripts they are redefined, but argparse can be used in uncommon environments, for example for emulating command line in the environment like IDLE which redefines standard streams. And I'm sure this will break third-party tests which main() with patched stdin/stdout for testing CLI. The initial solution proposed by Moritz is more reliable, although it doesn't fix an issues with closing stdin/stdout. But this is a different issue at all.
MojoVampire commentedon May 7, 2019
Serhiy: To be clear, Moritz's patch won't work if stdin/stdout are io.StringIO or the like either, since StringIO lacks a buffer attribute.
Personally, I'm content to:
I'm going to convert Moritz's proposal to a PR and hope I can get core developer approval for it.
MojoVampire commentedon May 7, 2019
I've created PR13165 to address this bug. It's 99% test updates; the actual code changes to argparse.py are trivial and should be equally trivial to review.
The only functional change beyond Moritz's proposal is that I added support for having accepting '-' when the mode string uses 'a' or 'x' instead of 'w'; for sys.stdout, they're all effectively equivalent ('x' is trying to prevent stomping an existing file, which borrowing sys.stdout won't do, and sys.stdout is already more closely equivalent to mode 'a' in any event). No working code should break as a result of that change (passing 'a' or 'x' previously just caused FileType to exit immediately with a ValueError, which in turn caused parse_args to kill the program, which I'm assuming isn't considered a valuable "feature").
In addition to testing binary mode with argument '-' properly, I also added complete test cases for mode 'x' and 'xb' (for all arguments, both file names and '-') since we had no such tests, and ensuring exclusive creation mode behaves correctly is fairly important.
Lekensteyn commentedon Jul 5, 2020
I just ran into this issue on Linux when piping a binary file to stdin resulted in a UnicodeDecodeError while trying to read a byte from the stream. Passing /dev/stdin is a workaround that does not require modifications to an application.
As for the proposed PR 13165, I'd suggest to gracefully fallback to normal stdout/stdin if the buffer is not available. That approach is also followed in the fileinput module, and takes care of the note for library developers in the documentation at https://docs.python.org/3/library/sys.html#sys.stdin
Not feeling particular strong about the graceful handling, but I hope that the test code can be simplified in that case.
woodruffw commentedon Mar 3, 2022
Nosying myself; this affects 3.9 and 3.10 as well.
serhiy-storchaka commentedon Mar 6, 2022
New changeset eafec26 by MojoVampire in branch 'main':
bpo-14156: Make argparse.FileType work correctly for binary file modes when argument is '-' (GH-13165)
eafec26
miss-islington commentedon Mar 6, 2022
New changeset ee18df4 by Miss Islington (bot) in branch '3.10':
bpo-14156: Make argparse.FileType work correctly for binary file modes when argument is '-' (GH-13165)
ee18df4
serhiy-storchaka commentedon Mar 18, 2022
New changeset 4d2099f by Serhiy Storchaka in branch '3.9':
[3.9] bpo-14156: Make argparse.FileType work correctly for binary file modes when argument is '-' (GH-13165) (GH-31979)
4d2099f