gh-138514: getpass: restrict echo_char to a single ASCII character

[benjaminjohnson01]

Fixes gh-138514.

---

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

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[picnixz]

Can you add some tests?

[benjaminjohnson01]

Yes! Thank you for the feedback and review. I intended to work on this some more. I will add tests and look for potential efficiencies made possible by no longer accepting multi-character strings.

[picnixz]

Note that we have 3.14-rc3 coming next week so I would like for this to be merged before if possible. If you don't have enough time, I can take over.

[benjaminjohnson01]

I can do this now! I apologize for the delay.

[benjaminjohnson01]

I notice on Windows when an arrow key is used during password entering, two echo_char characters appear, and the password string saves the arrow key presses as control-ish characters (like "àK") due to how msvcrt.getwch() handles arrow keys. Should I address this behavior in this pull request (by ignoring/not accepting characters created through arrow key presses)? Or should I make a separate issue for this?

[picnixz]

Does it also happen when there is no echo char?

[picnixz]

However, is this an issue I should open? Using a password involving arrow keys provides a different password on Windows versus Unix systems. For example, with three left arrow key presses as a password...

I don't think it's an "issue". When I use sudo to enter my password and press left/right arrows, they are not interpreted as left/right arrows. It could be considered a feature if you think it's worth it but OTOH, I think it'll be too complex and really too niche. If the password is wrong, or if made a typo, you usually erase using backspace or re-enter the password. Should this feature be supported, a preliminary thread in https://discuss.python.org/c/ideas/6 shouold be opened.

[picnixz]

Could you remove the s to rejects and just use reject. It saves 1 char and we usually have test_[verb]* instead of test_[conjugated form]*.

[benjaminjohnson01]

I don't think it's an "issue". When I use sudo to enter my password and press left/right arrows, they are not interpreted as left/right arrows. It could be considered a feature if you think it's worth it but OTOH, I think it'll be too complex and really too niche. If the password is wrong, or if made a typo, you usually erase using backspace or re-enter the password. Should this feature be supported, a preliminary thread in https://discuss.python.org/c/ideas/6 should be opened.

However, left/right arrows currently are being interpreted as left/right arrows in a way, right (since arrow keys are recorded as bytes/characters)? Instead of supporting this behavior, could we more simply remove it?

[benjaminjohnson01]

I removed the s characters. Thank you once again for your feedback!

[picnixz]

However, left/right arrows currently are being interpreted as left/right arrows in a way, right (since arrow keys are recorded as bytes/characters)?

I think they are interpreted that way because on Unix we use termios while on Windows it's provided by msvcrt.getwch() so we depend on that.

[picnixz]

Unless there is a conflict to solve or a CI test to fix (that your changes wouldn't affect), avoid merging main into your branch as it wastes CI resources. TiA!

[benjaminjohnson01]

Because I raise TypeError when not isinstance(echo_char, str), does the news blurb need to be updated?

[picnixz]

Mmh, no. If we say "we expect a string", and you give something that is not a string, usually it raises a TypeError or an AttributeError. Currently passing something like 2 raises AttributeError because of echo_char.isprintable().

[benjaminjohnson01]

@picnixz would you like me to make any additional modifications?

[picnixz]

I'll take a look tomorrow during the sprint!

[miss-islington-app]

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

[bedevere-app]

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