Skip to content

gh-55646: Do not crash IDLE on an invalid key binding - #152747

Merged
terryjreedy merged 5 commits into
python:mainfrom
serhiy-storchaka:gh-55646-invalid-keybinding-crash
Sep 25, 2026
Merged

terryjreedy merged 5 commits into
python:mainfrom
serhiy-storchaka:gh-55646-invalid-keybinding-crash

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Jul 1, 2026 •

Copy link
Copy Markdown
Member

A typo in a custom key binding entered via Advanced Key Binding (which is not validated), such as <Alt-Key-up> instead of <Alt-Key-Up>, crashed IDLE at startup:

  File ".../idlelib/multicall.py", line ..., in bind
    self.__binders[triplet[1]].bind(triplet, func)
  ...
_tkinter.TclError: bad event type or keysym "up"

MultiCall.event_add stores the parsed sequence without validating the keysym, so the TclError is raised later, when a virtual event is bound and MultiCall performs the actual Tk bind. With no window yet, IDLE just exits.

Catch TclError at both binder call sites (MultiCall.bind and MultiCall.event_add) and ignore the invalid binding with a warning naming it. The bind path also drops the bad sequence from the virtual event, which avoids a secondary ValueError if it is later rebound or unbound.

🤖 Generated with Claude Code

A typo in a key binding, such as <Alt-Key-up> for <Alt-Key-Up>, crashed IDLE
at startup.  It is now ignored with a warning.
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jul 1, 2026
serhiy-storchaka and others added 2 commits July 1, 2026 18:07
On macOS an invalid binding such as <Alt-Key-up> is not parsed into a
MultiCall triplet, so it falls back to Tk's event_add(), which was left
unguarded and still crashed.  Guard the fallback too, and test both the
parsed (bind()) and unparsed (event_add()) paths on every platform.

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is an example of making the good the enemy of the adequate. Unexpected crashes/exits are bad and this is definately better than no fix.

The merge conflict was due to this being the 2nd patch to add tests at the end of test_multicall. The local interface had a button to accept both additions.

I also looked at Serwy's patch: https://bugs.python.org/file24351/issue11437.patch.
His concern about repeated processing is mostly moot for most users. We merged all the IDLE feature extensions into the main code about a decade ago, leaving only a test extension.

Main question: Why catch errors in multicall rather than configparser? Does every sequence get run through multicall? Does this catch more errors?

@serhiy-storchaka

Copy link
Copy Markdown
Member Author

Note that "Accept both" eats a blank line between fragments. You have to add it after pressing "Accept both" while you are in the online editor. I fixed this.

Yes, every configured key binding goes through MultiCall. MultiCall is the single choke point where a binding string is actually handed to Tk.

Serwy's patch also works. But it pulls a Tk root plus a modal dialog into config code that also runs headless (tests, the subprocess). Naming the bad key and suggesting a reconfigure is a nice touch, which I've added to the warning.

idlelib is private, so its helpers need no leading underscore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@serhiy-storchaka

Copy link
Copy Markdown
Member Author

@terryjreedy, does my answer above address your question? Every configured key binding does go through MultiCall, so it is the single place where an invalid sequence can be caught, and it also catches sequences that the config code cannot check without a Tk root.

The same kind of problem exists for other config values. An invalid color in a user theme crashes the Settings dialog (#85604). A config file that cannot be parsed at all (a syntax or encoding error) was handled in #152764. Fonts are already tolerant: a bad size falls back to the default with a warning, and Tk substitutes an unknown family.

@terryjreedy
terryjreedy merged commit b954645 into python:main Sep 25, 2026
50 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Sep 25, 2026

Copy link
Copy Markdown

GH-158149 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Sep 25, 2026
@miss-islington-app

Copy link
Copy Markdown

Sorry, @serhiy-storchaka and @terryjreedy, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker b9546455ab0f65af65b0b69e86e7a2f65d287c16 3.13

@bedevere-app

bedevere-app Bot commented Sep 25, 2026

Copy link
Copy Markdown

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

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 25, 2026
@terryjreedy

Copy link
Copy Markdown
Member

Serhiy: Conflict stopped 3.13 backport.

terryjreedy added a commit that referenced this pull request Sep 25, 2026
…) (#158150)

gh-55646: Do not crash IDLE on an invalid key binding (GH-152747)

A typo in a key binding, such as <Alt-Key-up> for <Alt-Key-Up>, crashed IDLE
at startup.  It is now ignored with a warning.

On macOS an invalid binding such as <Alt-Key-up> is not parsed into a
MultiCall triplet, so it falls back to Tk's event_add(), which was left
unguarded and still crashed.  Guard the fallback too, and test both the
parsed (bind()) and unparsed (event_add()) paths on every platform.

---------
(cherry picked from commit b954645)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@bedevere-app

bedevere-app Bot commented Sep 27, 2026

Copy link
Copy Markdown

GH-158265 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Sep 27, 2026
terryjreedy added a commit that referenced this pull request Sep 27, 2026
…) (#158265)

A typo in a key binding, such as <Alt-Key-up> for <Alt-Key-Up>, crashed IDLE
at startup.  It is now ignored with a warning.

On macOS an invalid binding such as <Alt-Key-up> is not parsed into a
MultiCall triplet, so it falls back to Tk's event_add(), which was left
unguarded and still crashed.  Guard the fallback too, and test both the
parsed (bind()) and unparsed (event_add()) paths on every platform.

---------
(cherry picked from commit b954645)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants