This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9375

ui: ensure `getpass()` returns bytes
ClosedPublic

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

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG07b0a687c01a: ui: ensure `getpass()` returns bytes
rHG87e7dd8e7734: ui: ensure `getpass()` returns bytes
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
mharbison72 added a commit: rHG87e7dd8e7734: ui: ensure `getpass()` returns bytes.Nov 26 2020, 8:15 AM
Closed by commit rHG87e7dd8e7734: ui: ensure `getpass()` returns bytes (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.
mharbison72 added a comment.Nov 27 2020, 11:54 PM

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.

Revision Contents
Changeset List

PathPackages
Mmercurial/debugcommands.py (4 lines)
Mmercurial/mail.py (2 lines)
Mmercurial/ui.py (2 lines)

Diff 23698

mercurial/debugcommands.py

b'debuguigetpass', b'debuguigetpass',
[(b'p', b'prompt', b'', _(b'prompt text'), _(b'TEXT')),], [(b'p', b'prompt', b'', _(b'prompt text'), _(b'TEXT')),],
_(b'[-p TEXT]'), _(b'[-p TEXT]'),
norepo=True, norepo=True,
) )
def debuguigetpass(ui, prompt=b''): def debuguigetpass(ui, prompt=b''):
"""show prompt to type password""" """show prompt to type password"""
r = ui.getpass(prompt) r = ui.getpass(prompt)
if r is not None:
r = encoding.strtolocal(r)
else:
r = b"<default response>"
ui.writenoi18n(b'response: %s\n' % r) ui.writenoi18n(b'response: %s\n' % r)
@command( @command(
b'debuguiprompt', b'debuguiprompt',
[(b'p', b'prompt', b'', _(b'prompt text'), _(b'TEXT')),], [(b'p', b'prompt', b'', _(b'prompt text'), _(b'TEXT')),],
_(b'[-p TEXT]'), _(b'[-p TEXT]'),
norepo=True, norepo=True,

mercurial/mail.py

sslutil.validatesocket(s.sock) sslutil.validatesocket(s.sock)
username = ui.config(b'smtp', b'username') username = ui.config(b'smtp', b'username')
password = ui.config(b'smtp', b'password') password = ui.config(b'smtp', b'password')
if username: if username:
if password: if password:
password = encoding.strfromlocal(password) password = encoding.strfromlocal(password)
else: else:
password = ui.getpass() password = ui.getpass()
if password is not None:
password = encoding.strfromlocal(password)
if username and password: if username and password:
ui.note(_(b'(authenticating to mail server as %s)\n') % username) ui.note(_(b'(authenticating to mail server as %s)\n') % username)
username = encoding.strfromlocal(username) username = encoding.strfromlocal(username)
try: try:
s.login(username, password) s.login(username, password)
except smtplib.SMTPException as inst: except smtplib.SMTPException as inst:
raise error.Abort(inst) raise error.Abort(inst)

mercurial/ui.py

# to interact with tty even if fin is not a tty. # to interact with tty even if fin is not a tty.
with self.timeblockedsection(b'stdio'): with self.timeblockedsection(b'stdio'):
if self.configbool(b'ui', b'nontty'): if self.configbool(b'ui', b'nontty'):
l = self._fin.readline() l = self._fin.readline()
if not l: if not l:
raise EOFError raise EOFError
return l.rstrip(b'\n') return l.rstrip(b'\n')
else: else:
return getpass.getpass('') return encoding.strtolocal(getpass.getpass(''))
except EOFError: except EOFError:
raise error.ResponseExpected() raise error.ResponseExpected()
def status(self, *msg, **opts): def status(self, *msg, **opts):
'''write status message to output (if ui.quiet is False) '''write status message to output (if ui.quiet is False)
This adds an output label of "ui.status". This adds an output label of "ui.status".
''' '''