This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Some repr implementations don't check for self-referential structures
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: abacabadabacaba, benjamin.peterson, berker.peksag, pitrou, python-dev, rhettinger, scoder, serhiy.storchaka, skrah, stutzbach
Priority: high Keywords: patch

Created on 2015-10-21 19:17 by abacabadabacaba, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
partial_recursive_repr.patch serhiy.storchaka, 2015-11-03 08:48 review
io_recursive_repr.patch serhiy.storchaka, 2015-11-07 12:29 review
etree_recursive_repr.patch serhiy.storchaka, 2015-11-07 12:29 review
io_recursive_repr2.patch serhiy.storchaka, 2016-06-12 12:07 review
Pull Requests
URL Status Linked Edit
PR 514 closed serhiy.storchaka, 2017-03-06 12:19
PR 722 merged serhiy.storchaka, 2017-03-19 17:30
PR 727 merged serhiy.storchaka, 2017-03-19 21:59
Messages (26)
msg253309 - (view) Author: Evgeny Kapun (abacabadabacaba) Date: 2015-10-21 19:17
Implementations of repr for some of the types in the standard library doesn't check for self-referential structures. As a result, when calling repr() on such objects, Python crashes due to infinite recursion.

Example:
>>> import functools
>>> x = functools.partial(min)
>>> x.__setstate__((x, (), {}, {}))
>>> repr(x)
Segmentation fault

Another example:
>>> import xml.etree.ElementTree
>>> x = xml.etree.ElementTree.Element('')
>>> x.tag = x
>>> repr(x)
Segmentation fault

One more example:
>>> import io
>>> class X(io.TextIOWrapper): __slots__ = 'name'
>>> x = X(open('/dev/null'))
>>> x.name = x
>>> repr(x)
Segmentation fault
msg253872 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-01 20:05
The general solution is to make PyObject_Repr to detect recursive calls (as reprlib.recursive_repr does).

The straightforward way is to use thread local identity set. It can be implemented as a dict that maps id(obj) -> obj (creates an int object for key for every call, requires about 40-80 bytes for recurse level), or as specialized hash table (see Modules/hashtable.c) (faster, requires about 12-24 bytes for recurse level).

The fastest solution would be to set special flag inside proceeded object. For now general object has no place for such flag, but we can add it to GC head. On 64-bit this would increase the size of GC head from 24 to 32 bytes, on 32-bit there is a free place in 16-bytes GC head.

However the performance can be not critical here, because in any case repr() creates new object (resulting string). Using thread local hash table can be enough. In any case the patch will be enough complex to target it 3.6 only.
msg253965 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-03 06:02
Changing PyObject_Repr is too course; it affects a broad class of objects other than containers, and it risks unknown impacts to larges swaths of third-party code use this venerable API.  It is also a break with the long established history of recursion detection being a responsibility of the individual types (i.e. the code in sets, lists, dicts, etc.)

The three cases listed here should be fixed individually.
msg253969 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-03 07:47
Yet one example:

>>> import io
>>> class BR(io.BufferedReader):
...     @property
...     def name(self):
...         return self
... 
>>> repr(BR(io.BytesIO()))
Segmentation fault

The same is for other file-like objects.
msg253972 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-03 08:48
Recursive partial objects are legitimate. Here is a patch that makes partial's repr to support recursive partial objects. Also added a test for pickling.

Cases for Element and file-like objects are questionable. Recursive Element.tag and TextIOWrapper.name don't make a sense, and I don't think we should special support (and encourage) these cases. To avoid stack overflow we can add a restriction for tag to be str or None, but file's name attribute can be dynamic. We can omit name from repr if it is not None, str, bytes or int.
msg253973 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-03 09:00
Added ElementTree and io modules experts to the nosy list.
msg254267 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-07 12:29
Here are patches for io classes and for ElementTree.
msg254268 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-07 12:50
There is also a crash with Python implementation of TextIOWrapper.

>>> import _pyio as io
>>> t = io.TextIOWrapper(io.BytesIO())
>>> t.mode = t
>>> t
Fatal Python error: Cannot recover from stack overflow.

Current thread 0xb74a9700 (most recent call first):
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1982 in __repr__
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1992 in __repr__
[...]
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1992 in __repr__
  File "/home/serhiy/py/cpython/Lib/_pyio.py", line 1992 in __repr__
  ...
Aborted (core dumped)
msg255095 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-22 11:40
Raymond, could you please make a review of the first patch?
msg255110 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-22 19:47
>Raymond, could you please make a review of the first patch?
Will do.

Also, we should ask Antoine Pitrou to look at the TextIO patch and ask Stephan Krah to look at the ElementTree patch.
msg255174 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2015-11-23 15:13
I think you may have meant Eli Bendersky -- I'm not an elementree
expert (Eli, I'm adding you back just to clear this up).
msg255182 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2015-11-23 16:28
As I've mentioned elsewhere, I'll have to temporarily take myself off these issues as I don't have the time to work on them (even review patches). I think Raymond may have gotten his Stefans mixed up and meant Stefan Behnel, who's also been looking at etree patches.
msg267784 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-08 05:13
Ping.
msg268328 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2016-06-12 05:33
Etree patch looks straight forward to me, feel free to apply it.
msg268335 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 06:49
New changeset e44bd1259bda by Serhiy Storchaka in branch '3.5':
Issue #25455: Fixed a crash in repr of ElementTree.Element with recursive tag.
https://hg.python.org/cpython/rev/e44bd1259bda

New changeset e3671a684ea0 by Serhiy Storchaka in branch 'default':
Issue #25455: Fixed a crash in repr of ElementTree.Element with recursive tag.
https://hg.python.org/cpython/rev/e3671a684ea0
msg268349 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 08:57
New changeset c071da010053 by Serhiy Storchaka in branch '2.7':
Issue #25455: Fixed a crash in repr of cElementTree.Element with recursive tag.
https://hg.python.org/cpython/rev/c071da010053

New changeset 17e78918f608 by Serhiy Storchaka in branch '3.5':
Issue #25455: Fixed a crash in repr of recursive functools.partial objects.
https://hg.python.org/cpython/rev/17e78918f608

New changeset 86959c696ab7 by Serhiy Storchaka in branch 'default':
Issue #25455: Fixed a crash in repr of recursive functools.partial objects.
https://hg.python.org/cpython/rev/86959c696ab7
msg268365 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-12 11:40
I'm not sure if 17e78918f608 is relevant but test_gc is failing on several buildbots:

* http://buildbot.python.org/all/builders/PPC64%20Fedora%203.5/builds/716
  http://buildbot.python.org/all/builders/PPC64%20Fedora%203.5/builds/716/steps/test/logs/stdio

* http://buildbot.python.org/all/builders/s390x%20RHEL%203.5/builds/775
  http://buildbot.python.org/all/builders/s390x%20RHEL%203.5/builds/775/steps/test/logs/stdio
msg268367 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-12 12:07
The patch for io classes needed an update.
msg268369 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 12:09
New changeset 7859742826b2 by Serhiy Storchaka in branch '2.7':
Issue #25455: Backported tests for pickling recursive functools.partial objects.
https://hg.python.org/cpython/rev/7859742826b2
msg268371 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-12 12:40
Indeed, tests for recursive partial objects create reference loops and don't clean them. Thank you Berker. I'll fix this.
msg268372 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-12 12:53
New changeset 0323b33894f2 by Serhiy Storchaka in branch '3.5':
Issue #25455: Clean up reference loops created in tests for recursive
https://hg.python.org/cpython/rev/0323b33894f2

New changeset 688edc946ab9 by Serhiy Storchaka in branch '2.7':
Issue #25455: Clean up reference loops created in tests for recursive
https://hg.python.org/cpython/rev/688edc946ab9

New changeset 818a10534e44 by Serhiy Storchaka in branch 'default':
Issue #25455: Clean up reference loops created in tests for recursive
https://hg.python.org/cpython/rev/818a10534e44
msg272730 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-08-15 07:40
Antoine, are you fine with io_recursive_repr2.patch?
msg282295 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-03 19:30
New changeset ea1edf1bf362 by Benjamin Peterson in branch '2.7':
when you enter repr, you must leave, too (#25455)
https://hg.python.org/cpython/rev/ea1edf1bf362
msg290137 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-24 20:19
New changeset 08612ed6a91003cdba7617dee7787c26495c50d9 by Serhiy Storchaka in branch '3.5':
bpo-25455: Fixed crashes in repr of recursive buffered file-like objects. (#514) (#727)
https://github.com/python/cpython/commit/08612ed6a91003cdba7617dee7787c26495c50d9
msg290154 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-24 22:06
New changeset fca705d533970011e50b3f278aab81cead39b00d by Serhiy Storchaka in branch '3.6':
bpo-25455: Fixed crashes in repr of recursive buffered file-like objects. (#514) (#722)
https://github.com/python/cpython/commit/fca705d533970011e50b3f278aab81cead39b00d
msg290161 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-24 22:08
New changeset a5af6e1af77ee0f9294c5776478a9c24d9fbab94 by Serhiy Storchaka in branch 'master':
bpo-25455: Fixed crashes in repr of recursive buffered file-like objects. (#514)
https://github.com/python/cpython/commit/a5af6e1af77ee0f9294c5776478a9c24d9fbab94
History
Date User Action Args
2022-04-11 14:58:23adminsetgithub: 69641
2017-03-24 22:08:12serhiy.storchakasetmessages: + msg290161
2017-03-24 22:06:03serhiy.storchakasetmessages: + msg290154
2017-03-24 20:19:24serhiy.storchakasetmessages: + msg290137
2017-03-20 07:57:32serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-03-19 21:59:45serhiy.storchakasetpull_requests: + pull_request642
2017-03-19 17:30:53serhiy.storchakasetpull_requests: + pull_request637
2017-03-06 12:20:36serhiy.storchakasetversions: + Python 3.7, - Python 3.4
2017-03-06 12:19:44serhiy.storchakasetpull_requests: + pull_request423
2016-12-03 19:30:14python-devsetmessages: + msg282295
2016-08-15 07:40:20serhiy.storchakasetmessages: + msg272730
2016-06-12 12:53:34python-devsetmessages: + msg268372
2016-06-12 12:40:22serhiy.storchakasetmessages: + msg268371
2016-06-12 12:09:27python-devsetmessages: + msg268369
2016-06-12 12:07:48serhiy.storchakasetfiles: + io_recursive_repr2.patch

messages: + msg268367
2016-06-12 11:40:02berker.peksagsetnosy: + berker.peksag
messages: + msg268365
2016-06-12 08:57:41python-devsetmessages: + msg268349
2016-06-12 08:46:58serhiy.storchakasetassignee: serhiy.storchaka
2016-06-12 07:53:57rhettingersetassignee: rhettinger -> (no value)
2016-06-12 06:49:49python-devsetnosy: + python-dev
messages: + msg268335
2016-06-12 05:33:06scodersetmessages: + msg268328
2016-06-08 05:13:47serhiy.storchakasetmessages: + msg267784
2016-01-14 08:03:14serhiy.storchakasetassignee: rhettinger
2015-11-23 16:28:44eli.benderskysetnosy: - eli.bendersky
2015-11-23 16:28:28eli.benderskysetnosy: rhettinger, pitrou, scoder, benjamin.peterson, stutzbach, eli.bendersky, skrah, abacabadabacaba, serhiy.storchaka
messages: + msg255182
2015-11-23 15:13:59skrahsetnosy: + eli.bendersky
messages: + msg255174
2015-11-23 12:33:54eli.benderskysetnosy: - eli.bendersky
2015-11-22 19:47:02rhettingersetnosy: + skrah
messages: + msg255110
2015-11-22 11:40:28serhiy.storchakasetmessages: + msg255095
2015-11-07 12:50:58serhiy.storchakasetmessages: + msg254268
2015-11-07 12:29:41serhiy.storchakasetfiles: + etree_recursive_repr.patch
2015-11-07 12:29:22serhiy.storchakasetfiles: + io_recursive_repr.patch

stage: needs patch -> patch review
messages: + msg254267
versions: + Python 2.7, Python 3.4
2015-11-03 09:00:59serhiy.storchakasetnosy: + pitrou, scoder, benjamin.peterson, stutzbach, eli.bendersky
messages: + msg253973
2015-11-03 08:48:41serhiy.storchakasetfiles: + partial_recursive_repr.patch
keywords: + patch
messages: + msg253972
2015-11-03 07:47:30serhiy.storchakasetmessages: + msg253969
2015-11-03 06:02:21rhettingersetnosy: + rhettinger
messages: + msg253965
2015-11-01 20:05:17serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg253872
2015-10-24 07:25:30rhettingersetpriority: normal -> high
stage: needs patch
2015-10-21 19:17:18abacabadabacabacreate