12844: Refuse to unreference a directory by moving it into itself.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 7bbbaa492c71298157f3cfebc96dfade89966ae0..d8ee2a2b1c5175697bf39369274ff6c0a42e7310 100644 (file)
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -1078,6 +1078,10 @@ func (dn *dirnode) Rename(oldname, newname string) error {
        if !ok {
                return os.ErrNotExist
        }
+       if locked[oldinode] {
+               // oldinode cannot become a descendant of itself.
+               return ErrInvalidArgument
+       }
        if existing, ok := newdn.inodes[newname]; ok {
                // overwriting an existing file or dir
                if dn, ok := existing.(*dirnode); ok {

diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 57ba3255947439ded25834e6b8841cd51fe70f48..bd5d08bcf5e8f278606be6bac2037ce7b9215ecb 100644 (file)
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -599,6 +599,30 @@ func (s *CollectionFSSuite) TestRemove(c *check.C) {
        c.Check(err, check.IsNil)
 }
 
+func (s *CollectionFSSuite) TestRenameError(c *check.C) {
+       fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+       c.Assert(err, check.IsNil)
+       err = fs.Mkdir("first", 0755)
+       c.Assert(err, check.IsNil)
+       err = fs.Mkdir("first/second", 0755)
+       c.Assert(err, check.IsNil)
+       f, err := fs.OpenFile("first/second/file", os.O_CREATE|os.O_WRONLY, 0755)
+       c.Assert(err, check.IsNil)
+       f.Write([]byte{1, 2, 3, 4, 5})
+       f.Close()
+       err = fs.Rename("first", "first/second/third")
+       c.Check(err, check.Equals, ErrInvalidArgument)
+       err = fs.Rename("first", "first/third")
+       c.Check(err, check.Equals, ErrInvalidArgument)
+       err = fs.Rename("first/second", "second")
+       c.Check(err, check.IsNil)
+       f, err = fs.OpenFile("second/file", 0, 0)
+       c.Assert(err, check.IsNil)
+       data, err := ioutil.ReadAll(f)
+       c.Check(err, check.IsNil)
+       c.Check(data, check.DeepEquals, []byte{1, 2, 3, 4, 5})
+}
+
 func (s *CollectionFSSuite) TestRename(c *check.C) {
        fs, err := (&Collection{}).FileSystem(s.client, s.kc)
        c.Assert(err, check.IsNil)

diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index d4a89c844b567a4d2fb8d3a94f8138e365a672d7..eb323674b9013daa80b7ee7bc1d472dcdd21cf01 100644 (file)
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -131,6 +131,17 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
                        cmd:   "move newdir0/testfile newdir1/\n",
                        match: `(?ms).*Moving .* succeeded.*`,
                },
+               {
+                       path:  writePath,
+                       cmd:   "move newdir1 newdir1/\n",
+                       match: `(?ms).*Moving .* failed.*`,
+               },
+               {
+                       path:  writePath,
+                       cmd:   "get newdir1/testfile '" + checkfile.Name() + "'\n",
+                       match: `(?ms).*succeeded.*`,
+                       data:  testdata,
+               },
                {
                        path:  writePath,
                        cmd:   "put '" + localfile.Name() + "' newdir1/testfile1\n",