Page MenuHomePhabricator

ui: ensure `getpass()` returns bytes
ClosedPublic

Authored by mharbison72 on Nov 23 2020, 12:25 PM.

Details

Summary

Previously, this could return either bytes or str. I'm not sure which direction
we should go in, but since the input is bytes, I guess bytes makes sense as
output. debuguigetpass crashed because it assumed bytes would be returned,
sslcontext.load_cert_chain() is happy with bytes or str if the type info in
PyCharm is correct, and smtplib.SMTP.login() wants str.

I couldn't figure out how to test this, because the test stalls for input with
echo test | hg debuguigetpass --config ui.interactive=1, likely because it
drains stdin before prompting. The custom input reading with ui.nontty=1 does
not.

I'm also a bit concerned with all of this encoding/decoding. The existing code
in the mail module uses encoding.strfromlocal(), but the username and password
are ascii encoded/decoded in mercurial.url.passwordmgr.find_user_password()
with pycompat.{str,bytes}url(). I'm not sure if this inconsistency could
cause subtle compatability issues.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Nov 23 2020, 12:25 PM
pulkit accepted this revision.Nov 26 2020, 7:31 AM
This revision is now accepted and ready to land.Nov 26 2020, 7:31 AM
This revision was automatically updated to reflect the committed changes.

Should this be grafted to stable because of need for smtp.login() to have str? I don't care too much about the debug command crashing.