( )⚙ D2740 wireprotoserver: move all wire protocol handling logic out of hgweb

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2740

wireprotoserver: move all wire protocol handling logic out of hgweb
ClosedPublic

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

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG158d4ecc03c8: wireprotoserver: move all wire protocol handling logic out of hgweb
rHG09b9a9d4612b: wireprotoserver: move all wire protocol handling logic out of hgweb
Summary

Previous patches from several days ago worked to isolate processing
of HTTP wire protocol requests to wireprotoserver. We still had a
little logic in hgweb. If feels like the right time to finish the
job.

This commit moves WSGI request servicing from hgweb to wireprotoserver.
The ugly dict holding the parsed request is no more. I think the new
code is cleaner.

As part of this, we now process wire protocol requests before the
block to obtain the "query" variable. This makes it clear that this
wonky "query" variable is not used by the wire protocol.

The wonkiest part about this code is the HTTP 404. I'm actually not
sure what all is going on here. It looks like the code is trying to
prevent URL with path components that specify a command from not
working. That part I grok. What I don't grok is why we need to send
a 404. I would think it would be OK to no-op and let another handler
try to service the request. But if we do this, we get some subrepo
test failures. So it looks like something is expecting the HTTP 404
and reacting to it in a specific way. It /might/ be possible to
change the behavior here. But it isn't something I'm comfortable
doing because I don't understand the problem space.

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, 2:23 PM
This revision is now accepted and ready to land.Mar 9 2018, 2:23 PM
Closed by commit rHG09b9a9d4612b: wireprotoserver: move all wire protocol handling logic out of hgweb (authored by indygreg). · Explain WhyMar 9 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 6784

mercurial/hgweb/hgweb_mod.py

if rctx.csp: if rctx.csp:
# hgwebdir may have added CSP header. Since we generate our own, # hgwebdir may have added CSP header. Since we generate our own,
# replace it. # replace it.
wsgireq.headers = [h for h in wsgireq.headers wsgireq.headers = [h for h in wsgireq.headers
if h[0] != 'Content-Security-Policy'] if h[0] != 'Content-Security-Policy']
wsgireq.headers.append(('Content-Security-Policy', rctx.csp)) wsgireq.headers.append(('Content-Security-Policy', rctx.csp))
handled, res = wireprotoserver.handlewsgirequest(
rctx, wsgireq, req, self.check_perm)
if handled:
return res
if req.havepathinfo: if req.havepathinfo:
query = req.dispatchpath query = req.dispatchpath
else: else:
query = req.querystring.partition('&')[0].partition(';')[0] query = req.querystring.partition('&')[0].partition(';')[0]
# Route it to a wire protocol handler if it looks like a wire protocol
# request.
protohandler = wireprotoserver.parsehttprequest(rctx, wsgireq, req,
self.check_perm)
if protohandler:
try:
if query:
raise ErrorResponse(HTTP_NOT_FOUND)
return protohandler['dispatch']()
except ErrorResponse as inst:
return protohandler['handleerror'](inst)
# translate user-visible url structure to internal structure # translate user-visible url structure to internal structure
args = query.split('/', 2) args = query.split('/', 2)
if 'cmd' not in wsgireq.form and args and args[0]: if 'cmd' not in wsgireq.form and args and args[0]:
cmd = args.pop(0) cmd = args.pop(0)
style = cmd.rfind('-') style = cmd.rfind('-')
if style != -1: if style != -1:
wsgireq.form['style'] = [cmd[:style]] wsgireq.form['style'] = [cmd[:style]]

mercurial/wireprotoserver.py

# This method exists mostly so that extensions like remotefilelog can # This method exists mostly so that extensions like remotefilelog can
# disable a kludgey legacy method only over http. As of early 2018, # disable a kludgey legacy method only over http. As of early 2018,
# there are no other known users, so with any luck we can discard this # there are no other known users, so with any luck we can discard this
# hook if remotefilelog becomes a first-party extension. # hook if remotefilelog becomes a first-party extension.
def iscmd(cmd): def iscmd(cmd):
return cmd in wireproto.commands return cmd in wireproto.commands
def parsehttprequest(rctx, wsgireq, req, checkperm): def handlewsgirequest(rctx, wsgireq, req, checkperm):
"""Parse the HTTP request for a wire protocol request. """Possibly process a wire protocol request.
If the current request appears to be a wire protocol request, this If the current request is a wire protocol request, the request is
function returns a dict with details about that request, including processed by this function.
an ``abstractprotocolserver`` instance suitable for handling the
request. Otherwise, ``None`` is returned.
``wsgireq`` is a ``wsgirequest`` instance. ``wsgireq`` is a ``wsgirequest`` instance.
``req`` is a ``parsedrequest`` instance. ``req`` is a ``parsedrequest`` instance.
Returns a 2-tuple of (bool, response) where the 1st element indicates
whether the request was handled and the 2nd element is a return
value for a WSGI application (often a generator of bytes).
""" """
# Avoid cycle involving hg module.
from .hgweb import common as hgwebcommon
repo = rctx.repo repo = rctx.repo
# HTTP version 1 wire protocol requests are denoted by a "cmd" query # HTTP version 1 wire protocol requests are denoted by a "cmd" query
# string parameter. If it isn't present, this isn't a wire protocol # string parameter. If it isn't present, this isn't a wire protocol
# request. # request.
if 'cmd' not in req.querystringdict: if 'cmd' not in req.querystringdict:
return None return False, None
cmd = req.querystringdict['cmd'][0] cmd = req.querystringdict['cmd'][0]
# The "cmd" request parameter is used by both the wire protocol and hgweb. # The "cmd" request parameter is used by both the wire protocol and hgweb.
# While not all wire protocol commands are available for all transports, # While not all wire protocol commands are available for all transports,
# if we see a "cmd" value that resembles a known wire protocol command, we # if we see a "cmd" value that resembles a known wire protocol command, we
# route it to a protocol handler. This is better than routing possible # route it to a protocol handler. This is better than routing possible
# wire protocol requests to hgweb because it prevents hgweb from using # wire protocol requests to hgweb because it prevents hgweb from using
# known wire protocol commands and it is less confusing for machine # known wire protocol commands and it is less confusing for machine
# clients. # clients.
if not iscmd(cmd): if not iscmd(cmd):
return None return False, None
# The "cmd" query string argument is only valid on the root path of the
# repo. e.g. ``/?cmd=foo``, ``/repo?cmd=foo``. URL paths within the repo
# like ``/blah?cmd=foo`` are not allowed. So don't recognize the request
# in this case. We send an HTTP 404 for backwards compatibility reasons.
if req.dispatchpath:
res = _handlehttperror(
hgwebcommon.ErrorResponse(hgwebcommon.HTTP_NOT_FOUND), wsgireq,
cmd)
return True, res
proto = httpv1protocolhandler(wsgireq, repo.ui, proto = httpv1protocolhandler(wsgireq, repo.ui,
lambda perm: checkperm(rctx, wsgireq, perm)) lambda perm: checkperm(rctx, wsgireq, perm))
return { # The permissions checker should be the only thing that can raise an
'cmd': cmd, # ErrorResponse. It is kind of a layer violation to catch an hgweb
'proto': proto, # exception here. So consider refactoring into a exception type that
'dispatch': lambda: _callhttp(repo, wsgireq, proto, cmd), # is associated with the wire protocol.
'handleerror': lambda ex: _handlehttperror(ex, wsgireq, cmd), try:
} res = _callhttp(repo, wsgireq, proto, cmd)
except hgwebcommon.ErrorResponse as e:
res = _handlehttperror(e, wsgireq, cmd)
return True, res
def _httpresponsetype(ui, wsgireq, prefer_uncompressed): def _httpresponsetype(ui, wsgireq, prefer_uncompressed):
"""Determine the appropriate response type and compression settings. """Determine the appropriate response type and compression settings.
Returns a tuple of (mediatype, compengine, engineopts). Returns a tuple of (mediatype, compengine, engineopts).
""" """
# Determine the response media type and compression engine based # Determine the response media type and compression engine based
# on the request parameters. # on the request parameters.