Repository navigation
gh-94938: Fix errror detection of unexpected keyword arguments - #94999
Merged
serhiy-storchaka merged 1 commit intoJul 28, 2022
Merged
Conversation
When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in linearized arguments array. But since the number of expected initialized cells is determined as total number of passed arguments, this lead to reading NULL as keyword parameter value, that caused SysTemError or crash or other undesired behavior.
serhiy-storchaka
added
type-bug
Contributor
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
Contributor
|
Sorry @serhiy-storchaka, I had trouble checking out the |
Contributor
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
serhiy-storchaka
added
needs backport to 3.11
Contributor
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Contributor
|
Sorry @serhiy-storchaka, I had trouble checking out the |
serhiy-storchaka
added a commit
to serhiy-storchaka/cpython
that referenced
this pull request
Jul 28, 2022
…uments (pythonGH-94999) When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in the linearized arguments array. But since the number of expected initialized cells is determined as the total number of passed arguments, this lead to reading NULL as a keyword parameter value, that caused SystemError or crash or other undesired behavior. (cherry picked from commit ebad53a) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-95353 is a backport of this pull request to the 3.11 branch. |
serhiy-storchaka
added a commit
to serhiy-storchaka/cpython
that referenced
this pull request
Jul 28, 2022
…uments (pythonGH-94999) When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in the linearized arguments array. But since the number of expected initialized cells is determined as the total number of passed arguments, this lead to reading NULL as a keyword parameter value, that caused SystemError or crash or other undesired behavior.. (cherry picked from commit ebad53a) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-95354 is a backport of this pull request to the 3.10 branch. |
serhiy-storchaka
added a commit
that referenced
this pull request
Jul 28, 2022
…GH-94999) (GH-95353) When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in the linearized arguments array. But since the number of expected initialized cells is determined as the total number of passed arguments, this lead to reading NULL as a keyword parameter value, that caused SystemError or crash or other undesired behavior. (cherry picked from commit ebad53a) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka
added a commit
that referenced
this pull request
Jul 28, 2022
…GH-94999) (GH-95354) When keyword argument name is an instance of a str subclass with overloaded methods __eq__ and __hash__, the former code could not find the name of an extraneous keyword argument to report an error, and _PyArg_UnpackKeywords() returned success without setting the corresponding cell in the linearized arguments array. But since the number of expected initialized cells is determined as the total number of passed arguments, this lead to reading NULL as a keyword parameter value, that caused SystemError or crash or other undesired behavior.. (cherry picked from commit ebad53a) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
zware
reviewed
Jul 28, 2022
| def __eq__(self, other): | ||
| return False | ||
| def __hash__(self): | ||
| return str.__hash__(self) |
Member
There was a problem hiding this comment.
This looks a little odd; aren't these latter __eq__ and __hash__ definitions rendering the first ones pointless? Which set was supposed to be in this class?
Member
Author
There was a problem hiding this comment.
Good catch. The first definitions are correct, they cause a crash with non-patched code. The latter definitions do not cause any visible effects, Thank you, Zachary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When keyword argument name is an instance of a str subclass with
overloaded methods
__eq__and__hash__, the former code could not findthe name of an extraneous keyword argument to report an error, and
_PyArg_UnpackKeywords() returned success without setting the
corresponding cell in linearized arguments array. But since the number
of expected initialized cells is determined as total number of passed
arguments, this lead to reading NULL as keyword parameter value, that
caused SystemError or crash or other undesired behavior.