hgweb: change how dispatch path is reported

When I implemented the new request object, I carried forward some
ugly hacks until I could figure out what was happening. One of those
was the handling of PATH_INFO to determine how to route hgweb
requests.

Essentially, if we have PATH_INFO data, we route according to
that. But if we don't, we route by the query string. I question
if we still need to support query string routing. But that's for
another day, I suppose.

In this commit, we clean up the ugly "havepathinfo" hack and
replace it with a "dispatchpath" attribute that can hold None or
empty string to differentiate between the presence of PATH_INFO.
This is still a bit hacky. But at least the request parsing
and routing code is explicit about the meaning now.

Differential Revision: https://phab.mercurial-scm.org/D2820

mercurial/hgweb/hgweb_mod.py

@@ -324,7 +324,11 b' class hgweb(object):'
         if handled:
             return res.sendresponse()
 
-        if req.havepathinfo:
+        # Old implementations of hgweb supported dispatching the request via
+        # the initial query string parameter instead of using PATH_INFO.
+        # If PATH_INFO is present (signaled by ``req.dispatchpath`` having
+        # a value), we use it. Otherwise fall back to the query string.
+        if req.dispatchpath is not None:
             query = req.dispatchpath
         else:
             query = req.querystring.partition('&')[0].partition(';')[0]

mercurial/hgweb/request.py

@@ -138,11 +138,12 b' class parsedrequest(object):'
     apppath = attr.ib()
     # List of path parts to be used for dispatch.
     dispatchparts = attr.ib()
-    # URL path component (no query string) used for dispatch.
+    # URL path component (no query string) used for dispatch. Can be
+    # ``None`` to signal no path component given to the request, an
+    # empty string to signal a request to the application's root URL,
+    # or a string not beginning with ``/`` containing the requested
+    # path under the application.
     dispatchpath = attr.ib()
-    # Whether there is a path component to this request. This can be true
-    # when ``dispatchpath`` is empty due to REPO_NAME muckery.
-    havepathinfo = attr.ib()
     # The name of the repository being accessed.
     reponame = attr.ib()
     # Raw query string (part after "?" in URL).
@@ -246,12 +247,18 b' def parserequestfromenv(env, bodyfh, rep'
 
         apppath = apppath.rstrip('/') + repoprefix
         dispatchparts = dispatchpath.strip('/').split('/')
-    elif env.get('PATH_INFO', '').strip('/'):
+        dispatchpath = '/'.join(dispatchparts)
+
+    elif 'PATH_INFO' in env:
+        if env['PATH_INFO'].strip('/'):
         dispatchparts = env['PATH_INFO'].strip('/').split('/')
+            dispatchpath = '/'.join(dispatchparts)
     else:
         dispatchparts = []
-
-    dispatchpath = '/'.join(dispatchparts)
+            dispatchpath = ''
+    else:
+        dispatchparts = []
+        dispatchpath = None
 
     querystring = env.get('QUERY_STRING', '')
 
@@ -293,7 +300,6 b' def parserequestfromenv(env, bodyfh, rep'
                          remotehost=env.get('REMOTE_HOST'),
                          apppath=apppath,
                          dispatchparts=dispatchparts, dispatchpath=dispatchpath,
-                         havepathinfo='PATH_INFO' in env,
                          reponame=reponame,
                          querystring=querystring,
                          qsparams=qsparams,

tests/test-wsgirequest.py

@@ -42,8 +42,7 b' class ParseRequestTests(unittest.TestCas'
         self.assertIsNone(r.remotehost)
         self.assertEqual(r.apppath, b'')
         self.assertEqual(r.dispatchparts, [])
-        self.assertEqual(r.dispatchpath, b'')
-        self.assertFalse(r.havepathinfo)
+        self.assertIsNone(r.dispatchpath)
         self.assertIsNone(r.reponame)
         self.assertEqual(r.querystring, b'')
         self.assertEqual(len(r.qsparams), 0)
@@ -90,8 +89,7 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.advertisedbaseurl, b'http://testserver')
         self.assertEqual(r.apppath, b'')
         self.assertEqual(r.dispatchparts, [])
-        self.assertEqual(r.dispatchpath, b'')
-        self.assertFalse(r.havepathinfo)
+        self.assertIsNone(r.dispatchpath)
 
         r = parse(DEFAULT_ENV, extra={
             r'SCRIPT_NAME': r'/script',
@@ -103,8 +101,7 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.advertisedbaseurl, b'http://testserver')
         self.assertEqual(r.apppath, b'/script')
         self.assertEqual(r.dispatchparts, [])
-        self.assertEqual(r.dispatchpath, b'')
-        self.assertFalse(r.havepathinfo)
+        self.assertIsNone(r.dispatchpath)
 
         r = parse(DEFAULT_ENV, extra={
             r'SCRIPT_NAME': r'/multiple words',
@@ -116,8 +113,7 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.advertisedbaseurl, b'http://testserver')
         self.assertEqual(r.apppath, b'/multiple words')
         self.assertEqual(r.dispatchparts, [])
-        self.assertEqual(r.dispatchpath, b'')
-        self.assertFalse(r.havepathinfo)
+        self.assertIsNone(r.dispatchpath)
 
     def testpathinfo(self):
         r = parse(DEFAULT_ENV, extra={
@@ -131,7 +127,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'')
         self.assertEqual(r.dispatchparts, [])
         self.assertEqual(r.dispatchpath, b'')
-        self.assertTrue(r.havepathinfo)
 
         r = parse(DEFAULT_ENV, extra={
             r'PATH_INFO': r'/pathinfo',
@@ -144,7 +139,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'')
         self.assertEqual(r.dispatchparts, [b'pathinfo'])
         self.assertEqual(r.dispatchpath, b'pathinfo')
-        self.assertTrue(r.havepathinfo)
 
         r = parse(DEFAULT_ENV, extra={
             r'PATH_INFO': r'/one/two/',
@@ -157,7 +151,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'')
         self.assertEqual(r.dispatchparts, [b'one', b'two'])
         self.assertEqual(r.dispatchpath, b'one/two')
-        self.assertTrue(r.havepathinfo)
 
     def testscriptandpathinfo(self):
         r = parse(DEFAULT_ENV, extra={
@@ -172,7 +165,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'/script')
         self.assertEqual(r.dispatchparts, [b'pathinfo'])
         self.assertEqual(r.dispatchpath, b'pathinfo')
-        self.assertTrue(r.havepathinfo)
 
         r = parse(DEFAULT_ENV, extra={
             r'SCRIPT_NAME': r'/script1/script2',
@@ -188,7 +180,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'/script1/script2')
         self.assertEqual(r.dispatchparts, [b'path1', b'path2'])
         self.assertEqual(r.dispatchpath, b'path1/path2')
-        self.assertTrue(r.havepathinfo)
 
         r = parse(DEFAULT_ENV, extra={
             r'HTTP_HOST': r'hostserver',
@@ -203,7 +194,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'/script')
         self.assertEqual(r.dispatchparts, [b'pathinfo'])
         self.assertEqual(r.dispatchpath, b'pathinfo')
-        self.assertTrue(r.havepathinfo)
 
     def testreponame(self):
         """repository path components get stripped from URL."""
@@ -236,7 +226,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'/repo')
         self.assertEqual(r.dispatchparts, [b'path1', b'path2'])
         self.assertEqual(r.dispatchpath, b'path1/path2')
-        self.assertTrue(r.havepathinfo)
         self.assertEqual(r.reponame, b'repo')
 
         r = parse(DEFAULT_ENV, reponame=b'prefix/repo', extra={
@@ -251,7 +240,6 b' class ParseRequestTests(unittest.TestCas'
         self.assertEqual(r.apppath, b'/prefix/repo')
         self.assertEqual(r.dispatchparts, [b'path1', b'path2'])
         self.assertEqual(r.dispatchpath, b'path1/path2')
-        self.assertTrue(r.havepathinfo)
         self.assertEqual(r.reponame, b'prefix/repo')
 
 if __name__ == '__main__':