( )⚙ D2738 hgweb: only recognize wire protocol commands from query string (BC)

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2738

hgweb: only recognize wire protocol commands from query string (BC)
ClosedPublic

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

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG886fba199022: hgweb: only recognize wire protocol commands from query string (BC)
rHGad13301e6479: hgweb: only recognize wire protocol commands from query string (BC)
Summary

Previously, we attempted to parse the wire protocol command from
req.form. Data could have come from the query string or POST
form data.

The wire protocol states that the command must be declared in the
query string. And AFAICT all Mercurial releases from at least 1.0
send the command in the query string.

So let's actual require this behavior.

This is technically BC. But I'm not sure how anyone in the wild
would encounter this. POST has historically been used for sending
bundle data. So there's no opportunity to encode arguments there.
And the experimental HTTP POST args also takes over the body. So
the only way someone would be impacted by this is if they wrote
a custom client that both used POST for everything and sent arguments
via the HTTP body. I don't believe such a client exists.

.. bc::

The HTTP wire protocol server no longer accepts the ``cmd``
argument to control which command to run via HTTP POST bodies.
The ``cmd`` argument must be specified on the URL query string.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Mar 8 2018, 8:06 PM
durin42 accepted this revision.Mar 9 2018, 2:04 PM
This revision is now accepted and ready to land.Mar 9 2018, 2:04 PM
Closed by commit rHGad13301e6479: hgweb: only recognize wire protocol commands from query string (BC) (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 6744

mercurial/hgweb/hgweb_mod.py

parts = parts[len(repo_parts):] parts = parts[len(repo_parts):]
query = r'/'.join(parts) query = r'/'.join(parts)
else: else:
query = wsgireq.env[r'QUERY_STRING'].partition(r'&')[0] query = wsgireq.env[r'QUERY_STRING'].partition(r'&')[0]
query = query.partition(r';')[0] query = query.partition(r';')[0]
# Route it to a wire protocol handler if it looks like a wire protocol # Route it to a wire protocol handler if it looks like a wire protocol
# request. # request.
protohandler = wireprotoserver.parsehttprequest(rctx, wsgireq, query, protohandler = wireprotoserver.parsehttprequest(rctx, wsgireq, req,
self.check_perm) self.check_perm)
if protohandler: if protohandler:
try: try:
if query: if query:
raise ErrorResponse(HTTP_NOT_FOUND) raise ErrorResponse(HTTP_NOT_FOUND)
return protohandler['dispatch']() return protohandler['dispatch']()

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, query, checkperm): def parsehttprequest(rctx, wsgireq, req, checkperm):
"""Parse the HTTP request for a wire protocol request. """Parse the HTTP request for a wire protocol request.
If the current request appears to be a wire protocol request, this If the current request appears to be a wire protocol request, this
function returns a dict with details about that request, including function returns a dict with details about that request, including
an ``abstractprotocolserver`` instance suitable for handling the an ``abstractprotocolserver`` instance suitable for handling the
request. Otherwise, ``None`` is returned. request. Otherwise, ``None`` is returned.
``wsgireq`` is a ``wsgirequest`` instance. ``wsgireq`` is a ``wsgirequest`` instance.
``req`` is a ``parsedrequest`` instance.
""" """
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 wsgireq.form: if 'cmd' not in req.querystringdict:
return None return None
cmd = wsgireq.form['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.