gh-138577: Fix keyboard shortcuts in getpass with echo_char by CuriousLearner · Pull Request #141597 · python/cpython

CuriousLearner · 2025-11-15T18:07:47Z

When using getpass.getpass(echo_char='*'), keyboard shortcuts like Ctrl+U (kill line), Ctrl+W (erase word), and Ctrl+V (literal next) now work correctly by reading the terminal's control character settings and processing them in non-canonical mode.

Tested with:

import getpass

# Test Ctrl+U (clear line)
# Type "wrong", press Ctrl+U, type "correct"
password = getpass.getpass(echo_char="*")
print(f"Result: {password!r}")
# Expected: 'correct'

# Test Ctrl+W (erase word)
# Type "hello world", press Ctrl+W, type "python"
password = getpass.getpass(echo_char="*")
print(f"Result: {password!r}")
# Expected: 'hello python'

# Test Ctrl+V (literal next)
# Type "test", press Ctrl+V, press Ctrl+U, type "more"
password = getpass.getpass(echo_char="*")
print(f"Result: {password!r}")
# Expected: 'test\x15more' (contains literal Ctrl+U)

Issue: getpass's echo_char should not affect keyboard shortcuts #138577

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

When using getpass.getpass(echo_char='*'), keyboard shortcuts like Ctrl+U (kill line), Ctrl+W (erase word), and Ctrl+V (literal next) now work correctly by reading the terminal's control character settings and processing them in non-canonical mode.

picnixz

Thanks for doing this but this looks like a very complicate patch which is what I feared :( Also, I personally would use Ctrl+a/Ctrl+e for jumping at the start/end of what I write and ctrl+k (I think people are more familiar with ctrl+a/ctrl+k rather than ctrl+u for erasing an entire line in general...).

So I'm not really confident in actually changing this. I think deleting the last character that was typed (with DEL) is usually what people expect.

In addition, to make the feature more extendable in the future, I would prefer a tokenizer-based + dispatcher based approach instead (like we currently do for the REPL) because with many ifs like that it becomes hard to follow what's being done. But this would require an refactoring of this module which I don't really know if it's worth.

picnixz · 2025-11-15T18:16:21Z

Lib/getpass.py

+        # Control chars from termios are bytes, convert to str
+        erase_char = term_ctrl_chars['ERASE'].decode('latin-1') if isinstance(term_ctrl_chars['ERASE'], bytes) else term_ctrl_chars['ERASE']
+        kill_char = term_ctrl_chars['KILL'].decode('latin-1') if isinstance(term_ctrl_chars['KILL'], bytes) else term_ctrl_chars['KILL']
+        werase_char = term_ctrl_chars['WERASE'].decode('latin-1') if isinstance(term_ctrl_chars['WERASE'], bytes) else term_ctrl_chars['WERASE']
+        lnext_char = term_ctrl_chars['LNEXT'].decode('latin-1') if isinstance(term_ctrl_chars['LNEXT'], bytes) else term_ctrl_chars['LNEXT']


Those are too long loines and they are unreadable. All of that can be a single function that is given the action to perform and the current capabilities.

picnixz · 2025-11-15T18:16:46Z

Lib/getpass.py

+        erase_char = '\x7f'  # DEL
+        kill_char = '\x15'   # Ctrl+U
+        werase_char = '\x17' # Ctrl+W
+        lnext_char = '\x16'  # Ctrl+V


Ideally, we should have them defined in a global private dict instead to ease maintenance.

picnixz · 2025-11-15T18:17:17Z

Lib/getpass.py

+                    term_ctrl_chars = {
+                        'ERASE': cc[termios.VERASE] if termios.VERASE < len(cc) else b'\x7f',
+                        'KILL': cc[termios.VKILL] if termios.VKILL < len(cc) else b'\x15',
+                        'WERASE': cc[termios.VWERASE] if termios.VWERASE < len(cc) else b'\x17',
+                        'LNEXT': cc[termios.VLNEXT] if termios.VLNEXT < len(cc) else b'\x16',
+                    }


This code must be refactored with a combination of a global dict with the defaults and a function.

bedevere-app · 2025-11-15T18:17:26Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

CuriousLearner · 2025-11-15T21:31:19Z

Also, I personally would use Ctrl+a/Ctrl+e for jumping at the start/end of what I write and ctrl+k (I think people are more familiar with ctrl+a/ctrl+k rather than ctrl+u for erasing an entire line in general...).

Agree on this, since the original ticket asked for Ctrl+U, I would handle all these Ctrl characters.

In addition, to make the feature more extendable in the future, I would prefer a tokenizer-based + dispatcher based approach instead (like we currently do for the REPL) because with many ifs like that it becomes hard to follow what's being done

I'm refactoring the patch to be more extensible in the future, especially as more control characters are added.

CuriousLearner · 2025-11-16T03:36:22Z

It actually grew a lot bigger than I initially expected, but based on your review, I can iterate over this @picnixz

picnixz · 2025-11-16T09:03:08Z

It looks great. The patch itself is small but the tests are long so I think it is fine. Nonetheless I think it would be better to have it in 3.15 rather than in 3.14.1. WDYT? (i need to review it but not today)

Changes were made.

picnixz

Quick review.

Lib/getpass.py

picnixz · 2025-11-16T10:47:28Z

Lib/getpass.py

+
+    def refresh_display(self):
+        """Redraw the entire password line with asterisks."""
+        self.stream.write('\r' + ' ' * (len(self.passwd) + 20) + '\r')


Why are we adding 20 extra characters?

The +20 was an arbitrary buffer and has been removed. The current implementation now uses just len(self.passwd) to clear only the necessary characters:

self.stream.write('\r' + ' ' * len(self.passwd) + '\r')

picnixz · 2025-11-16T10:48:21Z

Lib/getpass.py

+        self.ctrl = {name: _decode_ctrl_char(value)
+                     for name, value in ctrl_chars.items()}


Can't we have the POSIX defaults already decoded?

Refactored this in b8609bd

Lib/getpass.py

CuriousLearner · 2025-11-30T18:42:04Z

Hi @picnixz 👋🏼

Sorry, it took some time to address the review here. I've refactored the patch & I think Python 3.15 would be a better choice for this, though I don't see any issues if it gets into Python 3.14.1 (unless I'm missing something).

Lib/getpass.py

picnixz · 2026-03-01T15:25:36Z

Lib/getpass.py

+        while self.cursor_pos > 0 and self.passwd[self.cursor_pos-1] == ' ':
+            self.cursor_pos -= 1


You can actually skip the trailing spaces as follows:

stripped = self.passwd.rstrip(' ') self.cursor_pos = self.cursor_pos - (len(self.passwd) - len(stripped))

picnixz · 2026-03-01T15:26:01Z

Lib/getpass.py

+        while self.cursor_pos > 0 and self.passwd[self.cursor_pos-1] != ' ':
+            self.cursor_pos -= 1


And here, use str.rfind using the start of the new cursor position.

Lib/getpass.py

* upstream/main: (1475 commits) Docs: replace all `datetime` imports with `import datetime as dt` (python#145640) pythongh-146153: Use `frozendict` in pure python fallback for `curses.has_key` (python#146154) pythongh-138234: clarify returncode behavior for subprocesses created with `shell=True` (python#138536) pythongh-140947: fix contextvars handling for server tasks in asyncio (python#141158) pythonGH-100108: Add async generators best practices section (python#141885) pythonGH-145667: Merge `GET_ITER` and `GET_YIELD_FROM_ITER` (pythonGH-146120) pythongh-146228: Better fork support in cached FastPath (python#146231) pythongh-146227: Fix wrong type in _Py_atomic_load_uint16 in pyatomic_std.h (pythongh-146229) pythongh-145980: Fix copy/paste mistake in binascii.c (python#146230) pythongh-146092: Raise MemoryError on allocation failure in _zoneinfo (python#146165) pythongh-91279: Note `SOURCE_DATE_EPOCH` support in `ZipFile.writestr()` doc (python#139396) pythongh-146196: Fix Undefined Behavior in _PyUnicodeWriter_WriteASCIIString() (python#146201) pythongh-143930: Reject leading dashes in webbrowser URLs pythongh-145916: Soft-deprecate ctypes.util.find_library (pythonGH-145919) pythongh-146205: Check the errno with != 0 in close impls in select module (python#146206) pythongh-146171: Fix nested AttributeError suggestions (python#146188) pythongh-146099: Optimize _GUARD_CODE_VERSION+IP via function version symbols (pythonGH-146101) pythongh-145980: Add support for alternative alphabets in the binascii module (pythonGH-145981) pythongh-145754: Update signature retrieval in unittest.mock to use forwardref annotation format (python#145756) pythongh-145177: Add emscripten run --test, uses test args from config.toml (python#146160) ...

CuriousLearner · 2026-03-22T00:51:58Z

Hey @picnixz 👋🏼

Thanks for the review :) I've addressed all your reviews. Can you please take another pass at this?

picnixz

This looks great but I will try it locally to see whether there are other finalizing touches to do. Thanks for the awesome work though! I should review it by the end of next week.

vstinner · 2026-03-23T12:05:42Z

I tried the reproducer: type "hello world", press Ctrl+W. I get:

******rd: ***********

I expected the output:

Password: *****

_readline_with_echo_char() is not called with prompt argument and _PasswordLineEditor._refresh_display() writes \r without rewriting the prompt.

There are ways to trigger this output error. For example, type "abc", press CTRL+A and then press "x":

****word: ***

I expected the output:

Password: ****

picnixz · 2026-03-23T12:18:08Z

Oh I think the problem is the prompt being counted inside the buffer. We should simply have a single buffer for the password and prepend the prompt everytime we refresh the line instead

Lib/test/test_getpass.py

Lib/getpass.py

Doc/library/getpass.rst

Lib/test/test_getpass.py

vstinner · 2026-03-24T01:15:10Z

Lib/getpass.py

+            '\b': self._handle_erase,                       # Backspace
+        }
+
+    def _refresh_display(self, prev_len=None):


The whole class is private, I don't think that it's useful to mark methods as private as well. Can you remove th underscore ("_") prefix from all methods?

Done, I’ve made the change. Just for my understanding and to follow the preferred style going forward: if a class is already internal/non-public, is it generally considered unnecessary to prefix all its methods with _ as well?

I don't know if it's a general rule, but yes, if a class is private, I consider that it's not needed to mark its methods as private as well.

…on into fix-pythongh-138577 * 'fix-pythongh-138577' of github.com:CuriousLearner/cpython: pythongh-146202: Create tmp_dir in regrtest worker (python#146347) pythongh-144319: obtain SeLockMemoryPrivilege on Windows (python#144928) pythongh-146199: Fix error handling in `code_richcompare` when `PyObject_RichCompareBool` fails (python#146200) pythongh-146197: Include a bit more information in sys._emscripten_info.runtime (python#146346) pythongh-135871: Reload lock internal state while spinning in `PyMutex_LockTimed` (pythongh-146064) pythongh-145719: Add `.efi` file detection in `mimetypes` (python#145720)

…8577 * 'main' of github.com:python/cpython: Remove inactive CODEOWNERS (python#145930) pythongh-140196: Added constructor behavior changes in ast.rst for python 3.13 (pythonGH-140243)

Doc/library/getpass.rst

Lib/test/test_getpass.py

Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner

LGTM.

@picnixz: Do you have to double check the change?

picnixz · 2026-03-24T12:02:08Z

Yes, I'd like to double-check this either this evening or this w-e (I am a bit tight with my daily job's schedule)

picnixz · 2026-03-29T09:21:00Z

I will now review this PR locally and play with the REPL a bit.

picnixz · 2026-03-29T11:57:03Z

I found some errors when handling Ctrl+A/E so I fixed them. I also moved handling Ctrl+C into its own handler and the last commit should have reduced the diff.

picnixz · 2026-03-29T12:07:34Z

As I have other tasks I want to address with CPython, I didn't add more tests but could you extend the tests to test the cursor position after a Ctrl+A/Ctrl+E as well please? I wasn't able to find other bugs.

FTR, my readline terminal is not capable of understanding control characters so maybe it could make sense to support echo_char='' for no-feedback but with term capabilities? (in contrast to echo_char=None which falls back to readline). This could be a follow-up though (for now echo_char='' is forbidden)

picnixz · 2026-03-29T12:04:31Z

Lib/getpass.py

-            elif char == self.ctrl['EOF']:
+            # Handle EOF now as Ctrl+D must be pressed twice
+            # consecutively to stop reading from the input.
+            elif self.is_eof(char):


For posterity, but this state transition was really the most annoying. I collasped the if but it's not equivalent!

The flow is:

EOF (first time) -> do nothing but record EOF pressed

EOF (second time) -> stop!

If I do something else:

EOF (first time) -> do nothing but record EOF pressed

Any other character that is not EOF/EOL (including Ctrl+V) -> fall through and set eof_pressed = False before reading the next character.

I considered having the state acceptance being in a function but I can't just encode three transitions ("break loop", "pass to handlers", "fall through") with just booleans so I decided against.

vstinner · 2026-03-30T09:11:40Z

Merged, thanks for your contribution.

bedevere-bot · 2026-03-30T09:12:24Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x (tier-3) has failed when building commit 7f0c4f6.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/6194) and take a look at the build logs.
Check if the failure is related to this commit (7f0c4f6) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/6194

Summary of the results of the build (if available):

Click to see traceback logs

remote: Enumerating objects: 14, done.        
remote: Counting objects:   8% (1/12)        
remote: Counting objects:  16% (2/12)        
remote: Counting objects:  25% (3/12)        
remote: Counting objects:  33% (4/12)        
remote: Counting objects:  41% (5/12)        
remote: Counting objects:  50% (6/12)        
remote: Counting objects:  58% (7/12)        
remote: Counting objects:  66% (8/12)        
remote: Counting objects:  75% (9/12)        
remote: Counting objects:  83% (10/12)        
remote: Counting objects:  91% (11/12)        
remote: Counting objects: 100% (12/12)        
remote: Counting objects: 100% (12/12), done.        
remote: Compressing objects:   9% (1/11)        
remote: Compressing objects:  18% (2/11)        
remote: Compressing objects:  27% (3/11)        
remote: Compressing objects:  36% (4/11)        
remote: Compressing objects:  45% (5/11)        
remote: Compressing objects:  54% (6/11)        
remote: Compressing objects:  63% (7/11)        
remote: Compressing objects:  72% (8/11)        
remote: Compressing objects:  81% (9/11)        
remote: Compressing objects:  90% (10/11)        
remote: Compressing objects: 100% (11/11)        
remote: Compressing objects: 100% (11/11), done.        
remote: Total 14 (delta 1), reused 1 (delta 1), pack-reused 2 (from 2)        
From https://github.com/python/cpython
 * branch                    main       -> FETCH_HEAD
Note: switching to '7f0c4f6a0b1d9a56fb8b915dcd0acd598a9d25e6'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 7f0c4f6a0b1 gh-138577: Fix keyboard shortcuts in getpass with echo_char (#141597)
Switched to and reset branch 'main'

Traceback (most recent call last):
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/buildbot/buildarea/3.x.rkm-arm64-ios-simulator.iOS-simulator.arm64/build/Platforms/Apple/__main__.py", line 57, in <module>
    ArgsT = Sequence[str | Path]
TypeError: unsupported operand type(s) for |: 'type' and 'type'

Traceback (most recent call last):
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/buildbot/buildarea/3.x.rkm-arm64-ios-simulator.iOS-simulator.arm64/build/Platforms/Apple/__main__.py", line 57, in <module>
    ArgsT = Sequence[str | Path]
TypeError: unsupported operand type(s) for |: 'type' and 'type'

vstinner · 2026-03-30T09:16:56Z

IMO this change is too big to be backported to 3.14. Also, I see it as a new feature and we don't backport features. So 3.14 has to keep its behavior. The echo_char parameter was added to Python 3.14. The workaround for 3.14 is to not use echo_char parameter.

picnixz · 2026-03-30T09:34:32Z

Yes, I documented the limitations already so I don't think we need to backport. I don't want to break people relying on this. In addition, it's a bit niche but there were complaints so... I don't mind adding the support as a new feature.

mhsmith · 2026-03-30T13:18:24Z

The Python version on the iOS buildbot should now be corrected.

bedevere-app bot added the awaiting review label Nov 15, 2025

bedevere-app bot mentioned this pull request Nov 15, 2025

getpass's echo_char should not affect keyboard shortcuts #138577

Closed

picnixz self-requested a review November 15, 2025 18:08

CuriousLearner requested a review from vstinner November 15, 2025 18:11

picnixz previously requested changes Nov 15, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting review labels Nov 15, 2025

Address reviews on dispatcher pattern + handle other ctrl chars

31e35e4

CuriousLearner added awaiting core review and removed awaiting changes labels Nov 16, 2025

bedevere-app bot removed the awaiting core review label Nov 16, 2025

picnixz self-requested a review November 16, 2025 10:26

bedevere-app bot added the awaiting review label Nov 16, 2025

picnixz reviewed Nov 16, 2025

View reviewed changes

Address reviews

b8609bd

CuriousLearner requested a review from picnixz December 6, 2025 22:00

picnixz reviewed Mar 1, 2026

View reviewed changes

CuriousLearner added 2 commits March 22, 2026 05:25

Address reviews

6a59f3b

picnixz reviewed Mar 22, 2026

View reviewed changes

vstinner reviewed Mar 23, 2026

View reviewed changes

Lib/test/test_getpass.py Outdated Show resolved Hide resolved

Lib/getpass.py Show resolved Hide resolved

Doc/library/getpass.rst Outdated Show resolved Hide resolved

Lib/test/test_getpass.py Outdated Show resolved Hide resolved

Lib/test/test_getpass.py Outdated Show resolved Hide resolved

CuriousLearner and others added 2 commits March 24, 2026 05:52

Address reviews

90da605

Merge branch 'main' into fix-pythongh-138577

ef1efcb

vstinner reviewed Mar 24, 2026

View reviewed changes

CuriousLearner added 3 commits March 24, 2026 07:41

Remove prefix from private class methods

a7c1de3

Merge branch 'main' of github.com:python/cpython into fix-pythongh-13…

e1461a7

…8577 * 'main' of github.com:python/cpython: Remove inactive CODEOWNERS (python#145930) pythongh-140196: Added constructor behavior changes in ast.rst for python 3.13 (pythonGH-140243)

vstinner reviewed Mar 24, 2026

View reviewed changes

Doc/library/getpass.rst Outdated Show resolved Hide resolved

Lib/test/test_getpass.py Show resolved Hide resolved

Apply suggestions from code review

741a817

Co-authored-by: Victor Stinner <vstinner@python.org>

vstinner approved these changes Mar 24, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Mar 24, 2026

picnixz added 2 commits March 29, 2026 12:21

chore! update some documentation

cc5dc99

fix! refresh screen on Ctrl+A/Ctrl-E

0f5f5c8

picnixz added 2 commits March 29, 2026 14:00

refactor! use more handlers

9d90dfa

chore! reduce diff against main

3b2ae38

picnixz force-pushed the fix-gh-138577 branch from 7dde9ed to 3b2ae38 Compare March 29, 2026 12:00

picnixz approved these changes Mar 29, 2026

View reviewed changes

test: add cursor position tests for Ctrl+A/Ctrl+E in getpass echo_char

919af5c

vstinner merged commit 7f0c4f6 into python:main Mar 30, 2026
48 checks passed

bedevere-app bot removed the awaiting merge label Mar 30, 2026

		self.ctrl = {name: _decode_ctrl_char(value)
		for name, value in ctrl_chars.items()}

		while self.cursor_pos > 0 and self.passwd[self.cursor_pos-1] == ' ':
		self.cursor_pos -= 1

Uh oh!

Conversation

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

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Nov 15, 2025

Uh oh!

CuriousLearner commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CuriousLearner commented Nov 16, 2025

Uh oh!

picnixz commented Nov 16, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CuriousLearner commented Nov 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CuriousLearner commented Mar 22, 2026

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Mar 23, 2026

Uh oh!

picnixz commented Mar 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz commented Mar 24, 2026

Uh oh!

picnixz commented Mar 29, 2026

Uh oh!

picnixz commented Mar 29, 2026

CuriousLearner commented Nov 15, 2025 •

edited by github-actions bot

Loading

CuriousLearner commented Nov 15, 2025 •

edited

Loading

picnixz commented Mar 30, 2026 •

edited

Loading