This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2744

hgweb: handle CONTENT_LENGTH
ClosedPublic

Authored by indygreg on Mar 8 2018, 8:06 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGed0456fde625: hgweb: handle CONTENT_LENGTH
Summary

PEP 3333 says CONTENT_LENGTH may be set. I /think/ WSGI servers are
allowed to invent this key even if the client didn't send it.

We had code in wireprotoserver looking for this key. So let's
just automagically convert this key to an HTTP request header
when parsing the request.

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

indygreg created this revision.Mar 8 2018, 8:06 PM
durin42 accepted this revision.Mar 9 2018, 5:14 PM
durin42 added a subscriber: durin42.
durin42 added inline comments.
mercurial/wireprotoserver.py
94–96

For our clients it always will, because we precompute the bundle to a file. It's gross.

This revision is now accepted and ready to land.Mar 9 2018, 5:14 PM
indygreg edited the summary of this revision. (Show Details)Mar 10 2018, 3:03 PM
indygreg updated this revision to Diff 6824.
mharbison72 added a subscriber: mharbison72.Mar 12 2018, 11:26 AM
mharbison72 added inline comments.
mercurial/hgweb/request.py
205

typo: "... will either either value..."

durin42 accepted this revision.Mar 12 2018, 4:52 PM
Closed by commit rHGed0456fde625: hgweb: handle CONTENT_LENGTH (authored by indygreg). · Explain WhyMar 12 2018, 5:32 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 6899

mercurial/hgweb/request.py

# so keys match what likely went over the wire. # so keys match what likely went over the wire.
headers = [] headers = []
for k, v in env.iteritems(): for k, v in env.iteritems():
if k.startswith('HTTP_'): if k.startswith('HTTP_'):
headers.append((k[len('HTTP_'):].replace('_', '-'), v)) headers.append((k[len('HTTP_'):].replace('_', '-'), v))
headers = wsgiheaders.Headers(headers) headers = wsgiheaders.Headers(headers)
# This is kind of a lie because the HTTP header wasn't explicitly
# sent. But for all intents and purposes it should be OK to lie about
# this, since a consumer will either either value to determine how many
mharbison72Unsubmitted
Not Done

typo: "... will either either value..."

mharbison72: typo: "... will either either value..."
# bytes are available to read.
if 'CONTENT_LENGTH' in env and 'HTTP_CONTENT_LENGTH' not in env:
headers['Content-Length'] = env['CONTENT_LENGTH']
return parsedrequest(url=fullurl, baseurl=baseurl, return parsedrequest(url=fullurl, baseurl=baseurl,
advertisedurl=advertisedfullurl, advertisedurl=advertisedfullurl,
advertisedbaseurl=advertisedbaseurl, advertisedbaseurl=advertisedbaseurl,
apppath=apppath, apppath=apppath,
dispatchparts=dispatchparts, dispatchpath=dispatchpath, dispatchparts=dispatchparts, dispatchpath=dispatchpath,
havepathinfo='PATH_INFO' in env, havepathinfo='PATH_INFO' in env,
querystring=querystring, querystring=querystring,
querystringlist=querystringlist, querystringlist=querystringlist,

mercurial/wireprotoserver.py

self._wsgireq.read(postlen), keep_blank_values=True)) self._wsgireq.read(postlen), keep_blank_values=True))
return args return args
argvalue = decodevaluefromheaders(self._req, b'X-HgArg') argvalue = decodevaluefromheaders(self._req, b'X-HgArg')
args.update(urlreq.parseqs(argvalue, keep_blank_values=True)) args.update(urlreq.parseqs(argvalue, keep_blank_values=True))
return args return args
def forwardpayload(self, fp): def forwardpayload(self, fp):
if b'Content-Length' in self._req.headers: # Existing clients *always* send Content-Length.
length = int(self._req.headers[b'Content-Length']) length = int(self._req.headers[b'Content-Length'])
else:
durin42Unsubmitted
Not Done

For our clients it always will, because we precompute the bundle to a file. It's gross.

durin42: For our clients it always will, because we precompute the bundle to a file. It's gross.
length = int(self._wsgireq.env[r'CONTENT_LENGTH'])
# If httppostargs is used, we need to read Content-Length # If httppostargs is used, we need to read Content-Length
# minus the amount that was consumed by args. # minus the amount that was consumed by args.
length -= int(self._req.headers.get(b'X-HgArgs-Post', 0)) length -= int(self._req.headers.get(b'X-HgArgs-Post', 0))
for s in util.filechunkiter(self._wsgireq, limit=length): for s in util.filechunkiter(self._wsgireq, limit=length):
fp.write(s) fp.write(s)
@contextlib.contextmanager @contextlib.contextmanager
def mayberedirectstdio(self): def mayberedirectstdio(self):