This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2854

hgweb: use our forked wsgiheaders module instead of stdlib one
ClosedPublic

Authored by durin42 on Mar 13 2018, 11:30 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGda84e26d85ed: hgweb: use our forked wsgiheaders module instead of stdlib one
Summary

Now we use bytes for headers, rather than native strings.

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.Mar 13 2018, 11:30 PM
indygreg requested changes to this revision.Mar 14 2018, 12:54 AM
indygreg added a subscriber: indygreg.

parsedrequest.headers is supposed to be bytes. One of the goals of this refactor was to make the WSGI code all bytes all the time. So if system strings are making their way to the new request or response objects (i.e. outside request.py), then something is wrong and we should fix that: we shouldn’t need to introduce raw literals outside of the very low level WSGI code.

This revision now requires changes to proceed.Mar 14 2018, 12:54 AM
indygreg added a comment.Mar 14 2018, 10:56 AM
In D2854#45911, @indygreg wrote:

parsedrequest.headers is supposed to be bytes. One of the goals of this refactor was to make the WSGI code all bytes all the time. So if system strings are making their way to the new request or response objects (i.e. outside request.py), then something is wrong and we should fix that: we shouldn’t need to introduce raw literals outside of the very low level WSGI code.

We're using wsgirequest.headers.Headers. Python 3's implementation insists that headers be specified as the native str type: https://github.com/python/cpython/blob/master/Lib/wsgiref/headers.py. Because of course it does. Ugh.

It looks like we'll have to implement a case insensitive + -/_ normalizing keys dict for header storage. Shouldn't be a big deal.

durin42 edited the summary of this revision. (Show Details)Apr 11 2018, 3:50 PM
durin42 retitled this revision from hgweb: str/bytes followups after indygreg's refactoring to hgweb: use our forked wsgiheaders module instead of stdlib one.
durin42 added a child revision: D3246: keepalive: rewrite readinto() to not use read().
durin42 updated this revision to Diff 8009.
indygreg accepted this revision.Apr 12 2018, 11:10 AM
This revision is now accepted and ready to land.Apr 12 2018, 11:10 AM
indygreg requested changes to this revision.Apr 12 2018, 11:30 AM

This one caused an import cycle: Import cycle: mercurial.hgweb.__init__ -> mercurial.hgweb.hgweb_mod -> mercurial.hgweb.request -> mercurial.hgweb.__init__

This revision now requires changes to proceed.Apr 12 2018, 11:30 AM
durin42 updated this revision to Diff 8074.Apr 12 2018, 12:47 PM
indygreg accepted this revision.Apr 12 2018, 12:59 PM
This revision is now accepted and ready to land.Apr 12 2018, 12:59 PM
Closed by commit rHGda84e26d85ed: hgweb: use our forked wsgiheaders module instead of stdlib one (authored by durin42). · Explain WhyApr 12 2018, 1:01 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/hgweb/request.py (3 lines)

Diff 8076

mercurial/hgweb/request.py

# hgweb/request.py - An http request from either CGI or the standalone server. # hgweb/request.py - An http request from either CGI or the standalone server.
# #
# Copyright 21 May 2005 - (c) 2005 Jake Edge <jake@edge2.net> # Copyright 21 May 2005 - (c) 2005 Jake Edge <jake@edge2.net>
# Copyright 2005, 2006 Matt Mackall <mpm@selenic.com> # Copyright 2005, 2006 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import wsgiref.headers as wsgiheaders
#import wsgiref.validate #import wsgiref.validate
from ..thirdparty import ( from ..thirdparty import (
attr, attr,
) )
from .. import ( from .. import (
error, error,
pycompat, pycompat,
# HTTP_* keys contain HTTP request headers. The Headers structure should # HTTP_* keys contain HTTP request headers. The Headers structure should
# perform case normalization for us. We just rewrite underscore to dash # perform case normalization for us. We just rewrite underscore to dash
# 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))
from . import wsgiheaders # avoid cycle
headers = wsgiheaders.Headers(headers) headers = wsgiheaders.Headers(headers)
# This is kind of a lie because the HTTP header wasn't explicitly # 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 # 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 # this, since a consumer will either either value to determine how many
# bytes are available to read. # bytes are available to read.
if 'CONTENT_LENGTH' in env and 'HTTP_CONTENT_LENGTH' not in env: if 'CONTENT_LENGTH' in env and 'HTTP_CONTENT_LENGTH' not in env:
headers['Content-Length'] = env['CONTENT_LENGTH'] headers['Content-Length'] = env['CONTENT_LENGTH']
``req`` is a ``parsedrequest``. ``startresponse`` is the ``req`` is a ``parsedrequest``. ``startresponse`` is the
``start_response`` function passed to the WSGI application. ``start_response`` function passed to the WSGI application.
""" """
self._req = req self._req = req
self._startresponse = startresponse self._startresponse = startresponse
self.status = None self.status = None
from . import wsgiheaders # avoid cycle
self.headers = wsgiheaders.Headers([]) self.headers = wsgiheaders.Headers([])
self._bodybytes = None self._bodybytes = None
self._bodygen = None self._bodygen = None
self._bodywillwrite = False self._bodywillwrite = False
self._started = False self._started = False
self._bodywritefn = None self._bodywritefn = None