This is an archive of the discontinued Mercurial Phabricator instance.

Differential D886

ui: convert to/from Optional[bytes] to Optional[str] in password manager
ClosedPublic

Authored by durin42 on Oct 1 2017, 12:43 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG75de5d456b60: ui: convert to/from Optional[bytes] to Optional[str] in password manager
Summary

This password manager proxy is roughly the right-looking layer to
convert between strings and bytes. Many of these arguments can be
None, so we have a helper method to make the conversion preserve Nones
without exploding.

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

durin42 created this revision.Oct 1 2017, 12:43 PM
yuja requested changes to this revision.Oct 2 2017, 5:38 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/ui.py
141

Nit: no need to check if ispy3.

165

No idea how user and passwd are encoded, but encoding.strfromlocal() might
be better because strurl() can raise unicode exception if non-ascii string is passed.

This revision now requires changes to proceed.Oct 2 2017, 5:38 AM
durin42 added inline comments.Oct 2 2017, 5:45 AM
mercurial/ui.py
141

Experimentally not true: if I don't do the ispy3 check, tests fail.

165

I think the password can come from either a URL or explicit end-user input. I'll have to look more closely.

yuja added inline comments.Oct 2 2017, 11:48 AM
mercurial/ui.py
141

No idea why. strurl() should just return the input on Py2.

durin42 marked 3 inline comments as done.Oct 4 2017, 12:26 PM
durin42 updated this revision to Diff 2421.
durin42 added a comment.Oct 4 2017, 12:27 PM

Oddly, this is now working correctly for me. I'm baffled why it was broken previously.

yuja accepted this revision.Oct 5 2017, 9:42 AM

Queued, thanks.

This revision is now accepted and ready to land.Oct 5 2017, 9:42 AM
Closed by commit rHG75de5d456b60: ui: convert to/from Optional[bytes] to Optional[str] in password manager (authored by durin42). · Explain WhyOct 5 2017, 10:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/ui.py (21 lines)

Diff 2461

mercurial/ui.py

# uncomment these lines to enable some popular extensions # uncomment these lines to enable some popular extensions
# (see 'hg help extensions' for more info) # (see 'hg help extensions' for more info)
# #
# blackbox = # blackbox =
# churn = # churn =
""", """,
} }
def _maybestrurl(maybebytes):
if maybebytes is None:
return None
return pycompat.strurl(maybebytes)
yujaUnsubmitted
Done

Nit: no need to check if ispy3.

yuja: Nit: no need to check if ispy3.
durin42AuthorUnsubmitted
Done

Experimentally not true: if I don't do the ispy3 check, tests fail.

durin42: Experimentally not true: if I don't do the ispy3 check, tests fail.
yujaUnsubmitted
Done

No idea why. strurl() should just return the input on Py2.

yuja: No idea why. strurl() should just return the input on Py2.
def _maybebytesurl(maybestr):
if maybestr is None:
return None
return pycompat.bytesurl(maybestr)
class httppasswordmgrdbproxy(object): class httppasswordmgrdbproxy(object):
"""Delays loading urllib2 until it's needed.""" """Delays loading urllib2 until it's needed."""
def __init__(self): def __init__(self):
self._mgr = None self._mgr = None
def _get_mgr(self): def _get_mgr(self):
if self._mgr is None: if self._mgr is None:
self._mgr = urlreq.httppasswordmgrwithdefaultrealm() self._mgr = urlreq.httppasswordmgrwithdefaultrealm()
return self._mgr return self._mgr
def add_password(self, realm, uris, user, passwd): def add_password(self, realm, uris, user, passwd):
return self._get_mgr().add_password(realm, uris, user, passwd) if isinstance(uris, tuple):
uris = tuple(_maybestrurl(u) for u in uris)
else:
uris = _maybestrurl(uris)
return self._get_mgr().add_password(
_maybestrurl(realm), uris,
_maybestrurl(user), _maybestrurl(passwd))
yujaUnsubmitted
Not Done

No idea how user and passwd are encoded, but encoding.strfromlocal() might
be better because strurl() can raise unicode exception if non-ascii string is passed.

yuja: No idea how user and passwd are encoded, but encoding.strfromlocal() might be better because…
durin42AuthorUnsubmitted
Not Done

I think the password can come from either a URL or explicit end-user input. I'll have to look more closely.

durin42: I think the password can come from either a URL or explicit end-user input. I'll have to look…
def find_user_password(self, realm, uri): def find_user_password(self, realm, uri):
return self._get_mgr().find_user_password(realm, uri) return tuple(_maybebytesurl(v) for v in
self._get_mgr().find_user_password(_maybestrurl(realm),
_maybestrurl(uri)))
def _catchterm(*args): def _catchterm(*args):
raise error.SignalInterrupt raise error.SignalInterrupt
# unique object used to detect no default value has been provided when # unique object used to detect no default value has been provided when
# retrieving configuration value. # retrieving configuration value.
_unset = object() _unset = object()