diff --git a/NEWS.rst b/NEWS.rst index e5dd54b48..c0612f46c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -16,8 +16,8 @@ New Features nodes is not affected. When server, introducer, and client are all upgraded, the welcome page will show node IDs that start with "v0-" instead of the old tubid. (`#466`_) -- The web-API has a new move operation that supports directly moving files - between directories. (`#1579`_) +- The web-API has a new ``relink`` operation that supports directly moving + files between directories. (`#1579`_) Security Improvements ''''''''''''''''''''' diff --git a/docs/frontends/webapi.rst b/docs/frontends/webapi.rst index 616eb8422..87ff15acc 100644 --- a/docs/frontends/webapi.rst +++ b/docs/frontends/webapi.rst @@ -29,7 +29,7 @@ The Tahoe REST-ful Web API 6. `Attaching An Existing File Or Directory (by URI)`_ 7. `Unlinking A Child`_ 8. `Renaming A Child`_ - 9. `Moving A Child`_ + 9. `Relinking a Child`_ 10. `Other Utilities`_ 11. `Debugging and Testing Features`_ @@ -1274,34 +1274,68 @@ Renaming A Child same child-cap under the new name, except that it preserves metadata. This operation cannot move the child to a different directory. - By default, this operation will replace any existing child of the new name, - making it behave like the UNIX "``mv -f``" command. Adding a "replace=false" - argument causes the command to throw an HTTP 409 Conflict error if there is - already a child with the new name. + The default behavior is to overwrite any existing link at the destination + (replace=true). To prevent this (and make the operation return an error + instead of overwriting), add a "replace=false" argument. With replace=false, + this operation will return an HTTP 409 "Conflict" error if the destination + is not the same link as the source and there is already a link at the + destination, rather than overwriting the existing link. To allow the + operation to overwrite a link to a file, but return an HTTP 409 error when + trying to overwrite a link to a directory, use "replace=only-files" (this + behavior is closer to the traditional UNIX "mv" command). Note that "true", + "t", and "1" are all synonyms for "True"; "false", "f", and "0" are synonyms + for "False"; and the parameter is case-insensitive. -Moving A Child ----------------- -``POST /uri/$DIRCAP/[SUBDIRS../]?t=move&from_name=OLD&to_dir=TARGETNAME[&target_type=name][&to_name=NEWNAME]`` -``POST /uri/$DIRCAP/[SUBDIRS../]?t=move&from_name=OLD&to_dir=TARGETURI&target_type=uri[&to_name=NEWNAME]`` +Relinking a Child +----------------- - This instructs the node to move a child of the given directory to a - different directory, both of which must be mutable. If target_type=name - or is omitted, the to_dir= parameter should contain the name of a - subdirectory of the child's current parent directory (multiple levels of - descent are supported). If target_uri=, then to_dir= will be treated as - a dircap, allowing the child to be moved to an unrelated directory. +``POST /uri/$DIRCAP/[SUBDIRS../]?t=rename&from_name=OLD&to_dir=$NEWDIRCAP/[NEWSUBDIRS../]&to_name=NEW`` + ``[&replace=true|false|only-files]`` (Tahoe >= v1.10) - The child can also be renamed in the process, by providing a new name in - the to_name= parameter. If omitted, the child will retain its existing - name. + This instructs the node to relink a child of the given source directory, + into a different directory and/or to a different name. The source and + destination directories must be writeable. If {{{to_dir}}} is not present, + the child link is renamed within the same directory. If {{{to_name}}} is + not present then it defaults to {{{from_name}}}. If the destination link + (directory and name) is the same as the source link, the operation has no + effect. + + Metadata from the source directory entry is preserved. Multiple levels of + descent in the source and destination paths are supported. + + This operation will return an HTTP 404 "Not Found" error if + ``$DIRCAP/[SUBDIRS../]``, the child being moved, or the destination + directory does not exist. It will return an HTTP 400 "Bad Request" error + if any entry in the source or destination paths is not a directory. + + The default behavior is to overwrite any existing link at the destination + (replace=true). To prevent this (and make the operation return an error + instead of overwriting), add a "replace=false" argument. With replace=false, + this operation will return an HTTP 409 "Conflict" error if the destination + is not the same link as the source and there is already a link at the + destination, rather than overwriting the existing link. To allow the + operation to overwrite a link to a file, but return an HTTP 409 error when + trying to overwrite a link to a directory, use "replace=only-files" (this + behavior is closer to the traditional UNIX "mv" command). Note that "true", + "t", and "1" are all synonyms for "True"; "false", "f", and "0" are synonyms + for "False"; and the parameter is case-insensitive. + + When relinking into a different directory, for safety, the child link is + not removed from the old directory until it has been successfully added to + the new directory. This implies that in case of a crash or failure, the + link to the child will not be lost, but it could be linked at both the old + and new locations. + + The source link should not be the same as any link (directory and child name) + in the ``to_dir`` path. This restriction is not enforced, but it may be + enforced in a future version. If it were violated then the result would be + to create a cycle in the directory structure that is not necessarily reachable + from the root of the destination path (``$NEWDIRCAP``), which could result in + data loss, as described in ticket `#943`_. + +.. _`#943`: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/943 - By default, this operation will replace any existing child of the new name, - making it behave like the UNIX "``mv -f``" command. Adding a "replace=false" - argument causes the command to throw an HTTP 409 Conflict error if there is - already a child with the new name. For safety, the child is not unlinked - from the old directory until its has been successfully added to the new - directory. Other Utilities --------------- diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py index 7941e623a..5fddec41a 100644 --- a/src/allmydata/dirnode.py +++ b/src/allmydata/dirnode.py @@ -134,6 +134,7 @@ class Adder: if entries is None: entries = {} precondition(isinstance(entries, dict), entries) + precondition(overwrite in (True, False, "only-files"), overwrite) # keys of 'entries' may not be normalized. self.entries = entries self.overwrite = overwrite @@ -160,7 +161,7 @@ class Adder: raise ExistingChildError("child %s already exists" % quote_output(name, encoding='utf-8')) if self.overwrite == "only-files" and IDirectoryNode.providedBy(children[name][0]): - raise ExistingChildError("child %s already exists" % quote_output(name, encoding='utf-8')) + raise ExistingChildError("child %s already exists as a directory" % quote_output(name, encoding='utf-8')) metadata = children[name][1].copy() metadata = update_metadata(metadata, new_metadata, now) @@ -642,22 +643,36 @@ class DirectoryNode: def move_child_to(self, current_child_namex, new_parent, new_child_namex=None, overwrite=True): - """I take one of my children and move them to a new parent. The child - is referenced by name. On the new parent, the child will live under - 'new_child_name', which defaults to 'current_child_name'. I return a - Deferred that fires when the operation finishes.""" + """ + I take one of my child links and move it to a new parent. The child + link is referenced by name. In the new parent, the child link will live + at 'new_child_namex', which defaults to 'current_child_namex'. I return + a Deferred that fires when the operation finishes. + 'new_child_namex' and 'current_child_namex' need not be normalized. + The overwrite parameter may be True (overwrite any existing child), + False (error if the new child link already exists), or "only-files" + (error if the new child link exists and points to a directory). + """ if self.is_readonly() or new_parent.is_readonly(): return defer.fail(NotWriteableError()) current_child_name = normalize(current_child_namex) if new_child_namex is None: - new_child_namex = current_child_name - d = self.get(current_child_name) - def sn(child): - return new_parent.set_node(new_child_namex, child, + new_child_name = current_child_name + else: + new_child_name = normalize(new_child_namex) + + from_uri = self.get_write_uri() + if new_parent.get_write_uri() == from_uri and new_child_name == current_child_name: + # needed for correctness, otherwise we would delete the child + return defer.succeed("redundant rename/relink") + + d = self.get_child_and_metadata(current_child_name) + def _got_child( (child, metadata) ): + return new_parent.set_node(new_child_name, child, metadata, overwrite=overwrite) - d.addCallback(sn) + d.addCallback(_got_child) d.addCallback(lambda child: self.delete(current_child_name)) return d diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index 1ffab54b2..00d050974 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -563,8 +563,10 @@ class WebMixin(object): res.trap(expected_failure) if substring: self.failUnlessIn(substring, str(res), - "'%s' not in '%s' for test '%s'" % \ - (substring, str(res), which)) + "'%s' not in '%s' (response is '%s') for test '%s'" % \ + (substring, str(res), + getattr(res.value, "response", ""), + which)) if response_substring: self.failUnlessIn(response_substring, res.value.response, "'%s' not in '%s' for test '%s'" % \ @@ -1560,7 +1562,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi r'\s+%d' % len(self.BAR_CONTENTS), ]) self.failUnless(re.search(get_bar, res), res) - for label in ['unlink', 'rename/move']: + for label in ['unlink', 'rename/relink']: for line in res.split("\n"): # find the line that contains the relevant button for bar.txt if ("form action" in line and @@ -3501,16 +3503,50 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsEmptyJSON) return d + def test_POST_rename_file_no_replace_same_link(self): + d = self.POST(self.public_url + "/foo", t="rename", + replace="false", from_name="bar.txt", to_name="bar.txt") + d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_rename_file_replace_only_files(self): + d = self.POST(self.public_url + "/foo", t="rename", + replace="only-files", from_name="bar.txt", + to_name="baz.txt") + d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_rename_file_replace_only_files_conflict(self): + d = self.shouldFail2(error.Error, "POST_relink_file_replace_only_files_conflict", + "409 Conflict", + "There was already a child by that name, and you asked me to not replace it.", + self.POST, self.public_url + "/foo", t="relink", + replace="only-files", from_name="bar.txt", + to_name="empty") + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + def failUnlessIsEmptyJSON(self, res): data = simplejson.loads(res) self.failUnlessEqual(data[0], "dirnode", data) self.failUnlessReallyEqual(len(data[1]["children"]), 0) - def test_POST_rename_file_slash_fail(self): + def test_POST_rename_file_to_slash_fail(self): d = self.POST(self.public_url + "/foo", t="rename", from_name="bar.txt", to_name='kirk/spock.txt') d.addBoth(self.shouldFail, error.Error, - "test_POST_rename_file_slash_fail", + "test_POST_rename_file_to_slash_fail", "400 Bad Request", "to_name= may not contain a slash", ) @@ -3518,6 +3554,18 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) return d + def test_POST_rename_file_from_slash_fail(self): + d = self.POST(self.public_url + "/foo", t="rename", + from_name="sub/bar.txt", to_name='spock.txt') + d.addBoth(self.shouldFail, error.Error, + "test_POST_rename_from_file_slash_fail", + "400 Bad Request", + "from_name= may not contain a slash", + ) + d.addCallback(lambda res: + self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) + return d + def test_POST_rename_dir(self): d = self.POST(self.public_url, t="rename", from_name="foo", to_name='plunk') @@ -3529,9 +3577,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsFooJSON) return d - def test_POST_move_file(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="sub") + def test_POST_relink_file(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_dir=self.public_root.get_uri() + "/foo/sub") d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: @@ -3542,9 +3591,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_new_name(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_name="wibble.txt", to_dir="sub") + def test_POST_relink_file_new_name(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_name="wibble.txt", to_dir=self.public_root.get_uri() + "/foo/sub") d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: @@ -3557,9 +3607,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_replace(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_name="baz.txt", to_dir="sub") + def test_POST_relink_file_replace(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_name="baz.txt", to_dir=self.public_root.get_uri() + "/foo/sub") d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: self.GET(self.public_url + "/foo/sub/baz.txt")) @@ -3568,13 +3619,13 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_no_replace(self): - d = self.shouldFail2(error.Error, "POST_move_file_no_replace", + def test_POST_relink_file_no_replace(self): + d = self.shouldFail2(error.Error, "POST_relink_file_no_replace", "409 Conflict", "There was already a child by that name, and you asked me to not replace it", - self.POST, self.public_url + "/foo", t="move", + self.POST, self.public_url + "/foo", t="relink", replace="false", from_name="bar.txt", - to_name="baz.txt", to_dir="sub") + to_name="baz.txt", to_dir=self.public_root.get_uri() + "/foo/sub") d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) d.addCallback(self.failUnlessIsBarDotTxt) d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) @@ -3583,13 +3634,49 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsSubBazDotTxt) return d - def test_POST_move_file_slash_fail(self): + def test_POST_relink_file_no_replace_explicitly_same_link(self): + d = self.POST(self.public_url + "/foo", t="relink", + replace="false", from_name="bar.txt", + to_name="bar.txt", to_dir=self.public_root.get_uri() + "/foo") + d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_relink_file_replace_only_files(self): + d = self.POST(self.public_url + "/foo", t="relink", + replace="only-files", from_name="bar.txt", + to_name="baz.txt", to_dir=self.public_root.get_uri() + "/foo/sub") + d.addCallback(lambda res: + self.failIfNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/sub/baz.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/sub/baz.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_relink_file_replace_only_files_conflict(self): + d = self.shouldFail2(error.Error, "POST_relink_file_replace_only_files_conflict", + "409 Conflict", + "There was already a child by that name, and you asked me to not replace it.", + self.POST, self.public_url + "/foo", t="relink", + replace="only-files", from_name="bar.txt", + to_name="sub", to_dir=self.public_root.get_uri() + "/foo") + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_relink_file_to_slash_fail(self): d = self.shouldFail2(error.Error, "test_POST_rename_file_slash_fail", "400 Bad Request", "to_name= may not contain a slash", - self.POST, self.public_url + "/foo", t="move", + self.POST, self.public_url + "/foo", t="relink", from_name="bar.txt", - to_name="slash/fail.txt", to_dir="sub") + to_name="slash/fail.txt", to_dir=self.public_root.get_uri() + "/foo/sub") d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: @@ -3600,38 +3687,58 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi "400 Bad Request", "from_name= may not contain a slash", self.POST, self.public_url + "/foo", - t="move", + t="relink", from_name="nope/bar.txt", - to_name="fail.txt", to_dir="sub")) + to_name="fail.txt", + to_dir=self.public_root.get_uri() + "/foo/sub")) return d - def test_POST_move_file_no_target(self): - d = self.shouldFail2(error.Error, "POST_move_file_no_target", - "400 Bad Request", - "move requires from_name and to_dir", - self.POST, self.public_url + "/foo", t="move", - from_name="bar.txt", to_name="baz.txt") + def test_POST_relink_file_explicitly_same_link(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_name="bar.txt", to_dir=self.public_root.get_uri() + "/foo") + d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) d.addCallback(self.failUnlessIsBarDotTxt) d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_relink_file_implicitly_same_link(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt") + d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) + return d + + def test_POST_relink_file_same_dir(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_name="baz.txt", to_dir=self.public_root.get_uri() + "/foo") + d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) + d.addCallback(lambda res: self.failUnlessNodeHasChild(self._sub_node, u"baz.txt")) d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt")) - d.addCallback(self.failUnlessIsBazDotTxt) + d.addCallback(self.failUnlessIsBarDotTxt) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt?t=json")) + d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_bad_target_type(self): - d = self.shouldFail2(error.Error, "test_POST_move_file_bad_target_type", - "400 Bad Request", "invalid target_type parameter", + def test_POST_relink_file_bad_replace(self): + d = self.shouldFail2(error.Error, "test_POST_relink_file_bad_replace", + "400 Bad Request", "invalid replace= argument: 'boogabooga'", self.POST, - self.public_url + "/foo", t="move", - target_type="*D", from_name="bar.txt", - to_dir="sub") + self.public_url + "/foo", t="relink", + replace="boogabooga", from_name="bar.txt", + to_dir=self.public_root.get_uri() + "/foo/sub") return d - def test_POST_move_file_multi_level(self): + def test_POST_relink_file_multi_level(self): d = self.POST(self.public_url + "/foo/sub/level2?t=mkdir", "") - d.addCallback(lambda res: self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="sub/level2")) + d.addCallback(lambda res: self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", to_dir=self.public_root.get_uri() + "/foo/sub/level2")) d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: self.failIfNodeHasChild(self._sub_node, u"bar.txt")) d.addCallback(lambda res: self.GET(self.public_url + "/foo/sub/level2/bar.txt")) @@ -3640,8 +3747,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_to_uri(self): - d = self.POST(self.public_url + "/foo", t="move", target_type="uri", + def test_POST_relink_file_to_uri(self): + d = self.POST(self.public_url + "/foo", t="relink", target_type="uri", from_name="bar.txt", to_dir=self._sub_uri) d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"bar.txt")) @@ -3651,18 +3758,20 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_to_nonexist_dir(self): - d = self.shouldFail2(error.Error, "POST_move_file_to_nonexist_dir", + def test_POST_relink_file_to_nonexistent_dir(self): + d = self.shouldFail2(error.Error, "POST_relink_file_to_nonexistent_dir", "404 Not Found", "No such child: nopechucktesta", - self.POST, self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="nopechucktesta") + self.POST, self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_dir=self.public_root.get_uri() + "/nopechucktesta") return d - def test_POST_move_file_into_file(self): - d = self.shouldFail2(error.Error, "POST_move_file_into_file", + def test_POST_relink_file_into_file(self): + d = self.shouldFail2(error.Error, "POST_relink_file_into_file", "400 Bad Request", "to_dir is not a directory", - self.POST, self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="baz.txt") + self.POST, self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_dir=self.public_root.get_uri() + "/foo/baz.txt") d.addCallback(lambda res: self.GET(self.public_url + "/foo/baz.txt")) d.addCallback(self.failUnlessIsBazDotTxt) d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) @@ -3671,11 +3780,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_file_to_bad_uri(self): - d = self.shouldFail2(error.Error, "POST_move_file_to_bad_uri", + def test_POST_relink_file_to_bad_uri(self): + d = self.shouldFail2(error.Error, "POST_relink_file_to_bad_uri", "400 Bad Request", "to_dir is not a directory", - self.POST, self.public_url + "/foo", t="move", - from_name="bar.txt", target_type="uri", + self.POST, self.public_url + "/foo", t="relink", + from_name="bar.txt", to_dir="URI:DIR2:mn5jlyjnrjeuydyswlzyui72i:rmneifcj6k6sycjljjhj3f6majsq2zqffydnnul5hfa4j577arma") d.addCallback(lambda res: self.GET(self.public_url + "/foo/bar.txt")) d.addCallback(self.failUnlessIsBarDotTxt) @@ -3683,11 +3792,13 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d.addCallback(self.failUnlessIsBarJSON) return d - def test_POST_move_dir(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="empty") + def test_POST_relink_dir(self): + d = self.POST(self.public_url + "/foo", t="relink", + from_name="bar.txt", + to_dir=self.public_root.get_uri() + "/foo/empty") d.addCallback(lambda res: self.POST(self.public_url + "/foo", - t="move", from_name="empty", to_dir="sub")) + t="relink", from_name="empty", + to_dir=self.public_root.get_uri() + "/foo/sub")) d.addCallback(lambda res: self.failIfNodeHasChild(self._foo_node, u"empty")) d.addCallback(lambda res: @@ -4335,8 +4446,7 @@ class Util(ShouldFailMixin, testutil.ReallyEqualMixin, unittest.TestCase): self.failUnlessReallyEqual(common.parse_replace_arg("false"), False) self.failUnlessReallyEqual(common.parse_replace_arg("only-files"), "only-files") - self.shouldFail(AssertionError, "test_parse_replace_arg", "", - common.parse_replace_arg, "only_fles") + self.failUnlessRaises(common.WebError, common.parse_replace_arg, "only_fles") def test_abbreviate_time(self): self.failUnlessReallyEqual(common.abbreviate_time(None), "") diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 13385c5f6..c3b94d2a3 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -24,14 +24,17 @@ def getxmlfile(name): def boolean_of_arg(arg): # TODO: "" - assert arg.lower() in ("true", "t", "1", "false", "f", "0", "on", "off") + if arg.lower() not in ("true", "t", "1", "false", "f", "0", "on", "off"): + raise WebError("invalid boolean argument: %r" % (arg,), http.BAD_REQUEST) return arg.lower() in ("true", "t", "1", "on") def parse_replace_arg(replace): if replace.lower() == "only-files": return replace - else: + try: return boolean_of_arg(replace) + except WebError: + raise WebError("invalid replace= argument: %r" % (replace,), http.BAD_REQUEST) def get_format(req, default="CHK"): diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 33ca25991..21f6d429c 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -13,6 +13,7 @@ from nevow.inevow import IRequest from foolscap.api import fireEventually from allmydata.util import base32, time_format +from allmydata.util.encodingutil import to_str from allmydata.uri import from_string_dirnode from allmydata.interfaces import IDirectoryNode, IFileNode, IFilesystemNode, \ IImmutableFileNode, IMutableFileNode, ExistingChildError, \ @@ -219,8 +220,8 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): d = self._POST_unlink(req) elif t == "rename": d = self._POST_rename(req) - elif t == "move": - d = self._POST_move(req) + elif t == "relink": + d = self._POST_relink(req) elif t == "check": d = self._POST_check(req) elif t == "start-deep-check": @@ -344,7 +345,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): raise WebError("set-uri requires a name") charset = get_arg(req, "_charset", "utf-8") name = name.decode(charset) - replace = boolean_of_arg(get_arg(req, "replace", "true")) + replace = parse_replace_arg(get_arg(req, "replace", "true")) # We mustn't pass childcap for the readcap argument because we don't # know whether it is a read cap. Passing a read cap as the writecap @@ -373,59 +374,36 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): return d def _POST_rename(self, req): + # rename is identical to relink, but to_dir is not allowed + # and to_name is required. + if get_arg(req, "to_dir") is not None: + raise WebError("to_dir= is not valid for rename") + if get_arg(req, "to_name") is None: + raise WebError("to_name= is required for rename") + return self._POST_relink(req) + + def _POST_relink(self, req): charset = get_arg(req, "_charset", "utf-8") + replace = parse_replace_arg(get_arg(req, "replace", "true")) + from_name = get_arg(req, "from_name") if from_name is not None: from_name = from_name.strip() from_name = from_name.decode(charset) assert isinstance(from_name, unicode) + else: + raise WebError("from_name= is required") + to_name = get_arg(req, "to_name") if to_name is not None: to_name = to_name.strip() to_name = to_name.decode(charset) assert isinstance(to_name, unicode) - if not from_name or not to_name: - raise WebError("rename requires from_name and to_name") - if from_name == to_name: - return defer.succeed("redundant rename") - - # allow from_name to contain slashes, so they can fix names that were - # accidentally created with them. But disallow them in to_name, to - # discourage the practice. - if "/" in to_name: - raise WebError("to_name= may not contain a slash", http.BAD_REQUEST) - - replace = boolean_of_arg(get_arg(req, "replace", "true")) - d = self.node.move_child_to(from_name, self.node, to_name, replace) - d.addCallback(lambda res: "thing renamed") - return d - - def _POST_move(self, req): - charset = get_arg(req, "_charset", "utf-8") - from_name = get_arg(req, "from_name") - if from_name is not None: - from_name = from_name.strip() - from_name = from_name.decode(charset) - assert isinstance(from_name, unicode) - to_name = get_arg(req, "to_name") - if to_name is not None: - to_name = to_name.strip() - to_name = to_name.decode(charset) - assert isinstance(to_name, unicode) - if not to_name: + else: to_name = from_name - to_dir = get_arg(req, "to_dir") - if to_dir is not None: - to_dir = to_dir.strip() - to_dir = to_dir.decode(charset) - assert isinstance(to_dir, unicode) - if not from_name or not to_dir: - raise WebError("move requires from_name and to_dir") - replace = boolean_of_arg(get_arg(req, "replace", "true")) # Disallow slashes in both from_name and to_name, that would only - # cause confusion. t=move is only for moving things from the - # *current* directory into a second directory named by to_dir= + # cause confusion. if "/" in from_name: raise WebError("from_name= may not contain a slash", http.BAD_REQUEST) @@ -433,22 +411,26 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): raise WebError("to_name= may not contain a slash", http.BAD_REQUEST) - target_type = get_arg(req, "target_type", "name") - if target_type == "name": - d = self.node.get_child_at_path(to_dir) - elif target_type == "uri": - d = defer.succeed(self.client.create_node_from_uri(str(to_dir))) - else: - raise WebError("invalid target_type parameter", http.BAD_REQUEST) - - def is_target_node_usable(target_node): - if not IDirectoryNode.providedBy(target_node): + to_dir = get_arg(req, "to_dir") + if to_dir is not None and to_dir != self.node.get_write_uri(): + to_dir = to_dir.strip() + to_dir = to_dir.decode(charset) + assert isinstance(to_dir, unicode) + to_path = to_dir.split(u"/") + to_root = self.client.nodemaker.create_from_cap(to_str(to_path[0])) + if not IDirectoryNode.providedBy(to_root): raise WebError("to_dir is not a directory", http.BAD_REQUEST) - return target_node - d.addCallback(is_target_node_usable) - d.addCallback(lambda new_parent: - self.node.move_child_to(from_name, new_parent, - to_name, replace)) + d = to_root.get_child_at_path(to_path[1:]) + else: + d = defer.succeed(self.node) + + def _got_new_parent(new_parent): + if not IDirectoryNode.providedBy(new_parent): + raise WebError("to_dir is not a directory", http.BAD_REQUEST) + + return self.node.move_child_to(from_name, new_parent, + to_name, replace) + d.addCallback(_got_new_parent) d.addCallback(lambda res: "thing moved") return d @@ -563,7 +545,7 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): return d def _POST_set_children(self, req): - replace = boolean_of_arg(get_arg(req, "replace", "true")) + replace = parse_replace_arg(get_arg(req, "replace", "true")) req.content.seek(0) body = req.content.read() try: @@ -711,7 +693,7 @@ class DirectoryAsHTML(rend.Page): T.input(type='hidden', name='t', value='rename-form'), T.input(type='hidden', name='name', value=name), T.input(type='hidden', name='when_done', value="."), - T.input(type='submit', value='rename/move', name="rename"), + T.input(type='submit', value='rename/relink', name="rename"), ] ctx.fillSlots("unlink", unlink)