classification
Title: Infinite recursion in curses.textpad.Textbox
Type: behavior Stage: resolved
Components: Library (Lib) 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: berker.peksag, brian.curtin, python-dev, rxcomm, serhiy.storchaka, tycho, vstinner
Priority: normal Keywords: patch

Created on 2011-09-28 00:47 by tycho, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
textpad_resize.patch tycho, 2011-09-28 00:47
textpad-recursion-fix.patch tycho, 2011-12-20 17:34 review
textpad-recursion-fix2.patch serhiy.storchaka, 2016-12-17 19:20 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (9)
msg144559 - (view) Author: Tycho Andersen (tycho) Date: 2011-09-28 00:47
The attached patch fixes two bugs which manifest as infinite recursion in _insert_printable_char() of Textbox. First, the previous implementation of _insert_printable_char() used recursion to move characters when inserting a character. Thus, any Textpad which had an area greater than the interpreter's maximum recursion limit would crash. A minimal test case is the following:

#!/usr/bin/python
import curses
from curses.textpad import Textbox

def main(stdscr):
    box = Textbox(stdscr, insert_mode=True)
    box.stripspaces = True
    while 1:
        cmd = box.edit()
        if cmd == 'q':
            break

curses.wrapper(main)

Run that script in a terminal with area (i.e. $LINES * $COLUMNS) > 1000 (the default max recursion limit), press any key and be greeted by a stack trace. The patch changes the implementation of _insert_printable_char() to be iterative, thus avoiding the infinite recursion.

Second, when the underlying curses window was resized to be smaller than it was when the Textpad was created, pressing any key would result in infinite recursion (or with the new method, an infinite loop). The patch also changes Textpad so that instead of keeping the underlying window's size as instance attributes of this Textpad, Textpad asks the underlying window its size every time Textpad needs to know, allowing the underlying window to be resized at will.

I've verified this bug is in 2.7.1 and 3.2. Let me know if you need anything else.
msg149892 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2011-12-19 21:35
Would you be able to produce a unit test which fails before your patch is applied, but succeeds after applying your changes? That'll make your changes more likely to get accepted.
msg149920 - (view) Author: Tycho Andersen (tycho) Date: 2011-12-20 17:34
Attached is a patch which contains a testcase as well. A few notes about this testcase:

1. I couldn't figure out how to get it to run correctly after all the other tests had run, so I had to run it first. This seems lame. One possible fix is to run each testcase in curses.wrapper; I'd be happy to change this patch to do that if it's more acceptable.

2. This testcase only tests one of the two bugs this patch fixes. The other seems much harder to write a testcase for, since you have to have a terminal such that curses.LINES * curses.COLUMS > sys.getrecursionlimit(). If there's a good way to guarantee this, I'd be happy to write a testcase for it.

Comments are appreciated!
msg151947 - (view) Author: Tycho Andersen (tycho) Date: 2012-01-25 14:28
Hi, any movement on this?
msg283397 - (view) Author: David Andersen (rxcomm) Date: 2016-12-16 12:00
Any progress on this? Its Dec 2016 and this bug is still around. There's a patch and a patch with tests. The problem is well-understood. I'm not sure what the holdup is, but it would be great to get fixed!
msg283458 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-12-17 00:45
The patch doesn't apply cleanly anymore so the next step would be to provide an updated patch.

By the way, I can reproduce the first bug, but not the second one. If the second bug can't be reproducible anymore, we might need a simpler patch.
msg283506 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-17 19:20
I can reproduce the second bug too.

Here is updated patch. The main difference is that it preserves maxx and maxy attributes and support them up to date.
msg284163 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-28 08:23
New changeset b446a4aab9cf by Serhiy Storchaka in branch '3.5':
Issue #13051: Fixed recursion errors in large or resized curses.textpad.Textbox.
https://hg.python.org/cpython/rev/b446a4aab9cf

New changeset d87771d1c1e6 by Serhiy Storchaka in branch '2.7':
Issue #13051: Fixed recursion errors in large or resized curses.textpad.Textbox.
https://hg.python.org/cpython/rev/d87771d1c1e6

New changeset ea87e00a3e89 by Serhiy Storchaka in branch '3.6':
Issue #13051: Fixed recursion errors in large or resized curses.textpad.Textbox.
https://hg.python.org/cpython/rev/ea87e00a3e89

New changeset ea7f22cf9c8c by Serhiy Storchaka in branch 'default':
Issue #13051: Fixed recursion errors in large or resized curses.textpad.Textbox.
https://hg.python.org/cpython/rev/ea7f22cf9c8c
msg284164 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-28 08:24
Thank you for your contribution Tycho.
History
Date User Action Args
2017-03-31 16:36:21dstufftsetpull_requests: + pull_request947
2016-12-28 08:24:41serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg284164

stage: patch review -> resolved
2016-12-28 08:23:42python-devsetnosy: + python-dev
messages: + msg284163
2016-12-28 08:10:08serhiy.storchakasetassignee: serhiy.storchaka
2016-12-17 19:20:21serhiy.storchakasetfiles: + textpad-recursion-fix2.patch

messages: + msg283506
2016-12-17 06:41:52serhiy.storchakasetnosy: + serhiy.storchaka
2016-12-17 00:45:20berker.peksagsetversions: + Python 3.5, Python 3.6, Python 3.7, - Python 3.2
nosy: + berker.peksag

messages: + msg283458

type: crash -> behavior
2016-12-16 12:00:14rxcommsetnosy: + rxcomm
messages: + msg283397
2015-02-02 01:39:57berker.peksaglinkissue23373 superseder
2012-01-25 14:28:02tychosetmessages: + msg151947
2011-12-20 17:34:24tychosetfiles: + textpad-recursion-fix.patch

messages: + msg149920
2011-12-19 21:35:34brian.curtinsetnosy: + brian.curtin
messages: + msg149892
2011-12-19 19:25:11ned.deilysetnosy: + vstinner

stage: patch review
2011-09-28 00:47:10tychocreate