| 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) *  |
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) * |
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 "", line 1, in 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) * |
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) * |
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) * |
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) * |
Date: 2012-07-22 02:36 |
| Done. |
|
|