This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2779

hgweb: use our new request object for "style" parameter
ClosedPublic

Authored by indygreg on Mar 10 2018, 3:03 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG1a1972b1a1ff: hgweb: use our new request object for "style" parameter
Summary

The "style" parameter is kind of wonky because it is explicitly
set and has lookups in random locations.

Let's port it to qsparams first because it isn't straightforward.

There is subtle change in behavior. But I don't think it is worth
calling out in a BC.

Our multidict's getitem returns the last set value for a key,
not the first. So if the query string set a variable multiple times,
before we would get the first value and now we would get the last
value. It makes no sense to specify these things multiple times.
And I think last write wins is more sensible than first write wins.

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 10 2018, 3:03 PM
durin42 accepted this revision.Mar 12 2018, 5:05 PM
This revision is now accepted and ready to land.Mar 12 2018, 5:05 PM
Closed by commit rHG1a1972b1a1ff: hgweb: use our new request object for "style" parameter (authored by indygreg). · Explain WhyMar 12 2018, 5:32 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 6915

mercurial/hgweb/hgweb_mod.py

archivespecs = util.sortdict(( archivespecs = util.sortdict((
('zip', ('application/zip', 'zip', '.zip', None)), ('zip', ('application/zip', 'zip', '.zip', None)),
('gz', ('application/x-gzip', 'tgz', '.tar.gz', None)), ('gz', ('application/x-gzip', 'tgz', '.tar.gz', None)),
('bz2', ('application/x-bzip2', 'tbz2', '.tar.bz2', None)), ('bz2', ('application/x-bzip2', 'tbz2', '.tar.bz2', None)),
)) ))
def getstyle(req, configfn, templatepath): def getstyle(req, configfn, templatepath):
fromreq = req.form.get('style', [None])[0]
styles = ( styles = (
fromreq, req.qsparams.get('style', None),
configfn('web', 'style'), configfn('web', 'style'),
'paper', 'paper',
) )
return styles, templater.stylemap(styles, templatepath) return styles, templater.stylemap(styles, templatepath)
def makebreadcrumb(url, prefix=''): def makebreadcrumb(url, prefix=''):
'''Return a 'URL breadcrumb' list '''Return a 'URL breadcrumb' list
# 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.req, 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', '')
# translate user-visible url structure to internal structure # translate user-visible url structure to internal structure
args = query.split('/', 2) args = query.split('/', 2)
if 'cmd' not in wsgireq.form and args and args[0]: if 'cmd' not in wsgireq.form and args and args[0]:
cmd = args.pop(0) cmd = args.pop(0)
style = cmd.rfind('-') style = cmd.rfind('-')
if style != -1: if style != -1:
wsgireq.form['style'] = [cmd[:style]] req.qsparams['style'] = cmd[:style]
cmd = cmd[style + 1:] cmd = cmd[style + 1:]
# avoid accepting e.g. style parameter as command # avoid accepting e.g. style parameter as command
if util.safehasattr(webcommands, cmd): if util.safehasattr(webcommands, cmd):
wsgireq.form['cmd'] = [cmd] wsgireq.form['cmd'] = [cmd]
if cmd == 'static': if cmd == 'static':
wsgireq.form['file'] = ['/'.join(args)] wsgireq.form['file'] = ['/'.join(args)]
else: else:
if args and args[0]: if args and args[0]:
node = args.pop(0).replace('%2F', '/') node = args.pop(0).replace('%2F', '/')
wsgireq.form['node'] = [node] wsgireq.form['node'] = [node]
if args: if args:
wsgireq.form['file'] = args wsgireq.form['file'] = args
ua = req.headers.get('User-Agent', '') ua = req.headers.get('User-Agent', '')
if cmd == 'rev' and 'mercurial' in ua: if cmd == 'rev' and 'mercurial' in ua:
wsgireq.form['style'] = ['raw'] req.qsparams['style'] = 'raw'
if cmd == 'archive': if cmd == 'archive':
fn = wsgireq.form['node'][0] fn = wsgireq.form['node'][0]
for type_, spec in rctx.archivespecs.iteritems(): for type_, spec in rctx.archivespecs.iteritems():
ext = spec[2] ext = spec[2]
if fn.endswith(ext): if fn.endswith(ext):
wsgireq.form['node'] = [fn[:-len(ext)]] wsgireq.form['node'] = [fn[:-len(ext)]]
wsgireq.form['type'] = [type_] wsgireq.form['type'] = [type_]
# Don't enable caching if using a CSP nonce because then it wouldn't # Don't enable caching if using a CSP nonce because then it wouldn't
# be a nonce. # be a nonce.
if rctx.configbool('web', 'cache') and not rctx.nonce: if rctx.configbool('web', 'cache') and not rctx.nonce:
caching(self, wsgireq) # sets ETag header or raises NOT_MODIFIED caching(self, wsgireq) # sets ETag header or raises NOT_MODIFIED
if cmd not in webcommands.__all__: if cmd not in webcommands.__all__:
msg = 'no such method: %s' % cmd msg = 'no such method: %s' % cmd
raise ErrorResponse(HTTP_BAD_REQUEST, msg) raise ErrorResponse(HTTP_BAD_REQUEST, msg)
elif cmd == 'file' and 'raw' in wsgireq.form.get('style', []): elif cmd == 'file' and req.qsparams.get('style') == 'raw':
rctx.ctype = ctype rctx.ctype = ctype
content = webcommands.rawfile(rctx, wsgireq, tmpl) content = webcommands.rawfile(rctx, wsgireq, tmpl)
else: else:
content = getattr(webcommands, cmd)(rctx, wsgireq, tmpl) content = getattr(webcommands, cmd)(rctx, wsgireq, tmpl)
wsgireq.respond(HTTP_OK, ctype) wsgireq.respond(HTTP_OK, ctype)
return content return content

mercurial/hgweb/hgwebdir_mod.py

self.updatereqenv(wsgireq.env) self.updatereqenv(wsgireq.env)
url = wsgireq.env.get('SCRIPT_NAME', '') url = wsgireq.env.get('SCRIPT_NAME', '')
if not url.endswith('/'): if not url.endswith('/'):
url += '/' url += '/'
vars = {} vars = {}
styles, (style, mapfile) = hgweb_mod.getstyle(wsgireq, config, styles, (style, mapfile) = hgweb_mod.getstyle(wsgireq.req, config,
self.templatepath) self.templatepath)
if style == styles[0]: if style == styles[0]:
vars['style'] = style vars['style'] = style
sessionvars = webutil.sessionvars(vars, r'?') sessionvars = webutil.sessionvars(vars, r'?')
logourl = config('web', 'logourl') logourl = config('web', 'logourl')
logoimg = config('web', 'logoimg') logoimg = config('web', 'logoimg')
staticurl = config('web', 'staticurl') or url + 'static/' staticurl = config('web', 'staticurl') or url + 'static/'

mercurial/hgweb/webcommands.py

raise raise
if fctx is not None: if fctx is not None:
path = fctx.path() path = fctx.path()
ctx = fctx.changectx() ctx = fctx.changectx()
basectx = ctx.p1() basectx = ctx.p1()
style = web.config('web', 'style') style = web.config('web', 'style')
if 'style' in req.form: if 'style' in req.req.qsparams:
style = req.form['style'][0] style = req.req.qsparams['style']
diffs = webutil.diffs(web, tmpl, ctx, basectx, [path], style) diffs = webutil.diffs(web, tmpl, ctx, basectx, [path], style)
if fctx is not None: if fctx is not None:
rename = webutil.renamelink(fctx) rename = webutil.renamelink(fctx)
ctx = fctx ctx = fctx
else: else:
rename = [] rename = []
ctx = ctx ctx = ctx
end = min(count, start + revcount) # last rev on this page end = min(count, start + revcount) # last rev on this page
parity = paritygen(web.stripecount, offset=start - end) parity = paritygen(web.stripecount, offset=start - end)
repo = web.repo repo = web.repo
revs = fctx.filelog().revs(start, end - 1) revs = fctx.filelog().revs(start, end - 1)
entries = [] entries = []
diffstyle = web.config('web', 'style') diffstyle = web.config('web', 'style')
if 'style' in req.form: if 'style' in req.req.qsparams:
diffstyle = req.form['style'][0] diffstyle = req.req.qsparams['style']
def diff(fctx, linerange=None): def diff(fctx, linerange=None):
ctx = fctx.changectx() ctx = fctx.changectx()
basectx = ctx.p1() basectx = ctx.p1()
path = fctx.path() path = fctx.path()
return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle, return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle,
linerange=linerange, linerange=linerange,
lineidprefix='%s-' % ctx.hex()[:12]) lineidprefix='%s-' % ctx.hex()[:12])

mercurial/hgweb/webutil.py

node=ctx.hex(), file=f, blockno=blockno + 1, node=ctx.hex(), file=f, blockno=blockno + 1,
parity=next(parity))) parity=next(parity)))
basectx = basechangectx(web.repo, req) basectx = basechangectx(web.repo, req)
if basectx is None: if basectx is None:
basectx = ctx.p1() basectx = ctx.p1()
style = web.config('web', 'style') style = web.config('web', 'style')
if 'style' in req.form: if 'style' in req.req.qsparams:
style = req.form['style'][0] style = req.req.qsparams['style']
diff = diffs(web, tmpl, ctx, basectx, None, style) diff = diffs(web, tmpl, ctx, basectx, None, style)
parity = paritygen(web.stripecount) parity = paritygen(web.stripecount)
diffstatsgen = diffstatgen(ctx, basectx) diffstatsgen = diffstatgen(ctx, basectx)
diffstats = diffstat(tmpl, ctx, diffstatsgen, parity) diffstats = diffstat(tmpl, ctx, diffstatsgen, parity)
return dict( return dict(