From a92385b54b8430b64e0dca422f461b02e2825607 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 9 May 2012 14:18:27 -0700 Subject: [PATCH] webapi 'move'-button cleanups test_web.py: use shouldFail2(), safer than old shouldFail() directory.py: forbid slashes in from_name=, return BAD_REQUEST instead of GONE when trying to move into a non-directory --- src/allmydata/test/test_web.py | 91 +++++++++++++++++----------------- src/allmydata/web/directory.py | 39 +++++++-------- 2 files changed, 64 insertions(+), 66 deletions(-) diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index 5103478f..54a30bee 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -3286,13 +3286,12 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_POST_move_file_no_replace(self): - d = self.POST(self.public_url + "/foo", t="move", replace="false", - from_name="bar.txt", to_name="baz.txt", to_dir="sub") - d.addBoth(self.shouldFail, error.Error, - "POST_move_file_no_replace", - "409 Conflict", - "There was already a child by that name, and you asked me " - "to not replace it") + d = self.shouldFail2(error.Error, "POST_move_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", + replace="false", from_name="bar.txt", + to_name="baz.txt", to_dir="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")) @@ -3302,26 +3301,33 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_POST_move_file_slash_fail(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_name="slash/fail.txt", to_dir="sub") - d.addBoth(self.shouldFail, error.Error, - "test_POST_rename_file_slash_fail", - "400 Bad Request", - "to_name= may not contain a slash", - ) + 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", + from_name="bar.txt", + to_name="slash/fail.txt", to_dir="sub") d.addCallback(lambda res: self.failUnlessNodeHasChild(self._foo_node, u"bar.txt")) d.addCallback(lambda res: self.failIfNodeHasChild(self._sub_node, u"slash/fail.txt")) + d.addCallback(lambda ign: + self.shouldFail2(error.Error, + "test_POST_rename_file_slash_fail2", + "400 Bad Request", + "from_name= may not contain a slash", + self.POST, self.public_url + "/foo", + t="move", + from_name="nope/bar.txt", + to_name="fail.txt", to_dir="sub")) return d def test_POST_move_file_no_target(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_name="baz.txt") - d.addBoth(self.shouldFail, error.Error, - "POST_move_file_no_target", - "400 Bad Request", - "move requires from_name and to_dir") + 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") 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")) @@ -3331,13 +3337,12 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_POST_move_file_bad_target_type(self): - d = self.POST(self.public_url + "/foo", t="move", target_type="*D", - from_name="bar.txt", to_dir="sub") - d.addBoth(self.shouldFail, error.Error, - "test_POST_rename_file_slash_fail", - "400 Bad Request", - "invalid target_type parameter", - ) + d = self.shouldFail2(error.Error, "test_POST_move_file_bad_target_type", + "400 Bad Request", "invalid target_type parameter", + self.POST, + self.public_url + "/foo", t="move", + target_type="*D", from_name="bar.txt", + to_dir="sub") return d def test_POST_move_file_multi_level(self): @@ -3364,21 +3369,17 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_POST_move_file_to_nonexist_dir(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="notchucktesta") - d.addBoth(self.shouldFail, error.Error, - "POST_move_file_to_nonexist_dir", - "404 Not Found", - "No such child: notchucktesta") + d = self.shouldFail2(error.Error, "POST_move_file_to_nonexist_dir", + "404 Not Found", "No such child: nopechucktesta", + self.POST, self.public_url + "/foo", t="move", + from_name="bar.txt", to_dir="nopechucktesta") return d def test_POST_move_file_into_file(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", to_dir="baz.txt") - d.addBoth(self.shouldFail, error.Error, - "POST_move_file_into_file", - "410 Gone", - "to_dir is not a usable directory") + d = self.shouldFail2(error.Error, "POST_move_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") 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")) @@ -3388,13 +3389,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_POST_move_file_to_bad_uri(self): - d = self.POST(self.public_url + "/foo", t="move", - from_name="bar.txt", target_type="uri", - to_dir="URI:DIR2:mn5jlyjnrjeuydyswlzyui72i:rmneifcj6k6sycjljjhj3f6majsq2zqffydnnul5hfa4j577arma") - d.addBoth(self.shouldFail, error.Error, - "POST_move_file_to_bad_uri", - "410 Gone", - "to_dir is not a usable directory") + d = self.shouldFail2(error.Error, "POST_move_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", + to_dir="URI:DIR2:mn5jlyjnrjeuydyswlzyui72i:rmneifcj6k6sycjljjhj3f6majsq2zqffydnnul5hfa4j577arma") 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")) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index c7f9af2d..112595cd 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -444,34 +444,33 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): 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")) - target_type = get_arg(req, "target_type", "name") - if not target_type in ["name", "uri"]: - raise WebError("invalid target_type parameter", - http.BAD_REQUEST) - # allow from_name to contain slashes, so they can fix names that - # were accidentally created with them. But disallow them in to_name - # (if it's specified), to discourage the practice. - if to_name and "/" in to_name: + # 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= + if "/" in from_name: + raise WebError("from_name= may not contain a slash", + http.BAD_REQUEST) + if "/" in to_name: raise WebError("to_name= may not contain a slash", http.BAD_REQUEST) - d = defer.Deferred() - def get_target_node(target_type): - if target_type == "name": - return self.node.get_child_at_path(to_dir) - elif target_type == "uri": - return self.client.create_node_from_uri(str(to_dir)) - d.addCallback(get_target_node) - d.callback(target_type) + 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): - raise WebError("to_dir is not a usable directory", - http.GONE) + 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.addCallback(lambda new_parent: + self.node.move_child_to(from_name, new_parent, + to_name, replace)) d.addCallback(lambda res: "thing moved") return d -- 2.45.2