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.

picnixz · 2025-11-16T10:34:09Z

Lib/getpass.py

+        # Handle literal next mode FIRST (Ctrl+V quotes next char)
+        if editor.literal_next:
+            editor.handle_literal_next(char)
+            continue


To reduce the number of continue, just use if-elif constructions here.

picnixz · 2025-11-16T10:35:25Z

Lib/getpass.py

+        handler = dispatch.get(char)
+        if handler:
+            handler()
        else:
-            passwd += char
-            stream.write(echo_char)
-            stream.flush()
-            eof_pressed = False
-    return passwd
+            editor.insert_char(char)
+            editor.eof_pressed = False


Here's a suggestion: don't build a dispatch table via a function but make the editor handles it internally:

editor.handle(char)

picnixz · 2025-11-16T10:42:29Z

Lib/getpass.py

+                     for name, value in ctrl_chars.items()}
+
+    def refresh_display(self):
+        """Redraw the entire password line with asterisks."""


Suggested change

"""Redraw the entire password line with asterisks."""

"""Redraw the entire password line with *echo_char*."""

picnixz · 2025-11-16T10:45:42Z

Lib/getpass.py

+    try:
+        old = termios.tcgetattr(fd)
+        cc = old[6]  # Index 6 is the control characters array
+        return {
+            'ERASE': cc[termios.VERASE] if termios.VERASE < len(cc) else _POSIX_CTRL_CHARS['ERASE'],
+            'KILL': cc[termios.VKILL] if termios.VKILL < len(cc) else _POSIX_CTRL_CHARS['KILL'],
+            'WERASE': cc[termios.VWERASE] if termios.VWERASE < len(cc) else _POSIX_CTRL_CHARS['WERASE'],
+            'LNEXT': cc[termios.VLNEXT] if termios.VLNEXT < len(cc) else _POSIX_CTRL_CHARS['LNEXT'],
+            'EOF': cc[termios.VEOF] if termios.VEOF < len(cc) else _POSIX_CTRL_CHARS['EOF'],
+            'INTR': cc[termios.VINTR] if termios.VINTR < len(cc) else _POSIX_CTRL_CHARS['INTR'],
+            # Ctrl+A/E/K are not in termios, use POSIX defaults
+            'SOH': _POSIX_CTRL_CHARS['SOH'],
+            'ENQ': _POSIX_CTRL_CHARS['ENQ'],
+            'VT': _POSIX_CTRL_CHARS['VT'],
+        }
+    except (termios.error, OSError):
+        return _POSIX_CTRL_CHARS.copy()


Suggested change

try:

old = termios.tcgetattr(fd)

cc = old[6] # Index 6 is the control characters array

return {

'ERASE': cc[termios.VERASE] if termios.VERASE < len(cc) else _POSIX_CTRL_CHARS['ERASE'],

'KILL': cc[termios.VKILL] if termios.VKILL < len(cc) else _POSIX_CTRL_CHARS['KILL'],

'WERASE': cc[termios.VWERASE] if termios.VWERASE < len(cc) else _POSIX_CTRL_CHARS['WERASE'],

'LNEXT': cc[termios.VLNEXT] if termios.VLNEXT < len(cc) else _POSIX_CTRL_CHARS['LNEXT'],

'EOF': cc[termios.VEOF] if termios.VEOF < len(cc) else _POSIX_CTRL_CHARS['EOF'],

'INTR': cc[termios.VINTR] if termios.VINTR < len(cc) else _POSIX_CTRL_CHARS['INTR'],

# Ctrl+A/E/K are not in termios, use POSIX defaults

'SOH': _POSIX_CTRL_CHARS['SOH'],

'ENQ': _POSIX_CTRL_CHARS['ENQ'],

'VT': _POSIX_CTRL_CHARS['VT'],

}

except (termios.error, OSError):

return _POSIX_CTRL_CHARS.copy()

res = _POSIX_CTRL_CHARS.copy()

try:

old = termios.tcgetattr(fd)

cc = old[6] # Index 6 is the control characters array

except (termios.error, OSError):

return res

# Ctrl+A/E/K are not in termios, use POSIX defaults

for name in ["ERASE", "KILL", "WERASE", "LNEXT", "EOF", "INTR"]

cap = getattr(termios, f"V{name}")

if cap < len(cc):

res[name] = cc[flag]

return res

Thanks for the suggestion, there was some minor issue here, which I've fixed in my refactoring

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

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

Lib/getpass.py

+            'WERASE': cc[termios.VWERASE] if termios.VWERASE < len(cc) else _POSIX_CTRL_CHARS['WERASE'],
+            'LNEXT': cc[termios.VLNEXT] if termios.VLNEXT < len(cc) else _POSIX_CTRL_CHARS['LNEXT'],
+            'EOF': cc[termios.VEOF] if termios.VEOF < len(cc) else _POSIX_CTRL_CHARS['EOF'],
+            'INTR': cc[termios.VINTR] if termios.VINTR < len(cc) else _POSIX_CTRL_CHARS['INTR'],


I'm assuming cc[termios.VINTR] gives bytes right? in this case we can already decode it.

Yes, cc[termios.VINTR] returns bytes. This is now decoded inline in the refactored implementation:

for name in ('ERASE', 'KILL', 'WERASE', 'LNEXT', 'EOF', 'INTR'): cap = getattr(termios, f'V{name}') if cap < len(cc): res[name] = cc[cap].decode('latin-1')

The _POSIX_CTRL_CHARS defaults are also now stored as already-decoded strings.

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).

picnixz · 2026-03-01T15:20:02Z

Lib/getpass.py

+    def _handle_erase(self):
+        """Delete character before cursor (Backspace/DEL)."""
+        if self.cursor_pos > 0:
+            self.passwd = self.passwd[:self.cursor_pos-1] + self.passwd[self.cursor_pos:]


Suggested change

self.passwd = self.passwd[:self.cursor_pos-1] + self.passwd[self.cursor_pos:]

self.passwd = (

self.passwd[:self.cursor_pos-1]

+ self.passwd[self.cursor_pos:]

)

picnixz · 2026-03-01T15:21:16Z

Lib/getpass.py

+
+    def _handle_erase(self):
+        """Delete character before cursor (Backspace/DEL)."""
+        if self.cursor_pos > 0:


Can we swap the ifs (so that we have less indentation)

picnixz · 2026-03-01T15:23:15Z

Lib/getpass.py

+        self.stream.flush()
+
+    def _erase_chars(self, count):
+        """Erase count echo characters from display."""


Suggested change

"""Erase count echo characters from display."""

"""Erase *count* echo characters from display."""

picnixz · 2026-03-01T15:23:29Z

Lib/getpass.py

+        self.stream.write("\b \b" * count)
+
+    def _insert_char(self, char):
+        """Insert character at cursor position."""


Suggested change

"""Insert character at cursor position."""

"""Insert *char* at cursor position."""

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.

picnixz · 2026-03-01T15:27:24Z

Lib/getpass.py

+    def __init__(self, stream, echo_char, ctrl_chars):
+        self.stream = stream
+        self.echo_char = echo_char
+        self.passwd = ""


Since we're manupulating the password, I'm almost tempted to actually use a bytearray instead of a string. Or use a deque() so that you can popleft() and popright() more easily. Then the password itself is just joined from that deque.

* 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.

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

	"""Redraw the entire password line with asterisks."""
	"""Redraw the entire password line with echo_char."""

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

	"""Erase count echo characters from display."""
	"""Erase count echo characters from display."""

	"""Insert character at cursor position."""
	"""Insert char at cursor position."""

		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!

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!

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!

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!

CuriousLearner commented Nov 30, 2025

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!

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!

CuriousLearner commented Mar 22, 2026

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CuriousLearner commented Nov 15, 2025 •

edited by github-actions bot

Loading

CuriousLearner commented Nov 15, 2025 •

edited

Loading