[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 0641ada9b78b6a06f539d4bde42c1ad1b90579a7 by Serhiy Storchaka 
(Michael Seifert) in branch '3.5':
bpo-29800: Fix crashes in partial.__repr__ if the keys of partial.keywords are 
not strings (#649) (#672)
https://github.com/python/cpython/commit/0641ada9b78b6a06f539d4bde42c1ad1b90579a7


--

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 6c3d5274687c97f9c13800ad50e73e15b54f629d by Serhiy Storchaka 
(Michael Seifert) in branch 'master':
bpo-29800: Fix crashes in partial.__repr__ if the keys of partial.keywords are 
not strings (#649)
https://github.com/python/cpython/commit/6c3d5274687c97f9c13800ad50e73e15b54f629d


--

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:


New changeset 53b2667dcf2a7d13af466a5fb91844f5125a920d by Serhiy Storchaka 
(Michael Seifert) in branch '3.6':
bpo-29800: Fix crashes in partial.__repr__ if the keys of partial.keywords are 
not strings (#649) (#671)
https://github.com/python/cpython/commit/53b2667dcf2a7d13af466a5fb91844f5125a920d


--

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Larry Hastings]

Changes by Larry Hastings :


--
pull_requests: +623

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your contribution Michael!

--
resolution:  -> fixed
stage: needs patch -> 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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Michael Seifert]

Changes by Michael Seifert :


--
pull_requests: +555

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Michael Seifert]

Changes by Michael Seifert :


--
pull_requests: +554

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Could you create PRs for 3.6 and 3.5 Michael?

--

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It would be nice if you provided the solution with %S. But if you don't do 
this, and nobody other provide other pull request, I'll merge the current 
solution.

When the dictionary keys are changed during iteration PyDict_Next() can skip 
some key-value pairs or return some key-value pairs multiple times. It never 
raises an exception, crashes or hangs in infinite loop. This is appropriate.

The problem is that after calling __str__ for the key, the borrowed reference 
to the value can become invalid, and __repr__ will be called for destroyed 
object. This can cause an undefined behavior, in particular a crash.

--
stage:  -> needs patch

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Michael Seifert]

Michael Seifert added the comment:

Given that this my first contribution to CPython I'm not too sure about the 
etiquette. When do I know (or who decides) when an agreement on a fix is 
reached? I wouldn't mind retracting the pull request if someone else wants to 
fix it differently.

I actually used the PyUnicode_Check on purpose because I always viewed the 
representation should be either unreadably (like objectxyz at 0xwhatever) or 
something that can should be copy Returning something that will 
definetly throw an exception when copied seemed inappropriate.

But the %S change definetly has it's attraction: shorter, also fixes the 
segfault and no need for type checking.

Given Serhiy's answer it seems to me there could be another lurking problem 
because any %R or %S is potentially a problem within PyDict_Next - because this 
C-API-Function states that "The dictionary p should not be mutated during 
iteration. It is safe to modify the values of the keys as you iterate over the 
dictionary, but only so long as the set of keys does not change." But arbitary 
__str__ or __repr__ functions could do just that. I'm mostly an end-user so I'm 
not sure if I understand that right?

--

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Łukasz Langa]

Łukasz Langa added the comment:

I'd also prefer %S instead of a hard-coded check.

--
nosy: +lukasz.langa

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

PyUnicode_FromFormat() crashes in debug build if the argument for %U is not a 
unicode string.

I thought about using %S. This would correspond possible Python implementation 
(`'%s=%r' % (key, value)`). But in that case we should increase refcounts of 
key and value (and maybe even pto->kw) since custom __str__ of the key can 
remove the value from the dict and make the value variable the hanging 
reference.

I would prefer never failing __repr__, but other core developers have other 
opinion. Currently partial.__repr__ raises an exception on deep recursion and 
when the repr of any argument raises an exception.

Current Michael's patch LGTM, but if his want to suggest the solution with %S 
it would LGTM too.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Josh Rosenberg]

Josh Rosenberg added the comment:

I feel like adding a type check to partial_repr might not be the correct fix 
here. If PyUnicode_FromFormat returned NULL and set an exception here, then the 
existing code would work as written (raising an exception, but not segfaulting).

Alternatively, if the code in question used %S for the key instead of %U, it 
would also work as written (not raising an exception or segfaulting).

It's extremely strange to have something accepted, then raise exceptions in the 
repr of all places, and adding extra special purpose code for that specific 
purpose seems odd, to say the least.

I think I'd be in favor of using %S personally, since %U should only be used 
when you have absolute guarantees that the object is a Unicode object, which we 
can't give here. Sure, an invalid state passes without notice in the repr, but 
I'm not sure that bothers me so much; if they actually try to call the invalid 
partial they'll get the TypeError they deserve at a time when the type finally 
matters.

--
nosy: +josh.r

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Michael Seifert]

Changes by Michael Seifert :


--
pull_requests: +536

___
Python tracker 

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

[issue29800] functools.partial segfaults in repr when keywords attribute is abused

[Michael Seifert]

New submission from Michael Seifert:

It's possible to create a segfault when one (inappropriatly) changes the 
functools.partial.keywords attribute manually. A minimal example reproducing 
the segfault is:

>>> from functools import partial
>>> p = partial(int)
>>> p.keywords[1] = 10
>>> repr(p)

System: Windows 10
Python: 3.5.3, 3.6.0, master-branch

--
components: Library (Lib)
messages: 289510
nosy: MSeifert
priority: normal
severity: normal
status: open
title: functools.partial segfaults in repr when keywords attribute is abused
type: crash
versions: Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

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