indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
The hgwebdir WSGI application detects when a requested URL is for
a known repository and it effectively forwards the request to the
hgweb WSGI application.
The hgweb WSGI application needs to route the request based on the
base URL for the repository. The way this normally works is
SCRIPT_NAME is used to resolve the base URL and PATH_INFO
contains the path after the script.
But with hgwebdir, SCRIPT_NAME refers to hgwebdir, not the base
URL for the repository. So, there was a hacky REPO_NAME environment
variable being set to convey the part of the URL that represented
the repository so hgweb could ignore this path component for
routing purposes.
The use of the environment variable for passing internal state
is pretty hacky. Plus, it wasn't clear from the perspective of
the URL parsing code what was going on.
This commit improves matters by making the repository name an
explicit argument to the request parser. The logic around
handling of this value has been shored up. We add various checks
that the argument is used properly - that the repository name
does represent the prefix of the PATH_INFO.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2819
AFFECTED FILES
mercurial/hgweb/hgwebdir_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
@@ -5,6 +5,9 @@
from mercurial.hgweb import (
request as requestmod,
)
+from mercurial import (
+error,
+)
DEFAULT_ENV = {
r'REQUEST_METHOD': r'GET',
@@ -20,11 +23,11 @@
r'wsgi.run_once': False,
}
-def parse(env, bodyfh=None, extra=None):
+def parse(env, bodyfh=None, reponame=None, extra=None):
env = dict(env)
env.update(extra or {})
-return requestmod.parserequestfromenv(env, bodyfh)
+return requestmod.parserequestfromenv(env, bodyfh, reponame=reponame)
class ParseRequestTests(unittest.TestCase):
def testdefault(self):
@@ -203,24 +206,26 @@
self.assertTrue(r.havepathinfo)
def testreponame(self):
-"""REPO_NAME path components get stripped from URL."""
-r = parse(DEFAULT_ENV, extra={
-r'REPO_NAME': r'repo',
-r'PATH_INFO': r'/path1/path2'
-})
+"""repository path components get stripped from URL."""
+
+with self.assertRaisesRegexp(error.ProgrammingError,
+ b'reponame requires PATH_INFO'):
+parse(DEFAULT_ENV, reponame=b'repo')
-self.assertEqual(r.url, b'http://testserver/path1/path2')
-self.assertEqual(r.baseurl, b'http://testserver')
-self.assertEqual(r.advertisedurl, b'http://testserver/path1/path2')
-self.assertEqual(r.advertisedbaseurl, b'http://testserver')
-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')
+with self.assertRaisesRegexp(error.ProgrammingError,
+ b'PATH_INFO does not begin with repo '
+ b'name'):
+parse(DEFAULT_ENV, reponame=b'repo', extra={
+r'PATH_INFO': r'/pathinfo',
+})
-r = parse(DEFAULT_ENV, extra={
-r'REPO_NAME': r'repo',
+with self.assertRaisesRegexp(error.ProgrammingError,
+ b'reponame prefix of PATH_INFO'):
+parse(DEFAULT_ENV, reponame=b'repo', extra={
+r'PATH_INFO': r'/repoextra/path',
+})
+
+r = parse(DEFAULT_ENV, reponame=b'repo', extra={
r'PATH_INFO': r'/repo/path1/path2',
})
@@ -234,8 +239,7 @@
self.assertTrue(r.havepathinfo)
self.assertEqual(r.reponame, b'repo')
-r = parse(DEFAULT_ENV, extra={
-r'REPO_NAME': r'prefix/repo',
+r = parse(DEFAULT_ENV, reponame=b'prefix/repo', extra={
r'PATH_INFO': r'/prefix/repo/path1/path2',
})
diff --git a/mercurial/hgweb/request.py b/mercurial/hgweb/request.py
--- a/mercurial/hgweb/request.py
+++ b/mercurial/hgweb/request.py
@@ -155,11 +155,16 @@
# Request body input stream.
bodyfh = attr.ib()
-def parserequestfromenv(env, bodyfh):
+def parserequestfromenv(env, bodyfh, reponame=None):
"""Parse URL components from environment variables.
WSGI defines request attributes via environment variables. This function
parses the environment variables into a data structure.
+
+If ``reponame`` is defined, the leading path components matching that
+string are effectively
