changeset 5098:ff588e9e24d6

simplify getAttachUrl: remove upload parameter upload=x didn't influence drawing url generation at all, for file url generation one can just give do='upload_form' if one wants the upload url. URL args for do='upload_form' were streamlined to use target=x instead of rename=x so it is more similar to the other code. Removed test for "tainted" file names in URLs. We must not "taint" file names for URLs. Filenames in URLs need to be url-quoted. If a URL is used in html, it needs to be escaped.
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Sun, 13 Sep 2009 00:06:04 +0200
parents c8ea1985d348
children 753ac1cbbc38
files MoinMoin/action/AttachFile.py MoinMoin/action/_tests/test_attachfile.py MoinMoin/action/anywikidraw.py MoinMoin/action/twikidraw.py MoinMoin/formatter/text_docbook.py MoinMoin/formatter/text_html.py MoinMoin/macro/EmbedObject.py
diffstat 7 files changed, 20 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/action/AttachFile.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/action/AttachFile.py	Sun Sep 13 00:06:04 2009 +0200
@@ -82,13 +82,12 @@
     else:
         return u"/".join(pieces[:-1]), pieces[-1]
 
-def getAttachUrl(pagename, filename, request, addts=0, escaped=0, do='get', drawing='', upload=False):
-    """ Get URL that points to attachment `filename` of page `pagename`. """
+def getAttachUrl(pagename, filename, request, addts=0, escaped=0, do='get', drawing=''):
+    """ Get URL that points to attachment `filename` of page `pagename`.
+        For upload url (files, not drawings), call with do='upload_form'.
+    """
     if not drawing:
-        if upload:
-            url = request.href(pagename, action=action_name, rename=wikiutil.taintfilename(filename))
-        else:
-            url = request.href(pagename, action=action_name, do=do, target=filename)
+        url = request.href(pagename, action=action_name, do=do, target=filename)
     else:
         url = request.href(pagename, action=request.cfg.drawing_action, target=drawing)
     return url
@@ -416,8 +415,8 @@
 <dl>
 <dt>%(upload_label_file)s</dt>
 <dd><input type="file" name="file" size="50"></dd>
-<dt>%(upload_label_rename)s</dt>
-<dd><input type="text" name="rename" size="50" value="%(rename)s"></dd>
+<dt>%(upload_label_target)s</dt>
+<dd><input type="text" name="target" size="50" value="%(target)s"></dd>
 <dt>%(upload_label_overwrite)s</dt>
 <dd><input type="checkbox" name="overwrite" value="1" %(overwrite_checked)s></dd>
 </dl>
@@ -432,8 +431,8 @@
     'url': request.href(pagename),
     'action_name': action_name,
     'upload_label_file': _('File to upload'),
-    'upload_label_rename': _('Rename to'),
-    'rename': wikiutil.escape(request.values.get('rename', ''), 1),
+    'upload_label_target': _('Rename to'),
+    'target': wikiutil.escape(request.values.get('target', ''), 1),
     'upload_label_overwrite': _('Overwrite existing attachment of same name'),
     'overwrite_checked': ('', 'checked')[request.form.get('overwrite', '0') == '1'],
     'upload_button': _('Upload'),
@@ -510,10 +509,8 @@
     if overwrite and not request.user.may.delete(pagename):
         return _('You are not allowed to overwrite a file attachment of this page.')
 
-    rename = form.get('rename', u'').strip()
-    if rename:
-        target = rename
-    else:
+    target = form.get('target', u'').strip()
+    if not target:
         target = file_upload.filename or u''
 
     target = wikiutil.clean_input(target)
--- a/MoinMoin/action/_tests/test_attachfile.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/action/_tests/test_attachfile.py	Sun Sep 13 00:06:04 2009 +0200
@@ -66,14 +66,4 @@
 
         assert file_exists
 
-    def test_getAttachUrl(self):
-        """
-        Tests if AttachFile.getAttachUrl taints a filename
-        """
-        filename = "<test2.txt>"
-        expect = "rename=_test2.txt_"
-        result = AttachFile.getAttachUrl(self.pagename, filename, self.request, upload=True)
-
-        assert expect in result
-
 coverage_modules = ['MoinMoin.action.AttachFile']
--- a/MoinMoin/action/anywikidraw.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/action/anywikidraw.py	Sun Sep 13 00:06:04 2009 +0200
@@ -44,7 +44,7 @@
     kw['title'] = "drawing:%s" % wikiutil.quoteWikinameURL(url)
     pagename, drawing = AttachFile.absoluteName(url, self.page.page_name)
     containername = wikiutil.taintfilename(drawing) + ".adraw"
-    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing, upload=True)
+    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing)
     ci = AttachFile.ContainerItem(self.request, pagename, containername)
     if not ci.exists():
         title = _('Create new drawing "%(filename)s (opens in new window)"') % {'filename': drawing}
@@ -61,7 +61,7 @@
     pagename, drawing = AttachFile.absoluteName(url, self.page.page_name)
     containername = wikiutil.taintfilename(drawing) + ".adraw"
 
-    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing, upload=True)
+    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing)
     ci = AttachFile.ContainerItem(self.request, pagename, containername)
     if not ci.exists():
         title = _('Create new drawing "%(filename)s (opens in new window)"') % {'filename': drawing}
--- a/MoinMoin/action/twikidraw.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/action/twikidraw.py	Sun Sep 13 00:06:04 2009 +0200
@@ -46,7 +46,7 @@
     kw['title'] = "drawing:%s" % wikiutil.quoteWikinameURL(url)
     pagename, drawing = AttachFile.absoluteName(url, self.page.page_name)
     containername = wikiutil.taintfilename(drawing) + ".tdraw"
-    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing, upload=True)
+    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing)
     ci = AttachFile.ContainerItem(self.request, pagename, containername)
     if not ci.exists():
         title = _('Create new drawing "%(filename)s (opens in new window)"') % {'filename': drawing}
@@ -63,7 +63,7 @@
     pagename, drawing = AttachFile.absoluteName(url, self.page.page_name)
     containername = wikiutil.taintfilename(drawing) + ".tdraw"
 
-    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing, upload=True)
+    drawing_url = AttachFile.getAttachUrl(pagename, containername, self.request, drawing=drawing)
     ci = AttachFile.ContainerItem(self.request, pagename, containername)
     if not ci.exists():
         title = _('Create new drawing "%(filename)s (opens in new window)"') % {'filename': drawing}
--- a/MoinMoin/formatter/text_docbook.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/formatter/text_docbook.py	Sun Sep 13 00:06:04 2009 +0200
@@ -422,8 +422,7 @@
             return self.text("[attachment:%s]" % url)
         else:
             return self.image(
-                src=AttachFile.getAttachUrl(pagename, filename,
-                                            self.request, addts=1),
+                src=AttachFile.getAttachUrl(pagename, filename, self.request, addts=1),
                 attachment_title=url,
                 **kw)
 
--- a/MoinMoin/formatter/text_html.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/formatter/text_html.py	Sun Sep 13 00:06:04 2009 +0200
@@ -623,7 +623,7 @@
                 title = "attachment:%s" % url
                 css = 'attachment'
             else:
-                target = AttachFile.getAttachUrl(pagename, fname, self.request, upload=True)
+                target = AttachFile.getAttachUrl(pagename, fname, self.request, do='upload_form')
                 title = _('Upload new attachment "%(filename)s"') % {'filename': fname}
                 css = 'attachment nonexistent'
             return self.url(on, target, css=css, title=title)
@@ -637,7 +637,7 @@
         exists = AttachFile.exists(self.request, pagename, fname)
         if exists:
             kw['css'] = 'attachment'
-            kw['src'] = AttachFile.getAttachUrl(pagename, filename, self.request, addts=1)
+            kw['src'] = AttachFile.getAttachUrl(pagename, fname, self.request, addts=1)
             title = _('Inlined image: %(url)s') % {'url': self.text(url)}
             if not 'title' in kw:
                 kw['title'] = title
@@ -649,7 +649,7 @@
             title = _('Upload new attachment "%(filename)s"') % {'filename': fname}
             img = self.icon('attachimg')
             css = 'nonexistent'
-            target = AttachFile.getAttachUrl(pagename, fname, self.request, upload=True)
+            target = AttachFile.getAttachUrl(pagename, fname, self.request, do='upload_form')
             return self.url(1, target, css=css, title=title) + img + self.url(0)
 
     def attachment_drawing(self, url, text, **kw):
--- a/MoinMoin/macro/EmbedObject.py	Sat Sep 12 21:42:55 2009 +0200
+++ b/MoinMoin/macro/EmbedObject.py	Sun Sep 13 00:06:04 2009 +0200
@@ -84,7 +84,7 @@
 
         if not AttachFile.exists(request, pagename, fname):
             linktext = _('Upload new attachment "%(filename)s"') % {'filename': fname}
-            target = AttachFile.getAttachUrl(pagename, fname, request, upload=True)
+            target = AttachFile.getAttachUrl(pagename, fname, request, do='upload_form')
             return (fmt.url(1, target) +
                     fmt.text(linktext) +
                     fmt.url(0))