D2781: hgweb: perform all parameter lookup via qsparams
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Sat Mar 10 20:57:54 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
I think I managed to update all call sites using wsgirequest.form
to use parsedrequest.qsparams.
Since behavior of qsparams is to retrieve last value, behavior will
change if a parameter was specified multiple times. But I think this
is acceptable.
I'm not a fan of the `req.req.qsparams` pattern. And some of the
modified code could be written better. But I was aiming for a
straight port with this change. Cleanup can come later.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2781
AFFECTED FILES
mercurial/hgweb/hgweb_mod.py
mercurial/hgweb/hgwebdir_mod.py
mercurial/hgweb/webcommands.py
mercurial/hgweb/webutil.py
tests/hgweberror.py
CHANGE DETAILS
diff --git a/tests/hgweberror.py b/tests/hgweberror.py
--- a/tests/hgweberror.py
+++ b/tests/hgweberror.py
@@ -10,7 +10,7 @@
'''Dummy web command that raises an uncaught Exception.'''
# Simulate an error after partial response.
- if 'partialresponse' in req.form:
+ if 'partialresponse' in req.req.qsparams:
req.respond(200, 'text/plain')
req.write('partial content\n')
diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -177,7 +177,7 @@
section=section, whitespace=True)
for k in ('ignorews', 'ignorewsamount', 'ignorewseol', 'ignoreblanklines'):
- v = req.form.get(k, [None])[0]
+ v = req.req.qsparams.get(k)
if v is not None:
v = util.parsebool(v)
setattr(diffopts, k, v if v is not None else True)
@@ -295,34 +295,34 @@
def changectx(repo, req):
changeid = "tip"
- if 'node' in req.form:
- changeid = req.form['node'][0]
+ if 'node' in req.req.qsparams:
+ changeid = req.req.qsparams['node']
ipos = changeid.find(':')
if ipos != -1:
changeid = changeid[(ipos + 1):]
- elif 'manifest' in req.form:
- changeid = req.form['manifest'][0]
+ elif 'manifest' in req.req.qsparams:
+ changeid = req.req.qsparams['manifest']
return changeidctx(repo, changeid)
def basechangectx(repo, req):
- if 'node' in req.form:
- changeid = req.form['node'][0]
+ if 'node' in req.req.qsparams:
+ changeid = req.req.qsparams['node']
ipos = changeid.find(':')
if ipos != -1:
changeid = changeid[:ipos]
return changeidctx(repo, changeid)
return None
def filectx(repo, req):
- if 'file' not in req.form:
+ if 'file' not in req.req.qsparams:
raise ErrorResponse(HTTP_NOT_FOUND, 'file not given')
- path = cleanpath(repo, req.form['file'][0])
- if 'node' in req.form:
- changeid = req.form['node'][0]
- elif 'filenode' in req.form:
- changeid = req.form['filenode'][0]
+ path = cleanpath(repo, req.req.qsparams['file'])
+ if 'node' in req.req.qsparams:
+ changeid = req.req.qsparams['node']
+ elif 'filenode' in req.req.qsparams:
+ changeid = req.req.qsparams['filenode']
else:
raise ErrorResponse(HTTP_NOT_FOUND, 'node or filenode not given')
try:
@@ -333,8 +333,8 @@
return fctx
def linerange(req):
- linerange = req.form.get('linerange')
- if linerange is None:
+ linerange = req.req.qsparams.getall('linerange')
+ if not linerange:
return None
if len(linerange) > 1:
raise ErrorResponse(HTTP_BAD_REQUEST,
@@ -412,8 +412,8 @@
return entry
def symrevorshortnode(req, ctx):
- if 'node' in req.form:
- return templatefilters.revescape(req.form['node'][0])
+ if 'node' in req.req.qsparams:
+ return templatefilters.revescape(req.req.qsparams['node'])
else:
return short(ctx.node())
diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -85,16 +85,16 @@
file will be shown. This form is equivalent to the ``filelog`` handler.
"""
- if 'file' in req.form and req.form['file'][0]:
+ if req.req.qsparams.get('file'):
return filelog(web, req, tmpl)
else:
return changelog(web, req, tmpl)
@webcommand('rawfile')
def rawfile(web, req, tmpl):
guessmime = web.configbool('web', 'guessmime')
- path = webutil.cleanpath(web.repo, req.form.get('file', [''])[0])
+ path = webutil.cleanpath(web.repo, req.req.qsparams.get('file', ''))
if not path:
content = manifest(web, req, tmpl)
req.respond(HTTP_OK, web.ctype)
@@ -173,7 +173,7 @@
If ``path`` is not defined, information about the root directory will
be rendered.
"""
- path = webutil.cleanpath(web.repo, req.form.get('file', [''])[0])
+ path = webutil.cleanpath(web.repo, req.req.qsparams.get('file', ''))
if not path:
return manifest(web, req, tmpl)
try:
@@ -289,11 +289,11 @@
if count >= revcount:
break
- query = req.form['rev'][0]
+ query = req.req.qsparams['rev']
revcount = web.maxchanges
- if 'revcount' in req.form:
+ if 'revcount' in req.req.qsparams:
try:
- revcount = int(req.form.get('revcount', [revcount])[0])
+ revcount = int(req.req.qsparams.get('revcount', revcount))
revcount = max(revcount, 1)
tmpl.defaults['sessionvars']['revcount'] = revcount
except ValueError:
@@ -308,7 +308,7 @@
mode, funcarg = getsearchmode(query)
- if 'forcekw' in req.form:
+ if 'forcekw' in req.req.qsparams:
showforcekw = ''
showunforcekw = searchfuncs[mode][1]
mode = MODE_KEYWORD
@@ -358,10 +358,10 @@
"""
query = ''
- if 'node' in req.form:
+ if 'node' in req.req.qsparams:
ctx = webutil.changectx(web.repo, req)
symrev = webutil.symrevorshortnode(req, ctx)
- elif 'rev' in req.form:
+ elif 'rev' in req.req.qsparams:
return _search(web, req, tmpl)
else:
ctx = web.repo['tip']
@@ -386,9 +386,9 @@
else:
revcount = web.maxchanges
- if 'revcount' in req.form:
+ if 'revcount' in req.req.qsparams:
try:
- revcount = int(req.form.get('revcount', [revcount])[0])
+ revcount = int(req.req.qsparams.get('revcount', revcount))
revcount = max(revcount, 1)
tmpl.defaults['sessionvars']['revcount'] = revcount
except ValueError:
@@ -481,13 +481,13 @@
The ``manifest`` template will be rendered for this handler.
"""
- if 'node' in req.form:
+ if 'node' in req.req.qsparams:
ctx = webutil.changectx(web.repo, req)
symrev = webutil.symrevorshortnode(req, ctx)
else:
ctx = web.repo['tip']
symrev = 'tip'
- path = webutil.cleanpath(web.repo, req.form.get('file', [''])[0])
+ path = webutil.cleanpath(web.repo, req.req.qsparams.get('file', ''))
mf = ctx.manifest()
node = ctx.node()
@@ -752,7 +752,7 @@
fctx = webutil.filectx(web.repo, req)
except LookupError:
ctx = webutil.changectx(web.repo, req)
- path = webutil.cleanpath(web.repo, req.form['file'][0])
+ path = webutil.cleanpath(web.repo, req.req.qsparams['file'])
if path not in ctx.files():
raise
@@ -799,13 +799,13 @@
The ``filecomparison`` template is rendered.
"""
ctx = webutil.changectx(web.repo, req)
- if 'file' not in req.form:
+ if 'file' not in req.req.qsparams:
raise ErrorResponse(HTTP_NOT_FOUND, 'file not given')
- path = webutil.cleanpath(web.repo, req.form['file'][0])
+ path = webutil.cleanpath(web.repo, req.req.qsparams['file'])
parsecontext = lambda v: v == 'full' and -1 or int(v)
- if 'context' in req.form:
- context = parsecontext(req.form['context'][0])
+ if 'context' in req.req.qsparams:
+ context = parsecontext(req.req.qsparams['context'])
else:
context = parsecontext(web.config('web', 'comparisoncontext', '5'))
@@ -964,7 +964,7 @@
f = fctx.path()
fl = fctx.filelog()
except error.LookupError:
- f = webutil.cleanpath(web.repo, req.form['file'][0])
+ f = webutil.cleanpath(web.repo, req.req.qsparams['file'])
fl = web.repo.file(f)
numrevs = len(fl)
if not numrevs: # file doesn't exist at all
@@ -979,9 +979,9 @@
fctx = web.repo.filectx(f, fl.linkrev(frev))
revcount = web.maxshortchanges
- if 'revcount' in req.form:
+ if 'revcount' in req.req.qsparams:
try:
- revcount = int(req.form.get('revcount', [revcount])[0])
+ revcount = int(req.req.qsparams.get('revcount', revcount))
revcount = max(revcount, 1)
tmpl.defaults['sessionvars']['revcount'] = revcount
except ValueError:
@@ -994,12 +994,12 @@
morevars = copy.copy(tmpl.defaults['sessionvars'])
morevars['revcount'] = revcount * 2
- patch = 'patch' in req.form
+ patch = 'patch' in req.req.qsparams
if patch:
- lessvars['patch'] = morevars['patch'] = req.form['patch'][0]
- descend = 'descend' in req.form
+ lessvars['patch'] = morevars['patch'] = req.req.qsparams['patch']
+ descend = 'descend' in req.req.qsparams
if descend:
- lessvars['descend'] = morevars['descend'] = req.form['descend'][0]
+ lessvars['descend'] = morevars['descend'] = req.req.qsparams['descend']
count = fctx.filerev() + 1
start = max(0, count - revcount) # first rev on this page
@@ -1103,9 +1103,9 @@
No template is used for this handler. Raw, binary content is generated.
"""
- type_ = req.form.get('type', [None])[0]
+ type_ = req.req.qsparams.get('type')
allowed = web.configlist("web", "allow_archive")
- key = req.form['node'][0]
+ key = req.req.qsparams['node']
if type_ not in web.archivespecs:
msg = 'Unsupported archive type: %s' % type_
@@ -1126,15 +1126,15 @@
ctx = webutil.changectx(web.repo, req)
pats = []
match = scmutil.match(ctx, [])
- file = req.form.get('file', None)
+ file = req.req.qsparams.get('file')
if file:
- pats = ['path:' + file[0]]
+ pats = ['path:' + file]
match = scmutil.match(ctx, pats, default='path')
if pats:
files = [f for f in ctx.manifest().keys() if match(f)]
if not files:
raise ErrorResponse(HTTP_NOT_FOUND,
- 'file(s) not found: %s' % file[0])
+ 'file(s) not found: %s' % file)
mimetype, artype, extension, encoding = web.archivespecs[type_]
headers = [
@@ -1153,7 +1153,7 @@
@webcommand('static')
def static(web, req, tmpl):
- fname = req.form['file'][0]
+ fname = req.req.qsparams['file']
# a repo owner may set web.static in .hg/hgrc to get any file
# readable by the user running the CGI script
static = web.config("web", "static", None, untrusted=False)
@@ -1189,7 +1189,7 @@
This handler will render the ``graph`` template.
"""
- if 'node' in req.form:
+ if 'node' in req.req.qsparams:
ctx = webutil.changectx(web.repo, req)
symrev = webutil.symrevorshortnode(req, ctx)
else:
@@ -1199,9 +1199,9 @@
bg_height = 39
revcount = web.maxshortchanges
- if 'revcount' in req.form:
+ if 'revcount' in req.req.qsparams:
try:
- revcount = int(req.form.get('revcount', [revcount])[0])
+ revcount = int(req.req.qsparams.get('revcount', revcount))
revcount = max(revcount, 1)
tmpl.defaults['sessionvars']['revcount'] = revcount
except ValueError:
@@ -1212,7 +1212,7 @@
morevars = copy.copy(tmpl.defaults['sessionvars'])
morevars['revcount'] = revcount * 2
- graphtop = req.form.get('graphtop', [ctx.hex()])[0]
+ graphtop = req.req.qsparams.get('graphtop', ctx.hex())
graphvars = copy.copy(tmpl.defaults['sessionvars'])
graphvars['graphtop'] = graphtop
@@ -1342,7 +1342,7 @@
"""
from .. import commands, help as helpmod # avoid cycle
- topicname = req.form.get('node', [None])[0]
+ topicname = req.req.qsparams.get('node')
if not topicname:
def topics(**map):
for entries, summary, _doc in helpmod.helptable:
diff --git a/mercurial/hgweb/hgwebdir_mod.py b/mercurial/hgweb/hgwebdir_mod.py
--- a/mercurial/hgweb/hgwebdir_mod.py
+++ b/mercurial/hgweb/hgwebdir_mod.py
@@ -229,6 +229,8 @@
yield r
def _runwsgi(self, wsgireq):
+ req = wsgireq.req
+
try:
self.refresh()
@@ -242,11 +244,11 @@
ctype = templater.stringify(ctype)
# a static file
- if virtual.startswith('static/') or 'static' in wsgireq.form:
+ if virtual.startswith('static/') or 'static' in req.qsparams:
if virtual.startswith('static/'):
fname = virtual[7:]
else:
- fname = wsgireq.form['static'][0]
+ fname = req.qsparams['static']
static = self.ui.config("web", "static", None,
untrusted=False)
if not static:
@@ -471,8 +473,8 @@
self.refresh()
sortable = ["name", "description", "contact", "lastchange"]
sortcolumn, descending = sortdefault
- if 'sort' in wsgireq.form:
- sortcolumn = wsgireq.form['sort'][0]
+ if 'sort' in wsgireq.req.qsparams:
+ sortcolum = wsgireq.req.qsparams['sort']
descending = sortcolumn.startswith('-')
if descending:
sortcolumn = sortcolumn[1:]
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
@@ -332,7 +332,7 @@
# translate user-visible url structure to internal structure
args = query.split('/', 2)
- if 'cmd' not in wsgireq.form and args and args[0]:
+ if 'cmd' not in req.qsparams and args and args[0]:
cmd = args.pop(0)
style = cmd.rfind('-')
if style != -1:
@@ -364,16 +364,16 @@
req.qsparams['style'] = 'raw'
if cmd == 'archive':
- fn = wsgireq.form['node'][0]
+ fn = req.qsparams['node']
for type_, spec in rctx.archivespecs.iteritems():
ext = spec[2]
if fn.endswith(ext):
wsgireq.form['node'] = [fn[:-len(ext)]]
- req.qsparams['node'] = fn[:-len(next)]
+ req.qsparams['node'] = fn[:-len(ext)]
wsgireq.form['type'] = [type_]
req.qsparams['type'] = type_
else:
- cmd = wsgireq.form.get('cmd', [''])[0]
+ cmd = req.qsparams.get('cmd', '')
# process the web interface request
@@ -389,7 +389,7 @@
if cmd == '':
wsgireq.form['cmd'] = [tmpl.cache['default']]
req.qsparams['cmd'] = tmpl.cache['default']
- cmd = wsgireq.form['cmd'][0]
+ cmd = req.qsparams['cmd']
# Don't enable caching if using a CSP nonce because then it wouldn't
# be a nonce.
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list