From 35f37cc5b86465407a0eb85a7760f8442f5a969f Mon Sep 17 00:00:00 2001
From: Daira Hopwood <david-sarah@jacaranda.org>
Date: Fri, 5 Apr 2013 06:36:14 +0100
Subject: [PATCH] Change web-API to support t=relink instead of t=move (+ docs
 and tests). fixes #1732

Signed-off-by: Daira Hopwood <david-sarah@jacaranda.org>
---
 NEWS.rst                       |   4 +-
 docs/frontends/webapi.rst      |  88 +++++++++----
 src/allmydata/dirnode.py       |  37 ++++--
 src/allmydata/test/test_web.py | 228 ++++++++++++++++++++++++---------
 src/allmydata/web/common.py    |   7 +-
 src/allmydata/web/directory.py |  94 ++++++--------
 6 files changed, 301 insertions(+), 157 deletions(-)

diff --git a/NEWS.rst b/NEWS.rst
index e5dd54b4..c0612f46 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 616eb842..87ff15ac 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]``
-
- 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.
-
- 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.
-
- 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.
+Relinking a Child
+-----------------
+
+``POST /uri/$DIRCAP/[SUBDIRS../]?t=rename&from_name=OLD&to_dir=$NEWDIRCAP/[NEWSUBDIRS../]&to_name=NEW``
+ ``[&replace=true|false|only-files]``    (Tahoe >= v1.10)
+
+ 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
+
 
 Other Utilities
 ---------------
diff --git a/src/allmydata/dirnode.py b/src/allmydata/dirnode.py
index 7941e623..5fddec41 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 1ffab54b..00d05097 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+<td align="right">%d</td>' % 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 13385c5f..c3b94d2a 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 33ca2599..21f6d429 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")
-        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 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
+        replace = parse_replace_arg(get_arg(req, "replace", "true"))
 
-    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)
+        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 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)))
+        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)
+            d = to_root.get_child_at_path(to_path[1:])
         else:
-            raise WebError("invalid target_type parameter", http.BAD_REQUEST)
+            d = defer.succeed(self.node)
 
-        def is_target_node_usable(target_node):
-            if not IDirectoryNode.providedBy(target_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 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))
+
+            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)
-- 
2.45.2