Address comments by Kevan on 833 and add test for stripping spaces
authordavid-sarah <david-sarah@jacaranda.org>
Wed, 27 Jan 2010 23:06:42 +0000 (15:06 -0800)
committerdavid-sarah <david-sarah@jacaranda.org>
Wed, 27 Jan 2010 23:06:42 +0000 (15:06 -0800)
docs/frontends/webapi.txt
src/allmydata/dirnode.py
src/allmydata/test/test_dirnode.py
src/allmydata/test/test_filenode.py
src/allmydata/test/test_web.py

index 12435f4e4a030c3b69fa5598fad1064921988660..6e6989903900f7c9a66622d7f1a6c6523e75b70f 100644 (file)
@@ -743,8 +743,14 @@ PUT /uri/$DIRCAP/[SUBDIRS../]CHILDNAME?t=uri
   
   Note that this operation does not take its child cap in the form of
   separate "rw_uri" and "ro_uri" fields. Therefore, it cannot accept a
-  child cap in a format unknown to the webapi server, because the server
-  is not able to attenuate an unknown write cap to a read cap.
+  child cap in a format unknown to the webapi server, unless its URI
+  starts with "ro." or "imm.". This restriction is necessary because the
+  server is not able to attenuate an unknown write cap to a read cap.
+  Unknown URIs starting with "ro." or "imm.", on the other hand, are
+  assumed to represent read caps. The client should not prefix a write
+  cap with "ro." or "imm." and pass it to this operation, since that
+  would result in granting the cap's write authority to holders of the
+  directory read cap.
 
 === Adding multiple files or directories to a parent directory at once ===
 
@@ -1028,7 +1034,9 @@ POST /uri/$DIRCAP/[SUBDIRS../]?t=uri&name=CHILDNAME&uri=CHILDCAP
 
  This attaches a given read- or write- cap "CHILDCAP" to the designated
  directory, with a specified child name. This behaves much like the PUT t=uri
- operation, and is a lot like a UNIX hardlink.
+ operation, and is a lot like a UNIX hardlink. It is subject to the same
+ restrictions as that operation on the use of cap formats unknown to the
+ webapi server.
 
  This will create additional intermediate directories as necessary, although
  since it is expected to be triggered by a form that was retrieved by "GET
index 2c1f0dd8ab246ab7abb0148e2cac79aef4173573..ebdc0fa75c87988004b7120a3dc14a4f9a1a3128 100644 (file)
@@ -265,23 +265,23 @@ class DirectoryNode:
         while position < len(data):
             entries, position = split_netstring(data, 1, position)
             entry = entries[0]
-            (name, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
+            (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
             if not mutable and len(rwcapdata) > 0:
                 raise ValueError("the rwcapdata field of a dirnode in an immutable directory was not empty")
-            name = name.decode("utf-8")
+            name = name_utf8.decode("utf-8")
             rw_uri = ""
             if writeable:
                 rw_uri = self._decrypt_rwcapdata(rwcapdata)
 
             # Since the encryption uses CTR mode, it currently leaks the length of the
             # plaintext rw_uri -- and therefore whether it is present, i.e. whether the
-            # dirnode is writeable (ticket #925). By stripping spaces in Tahoe >= 1.6.0,
-            # we may make it easier for future versions to plug this leak.
+            # dirnode is writeable (ticket #925). By stripping trailing spaces in
+            # Tahoe >= 1.6.0, we may make it easier for future versions to plug this leak.
             # ro_uri is treated in the same way for consistency.
             # rw_uri and ro_uri will be either None or a non-empty string.
 
-            rw_uri = rw_uri.strip(' ') or None
-            ro_uri = ro_uri.strip(' ') or None
+            rw_uri = rw_uri.rstrip(' ') or None
+            ro_uri = ro_uri.rstrip(' ') or None
 
             try:
                 child = self._create_and_validate_node(rw_uri, ro_uri, name)
@@ -292,11 +292,11 @@ class DirectoryNode:
                     children.set_with_aux(name, (child, metadata), auxilliary=entry)
                 else:
                     log.msg(format="mutable cap for child '%(name)s' unpacked from an immutable directory",
-                                   name=name.encode("utf-8"),
+                                   name=name_utf8,
                                    facility="tahoe.webish", level=log.UNUSUAL)
             except CapConstraintError, e:
                 log.msg(format="unmet constraint on cap for child '%(name)s' unpacked from a directory:\n"
-                               "%(message)s", message=e.args[0], name=name.encode("utf-8"),
+                               "%(message)s", message=e.args[0], name=name_utf8,
                                facility="tahoe.webish", level=log.UNUSUAL)
 
         return children
index 1acfdb5f09192770022acb95137a357699578931..6adf91ab82ad1377791418edd72864b7ede6a0f6 100644 (file)
@@ -13,6 +13,7 @@ from allmydata.interfaces import IImmutableFileNode, IMutableFileNode, \
 from allmydata.mutable.filenode import MutableFileNode
 from allmydata.mutable.common import UncoordinatedWriteError
 from allmydata.util import hashutil, base32
+from allmydata.util.netstring import split_netstring
 from allmydata.monitor import Monitor
 from allmydata.test.common import make_chk_file_uri, make_mutable_file_uri, \
      ErrorMixin
@@ -48,6 +49,7 @@ class Dirnode(GridTestMixin, unittest.TestCase,
         self.set_up_grid()
         c = self.g.clients[0]
         nm = c.nodemaker
+
         setup_py_uri = "URI:CHK:n7r3m6wmomelk4sep3kw5cvduq:os7ijw5c3maek7pg65e5254k2fzjflavtpejjyhshpsxuqzhcwwq:3:20:14861"
         one_uri = "URI:LIT:n5xgk" # LIT for "one"
         mut_write_uri = "URI:SSK:vfvcbdfbszyrsaxchgevhmmlii:euw4iw7bbnkrrwpzuburbhppuxhc3gwxv26f6imekhz7zyw2ojnq"
@@ -115,6 +117,8 @@ class Dirnode(GridTestMixin, unittest.TestCase,
 
         bad_future_node = UnknownNode(future_write_uri, None)
         bad_kids1 = {u"one": (bad_future_node, {})}
+        # This should fail because we don't know how to diminish the future_write_uri
+        # cap (given in a write slot and not prefixed with "ro." or "imm.") to a readcap.
         d.addCallback(lambda ign:
                       self.shouldFail(MustNotBeUnknownRWError, "bad_kids1",
                                       "cannot attach unknown",
@@ -133,6 +137,7 @@ class Dirnode(GridTestMixin, unittest.TestCase,
         self.set_up_grid()
         c = self.g.clients[0]
         nm = c.nodemaker
+
         setup_py_uri = "URI:CHK:n7r3m6wmomelk4sep3kw5cvduq:os7ijw5c3maek7pg65e5254k2fzjflavtpejjyhshpsxuqzhcwwq:3:20:14861"
         one_uri = "URI:LIT:n5xgk" # LIT for "one"
         mut_write_uri = "URI:SSK:vfvcbdfbszyrsaxchgevhmmlii:euw4iw7bbnkrrwpzuburbhppuxhc3gwxv26f6imekhz7zyw2ojnq"
@@ -280,6 +285,75 @@ class Dirnode(GridTestMixin, unittest.TestCase,
         d.addCallback(_made_parent)
         return d
 
+    def test_spaces_are_stripped_on_the_way_out(self):
+        self.basedir = "dirnode/Dirnode/test_spaces_are_stripped_on_the_way_out"
+        self.set_up_grid()
+        c = self.g.clients[0]
+        nm = c.nodemaker
+
+        # This test checks that any trailing spaces in URIs are retained in the
+        # encoded directory, but stripped when we get them out of the directory.
+        # See ticket #925 for why we want that.
+
+        stripped_write_uri = "lafs://from_the_future\t"
+        stripped_read_uri = "lafs://readonly_from_the_future\t"
+        spacedout_write_uri = stripped_write_uri + "  "
+        spacedout_read_uri = stripped_read_uri + "  "
+
+        child = nm.create_from_cap(spacedout_write_uri, spacedout_read_uri)
+        self.failUnlessEqual(child.get_write_uri(), spacedout_write_uri)
+        self.failUnlessEqual(child.get_readonly_uri(), "ro." + spacedout_read_uri)
+
+        kids = {u"child": (child, {})}
+        d = c.create_dirnode(kids)
+        
+        def _created(dn):
+            self.failUnless(isinstance(dn, dirnode.DirectoryNode))
+            self.failUnless(dn.is_mutable())
+            self.failIf(dn.is_readonly())
+            dn.raise_error()
+            self.cap = dn.get_cap()
+            self.rootnode = dn
+            return dn._node.download_best_version()
+        d.addCallback(_created)
+
+        def _check_data(data):
+            # Decode the netstring representation of the directory to check that the
+            # spaces are retained when the URIs are stored.
+            position = 0
+            numkids = 0
+            while position < len(data):
+                entries, position = split_netstring(data, 1, position)
+                entry = entries[0]
+                (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
+                name = name_utf8.decode("utf-8")
+                rw_uri = self.rootnode._decrypt_rwcapdata(rwcapdata)
+                self.failUnless(name in kids)
+                (expected_child, ign) = kids[name]
+                self.failUnlessEqual(rw_uri, expected_child.get_write_uri())
+                self.failUnlessEqual("ro." + ro_uri, expected_child.get_readonly_uri())
+                numkids += 1
+
+            self.failUnlessEqual(numkids, 1)
+            return self.rootnode.list()
+        d.addCallback(_check_data)
+        
+        # Now when we use the real directory listing code, the trailing spaces
+        # should have been stripped (and "ro." should have been prepended to the
+        # ro_uri, since it's unknown).
+        def _check_kids(children):
+            self.failUnlessEqual(sorted(children.keys()), [u"child"])
+            child_node, child_metadata = children[u"child"]
+
+            self.failUnlessEqual(child_node.get_write_uri(), stripped_write_uri)
+            self.failUnlessEqual(child_node.get_readonly_uri(), "ro." + stripped_read_uri)
+        d.addCallback(_check_kids)
+
+        d.addCallback(lambda ign: nm.create_from_cap(self.cap.to_string()))
+        d.addCallback(lambda n: n.list())
+        d.addCallback(_check_kids)  # again with dirnode recreated from cap
+        return d
+
     def test_check(self):
         self.basedir = "dirnode/Dirnode/test_check"
         self.set_up_grid()
@@ -1121,6 +1195,9 @@ class FakeMutableFile:
     def is_allowed_in_immutable_directory(self):
         return False
 
+    def raise_error(self):
+        pass
+
     def modify(self, modifier):
         self.data = modifier(self.data, None, True)
         return defer.succeed(None)
index 6106a2f8c776c617495f80ba0bdfc5350edb3f8d..983896f43d8f212c0ba6c8127b440233049ad71e 100644 (file)
@@ -39,10 +39,10 @@ class Node(unittest.TestCase):
         self.failUnlessEqual(fn1.get_uri(), u.to_string())
         self.failUnlessEqual(fn1.get_cap(), u)
         self.failUnlessEqual(fn1.get_readcap(), u)
-        self.failUnlessEqual(fn1.is_readonly(), True)
-        self.failUnlessEqual(fn1.is_mutable(), False)
-        self.failUnlessEqual(fn1.is_unknown(), False)
-        self.failUnlessEqual(fn1.is_allowed_in_immutable_directory(), True)
+        self.failUnless(fn1.is_readonly())
+        self.failIf(fn1.is_mutable())
+        self.failIf(fn1.is_unknown())
+        self.failUnless(fn1.is_allowed_in_immutable_directory())
         self.failUnlessEqual(fn1.get_write_uri(), None)
         self.failUnlessEqual(fn1.get_readonly_uri(), u.to_string())
         self.failUnlessEqual(fn1.get_size(), 1000)
@@ -54,8 +54,8 @@ class Node(unittest.TestCase):
         v = fn1.get_verify_cap()
         self.failUnless(isinstance(v, uri.CHKFileVerifierURI))
         self.failUnlessEqual(fn1.get_repair_cap(), v)
-        self.failUnlessEqual(v.is_readonly(), True)
-        self.failUnlessEqual(v.is_mutable(), False)
+        self.failUnless(v.is_readonly())
+        self.failIf(v.is_mutable())
 
 
     def test_literal_filenode(self):
@@ -69,10 +69,10 @@ class Node(unittest.TestCase):
         self.failUnlessEqual(fn1.get_uri(), u.to_string())
         self.failUnlessEqual(fn1.get_cap(), u)
         self.failUnlessEqual(fn1.get_readcap(), u)
-        self.failUnlessEqual(fn1.is_readonly(), True)
-        self.failUnlessEqual(fn1.is_mutable(), False)
-        self.failUnlessEqual(fn1.is_unknown(), False)
-        self.failUnlessEqual(fn1.is_allowed_in_immutable_directory(), True)
+        self.failUnless(fn1.is_readonly())
+        self.failIf(fn1.is_mutable())
+        self.failIf(fn1.is_unknown())
+        self.failUnless(fn1.is_allowed_in_immutable_directory())
         self.failUnlessEqual(fn1.get_write_uri(), None)
         self.failUnlessEqual(fn1.get_readonly_uri(), u.to_string())
         self.failUnlessEqual(fn1.get_size(), len(DATA))
@@ -122,10 +122,10 @@ class Node(unittest.TestCase):
         self.failUnlessEqual(n.get_readonly_uri(), u.get_readonly().to_string())
         self.failUnlessEqual(n.get_cap(), u)
         self.failUnlessEqual(n.get_readcap(), u.get_readonly())
-        self.failUnlessEqual(n.is_mutable(), True)
-        self.failUnlessEqual(n.is_readonly(), False)
-        self.failUnlessEqual(n.is_unknown(), False)
-        self.failUnlessEqual(n.is_allowed_in_immutable_directory(), False)
+        self.failUnless(n.is_mutable())
+        self.failIf(n.is_readonly())
+        self.failIf(n.is_unknown())
+        self.failIf(n.is_allowed_in_immutable_directory())
         n.raise_error()
 
         n2 = MutableFileNode(None, None, client.get_encoding_parameters(),
@@ -144,10 +144,10 @@ class Node(unittest.TestCase):
         self.failUnlessEqual(nro.get_readonly(), nro)
         self.failUnlessEqual(nro.get_cap(), u.get_readonly())
         self.failUnlessEqual(nro.get_readcap(), u.get_readonly())
-        self.failUnlessEqual(nro.is_mutable(), True)
-        self.failUnlessEqual(nro.is_readonly(), True)
-        self.failUnlessEqual(nro.is_unknown(), False)
-        self.failUnlessEqual(nro.is_allowed_in_immutable_directory(), False)
+        self.failUnless(nro.is_mutable())
+        self.failUnless(nro.is_readonly())
+        self.failIf(nro.is_unknown())
+        self.failIf(nro.is_allowed_in_immutable_directory())
         nro_u = nro.get_uri()
         self.failUnlessEqual(nro_u, nro.get_readonly_uri())
         self.failUnlessEqual(nro_u, u.get_readonly().to_string())
index 416459b6e33339f3059ccb9dfc96b8eac8f03777..ad63338404cc5735f28f5b0a382996f29bcf8cae 100644 (file)
@@ -2376,7 +2376,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, unittest.TestCase):
     def test_POST_set_children_with_hyphen(self):
         return self.test_POST_set_children(command_name="set-children")
 
-    def test_POST_put_uri(self):
+    def test_POST_link_uri(self):
         contents, n, newuri = self.makefile(8)
         d = self.POST(self.public_url + "/foo", t="uri", name="new.txt", uri=newuri)
         d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"new.txt")
@@ -2385,7 +2385,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, unittest.TestCase):
                                                       contents))
         return d
 
-    def test_POST_put_uri_replace(self):
+    def test_POST_link_uri_replace(self):
         contents, n, newuri = self.makefile(8)
         d = self.POST(self.public_url + "/foo", t="uri", name="bar.txt", uri=newuri)
         d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"bar.txt")
@@ -2394,12 +2394,33 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, unittest.TestCase):
                                                       contents))
         return d
 
-    def test_POST_put_uri_no_replace_queryarg(self):
+    def test_POST_link_uri_unknown_bad(self):
+        newuri = "lafs://from_the_future"
+        d = self.POST(self.public_url + "/foo", t="uri", name="future.txt", uri=newuri)
+        d.addBoth(self.shouldFail, error.Error,
+                  "POST_link_uri_unknown_bad",
+                  "400 Bad Request",
+                  "unknown cap in a write slot")
+        return d
+
+    def test_POST_link_uri_unknown_ro_good(self):
+        newuri = "ro.lafs://readonly_from_the_future"
+        d = self.POST(self.public_url + "/foo", t="uri", name="future-ro.txt", uri=newuri)
+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"future-ro.txt")
+        return d
+
+    def test_POST_link_uri_unknown_imm_good(self):
+        newuri = "imm.lafs://immutable_from_the_future"
+        d = self.POST(self.public_url + "/foo", t="uri", name="future-imm.txt", uri=newuri)
+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"future-imm.txt")
+        return d
+
+    def test_POST_link_uri_no_replace_queryarg(self):
         contents, n, newuri = self.makefile(8)
         d = self.POST(self.public_url + "/foo?replace=false", t="uri",
                       name="bar.txt", uri=newuri)
         d.addBoth(self.shouldFail, error.Error,
-                  "POST_put_uri_no_replace_queryarg",
+                  "POST_link_uri_no_replace_queryarg",
                   "409 Conflict",
                   "There was already a child by that name, and you asked me "
                   "to not replace it")
@@ -2407,12 +2428,12 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, unittest.TestCase):
         d.addCallback(self.failUnlessIsBarDotTxt)
         return d
 
-    def test_POST_put_uri_no_replace_field(self):
+    def test_POST_link_uri_no_replace_field(self):
         contents, n, newuri = self.makefile(8)
         d = self.POST(self.public_url + "/foo", t="uri", replace="false",
                       name="bar.txt", uri=newuri)
         d.addBoth(self.shouldFail, error.Error,
-                  "POST_put_uri_no_replace_field",
+                  "POST_link_uri_no_replace_field",
                   "409 Conflict",
                   "There was already a child by that name, and you asked me "
                   "to not replace it")
@@ -2680,6 +2701,29 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, unittest.TestCase):
                   "to not replace it")
         return d
 
+    def test_PUT_NEWFILEURL_uri_unknown_bad(self):
+        new_uri = "lafs://from_the_future"
+        d = self.PUT(self.public_url + "/foo/put-future.txt?t=uri", new_uri)
+        d.addBoth(self.shouldFail, error.Error,
+                  "POST_put_uri_unknown_bad",
+                  "400 Bad Request",
+                  "unknown cap in a write slot")
+        return d
+
+    def test_PUT_NEWFILEURL_uri_unknown_ro_good(self):
+        new_uri = "ro.lafs://readonly_from_the_future"
+        d = self.PUT(self.public_url + "/foo/put-future-ro.txt?t=uri", new_uri)
+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node,
+                      u"put-future-ro.txt")
+        return d
+
+    def test_PUT_NEWFILEURL_uri_unknown_imm_good(self):
+        new_uri = "imm.lafs://immutable_from_the_future"
+        d = self.PUT(self.public_url + "/foo/put-future-imm.txt?t=uri", new_uri)
+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node,
+                      u"put-future-imm.txt")
+        return d
+
     def test_PUT_NEWFILE_URI(self):
         file_contents = "New file contents here\n"
         d = self.PUT("/uri", file_contents)
@@ -3360,15 +3404,13 @@ class Grid(GridTestMixin, WebErrorMixin, unittest.TestCase, ShouldFailMixin):
             while position < len(data):
                 entries, position = split_netstring(data, 1, position)
                 entry = entries[0]
-                (name, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
-                name = name.decode("utf-8")
+                (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
+                name = name_utf8.decode("utf-8")
                 self.failUnless(rwcapdata == "")
-                ro_uri = ro_uri.strip()
-                if name in kids:
-                    self.failIfEqual(ro_uri, "")
-                    (expected_child, ign) = kids[name]
-                    self.failUnlessEqual(ro_uri, expected_child.get_readonly_uri())
-                    numkids += 1
+                self.failUnless(name in kids)
+                (expected_child, ign) = kids[name]
+                self.failUnlessEqual(ro_uri, expected_child.get_readonly_uri())
+                numkids += 1
 
             self.failUnlessEqual(numkids, 3)
             return self.rootnode.list()