hgweb: perform all parameter lookup via qsparams
authorGregory Szorc <gregory.szorc@gmail.com>
Sat, 10 Mar 2018 12:36:36 -0800
changeset 36865 3d60a22e27f5
parent 36864 4e06e8336634
child 36866 cf69df7ea385
hgweb: perform all parameter lookup via qsparams 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. Differential Revision: https://phab.mercurial-scm.org/D2781
mercurial/hgweb/hgweb_mod.py
mercurial/hgweb/hgwebdir_mod.py
mercurial/hgweb/webcommands.py
mercurial/hgweb/webutil.py
tests/hgweberror.py
--- a/mercurial/hgweb/hgweb_mod.py	Sat Mar 10 12:11:26 2018 -0800
+++ b/mercurial/hgweb/hgweb_mod.py	Sat Mar 10 12:36:36 2018 -0800
@@ -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.
--- a/mercurial/hgweb/hgwebdir_mod.py	Sat Mar 10 12:11:26 2018 -0800
+++ b/mercurial/hgweb/hgwebdir_mod.py	Sat Mar 10 12:36:36 2018 -0800
@@ -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:]
--- a/mercurial/hgweb/webcommands.py	Sat Mar 10 12:11:26 2018 -0800
+++ b/mercurial/hgweb/webcommands.py	Sat Mar 10 12:36:36 2018 -0800
@@ -85,7 +85,7 @@
     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)
@@ -94,7 +94,7 @@
 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:
--- a/mercurial/hgweb/webutil.py	Sat Mar 10 12:11:26 2018 -0800
+++ b/mercurial/hgweb/webutil.py	Sat Mar 10 12:36:36 2018 -0800
@@ -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,19 +295,19 @@
 
 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]
@@ -316,13 +316,13 @@
     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())
 
--- a/tests/hgweberror.py	Sat Mar 10 12:11:26 2018 -0800
+++ b/tests/hgweberror.py	Sat Mar 10 12:36:36 2018 -0800
@@ -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')