The trac seems to be throwing a 500 Internal Server Error whenever I try to add a comment or attachment. So here is a comment for Kevan that I was going to add to #833. The relevant unified diff (to be applied on top of mutable-in-immutable-darcspatch and misc-tweaks-darcspatch) is attached.
Replying to [comment:50 kevan]:
> test/test_dirnode.py:
> {{{
> + bad_future_node = UnknownNode(future_write_uri, None)
> + bad_kids1 = {u"one": (bad_future_node, {})}
> d.addCallback(lambda ign:
> + self.shouldFail(MustNotBeUnknownRWError, "bad_kids1",
> + "cannot attach unknown",
> nm.create_new_mutable_directory,
> bad_kids1))
> }}}
> Could you add a comment here explaining why this should fail? If I
understand correctly, it is because the cap you provide doesn't start with
ro. or imm. and since it is unknown we don't know how to diminish it to a
readonly cap?
Yes, exactly. Comment added.
> (Few of the tests so far in test_dirnode.py have explanations, but I've
been able to understand them without. Explanations aren't a bad idea, though)
Well, they didn't start with explanations :-) This is a good point, but I
think I'll need to address it after the 1.6 release.
> I think that FakeMutableFile? needs to implement raise_error.
Actually the tests never call it, but it should have this method for
consistency. Fixed.
> test_filenode.py:
>
> around line 41,
> {{{
> + self.failUnlessEqual(fn1.is_unknown(), False)
> + self.failUnlessEqual(fn1.is_allowed_in_immutable_directory(), True)
> }}}
> could be re-written as
>
> {{{
> self.failIf(fn1.is_unknown())
> self.failUnless(fn1.is_allowed_in_mutable_directory())
> }}}
Changed for all cases in test_filenode.py.
> test/test_uri.py:
>
> It would be nice to have a comment explaining why the invalid uris are
invalid, though it's easy enough to figure out after a few seconds of
looking at it. Just a thought.
You mean the ones that result in instances of UnknownURI? (Technically there
are no invalid uris any more, because uri.from_string will catch the
BadURIError and construct an UnknownURI that remembers it. The BadURIError
might be reraised later, though.)
I think that if it's easy enough to work out, that's as it should be. The
trouble with putting too many comments in is that reviewers trust the
comments, when they shouldn't.
> test_web.py:
>
> I'm assuming that existing tests will already fail if we attempt to add an
unknown node using PUT /uri/$DIRCAP/SUBDIRS../CHILDNAME?t=uri and POST
/uri/$DIRCAP/SUBDIRS../?t=uri&name=CHILDNAME&uri=CHILDCAP, since that was
the existing behavior before your changes.
Don't make that assumption! The patch removed some of the restrictions on
UnknownNodes, so there's definitely a need to explicitly test this.
In fact the addition should succeed (for a mutable directory) when the URI
is prefixed with "ro." or "imm." -- in those cases we don't need to be able
to diminish it to a readcap, because it already is one. Tests for the six
possible cases (test_{PUT_NEWFILEURL, POST_link}_uri_unknown_{bad, ro_good,
imm_good) added.
> You've changed the _create_initial_children and _create_immutable_children
to include unknown URIs, which threads tests for those nicely through the
existing tests.
Yes, that made it a lot easier to see what needed to be tested.
> Other comments:
>
> Can you add a test for the change you made in the chunk starting around
line 230 of dirnode.py that references #925?
I'll do that separately tomorrow.
--
David-Sarah Hopwood ⚥ http://davidsarah.livejournal.com
diff -rN -u old-tahoe/docs/frontends/webapi.txt
new-tahoe/docs/frontends/webapi.txt
--- old-tahoe/docs/frontends/webapi.txt 2010-01-27 09:17:01.212000000 +0000
+++ new-tahoe/docs/frontends/webapi.txt 2010-01-27 09:17:05.907000000 +0000
@@ -743,8 +743,14 @@
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 @@
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
diff -rN -u old-tahoe/src/allmydata/test/test_dirnode.py
new-tahoe/src/allmydata/test/test_dirnode.py
--- old-tahoe/src/allmydata/test/test_dirnode.py 2010-01-27
09:17:04.414000000 +0000
+++ new-tahoe/src/allmydata/test/test_dirnode.py 2010-01-27
09:17:09.952000000 +0000
@@ -115,6 +115,8 @@
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",
@@ -1121,6 +1123,9 @@
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)
diff -rN -u old-tahoe/src/allmydata/test/test_filenode.py
new-tahoe/src/allmydata/test/test_filenode.py
--- old-tahoe/src/allmydata/test/test_filenode.py 2010-01-27
09:17:04.435000000 +0000
+++ new-tahoe/src/allmydata/test/test_filenode.py 2010-01-27
09:17:09.987000000 +0000
@@ -39,10 +39,10 @@
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 @@
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 @@
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 @@
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 @@
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())
diff -rN -u old-tahoe/src/allmydata/test/test_web.py
new-tahoe/src/allmydata/test/test_web.py
--- old-tahoe/src/allmydata/test/test_web.py 2010-01-27 09:17:04.653000000
+0000
+++ new-tahoe/src/allmydata/test/test_web.py 2010-01-27 09:17:10.289000000
+0000
@@ -2376,7 +2376,7 @@
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 @@
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 @@
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 @@
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 @@
"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)
signature.asc
Description: OpenPGP digital signature
_______________________________________________ tahoe-dev mailing list [email protected] http://allmydata.org/cgi-bin/mailman/listinfo/tahoe-dev
