D2820: hgweb: change how dispatch path is reported
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Mon Mar 12 21:16:50 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2820
AFFECTED FILES
mercurial/hgweb/hgweb_mod.py
mercurial/hgweb/request.py
tests/test-wsgirequest.py
CHANGE DETAILS
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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__':
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 @@
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 @@
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 @@
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/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 @@
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]
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list