gh-134209: use heap-allocated memory in `_curses.window.{instr,getstr}` by zydtiger · Pull Request #134283 · python/cpython

zydtiger · 2025-05-19T20:56:07Z

Allocated curses buffer on heap and modified documentation to show this change.

Issue: Possible loss of large text data in _curses.window.{instr,getstr} #134209

📚 Documentation preview 📚: https://cpython-previews--134283.org.readthedocs.build/

python-cla-bot · 2025-05-19T20:56:10Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2025-05-19T20:56:13Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Modules/_cursesmodule.c

Doc/library/curses.rst

Misc/NEWS.d/next/Library/2025-05-19-20-59-06.gh-issue-134209.anhTcF.rst

Modules/_cursesmodule.c

picnixz · 2025-05-20T08:39:40Z

Ha. As I feared:

Traceback (most recent call last):
  File "/Users/runner/work/cpython/cpython/Lib/test/test_curses.py", line 379, in test_getstr
    self.assertEqual(win.getstr(3, 1, 2), b'Lo')
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: b'' != b'Lo'

I suspect that the value being returned is not reliable for some reason. Unfortunately, we'll need to use strlen I think. That's one thing I hate with libcurses, namely most of the return values semantics are implementation-defined -_-

add version changed message Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

…nhTcF.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz · 2025-05-20T15:32:07Z

By the way, no need to hit the "update branch" button if the CI is green. Usually it's only to pull newest (infra-related) changes and/or patches for tests that were already failing.

picnixz

Final nitpick. Since I'll be re-changing the logic a bit in #134325, I think this one is good. After the commit, I'll run the build bots.

Modules/_cursesmodule.c

bedevere-bot · 2025-05-20T15:46:33Z

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 6510d09 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134283%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot · 2025-05-20T15:46:33Z

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 6510d09 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134283%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

picnixz · 2025-05-20T15:47:09Z

Now, do NOT commit anything until all build bots are done (so don't hit "update branch") otherwise the report will be lost on GH.

Thanks for your first contribution!

zydtiger · 2025-05-20T15:48:05Z

Thank you so much to guide me through the process! Glad that I'm able to contribute.

miss-islington-app · 2025-05-20T20:36:08Z

Thanks @zydtiger for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

…getstr}` (pythonGH-134283) * made curses buffer heap allocated instead of stack * change docs to explicitly mention the max buffer size * changing GetStr() function to behave similarly too * Update Doc/library/curses.rst * Update instr with proper return error handling * Update Modules/_cursesmodule.c * change to strlen and better memory safety * change from const int to Py_ssize_t * add mem allocation guard * update versionchanged to mention it was an increase. * explicitly use versionchanged 3.14 as that is its own branch now. TESTED: `python -m test -u curses test_curses` --------- (cherry picked from commit aadda87) Co-authored-by: tigerding <43339228+zydtiger@users.noreply.github.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

bedevere-app · 2025-05-20T20:36:19Z

GH-134391 is a backport of this pull request to the 3.14 branch.

…,getstr}` (GH-134283) (#134391) gh-134209: use heap-allocated memory in `_curses.window.{instr,getstr}` (GH-134283) * made curses buffer heap allocated instead of stack * change docs to explicitly mention the max buffer size * changing GetStr() function to behave similarly too * Update Doc/library/curses.rst * Update instr with proper return error handling * Update Modules/_cursesmodule.c * change to strlen and better memory safety * change from const int to Py_ssize_t * add mem allocation guard * update versionchanged to mention it was an increase. * explicitly use versionchanged 3.14 as that is its own branch now. TESTED: `python -m test -u curses test_curses` --------- (cherry picked from commit aadda87) Co-authored-by: tigerding <43339228+zydtiger@users.noreply.github.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz · 2025-05-22T12:26:32Z

Oh so you treated it as a bug fix for 3.14/3.15 only? I'm ok although I would have considered this a feature but since it's scope is really small, it's fine IMO.

…getstr}` (pythonGH-134283) * made curses buffer heap allocated instead of stack * change docs to explicitly mention the max buffer size * changing GetStr() function to behave similarly too * Update Doc/library/curses.rst * Update instr with proper return error handling * Update Modules/_cursesmodule.c * change to strlen and better memory safety * change from const int to Py_ssize_t * add mem allocation guard * update versionchanged to mention it was an increase. * explicitly use versionchanged 3.14 as that is its own branch now. TESTED: `python -m test -u curses test_curses` --------- Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

zydtiger added 3 commits May 19, 2025 16:43

made curses buffer heap allocated instead of stack

46a6f47

change docs to explicitly mention the max buffer size

7eeb8f6

remove unintentional changes

d5aa3db

bedevere-app bot mentioned this pull request May 19, 2025

Possible loss of large text data in _curses.window.{instr,getstr} #134209

Closed

bedevere-app bot added the awaiting review label May 19, 2025

blurb-it bot and others added 3 commits May 19, 2025 20:59

📜🤖 Added by blurb_it.

020d798

reword news to satify linter

823fa38

Merge branch 'main' into fix-issue-134209

571091d

gpshead added the sprint label May 19, 2025

gpshead self-assigned this May 19, 2025

Merge branch 'main' into fix-issue-134209

1e7edf4

picnixz reviewed May 19, 2025

View reviewed changes

Modules/_cursesmodule.c Outdated Show resolved Hide resolved

Modules/_cursesmodule.c Outdated Show resolved Hide resolved

Modules/_cursesmodule.c Outdated Show resolved Hide resolved

Reword the ReST news entry.

0cb5fe0

gpshead reviewed May 20, 2025

View reviewed changes

Modules/_cursesmodule.c Show resolved Hide resolved

zydtiger and others added 4 commits May 19, 2025 22:42

use rtn value from winnstr instead of strlen

63a3ed2

changing GetStr() function to behave similarly too

58d6cde

updated curses doc for getstr function

82c9b1c

Merge branch 'main' into fix-issue-134209

04a9b14

picnixz changed the title ~~gh-134209: Allocating curses buffer on heap~~ gh-134209: use heap-allocated memory in _curses.window.{instr,getstr} May 20, 2025

picnixz reviewed May 20, 2025

View reviewed changes

picnixz mentioned this pull request May 20, 2025

gh-134208: remove dead AC directives for _curses.window.{chgat,getstr,instr} #134325

Merged

zydtiger and others added 6 commits May 20, 2025 10:40

Update Doc/library/curses.rst

cbce07b

add version changed message Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Doc/library/curses.rst

dfd210d

add version changed message Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Misc/NEWS.d/next/Library/2025-05-19-20-59-06.gh-issue-134209.a…

ad832bd

…nhTcF.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Modules/_cursesmodule.c

f7a51f9

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Modules/_cursesmodule.c

e5597a2

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Modules/_cursesmodule.c

5dd4304

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Merge branch 'main' into fix-issue-134209

5bfa810

picnixz approved these changes May 20, 2025

View reviewed changes

Modules/_cursesmodule.c Show resolved Hide resolved

bedevere-app bot added awaiting merge and removed awaiting review labels May 20, 2025

fix indent

6510d09

picnixz added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels May 20, 2025

bedevere-bot removed 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels May 20, 2025

update versionchanged to mention it was an increase.

6eccdc9

gpshead approved these changes May 20, 2025

View reviewed changes

explicitly use versionchanged 3.14 as that is its own branch now.

769bb87

gpshead enabled auto-merge (squash) May 20, 2025 20:14

gpshead added the needs backport to 3.14 bugs and security fixes label May 20, 2025

gpshead merged commit aadda87 into python:main May 20, 2025
41 checks passed

bedevere-app bot removed the awaiting merge label May 20, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 20, 2025

Uh oh!

Conversation

zydtiger commented May 19, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz commented May 20, 2025

Uh oh!

picnixz commented May 20, 2025

Uh oh!

picnixz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-bot commented May 20, 2025

Uh oh!

bedevere-bot commented May 20, 2025

Uh oh!

picnixz commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zydtiger commented May 20, 2025

Uh oh!

Uh oh!

miss-islington-app bot commented May 20, 2025

Uh oh!

bedevere-app bot commented May 20, 2025

Uh oh!

picnixz commented May 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zydtiger commented May 19, 2025 •

edited by github-actions bot

Loading

python-cla-bot bot commented May 19, 2025 •

edited

Loading

picnixz left a comment •

edited

Loading

picnixz commented May 20, 2025 •

edited

Loading