| msg258905 - (view) |
Author: Hrvoje Nikšić (hniksic) * |
Date: 2016-01-25 16:13 |
| The documentation for the "es#" format (and the "et#" that derives from it) documents the possibility of providing an already allocated buffer. Buffer overflow is detected and handled as follows: "If the buffer is not large enough, a ValueError will be set." However, the actual behavior is to raise a TypeError. Inspecting the code in getargs.c reveals that convertsimple() handles buffer overflow by returning a formatted message to its caller, convertitem(). Calls to convertitem() that return an error call seterror() to set the error, and seterror() unconditionally sets the PyExc_TypeError. This is not a big issue in practice, and since the behavior is not new, it might be best to simply update the documentation to match the existing practice instead of changing the behavior and risking breaking working code. |
|
|
| msg258907 - (view) |
Author: Hrvoje Nikšić (hniksic) * |
Date: 2016-01-25 16:24 |
| The problem can be encountered and easily reproduced by calling os.path functions, such as os.path.abspath, with a sufficiently large string on Windows: >>> os.path.abspath("a" * 1024) Traceback (most recent call last): File "", line 1, in File "P:\...\lib\ntpath.py", line 471, in abspath TypeError: must be (buffer overflow), not str The error message is somewhat confusing, making it look like the "must be" and "not" arguments are swapped. Ideally, the message could be along the lines of "must be a string of no more than X characters". |
|
|
| msg258915 - (view) |
Author: Barun Parruck (Barun Parruck) * |
Date: 2016-01-25 18:10 |
| I just changed the ValueError to TypeError. This is my first attempt at fixing anything, so let me know if I've screwed up anywhere. |
|
|
| msg258917 - (view) |
Author: Barun Parruck (Barun Parruck) * |
Date: 2016-01-25 19:05 |
| Added a patch that changes the documentation to reflect TypeError instead of ValueError* |
|
|
| msg258957 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2016-01-26 14:08 |
| Proposed patch fixes minor bugs in Python/getargs.c: 1. Replaces TypeError with confusing error message for buffer overflow for "es#" and "et#" format units to ValueError with correct error message. Due to the risk to break working code, may be we will left TypeError in maintained releases, but error message should be fixed in any case. 2. Replaces all other TypeError with confusing error message to SystemError with correct error message. All this errors are programming errors (incorrect use of PyArg_Parse* functions) and aren't occurred in valid extensions. 3. Fixes error messages for "k" and "K" format units. 4. Fixes error messages for "es" and "et" format units. 5. Fixes error messages for "compat" mode (looks as this mode is not used and can be freely removed in Python 3). |
|
|
| msg258960 - (view) |
Author: Hrvoje Nikšić (hniksic) * |
Date: 2016-01-26 14:29 |
| Barun, Serhiy, thanks for picking this up so quickly. I would further suggest to avoid using a fixed buffer in abspath (_getfullpathname, but only abspath seems to call it). Other filesystem calls are using the interface where PyArg_ParseTuple allocates the buffer. This makes it easier to handling errors from the native layer by catching OSError or similar, instead of the very generic TypeError (or ValueError). |
|
|
| msg259151 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-01-28 17:58 |
| New changeset 1c690cb4def8 by Serhiy Storchaka in branch '3.5': Issue #26198: Added tests for "es", "et", "es#", "et#" and "C" format units https://hg.python.org/cpython/rev/1c690cb4def8 New changeset 745ad3010384 by Serhiy Storchaka in branch 'default': Issue #26198: Added tests for "es", "et", "es#", "et#" and "C" format units https://hg.python.org/cpython/rev/745ad3010384 New changeset 60a2d67dacb3 by Serhiy Storchaka in branch '2.7': Issue #26198: Added tests for string-related format units of PyArg_Parse*() https://hg.python.org/cpython/rev/60a2d67dacb3 |
|
|
| msg259163 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-01-28 21:53 |
| One review comment |
|
|
| msg259167 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-01-28 22:36 |
| Also, test_getargs2 seems faulty on 2.7. I am seeing MemoryError; some of the buildbots are segfaulting. |
|
|
| msg259170 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-01-28 22:56 |
| New changeset d0b4be7d2134 by Serhiy Storchaka in branch '2.7': Fixed a crash in new tests in test_getargs2 added in 60a2d67dacb3 (issue #26198). https://hg.python.org/cpython/rev/d0b4be7d2134 |
|
|
| msg259171 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-01-28 23:03 |
| Thank you Martin. This was 64-bit specific bug. |
|
|
| msg259172 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-01-28 23:08 |
| Here is updated patch. |
|
|
| msg259809 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-02-07 23:23 |
| New changeset ab25ce400abd by Serhiy Storchaka in branch '2.7': Issue #26198: Fixed error messages for some argument parsing errors. https://hg.python.org/cpython/rev/ab25ce400abd New changeset 9f998e24d8d8 by Serhiy Storchaka in branch '3.5': Issue #26198: Fixed error messages for some argument parsing errors. https://hg.python.org/cpython/rev/9f998e24d8d8 New changeset f8bdc0ea3bcf by Serhiy Storchaka in branch 'default': Issue #26198: Fixed error messages for some argument parsing errors. https://hg.python.org/cpython/rev/f8bdc0ea3bcf New changeset 53d66a554beb by Serhiy Storchaka in branch 'default': Issue #26198: ValueError is now raised instead of TypeError on buffer https://hg.python.org/cpython/rev/53d66a554beb |
|
|
| msg259820 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2016-02-08 05:55 |
| test_datetime is broken on 3.5 and default: ====================================================================== FAIL: test_format (test.datetimetester.TestSubclassDateTime) ---------------------------------------------------------------------- TypeError: __format__() argument 1 must be str, not int During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/var/lib/buildbot/slaves/profile-opt-bot/3.5.gps-debian-profile-opt/build/Lib/test/datetimetester.py", line 1578, in test_format dt.__format__(123) AssertionError: "^must be str, not int$" does not match "__format__() argument 1 must be str, not int" http://buildbot.python.org/all/builders/AMD64%20Debian%20PGO%203.5/builds/632/steps/test/logs/stdio http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/10486/steps/test/logs/stdio |
|
|
| msg259825 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-02-08 07:27 |
| New changeset 22b0a63808f8 by Serhiy Storchaka in branch '3.5': Issue #26198: Make datetime error tests more lenient. https://hg.python.org/cpython/rev/22b0a63808f8 New changeset a9c9e4054f3f by Serhiy Storchaka in branch 'default': Issue #26198: Make datetime error tests more lenient. https://hg.python.org/cpython/rev/a9c9e4054f3f |
|
|
| msg259826 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-02-08 07:30 |
| Thanks Berker. |
|
|