[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your contribution Oren!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset bc80fd1bd2e8b6817accbc101e7fe5e50ba8f768 by Serhiy Storchaka 
(Oren Milman) in branch '2.7':
[2.7] bpo-28261: Prevent raising SystemError where PyArg_ParseTuple is used to 
parse non-args. (#3213)
https://github.com/python/cpython/commit/bc80fd1bd2e8b6817accbc101e7fe5e50ba8f768


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


--
pull_requests: +3251

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 8e67981fc8e1bf3cb9774b5fbf4a39b8d65ba4ff by Serhiy Storchaka 
(Oren Milman) in branch '3.6':
[3.6] bpo-28261: Prevent raising SystemError where PyArg_ParseTuple is used to 
parse non-args. (#3210)
https://github.com/python/cpython/commit/8e67981fc8e1bf3cb9774b5fbf4a39b8d65ba4ff


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


--
pull_requests: +3248

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

sure

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oren, do you mind to backport the part of your changes that adds explicit 
checks for tuples to 3.6 and 2.7? Raising SystemError looks like a bug to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 13614e375cc3637cf1311733d453df6107e964ea by Serhiy Storchaka 
(Oren Milman) in branch 'master':
bpo-28261: fix err msgs where PyArg_ParseTuple is used to parse normal tuples 
(leftovers) (#3198)
https://github.com/python/cpython/commit/13614e375cc3637cf1311733d453df6107e964ea


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


--
pull_requests: +3237

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

it seems that I have missed some places which are part of this issue, at least
in Modules/_io/textio.c (one of them is mentioned in #31243).

also, when fixing these, we should also add a check before the call to
PyArg_ParseTuple (in case such check doesn't already exist), to verify the
object is indeed a tuple.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 1d1d3e9db882d78433f5bc8dbe7df929f4b6b5e1 by Serhiy Storchaka 
(Oren Milman) in branch 'master':
bpo-28261: Fixed err msgs where PyArg_ParseTuple is used to parse normal 
tuples. (#3119)
https://github.com/python/cpython/commit/1d1d3e9db882d78433f5bc8dbe7df929f4b6b5e1


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


--
pull_requests: +3157

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Answered questions on Rietveld. I'm not sure email notification from Rietveld 
works now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> also, i found two places with a quite similar issue:
> may I fix them also as part of this issue?

This looks as a different issue to me. All these cases are similar, but 
different from the original issue. Please open a new issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

After more looking, I found this issue in two more places:
- in Modules/itertoolsmodule.c in product_new:
>>> itertools.product(0, a=1, b=2, c=3, d=4, e=5, f=6)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: product() takes at most 1 argument (6 given)
- in Python/bltinmodule.c in builtin_print: 
>>> print(0, a=1, b=2, c=3, d=4, e=5, f=6)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: print() takes at most 4 arguments (6 given)

what do you think?
should I open another issue for these and the other two I
mentioned in msg300366?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

I replied to your comments in Rietveld, Serhiy. 
(http://bugs.python.org/review/28261)

also, i found two places with a quite similar issue:
- in Objects/exceptions.c in ImportError_init:
>>> ImportError(1, 2, 3, 4, a=5, b=6, c=7)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: ImportError() takes at most 2 arguments (3 given)
- in Python/bltinmodule.c in min_max:
>>> min(1, 2, 3, 4, a=5, b=6, c=7)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: function takes at most 2 arguments (3 given)

may I fix them also as part of this issue?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

as Serhiy pointed out in PR 668, here are some more functions that produce the
same kind of wrong error messages:
- Modules/timemodule.c - gettmarg()
- Modules/socketmodule.c:
* getsockaddrarg()
* socket_getnameinfo()

ISTM that searching for 'PyTuple_Check(' might be a good way to find more such
functions, as they sometimes raise an error in case a type other than tuple was
received (and after that, PyArg_ParseTuple is called).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I suggest to withdraw changes in itertoolsmodule.c and avoid using "dunder" 
names like __setstate__ and __new__ in error messages.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

Raymond? any suggestions for how to make the code less ugly?
or do you have in mind a different approach for solving this issue?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Would it look less awful if remove "xx.__setstate__: "?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Raymond Hettinger]

Raymond Hettinger added the comment:

-0 The new code looks awful, and in the case of the itertools module, these are 
messages that users are unlikely to see (the methods are typically only called 
by pickle, and pickling itself is rare for itertools).

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oren, could you write reproducers for all affected cases? I don't think we need 
to add them as regular tests, but we should be able to check changes manually.

There are conflicts in Modules/itertoolsmodule.c.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

An error message is not passed in the function name. PyArg_ParseTuple() allows 
you to pass an arbitrary error message. I think this feature is specially 
designed for these cases.

I didn't look the patch close, but Oren's approach LGTM in general.

--
assignee:  -> serhiy.storchaka
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Oren Milman added the comment:

Do you mean that in each case PyArg_ParseTuple fails, we should chain
to the exception raised by PyArg_ParseTuple an exception that specifies
the name of the tuple that PyArg_ParseTuple failed to parse, without
specifying the function name?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[STINNER Victor]

STINNER Victor added the comment:

I dislike passing a error message in the function name. I suggest to instead 
raise a new exception with a better error message and chain it with the 
previous exception.

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


Added file: http://bugs.python.org/file44798/CPythonTestOutput.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


--
keywords: +patch
Added file: http://bugs.python.org/file44796/issue28261_ver1.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

Changes by Oren Milman :


Added file: http://bugs.python.org/file44797/patchedCPythonTestOutput_ver1.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28261] wrong error messages when using PyArg_ParseTuple to parse normal tuples

[Oren Milman]

New submission from Oren Milman:

 current state 
In few places in the codebase, PyArg_ParseTuple is called in order to parse a 
normal tuple (instead of the usual case of parsing the arguments tuple of a 
function).
In some of these places, failure of PyArg_ParseTuple is handled simply by 
cleanup (if needed) and returning an error code, and so an exception with the 
generic error message is raised.
The generic error message is appropriate in case the parsed tuple is the 
arguments tuple of a function, but might be really wrong in case it is a normal 
tuple.

For example, such case in Modules/socketmodule.c in socket_getnameinfo leads to 
the following:
>>> import socket
>>> socket.getnameinfo(tuple(), 1)
Traceback (most recent call last):
  File "", line 1, in 
TypeError: function takes at least 2 arguments (0 given)
>>>


 proposed changes 
In each of these places, add to the format string (which is passed to 
PyArg_ParseTuple) a ';', followed by an appropriate error message, as already 
done in Modules/audioop.c in audioop_ratecv_impl:
if (!PyArg_ParseTuple(state,
"iO!;audioop.ratecv: illegal state argument",
, _Type, ))
goto exit;


 diff 
The proposed patches diff file is attached.


 tests 
I wrote an ugly script to verify the wrong error messages on CPython without my 
patches, and to test the patches on CPython with my patches. The script is 
attached (but it might fail on a non-Windows machine).

In addition, I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with 
and without the patches, and got quite the same output.
The outputs of both runs are attached.

--
components: IO
files: testBugsOrPatches.py
messages: 277296
nosy: Oren Milman
priority: normal
severity: normal
status: open
title: wrong error messages when using PyArg_ParseTuple to parse normal tuples
type: behavior
versions: Python 3.7
Added file: http://bugs.python.org/file44795/testBugsOrPatches.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com