Ticket #833: changes-since-kevans-review-diff.txt

File changes-since-kevans-review-diff.txt, 19.2 KB (added by davidsarah, at 2010-01-27T23:26:08Z)

Changes since Kevan's review

Line 
1diff -rN -u old-tahoe/docs/frontends/webapi.txt new-tahoe/docs/frontends/webapi.txt
2--- old-tahoe/docs/frontends/webapi.txt 2010-01-27 23:20:14.922000000 +0000
3+++ new-tahoe/docs/frontends/webapi.txt 2010-01-27 23:20:18.929000000 +0000
4@@ -743,8 +743,14 @@
5   
6   Note that this operation does not take its child cap in the form of
7   separate "rw_uri" and "ro_uri" fields. Therefore, it cannot accept a
8-  child cap in a format unknown to the webapi server, because the server
9-  is not able to attenuate an unknown write cap to a read cap.
10+  child cap in a format unknown to the webapi server, unless its URI
11+  starts with "ro." or "imm.". This restriction is necessary because the
12+  server is not able to attenuate an unknown write cap to a read cap.
13+  Unknown URIs starting with "ro." or "imm.", on the other hand, are
14+  assumed to represent read caps. The client should not prefix a write
15+  cap with "ro." or "imm." and pass it to this operation, since that
16+  would result in granting the cap's write authority to holders of the
17+  directory read cap.
18 
19 === Adding multiple files or directories to a parent directory at once ===
20 
21@@ -1028,7 +1034,9 @@
22 
23  This attaches a given read- or write- cap "CHILDCAP" to the designated
24  directory, with a specified child name. This behaves much like the PUT t=uri
25- operation, and is a lot like a UNIX hardlink.
26+ operation, and is a lot like a UNIX hardlink. It is subject to the same
27+ restrictions as that operation on the use of cap formats unknown to the
28+ webapi server.
29 
30  This will create additional intermediate directories as necessary, although
31  since it is expected to be triggered by a form that was retrieved by "GET
32diff -rN -u old-tahoe/src/allmydata/dirnode.py new-tahoe/src/allmydata/dirnode.py
33--- old-tahoe/src/allmydata/dirnode.py  2010-01-27 23:20:16.828000000 +0000
34+++ new-tahoe/src/allmydata/dirnode.py  2010-01-27 23:20:20.898000000 +0000
35@@ -265,23 +265,23 @@
36         while position < len(data):
37             entries, position = split_netstring(data, 1, position)
38             entry = entries[0]
39-            (name, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
40+            (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
41             if not mutable and len(rwcapdata) > 0:
42                 raise ValueError("the rwcapdata field of a dirnode in an immutable directory was not empty")
43-            name = name.decode("utf-8")
44+            name = name_utf8.decode("utf-8")
45             rw_uri = ""
46             if writeable:
47                 rw_uri = self._decrypt_rwcapdata(rwcapdata)
48 
49             # Since the encryption uses CTR mode, it currently leaks the length of the
50             # plaintext rw_uri -- and therefore whether it is present, i.e. whether the
51-            # dirnode is writeable (ticket #925). By stripping spaces in Tahoe >= 1.6.0,
52-            # we may make it easier for future versions to plug this leak.
53+            # dirnode is writeable (ticket #925). By stripping trailing spaces in
54+            # Tahoe >= 1.6.0, we may make it easier for future versions to plug this leak.
55             # ro_uri is treated in the same way for consistency.
56             # rw_uri and ro_uri will be either None or a non-empty string.
57 
58-            rw_uri = rw_uri.strip(' ') or None
59-            ro_uri = ro_uri.strip(' ') or None
60+            rw_uri = rw_uri.rstrip(' ') or None
61+            ro_uri = ro_uri.rstrip(' ') or None
62 
63             try:
64                 child = self._create_and_validate_node(rw_uri, ro_uri, name)
65@@ -292,11 +292,11 @@
66                     children.set_with_aux(name, (child, metadata), auxilliary=entry)
67                 else:
68                     log.msg(format="mutable cap for child '%(name)s' unpacked from an immutable directory",
69-                                   name=name.encode("utf-8"),
70+                                   name=name_utf8,
71                                    facility="tahoe.webish", level=log.UNUSUAL)
72             except CapConstraintError, e:
73                 log.msg(format="unmet constraint on cap for child '%(name)s' unpacked from a directory:\n"
74-                               "%(message)s", message=e.args[0], name=name.encode("utf-8"),
75+                               "%(message)s", message=e.args[0], name=name_utf8,
76                                facility="tahoe.webish", level=log.UNUSUAL)
77 
78         return children
79diff -rN -u old-tahoe/src/allmydata/test/test_dirnode.py new-tahoe/src/allmydata/test/test_dirnode.py
80--- old-tahoe/src/allmydata/test/test_dirnode.py        2010-01-27 23:20:17.690000000 +0000
81+++ new-tahoe/src/allmydata/test/test_dirnode.py        2010-01-27 23:20:21.660000000 +0000
82@@ -13,6 +13,7 @@
83 from allmydata.mutable.filenode import MutableFileNode
84 from allmydata.mutable.common import UncoordinatedWriteError
85 from allmydata.util import hashutil, base32
86+from allmydata.util.netstring import split_netstring
87 from allmydata.monitor import Monitor
88 from allmydata.test.common import make_chk_file_uri, make_mutable_file_uri, \
89      ErrorMixin
90@@ -48,6 +49,7 @@
91         self.set_up_grid()
92         c = self.g.clients[0]
93         nm = c.nodemaker
94+
95         setup_py_uri = "URI:CHK:n7r3m6wmomelk4sep3kw5cvduq:os7ijw5c3maek7pg65e5254k2fzjflavtpejjyhshpsxuqzhcwwq:3:20:14861"
96         one_uri = "URI:LIT:n5xgk" # LIT for "one"
97         mut_write_uri = "URI:SSK:vfvcbdfbszyrsaxchgevhmmlii:euw4iw7bbnkrrwpzuburbhppuxhc3gwxv26f6imekhz7zyw2ojnq"
98@@ -115,6 +117,8 @@
99 
100         bad_future_node = UnknownNode(future_write_uri, None)
101         bad_kids1 = {u"one": (bad_future_node, {})}
102+        # This should fail because we don't know how to diminish the future_write_uri
103+        # cap (given in a write slot and not prefixed with "ro." or "imm.") to a readcap.
104         d.addCallback(lambda ign:
105                       self.shouldFail(MustNotBeUnknownRWError, "bad_kids1",
106                                       "cannot attach unknown",
107@@ -133,6 +137,7 @@
108         self.set_up_grid()
109         c = self.g.clients[0]
110         nm = c.nodemaker
111+
112         setup_py_uri = "URI:CHK:n7r3m6wmomelk4sep3kw5cvduq:os7ijw5c3maek7pg65e5254k2fzjflavtpejjyhshpsxuqzhcwwq:3:20:14861"
113         one_uri = "URI:LIT:n5xgk" # LIT for "one"
114         mut_write_uri = "URI:SSK:vfvcbdfbszyrsaxchgevhmmlii:euw4iw7bbnkrrwpzuburbhppuxhc3gwxv26f6imekhz7zyw2ojnq"
115@@ -280,6 +285,75 @@
116         d.addCallback(_made_parent)
117         return d
118 
119+    def test_spaces_are_stripped_on_the_way_out(self):
120+        self.basedir = "dirnode/Dirnode/test_spaces_are_stripped_on_the_way_out"
121+        self.set_up_grid()
122+        c = self.g.clients[0]
123+        nm = c.nodemaker
124+
125+        # This test checks that any trailing spaces in URIs are retained in the
126+        # encoded directory, but stripped when we get them out of the directory.
127+        # See ticket #925 for why we want that.
128+
129+        stripped_write_uri = "lafs://from_the_future\t"
130+        stripped_read_uri = "lafs://readonly_from_the_future\t"
131+        spacedout_write_uri = stripped_write_uri + "  "
132+        spacedout_read_uri = stripped_read_uri + "  "
133+
134+        child = nm.create_from_cap(spacedout_write_uri, spacedout_read_uri)
135+        self.failUnlessEqual(child.get_write_uri(), spacedout_write_uri)
136+        self.failUnlessEqual(child.get_readonly_uri(), "ro." + spacedout_read_uri)
137+
138+        kids = {u"child": (child, {})}
139+        d = c.create_dirnode(kids)
140+       
141+        def _created(dn):
142+            self.failUnless(isinstance(dn, dirnode.DirectoryNode))
143+            self.failUnless(dn.is_mutable())
144+            self.failIf(dn.is_readonly())
145+            dn.raise_error()
146+            self.cap = dn.get_cap()
147+            self.rootnode = dn
148+            return dn._node.download_best_version()
149+        d.addCallback(_created)
150+
151+        def _check_data(data):
152+            # Decode the netstring representation of the directory to check that the
153+            # spaces are retained when the URIs are stored.
154+            position = 0
155+            numkids = 0
156+            while position < len(data):
157+                entries, position = split_netstring(data, 1, position)
158+                entry = entries[0]
159+                (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
160+                name = name_utf8.decode("utf-8")
161+                rw_uri = self.rootnode._decrypt_rwcapdata(rwcapdata)
162+                self.failUnless(name in kids)
163+                (expected_child, ign) = kids[name]
164+                self.failUnlessEqual(rw_uri, expected_child.get_write_uri())
165+                self.failUnlessEqual("ro." + ro_uri, expected_child.get_readonly_uri())
166+                numkids += 1
167+
168+            self.failUnlessEqual(numkids, 1)
169+            return self.rootnode.list()
170+        d.addCallback(_check_data)
171+       
172+        # Now when we use the real directory listing code, the trailing spaces
173+        # should have been stripped (and "ro." should have been prepended to the
174+        # ro_uri, since it's unknown).
175+        def _check_kids(children):
176+            self.failUnlessEqual(sorted(children.keys()), [u"child"])
177+            child_node, child_metadata = children[u"child"]
178+
179+            self.failUnlessEqual(child_node.get_write_uri(), stripped_write_uri)
180+            self.failUnlessEqual(child_node.get_readonly_uri(), "ro." + stripped_read_uri)
181+        d.addCallback(_check_kids)
182+
183+        d.addCallback(lambda ign: nm.create_from_cap(self.cap.to_string()))
184+        d.addCallback(lambda n: n.list())
185+        d.addCallback(_check_kids)  # again with dirnode recreated from cap
186+        return d
187+
188     def test_check(self):
189         self.basedir = "dirnode/Dirnode/test_check"
190         self.set_up_grid()
191@@ -1121,6 +1195,9 @@
192     def is_allowed_in_immutable_directory(self):
193         return False
194 
195+    def raise_error(self):
196+        pass
197+
198     def modify(self, modifier):
199         self.data = modifier(self.data, None, True)
200         return defer.succeed(None)
201diff -rN -u old-tahoe/src/allmydata/test/test_filenode.py new-tahoe/src/allmydata/test/test_filenode.py
202--- old-tahoe/src/allmydata/test/test_filenode.py       2010-01-27 23:20:17.705000000 +0000
203+++ new-tahoe/src/allmydata/test/test_filenode.py       2010-01-27 23:20:21.672000000 +0000
204@@ -39,10 +39,10 @@
205         self.failUnlessEqual(fn1.get_uri(), u.to_string())
206         self.failUnlessEqual(fn1.get_cap(), u)
207         self.failUnlessEqual(fn1.get_readcap(), u)
208-        self.failUnlessEqual(fn1.is_readonly(), True)
209-        self.failUnlessEqual(fn1.is_mutable(), False)
210-        self.failUnlessEqual(fn1.is_unknown(), False)
211-        self.failUnlessEqual(fn1.is_allowed_in_immutable_directory(), True)
212+        self.failUnless(fn1.is_readonly())
213+        self.failIf(fn1.is_mutable())
214+        self.failIf(fn1.is_unknown())
215+        self.failUnless(fn1.is_allowed_in_immutable_directory())
216         self.failUnlessEqual(fn1.get_write_uri(), None)
217         self.failUnlessEqual(fn1.get_readonly_uri(), u.to_string())
218         self.failUnlessEqual(fn1.get_size(), 1000)
219@@ -54,8 +54,8 @@
220         v = fn1.get_verify_cap()
221         self.failUnless(isinstance(v, uri.CHKFileVerifierURI))
222         self.failUnlessEqual(fn1.get_repair_cap(), v)
223-        self.failUnlessEqual(v.is_readonly(), True)
224-        self.failUnlessEqual(v.is_mutable(), False)
225+        self.failUnless(v.is_readonly())
226+        self.failIf(v.is_mutable())
227 
228 
229     def test_literal_filenode(self):
230@@ -69,10 +69,10 @@
231         self.failUnlessEqual(fn1.get_uri(), u.to_string())
232         self.failUnlessEqual(fn1.get_cap(), u)
233         self.failUnlessEqual(fn1.get_readcap(), u)
234-        self.failUnlessEqual(fn1.is_readonly(), True)
235-        self.failUnlessEqual(fn1.is_mutable(), False)
236-        self.failUnlessEqual(fn1.is_unknown(), False)
237-        self.failUnlessEqual(fn1.is_allowed_in_immutable_directory(), True)
238+        self.failUnless(fn1.is_readonly())
239+        self.failIf(fn1.is_mutable())
240+        self.failIf(fn1.is_unknown())
241+        self.failUnless(fn1.is_allowed_in_immutable_directory())
242         self.failUnlessEqual(fn1.get_write_uri(), None)
243         self.failUnlessEqual(fn1.get_readonly_uri(), u.to_string())
244         self.failUnlessEqual(fn1.get_size(), len(DATA))
245@@ -122,10 +122,10 @@
246         self.failUnlessEqual(n.get_readonly_uri(), u.get_readonly().to_string())
247         self.failUnlessEqual(n.get_cap(), u)
248         self.failUnlessEqual(n.get_readcap(), u.get_readonly())
249-        self.failUnlessEqual(n.is_mutable(), True)
250-        self.failUnlessEqual(n.is_readonly(), False)
251-        self.failUnlessEqual(n.is_unknown(), False)
252-        self.failUnlessEqual(n.is_allowed_in_immutable_directory(), False)
253+        self.failUnless(n.is_mutable())
254+        self.failIf(n.is_readonly())
255+        self.failIf(n.is_unknown())
256+        self.failIf(n.is_allowed_in_immutable_directory())
257         n.raise_error()
258 
259         n2 = MutableFileNode(None, None, client.get_encoding_parameters(),
260@@ -144,10 +144,10 @@
261         self.failUnlessEqual(nro.get_readonly(), nro)
262         self.failUnlessEqual(nro.get_cap(), u.get_readonly())
263         self.failUnlessEqual(nro.get_readcap(), u.get_readonly())
264-        self.failUnlessEqual(nro.is_mutable(), True)
265-        self.failUnlessEqual(nro.is_readonly(), True)
266-        self.failUnlessEqual(nro.is_unknown(), False)
267-        self.failUnlessEqual(nro.is_allowed_in_immutable_directory(), False)
268+        self.failUnless(nro.is_mutable())
269+        self.failUnless(nro.is_readonly())
270+        self.failIf(nro.is_unknown())
271+        self.failIf(nro.is_allowed_in_immutable_directory())
272         nro_u = nro.get_uri()
273         self.failUnlessEqual(nro_u, nro.get_readonly_uri())
274         self.failUnlessEqual(nro_u, u.get_readonly().to_string())
275diff -rN -u old-tahoe/src/allmydata/test/test_web.py new-tahoe/src/allmydata/test/test_web.py
276--- old-tahoe/src/allmydata/test/test_web.py    2010-01-27 23:20:17.913000000 +0000
277+++ new-tahoe/src/allmydata/test/test_web.py    2010-01-27 23:20:21.879000000 +0000
278@@ -2376,7 +2376,7 @@
279     def test_POST_set_children_with_hyphen(self):
280         return self.test_POST_set_children(command_name="set-children")
281 
282-    def test_POST_put_uri(self):
283+    def test_POST_link_uri(self):
284         contents, n, newuri = self.makefile(8)
285         d = self.POST(self.public_url + "/foo", t="uri", name="new.txt", uri=newuri)
286         d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"new.txt")
287@@ -2385,7 +2385,7 @@
288                                                       contents))
289         return d
290 
291-    def test_POST_put_uri_replace(self):
292+    def test_POST_link_uri_replace(self):
293         contents, n, newuri = self.makefile(8)
294         d = self.POST(self.public_url + "/foo", t="uri", name="bar.txt", uri=newuri)
295         d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"bar.txt")
296@@ -2394,12 +2394,33 @@
297                                                       contents))
298         return d
299 
300-    def test_POST_put_uri_no_replace_queryarg(self):
301+    def test_POST_link_uri_unknown_bad(self):
302+        newuri = "lafs://from_the_future"
303+        d = self.POST(self.public_url + "/foo", t="uri", name="future.txt", uri=newuri)
304+        d.addBoth(self.shouldFail, error.Error,
305+                  "POST_link_uri_unknown_bad",
306+                  "400 Bad Request",
307+                  "unknown cap in a write slot")
308+        return d
309+
310+    def test_POST_link_uri_unknown_ro_good(self):
311+        newuri = "ro.lafs://readonly_from_the_future"
312+        d = self.POST(self.public_url + "/foo", t="uri", name="future-ro.txt", uri=newuri)
313+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"future-ro.txt")
314+        return d
315+
316+    def test_POST_link_uri_unknown_imm_good(self):
317+        newuri = "imm.lafs://immutable_from_the_future"
318+        d = self.POST(self.public_url + "/foo", t="uri", name="future-imm.txt", uri=newuri)
319+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node, u"future-imm.txt")
320+        return d
321+
322+    def test_POST_link_uri_no_replace_queryarg(self):
323         contents, n, newuri = self.makefile(8)
324         d = self.POST(self.public_url + "/foo?replace=false", t="uri",
325                       name="bar.txt", uri=newuri)
326         d.addBoth(self.shouldFail, error.Error,
327-                  "POST_put_uri_no_replace_queryarg",
328+                  "POST_link_uri_no_replace_queryarg",
329                   "409 Conflict",
330                   "There was already a child by that name, and you asked me "
331                   "to not replace it")
332@@ -2407,12 +2428,12 @@
333         d.addCallback(self.failUnlessIsBarDotTxt)
334         return d
335 
336-    def test_POST_put_uri_no_replace_field(self):
337+    def test_POST_link_uri_no_replace_field(self):
338         contents, n, newuri = self.makefile(8)
339         d = self.POST(self.public_url + "/foo", t="uri", replace="false",
340                       name="bar.txt", uri=newuri)
341         d.addBoth(self.shouldFail, error.Error,
342-                  "POST_put_uri_no_replace_field",
343+                  "POST_link_uri_no_replace_field",
344                   "409 Conflict",
345                   "There was already a child by that name, and you asked me "
346                   "to not replace it")
347@@ -2680,6 +2701,29 @@
348                   "to not replace it")
349         return d
350 
351+    def test_PUT_NEWFILEURL_uri_unknown_bad(self):
352+        new_uri = "lafs://from_the_future"
353+        d = self.PUT(self.public_url + "/foo/put-future.txt?t=uri", new_uri)
354+        d.addBoth(self.shouldFail, error.Error,
355+                  "POST_put_uri_unknown_bad",
356+                  "400 Bad Request",
357+                  "unknown cap in a write slot")
358+        return d
359+
360+    def test_PUT_NEWFILEURL_uri_unknown_ro_good(self):
361+        new_uri = "ro.lafs://readonly_from_the_future"
362+        d = self.PUT(self.public_url + "/foo/put-future-ro.txt?t=uri", new_uri)
363+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node,
364+                      u"put-future-ro.txt")
365+        return d
366+
367+    def test_PUT_NEWFILEURL_uri_unknown_imm_good(self):
368+        new_uri = "imm.lafs://immutable_from_the_future"
369+        d = self.PUT(self.public_url + "/foo/put-future-imm.txt?t=uri", new_uri)
370+        d.addCallback(self.failUnlessURIMatchesROChild, self._foo_node,
371+                      u"put-future-imm.txt")
372+        return d
373+
374     def test_PUT_NEWFILE_URI(self):
375         file_contents = "New file contents here\n"
376         d = self.PUT("/uri", file_contents)
377@@ -3360,15 +3404,13 @@
378             while position < len(data):
379                 entries, position = split_netstring(data, 1, position)
380                 entry = entries[0]
381-                (name, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
382-                name = name.decode("utf-8")
383+                (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
384+                name = name_utf8.decode("utf-8")
385                 self.failUnless(rwcapdata == "")
386-                ro_uri = ro_uri.strip()
387-                if name in kids:
388-                    self.failIfEqual(ro_uri, "")
389-                    (expected_child, ign) = kids[name]
390-                    self.failUnlessEqual(ro_uri, expected_child.get_readonly_uri())
391-                    numkids += 1
392+                self.failUnless(name in kids)
393+                (expected_child, ign) = kids[name]
394+                self.failUnlessEqual(ro_uri, expected_child.get_readonly_uri())
395+                numkids += 1
396 
397             self.failUnlessEqual(numkids, 3)
398             return self.rootnode.list()