This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2736

hgweb: use the parsed application path directly
ClosedPublic

Authored by indygreg on Mar 8 2018, 8:06 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG0031e972ded2: hgweb: use the parsed application path directly
rHG328d665ef23d: hgweb: use the parsed application path directly
Summary

Previously, we assigned a custom system string with a trailing slash
to wsgirequest.url.

The addition of the trailing slash felt arbitrary and seems to go
against how things typically work in WSGI.

We also want our URLs to be bytes, not system strings.

And, assigning a custom attribute to wsgirequest felt wrong.

This commit fixes all those things by removing the trailing
slash from the app path, changing consumers to use that variable
and to use it without a trailing slash, and removing the custom
attribute from wsgirequest.

We preserve the trailing slash on {url}. Also, makebreadcrumb
strips the trailing slash. So no change to it was needed.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Mar 8 2018, 8:06 PM
durin42 accepted this revision.Mar 9 2018, 2:00 PM
This revision is now accepted and ready to land.Mar 9 2018, 2:00 PM
Closed by commit rHG328d665ef23d: hgweb: use the parsed application path directly (authored by indygreg). · Explain WhyMar 9 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 6781

mercurial/hgweb/hgweb_mod.py

yield {'type': typ, 'extension': spec[2], 'node': nodeid} yield {'type': typ, 'extension': spec[2], 'node': nodeid}
def templater(self, wsgireq, req): def templater(self, wsgireq, req):
# determine scheme, port and server name # determine scheme, port and server name
# this is needed to create absolute urls # this is needed to create absolute urls
logourl = self.config('web', 'logourl') logourl = self.config('web', 'logourl')
logoimg = self.config('web', 'logoimg') logoimg = self.config('web', 'logoimg')
staticurl = (self.config('web', 'staticurl') staticurl = (self.config('web', 'staticurl')
or pycompat.sysbytes(wsgireq.url) + 'static/') or req.apppath + '/static/')
if not staticurl.endswith('/'): if not staticurl.endswith('/'):
staticurl += '/' staticurl += '/'
# some functions for the templater # some functions for the templater
def motd(**map): def motd(**map):
yield self.config('web', 'motd') yield self.config('web', 'motd')
# figure out which style to use # figure out which style to use
vars = {} vars = {}
styles, (style, mapfile) = getstyle(wsgireq, self.config, styles, (style, mapfile) = getstyle(wsgireq, self.config,
self.templatepath) self.templatepath)
if style == styles[0]: if style == styles[0]:
vars['style'] = style vars['style'] = style
sessionvars = webutil.sessionvars(vars, '?') sessionvars = webutil.sessionvars(vars, '?')
if not self.reponame: if not self.reponame:
self.reponame = (self.config('web', 'name', '') self.reponame = (self.config('web', 'name', '')
or wsgireq.env.get('REPO_NAME') or wsgireq.env.get('REPO_NAME')
or wsgireq.url.strip(r'/') or self.repo.root) or req.apppath or self.repo.root)
def websubfilter(text): def websubfilter(text):
return templatefilters.websub(text, self.websubtable) return templatefilters.websub(text, self.websubtable)
# create the templater # create the templater
# TODO: export all keywords: defaults = templatekw.keywords.copy() # TODO: export all keywords: defaults = templatekw.keywords.copy()
defaults = { defaults = {
'url': pycompat.sysbytes(wsgireq.url), 'url': req.apppath + '/',
'logourl': logourl, 'logourl': logourl,
'logoimg': logoimg, 'logoimg': logoimg,
'staticurl': staticurl, 'staticurl': staticurl,
'urlbase': req.advertisedbaseurl, 'urlbase': req.advertisedbaseurl,
'repo': self.reponame, 'repo': self.reponame,
'encoding': encoding.encoding, 'encoding': encoding.encoding,
'motd': motd, 'motd': motd,
'sessionvars': sessionvars, 'sessionvars': sessionvars,
'pathdef': makebreadcrumb(pycompat.sysbytes(wsgireq.url)), 'pathdef': makebreadcrumb(req.apppath),
'style': style, 'style': style,
'nonce': self.nonce, 'nonce': self.nonce,
} }
tres = formatter.templateresources(self.repo.ui, self.repo) tres = formatter.templateresources(self.repo.ui, self.repo)
tmpl = templater.templater.frommapfile(mapfile, tmpl = templater.templater.frommapfile(mapfile,
filters={'websub': websubfilter}, filters={'websub': websubfilter},
defaults=defaults, defaults=defaults,
resources=tres) resources=tres)
if rctx.csp: if rctx.csp:
# hgwebdir may have added CSP header. Since we generate our own, # hgwebdir may have added CSP header. Since we generate our own,
# replace it. # replace it.
wsgireq.headers = [h for h in wsgireq.headers wsgireq.headers = [h for h in wsgireq.headers
if h[0] != 'Content-Security-Policy'] if h[0] != 'Content-Security-Policy']
wsgireq.headers.append(('Content-Security-Policy', rctx.csp)) wsgireq.headers.append(('Content-Security-Policy', rctx.csp))
wsgireq.url = pycompat.sysstr(req.apppath)
if r'PATH_INFO' in wsgireq.env: if r'PATH_INFO' in wsgireq.env:
parts = wsgireq.env[r'PATH_INFO'].strip(r'/').split(r'/') parts = wsgireq.env[r'PATH_INFO'].strip(r'/').split(r'/')
repo_parts = wsgireq.env.get(r'REPO_NAME', r'').split(r'/') repo_parts = wsgireq.env.get(r'REPO_NAME', r'').split(r'/')
if parts[:len(repo_parts)] == repo_parts: if parts[:len(repo_parts)] == repo_parts:
parts = parts[len(repo_parts):] parts = parts[len(repo_parts):]
query = r'/'.join(parts) query = r'/'.join(parts)
else: else:
query = wsgireq.env[r'QUERY_STRING'].partition(r'&')[0] query = wsgireq.env[r'QUERY_STRING'].partition(r'&')[0]

mercurial/hgweb/request.py

# When dispatching requests, we look at the URL components (PATH_INFO # When dispatching requests, we look at the URL components (PATH_INFO
# and QUERY_STRING) after the application root (SCRIPT_NAME). But hgwebdir # and QUERY_STRING) after the application root (SCRIPT_NAME). But hgwebdir
# has the concept of "virtual" repositories. This is defined via REPO_NAME. # has the concept of "virtual" repositories. This is defined via REPO_NAME.
# If REPO_NAME is defined, we append it to SCRIPT_NAME to form a new app # If REPO_NAME is defined, we append it to SCRIPT_NAME to form a new app
# root. We also exclude its path components from PATH_INFO when resolving # root. We also exclude its path components from PATH_INFO when resolving
# the dispatch path. # the dispatch path.
# TODO the use of trailing slashes in apppath is arguably wrong. We need it
# to appease low-level parts of hgweb_mod for now.
apppath = env['SCRIPT_NAME'] apppath = env['SCRIPT_NAME']
if env.get('REPO_NAME'):
if not apppath.endswith('/'): if not apppath.endswith('/'):
apppath += '/' apppath += '/'
if env.get('REPO_NAME'): apppath += env.get('REPO_NAME')
apppath += env.get('REPO_NAME') + '/'
if 'PATH_INFO' in env: if 'PATH_INFO' in env:
dispatchparts = env['PATH_INFO'].strip('/').split('/') dispatchparts = env['PATH_INFO'].strip('/').split('/')
# Strip out repo parts. # Strip out repo parts.
repoparts = env.get('REPO_NAME', '').split('/') repoparts = env.get('REPO_NAME', '').split('/')
if dispatchparts[:len(repoparts)] == repoparts: if dispatchparts[:len(repoparts)] == repoparts:
dispatchparts = dispatchparts[len(repoparts):] dispatchparts = dispatchparts[len(repoparts):]