classification
Title: argparse cannot handle empty arguments
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: bethard, bjacobs, python-dev, r.david.murray, torsten, zbysz
Priority: normal Keywords: patch

Created on 2011-06-17 15:40 by bjacobs, last changed 2012-07-22 02:36 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
issue12353_test.diff torsten, 2011-06-23 22:25 Patch w/ unit test review
modify_test_empty.diff torsten, 2011-06-24 06:27 Trivial change to the TestEmptyAndSpaceContainingArguments unit test review
Messages (12)
msg138515 - (view) Author: Bryan Jacobs (bjacobs) Date: 2011-06-17 15:40
Parsing arguments with argparse fails with an IndexError when one of the arguments is the empty string (''). This is caused by an access to the zero'th element of the argument value, without a preceding length check.

Fixed by the below patch:

Index: Lib/argparse.py
===================================================================
--- Lib/argparse.py
+++ Lib/argparse.py
@@ -1967,7 +1967,7 @@ class ArgumentParser(_AttributeHolder, _
         for arg_string in arg_strings:
 
             # for regular arguments, just add them back into the list
-            if arg_string[0] not in self.fromfile_prefix_chars:
+            if len(arg_string) == 0 or arg_string[0] not in self.fromfile_prefix_chars:
                 new_arg_strings.append(arg_string)
 
             # replace arguments referencing files with the file content
msg138522 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-17 16:32
Thanks for the report and patch.  I'm setting this to test needed since the final patch will need a unit test.

The idiomatic way to do this kind of check is 'if not argstring or arg_string[0] not in self.fromfile_prefix_chars):'
msg138874 - (view) Author: Torsten Landschoff (torsten) * Date: 2011-06-23 22:25
Here is an updated patch including unit test coverage.
msg138883 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 01:01
Your unit test isn't consistent with the other unit tests in that set, which makes me suspicious that it isn't testing what we need to test.  Also, there are unit tests for this case further up in the test file (TestEmptyAndSpaceContainingArguments).  I haven't been able to reproduce the bug.

Can you post a short program that reproduces the failure?
msg138889 - (view) Author: Torsten Landschoff (torsten) * Date: 2011-06-24 06:21
> Your unit test isn't consistent with the other unit tests in that set, which makes me suspicious that it isn't testing what we need to test. 

That is because I did not try to understand the machinery behind the argparse unit tests completely. I did not want to create an extra unit test class just for this one test.

> Also, there are unit tests for this case further up in the test file (TestEmptyAndSpaceContainingArguments).  I haven't been able to reproduce the bug.

Did you run the unit tests from my patch?

> Can you post a short program that reproduces the failure?

Here you go:

from argparse import ArgumentParser
parser = ArgumentParser(fromfile_prefix_chars="@")
parser.parse_args([""])

This gives me

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/python3/lib/python3.3/argparse.py", line 1726, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "/opt/python3/lib/python3.3/argparse.py", line 1758, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
  File "/opt/python3/lib/python3.3/argparse.py", line 1770, in _parse_known_args
    arg_strings = self._read_args_from_files(arg_strings)
  File "/opt/python3/lib/python3.3/argparse.py", line 2003, in _read_args_from_files
    if arg_string[0] not in self.fromfile_prefix_chars:
IndexError: string index out of range
msg138890 - (view) Author: Torsten Landschoff (torsten) * Date: 2011-06-24 06:27
Here is another possible patch that will catch the problem.

But this enables the fromfile_prefix_chars option for all tests checking empty and space arguments. This way a problem that occurs only without that option might be hidden.

We would need to run those tests with and without fromfile_prefix_chars.
msg138946 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 14:02
Ah, I see now.  I misread the original traceback.

Creating a new test case would be the appropriate way to go, given the structure of the argparse test suite.
msg138947 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-24 14:05
Actually, your original test might be fine.  Let me double check the test implementation.
msg143560 - (view) Author: Torsten Landschoff (torsten) * Date: 2011-09-05 20:30
ping!?

I think this should be either applied or dropped. It does not make sense to have such a simple issue rot in the bug tracker...
msg166078 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2012-07-21 21:41
Yes, the original patch looks fine to me. I applied and tested it, and it works as expected.

Please go ahead and apply.
msg166097 - (view) Author: Roundup Robot (python-dev) Date: 2012-07-22 02:35
New changeset ac53876d1cc8 by R David Murray in branch '3.2':
#12353: argparse now correctly handles null argument values.
http://hg.python.org/cpython/rev/ac53876d1cc8

New changeset c4ad8a6eb0df by R David Murray in branch 'default':
Merge #12353: argparse now correctly handles null argument values.
http://hg.python.org/cpython/rev/c4ad8a6eb0df

New changeset c9806f0aaefb by R David Murray in branch '2.7':
#12353: argparse now correctly handles null argument values.
http://hg.python.org/cpython/rev/c9806f0aaefb
msg166098 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-07-22 02:36
Done.
History
Date User Action Args
2012-07-22 02:36:39r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg166098

stage: test needed -> resolved
2012-07-22 02:35:29python-devsetnosy: + python-dev
messages: + msg166097
2012-07-21 21:41:20bethardsetmessages: + msg166078
2011-09-24 21:37:28zbyszsetnosy: + zbysz
2011-09-05 20:30:39torstensetmessages: + msg143560
2011-06-24 14:05:33r.david.murraysetmessages: + msg138947
2011-06-24 14:02:59r.david.murraysetmessages: + msg138946
2011-06-24 06:27:24torstensetfiles: + modify_test_empty.diff

messages: + msg138890
2011-06-24 06:21:30torstensetmessages: + msg138889
2011-06-24 01:01:24r.david.murraysetmessages: + msg138883
2011-06-23 22:25:26torstensetfiles: + issue12353_test.diff

nosy: + torsten
messages: + msg138874

keywords: + patch
2011-06-17 16:32:18r.david.murraysetversions: + Python 3.2, Python 3.3
nosy: + r.david.murray, bethard

messages: + msg138522

stage: test needed
2011-06-17 15:40:06bjacobscreate