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

[indygreg (Gregory Szorc)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG09b9a9d4612b: wireprotoserver: move all wire protocol handling logic out of hgweb (authored by indygreg, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2740?vs=6746&id=6784

REVISION DETAIL
  https://phab.mercurial-scm.org/D2740

AFFECTED FILES
  mercurial/hgweb/hgweb_mod.py
  mercurial/wireprotoserver.py

CHANGE DETAILS

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -150,24 +150,29 @@
 def iscmd(cmd):
     return cmd in wireproto.commands
 
-def parsehttprequest(rctx, wsgireq, req, checkperm):
-    """Parse the HTTP request for a wire protocol request.
+def handlewsgirequest(rctx, wsgireq, req, checkperm):
+    """Possibly process a wire protocol request.
 
-    If the current request appears to be a wire protocol request, this
-    function returns a dict with details about that request, including
-    an ``abstractprotocolserver`` instance suitable for handling the
-    request. Otherwise, ``None`` is returned.
+    If the current request is a wire protocol request, the request is
+    processed by this function.
 
     ``wsgireq`` is a ``wsgirequest`` 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
 
     # 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
     # request.
     if 'cmd' not in req.querystringdict:
-        return None
+        return False, None
 
     cmd = req.querystringdict['cmd'][0]
 
@@ -179,17 +184,32 @@
     # known wire protocol commands and it is less confusing for machine
     # clients.
     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,
                                   lambda perm: checkperm(rctx, wsgireq, perm))
 
-    return {
-        'cmd': cmd,
-        'proto': proto,
-        'dispatch': lambda: _callhttp(repo, wsgireq, proto, cmd),
-        'handleerror': lambda ex: _handlehttperror(ex, wsgireq, cmd),
-    }
+    # The permissions checker should be the only thing that can raise an
+    # ErrorResponse. It is kind of a layer violation to catch an hgweb
+    # exception here. So consider refactoring into a exception type that
+    # is associated with the wire protocol.
+    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):
     """Determine the appropriate response type and compression settings.
diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -318,25 +318,16 @@
                                if h[0] != 'Content-Security-Policy']
             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:
             query = req.dispatchpath
         else:
             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
 
         args = query.split('/', 2)



To: indygreg, #hg-reviewers, durin42
Cc: mercurial-devel