This is an archive of the discontinued Mercurial Phabricator instance.

Differential D887

httppeer: use native strings for headers
ClosedPublic

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

Details

Reviewers
yuja
indygreg
Group Reviewers
hg-reviewers
Commits
rHG6db536bed7ec: httppeer: use native strings for headers
Summary

On Python 3, we need to use unicodes, rather than bytes. This lets
test-pull.t get a lot further along.

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, 11:52 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/httppeer.py
233

Appears that encodevalueinheaders() takes bytes.

I don't know which would be better to pass unicodes around or convert them
at one place.

241

Perhaps this should be handled in urlencode() because it converts the output
from str to bytes.

This revision now requires changes to proceed.Oct 2 2017, 11:52 AM
durin42 marked 2 inline comments as done.Oct 14 2017, 2:52 AM
durin42 updated this revision to Diff 2720.
durin42 added inline comments.Oct 14 2017, 2:52 AM
mercurial/httppeer.py
233

I'm going to go ahead and not mess with this until I hit some test coverage for it. I think we should probably just leave encodevalueinheaders() as bytes-in but make it native-str-out, but we'll see when we get there.

241

Turns out urlencode already copes with bytes input even on Python 3. A small victory.

yuja added inline comments.Oct 14 2017, 7:37 AM
mercurial/httppeer.py
241

Nice. So we have to concatenate bytes url and bytes qs, then covert the result to unicode.

yuja requested changes to this revision.Oct 14 2017, 7:38 AM
This revision now requires changes to proceed.Oct 14 2017, 7:38 AM
durin42 updated this revision to Diff 2737.Oct 14 2017, 10:49 AM
durin42 marked an inline comment as done.Oct 14 2017, 4:17 PM

Good catch. I've now got things wired up so that (after another ~17 patches after this) some basic wireproto stuff is starting to work!

There's a templater bug I can't figure out, which I'll point out when the requisite patches to get that far land.

indygreg accepted this revision.Oct 14 2017, 4:52 PM
Closed by commit rHG6db536bed7ec: httppeer: use native strings for headers (authored by durin42). · Explain WhyOct 14 2017, 5:03 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/httppeer.py (12 lines)

Diff 2752

mercurial/httppeer.py

else: else:
if isinstance(data, basestring): if isinstance(data, basestring):
i = io.BytesIO(data) i = io.BytesIO(data)
i.length = len(data) i.length = len(data)
data = i data = i
argsio = io.BytesIO(strargs) argsio = io.BytesIO(strargs)
argsio.length = len(strargs) argsio.length = len(strargs)
data = _multifile(argsio, data) data = _multifile(argsio, data)
headers['X-HgArgs-Post'] = len(strargs) headers[r'X-HgArgs-Post'] = len(strargs)
else: else:
if len(args) > 0: if len(args) > 0:
httpheader = self.capable('httpheader') httpheader = self.capable('httpheader')
if httpheader: if httpheader:
headersize = int(httpheader.split(',', 1)[0]) headersize = int(httpheader.split(',', 1)[0])
if headersize > 0: if headersize > 0:
# The headers can typically carry more data than the URL. # The headers can typically carry more data than the URL.
encargs = urlreq.urlencode(sorted(args.items())) encargs = urlreq.urlencode(sorted(args.items()))
for header, value in encodevalueinheaders(encargs, 'X-HgArg', for header, value in encodevalueinheaders(encargs, 'X-HgArg',
yujaUnsubmitted
Done

Appears that encodevalueinheaders() takes bytes.

I don't know which would be better to pass unicodes around or convert them
at one place.

yuja: Appears that encodevalueinheaders() takes bytes. I don't know which would be better to pass…
durin42AuthorUnsubmitted
Not Done

I'm going to go ahead and not mess with this until I hit some test coverage for it. I think we should probably just leave encodevalueinheaders() as bytes-in but make it native-str-out, but we'll see when we get there.

durin42: I'm going to go ahead and not mess with this until I hit some test coverage for it. I think we…
headersize): headersize):
headers[header] = value headers[header] = value
varyheaders.append(header) varyheaders.append(header)
else: else:
q += sorted(args.items()) q += sorted(args.items())
qs = '?%s' % urlreq.urlencode(q) qs = '?%s' % urlreq.urlencode(q)
cu = "%s%s" % (self._url, qs) cu = "%s%s" % (self._url, qs)
size = 0 size = 0
yujaUnsubmitted
Done

Perhaps this should be handled in urlencode() because it converts the output
from str to bytes.

yuja: Perhaps this should be handled in urlencode() because it converts the output from str to bytes.
durin42AuthorUnsubmitted
Not Done

Turns out urlencode already copes with bytes input even on Python 3. A small victory.

durin42: Turns out urlencode already copes with bytes input even on Python 3. A small victory.
yujaUnsubmitted
Done

Nice. So we have to concatenate bytes url and bytes qs, then covert the result to unicode.

yuja: Nice. So we have to concatenate bytes url and bytes qs, then covert the result to unicode.
if util.safehasattr(data, 'length'): if util.safehasattr(data, 'length'):
size = data.length size = data.length
elif data is not None: elif data is not None:
size = len(data) size = len(data)
if size and self.ui.configbool('ui', 'usehttp2'): if size and self.ui.configbool('ui', 'usehttp2'):
headers['Expect'] = '100-Continue' headers[r'Expect'] = r'100-Continue'
headers['X-HgHttp2'] = '1' headers[r'X-HgHttp2'] = r'1'
if data is not None and 'Content-Type' not in headers: if data is not None and r'Content-Type' not in headers:
headers['Content-Type'] = 'application/mercurial-0.1' headers[r'Content-Type'] = r'application/mercurial-0.1'
# Tell the server we accept application/mercurial-0.2 and multiple # Tell the server we accept application/mercurial-0.2 and multiple
# compression formats if the server is capable of emitting those # compression formats if the server is capable of emitting those
# payloads. # payloads.
protoparams = [] protoparams = []
mediatypes = set() mediatypes = set()
if self._caps is not None: if self._caps is not None:
protoheaders = encodevalueinheaders(' '.join(protoparams), protoheaders = encodevalueinheaders(' '.join(protoparams),
'X-HgProto', 'X-HgProto',
headersize or 1024) headersize or 1024)
for header, value in protoheaders: for header, value in protoheaders:
headers[header] = value headers[header] = value
varyheaders.append(header) varyheaders.append(header)
if varyheaders: if varyheaders:
headers['Vary'] = ','.join(varyheaders) headers[r'Vary'] = r','.join(varyheaders)
req = self._requestbuilder(pycompat.strurl(cu), data, headers) req = self._requestbuilder(pycompat.strurl(cu), data, headers)
if data is not None: if data is not None:
self.ui.debug("sending %s bytes\n" % size) self.ui.debug("sending %s bytes\n" % size)
req.add_unredirected_header('Content-Length', '%d' % size) req.add_unredirected_header('Content-Length', '%d' % size)
try: try:
resp = self._urlopener.open(req) resp = self._urlopener.open(req)