Changeset 1df7f11 in trunk

Timestamp:

2012-10-25T00:01:25Z (13 years ago)

Author:

David-Sarah Hopwood <david-sarah@…>

Branches:

master

Children:

b3bf207

Parents:

05d0b8b

Message:

src/allmydata/web/directory.py: fix HTML double-encoding issue for filenames.

Nevow automatically HTML-escapes strings passed in stan without a raw marker.
Written by MK_FG. fixes #1143

Signed-off-by: David-Sarah Hopwood <david-sarah@…>

Location:

src/allmydata

Files:

• test/test_web.py (modified) (15 diffs)
• web/directory.py (modified) (5 diffs)

src/allmydata/test/test_web.py ¶

@@ -7 +7 @@
 from twisted.internet import defer, reactor
 from twisted.internet.task import Clock
-from twisted.web import client, error, http
+from twisted.web import client, error, http, html
 from twisted.python import failure, log
 
 from foolscap.api import fireEventually, flushEventualQueue
 
+from nevow.util import escapeToXML
 from nevow import rend
 
@@ -257 +258 @@
             foo.set_uri(u"blockingfile", blocking_uri, blocking_uri)
 
+            # filenode to test for html encoding issues
+            self._htmlname_unicode = u"<&weirdly'named\"file>>>_<iframe />.txt"
+            self._htmlname_raw = self._htmlname_unicode.encode('utf-8')
+            self._htmlname_urlencoded = urllib.quote(self._htmlname_raw, '')
+            self._htmlname_escaped = escapeToXML(self._htmlname_raw)
+            self._htmlname_escaped_attr = html.escape(self._htmlname_raw)
+            self._htmlname_escaped_double = escapeToXML(html.escape(self._htmlname_raw))
+            self.HTMLNAME_CONTENTS, n, self._htmlname_txt_uri = self.makefile(0)
+            foo.set_uri(self._htmlname_unicode, self._htmlname_txt_uri, self._htmlname_txt_uri)
+
             unicode_filename = u"n\u00fc.txt" # n u-umlaut . t x t
             # ok, unicode calls it LATIN SMALL LETTER U WITH DIAERESIS but I
@@ -281 +292 @@
             # public/foo/quux.txt
             # public/foo/blockingfile
+            # public/foo/<&weirdly'named\"file>>>_<iframe />.txt
             # public/foo/empty/
             # public/foo/sub/
@@ -366 +378 @@
         kidnames = sorted([unicode(n) for n in data[1]["children"]])
         self.failUnlessEqual(kidnames,
-                             [u"bar.txt", u"baz.txt", u"blockingfile",
-                              u"empty", u"n\u00fc.txt", u"quux.txt", u"sub"])
+                             [self._htmlname_unicode, u"bar.txt", u"baz.txt",
+                              u"blockingfile", u"empty", u"n\u00fc.txt", u"quux.txt", u"sub"])
         kids = dict( [(unicode(name),value)
                       for (name,value)
@@ -1342 +1354 @@
         return d
 
+    def test_GET_DIRECTORY_html_filenode_encoding(self):
+        d = self.GET(self.public_url + "/foo", followRedirect=True)
+        def _check(html):
+            # Check if encoded entries are there
+            self.failUnlessIn('@@named=/' + self._htmlname_urlencoded + '">'
+                              + self._htmlname_escaped + '</a>', html)
+            self.failUnlessIn('value="' + self._htmlname_escaped_attr + '"', html)
+            self.failIfIn(self._htmlname_escaped_double, html)
+            # Make sure that Nevow escaping actually works by checking for unsafe characters
+            # and that '&' is escaped.
+            for entity in '<>':
+                self.failUnlessIn(entity, self._htmlname_raw)
+                self.failIfIn(entity, self._htmlname_escaped)
+            self.failUnlessIn('&', re.sub(r'&(amp|lt|gt|quot|apos);', '', self._htmlname_raw))
+            self.failIfIn('&', re.sub(r'&(amp|lt|gt|quot|apos);', '', self._htmlname_escaped))
+        d.addCallback(_check)
+        return d
+
     def test_GET_root_html(self):
         d = self.GET("/")
@@ -1564 +1594 @@
         d.addCallback(self.get_operation_results, "127", "json")
         def _got_json(stats):
-            expected = {"count-immutable-files": 3,
+            expected = {"count-immutable-files": 4,
                         "count-mutable-files": 2,
                         "count-literal-files": 0,
-                        "count-files": 5,
+                        "count-files": 6,
                         "count-directories": 3,
-                        "size-immutable-files": 57,
+                        "size-immutable-files": 76,
                         "size-literal-files": 0,
                         #"size-directories": 1912, # varies
                         #"largest-directory": 1590,
-                        "largest-directory-children": 7,
+                        "largest-directory-children": 8,
                         "largest-immutable-file": 19,
                         }
@@ -1581 +1611 @@
                                            (k, stats[k], v))
             self.failUnlessReallyEqual(stats["size-files-histogram"],
-                                       [ [11, 31, 3] ])
+                                       [ [11, 31, 4] ])
         d.addCallback(_got_json)
         return d
@@ -1590 +1620 @@
             self.failUnless(res.endswith("\n"))
             units = [simplejson.loads(t) for t in res[:-1].split("\n")]
-            self.failUnlessReallyEqual(len(units), 9)
+            self.failUnlessReallyEqual(len(units), 10)
             self.failUnlessEqual(units[-1]["type"], "stats")
             first = units[0]
@@ -2435 +2465 @@
         d.addCallback(lambda res:
                       self.failUnlessNodeKeysAre(self._foo_node,
-                                                 [u"bar.txt", u"baz.txt", u"blockingfile",
+                                                 [self._htmlname_unicode,
+                                                  u"bar.txt", u"baz.txt", u"blockingfile",
                                                   u"empty", u"n\u00fc.txt", u"quux.txt",
                                                   u"sub"]))
@@ -2627 +2658 @@
         def _check_json(data):
             self.failUnlessReallyEqual(data["finished"], True)
-            self.failUnlessReallyEqual(data["count-objects-checked"], 10)
-            self.failUnlessReallyEqual(data["count-objects-healthy"], 10)
+            self.failUnlessReallyEqual(data["count-objects-checked"], 11)
+            self.failUnlessReallyEqual(data["count-objects-healthy"], 11)
         d.addCallback(_check_json)
         d.addCallback(self.get_operation_results, "123", "html")
         def _check_html(res):
-            self.failUnlessIn("Objects Checked: <span>10</span>", res)
-            self.failUnlessIn("Objects Healthy: <span>10</span>", res)
+            self.failUnlessIn("Objects Checked: <span>11</span>", res)
+            self.failUnlessIn("Objects Healthy: <span>11</span>", res)
             self.failUnlessIn(FAVICON_MARKUP, res)
         d.addCallback(_check_html)
@@ -2663 +2694 @@
         def _check_json(data):
             self.failUnlessReallyEqual(data["finished"], True)
-            self.failUnlessReallyEqual(data["count-objects-checked"], 10)
-            self.failUnlessReallyEqual(data["count-objects-healthy-pre-repair"], 10)
+            self.failUnlessReallyEqual(data["count-objects-checked"], 11)
+            self.failUnlessReallyEqual(data["count-objects-healthy-pre-repair"], 11)
             self.failUnlessReallyEqual(data["count-objects-unhealthy-pre-repair"], 0)
             self.failUnlessReallyEqual(data["count-corrupt-shares-pre-repair"], 0)
@@ -2670 +2701 @@
             self.failUnlessReallyEqual(data["count-repairs-successful"], 0)
             self.failUnlessReallyEqual(data["count-repairs-unsuccessful"], 0)
-            self.failUnlessReallyEqual(data["count-objects-healthy-post-repair"], 10)
+            self.failUnlessReallyEqual(data["count-objects-healthy-post-repair"], 11)
             self.failUnlessReallyEqual(data["count-objects-unhealthy-post-repair"], 0)
             self.failUnlessReallyEqual(data["count-corrupt-shares-post-repair"], 0)
@@ -2676 +2707 @@
         d.addCallback(self.get_operation_results, "124", "html")
         def _check_html(res):
-            self.failUnlessIn("Objects Checked: <span>10</span>", res)
-
-            self.failUnlessIn("Objects Healthy (before repair): <span>10</span>", res)
+            self.failUnlessIn("Objects Checked: <span>11</span>", res)
+
+            self.failUnlessIn("Objects Healthy (before repair): <span>11</span>", res)
             self.failUnlessIn("Objects Unhealthy (before repair): <span>0</span>", res)
             self.failUnlessIn("Corrupt Shares (before repair): <span>0</span>", res)
@@ -2686 +2717 @@
             self.failUnlessIn("Repairs Unsuccessful: <span>0</span>", res)
 
-            self.failUnlessIn("Objects Healthy (after repair): <span>10</span>", res)
+            self.failUnlessIn("Objects Healthy (after repair): <span>11</span>", res)
             self.failUnlessIn("Objects Unhealthy (after repair): <span>0</span>", res)
             self.failUnlessIn("Corrupt Shares (after repair): <span>0</span>", res)
@@ -3679 +3710 @@
 
     def test_PUT_NEWFILEURL_bad_format(self):
-       new_contents = self.NEWFILE_CONTENTS * 300000
-       return self.shouldHTTPError("PUT_NEWFILEURL_bad_format",
-                                   400, "Bad Request", "Unknown format: foo",
-                                   self.PUT, self.public_url + \
-                                   "/foo/foo.txt?format=foo",
-                                   new_contents)
+        new_contents = self.NEWFILE_CONTENTS * 300000
+        return self.shouldHTTPError("PUT_NEWFILEURL_bad_format",
+                                    400, "Bad Request", "Unknown format: foo",
+                                    self.PUT, self.public_url + \
+                                    "/foo/foo.txt?format=foo",
+                                    new_contents)
 
     def test_PUT_NEWFILEURL_uri_replace(self):

src/allmydata/web/directory.py ¶

@@ -7 +7 @@
 from twisted.internet.interfaces import IPushProducer
 from twisted.python.failure import Failure
-from twisted.web import http, html
+from twisted.web import http
 from nevow import url, rend, inevow, tags as T
 from nevow.inevow import IRequest
@@ -751 +751 @@
             dlurl = "%s/file/%s/@@named=/%s" % (root, quoted_uri, nameurl)
 
-            ctx.fillSlots("filename",
-                          T.a(href=dlurl)[html.escape(name)])
+            ctx.fillSlots("filename", T.a(href=dlurl)[name])
             ctx.fillSlots("type", "SSK")
 
@@ -762 +761 @@
             dlurl = "%s/file/%s/@@named=/%s" % (root, quoted_uri, nameurl)
 
-            ctx.fillSlots("filename",
-                          T.a(href=dlurl)[html.escape(name)])
+            ctx.fillSlots("filename", T.a(href=dlurl)[name])
             ctx.fillSlots("type", "FILE")
 
@@ -773 +771 @@
             # directory
             uri_link = "%s/uri/%s/" % (root, urllib.quote(target_uri))
-            ctx.fillSlots("filename",
-                          T.a(href=uri_link)[html.escape(name)])
+            ctx.fillSlots("filename", T.a(href=uri_link)[name])
             if not target.is_mutable():
                 dirtype = "DIR-IMM"
@@ -798 +795 @@
         else:
             # unknown
-            ctx.fillSlots("filename", html.escape(name))
+            ctx.fillSlots("filename", name)
             if target.get_write_uri() is not None:
                 unknowntype = "?"