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

…dling

Lib/getpass.py

Lib/test/test_getpass.py

Lib/getpass.py

…8577 * 'main' of github.com:python/cpython: pythongh-146197: Run -m test.pythoninfo on the Emscripten CI (python#146332) pythongh-146325: Use `test.support.requires_fork` in test_fastpath_cache_cleared_in_forked_child (python#146330) pythongh-146197: Add Emscripten to CI (python#146198) pythongh-143387: Raise an exception instead of returning None when metadata file is missing. (python#146234) pythongh-108907: ctypes: Document _type_ codes (pythonGH-145837) pythongh-146175: Soft-deprecate outdated macros; convert internal usage (pythonGH-146178) pythongh-146056: Rework ref counting in treebuilder_handle_end() (python#146167) Add a warning about untrusted input to `configparser` docs (python#146276) pythongh-145264: Do not ignore excess Base64 data after the first padded quad (pythonGH-145267) pythongh-146308: Fix error handling issues in _remote_debugging module (python#146309) pythongh-146192: Add base32 support to binascii (pythonGH-146193) pythongh-135953: Properly obtain main thread identifier in Gecko Collector (python#146045) pythongh-143414: Implement unique reference tracking for JIT, optimize unpacking of such tuples (pythonGH-144300) pythongh-146261: Fix bug in `_Py_uop_sym_set_func_version` (pythonGH-146291) pythongh-145144: Add more tests for UserList, UserDict, etc (pythonGH-145145) pythongh-143959: Fix test_datetime if _datetime is unavailable (pythonGH-145248) pythongh-146245: Fix reference and buffer leaks via audit hook in socket module (pythonGH-146248) pythongh-140049: Colorize exception notes in `traceback.py` (python#140051) Update docs for pythongh-146056 (pythonGH-146213)

CuriousLearner · 2026-03-23T18:10:23Z

Thank you for your input @vstinner . I've addressed your reviews. Do you mind taking another pass?

cc @picnixz

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)

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

Open

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

CuriousLearner added 2 commits March 23, 2026 20:39

fix: prevent prompt corruption during getpass echo_char line editing

d280ce9

fix: disable IEXTEN in non-canonical mode to allow Ctrl+V (LNEXT) han…

3f1a861

…dling

vstinner reviewed Mar 23, 2026

View reviewed changes

CuriousLearner added 3 commits March 23, 2026 23:27

refactor: address review on getpass echo_char line editing

537392c

fix: Add comments about ICANON and IEXTEN

e1e4aa3

CuriousLearner requested review from picnixz and vstinner March 23, 2026 18:10

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

		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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CuriousLearner 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!

CuriousLearner commented Nov 15, 2025 •

edited by github-actions bot

Loading

CuriousLearner commented Nov 15, 2025 •

edited

Loading