# HG changeset patch # User Gregory Szorc # Date 2018-03-11 20:38:56 # Node ID d0b0fedbfb5374fbe1d94baa25523da2bacd2e1b # Parent d7fd203e36cc3b4cb20c4c2236428a5e8a0c706b 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 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 @@ -324,7 +324,11 @@ 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] diff --git a/mercurial/hgweb/request.py b/mercurial/hgweb/request.py --- a/mercurial/hgweb/request.py +++ b/mercurial/hgweb/request.py @@ -138,11 +138,12 @@ 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 @@ def parserequestfromenv(env, bodyfh, rep apppath = apppath.rstrip('/') + repoprefix dispatchparts = dispatchpath.strip('/').split('/') - elif env.get('PATH_INFO', '').strip('/'): - dispatchparts = env['PATH_INFO'].strip('/').split('/') + 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 = '' else: dispatchparts = [] - - dispatchpath = '/'.join(dispatchparts) + dispatchpath = None querystring = env.get('QUERY_STRING', '') @@ -293,7 +300,6 @@ 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, diff --git a/tests/test-wsgirequest.py b/tests/test-wsgirequest.py --- a/tests/test-wsgirequest.py +++ b/tests/test-wsgirequest.py @@ -42,8 +42,7 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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__':