diff options
author | Nigel Tao <nigeltao@golang.org> | 2015-02-10 16:53:23 +1100 |
---|---|---|
committer | Nigel Tao <nigeltao@golang.org> | 2015-02-10 06:42:32 +0000 |
commit | 5b4754d96d73efe14f882d2f14ae3d29f7b2a67d (patch) | |
tree | f386082d92a14e26987c90f2dca6acd94210f965 | |
parent | f090b05f9bc9fe228db5cef02d762c4fe3a87547 (diff) | |
download | net-5b4754d96d73efe14f882d2f14ae3d29f7b2a67d.tar.gz |
webdav: factor out a moveFiles function, and make the tests call that
instead of FileSystem.Rename directly.
Dir.Rename's behavior wrt overwriting existing files and directories is
OS-dependent.
Fixes golang/go#9786
Change-Id: If42728caa6f0f38f8e3d6b1fcdda8c2d272080d6
Reviewed-on: https://go-review.googlesource.com/4341
Reviewed-by: Nick Cooper <nmvc@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
-rw-r--r-- | webdav/file.go | 34 | ||||
-rw-r--r-- | webdav/file_test.go | 40 | ||||
-rw-r--r-- | webdav/webdav.go | 32 |
3 files changed, 52 insertions, 54 deletions
diff --git a/webdav/file.go b/webdav/file.go index 0f67853..aa4ea6a 100644 --- a/webdav/file.go +++ b/webdav/file.go @@ -30,6 +30,10 @@ func slashClean(name string) string { // // Each method has the same semantics as the os package's function of the same // name. +// +// Note that the os.Rename documentation says that "OS-specific restrictions +// might apply". In particular, whether or not renaming a file or directory +// overwriting another existing file or directory is an error is OS-dependent. type FileSystem interface { Mkdir(name string, perm os.FileMode) error OpenFile(name string, flag int, perm os.FileMode) (File, error) @@ -548,6 +552,36 @@ func (f *memFile) Write(p []byte) (int, error) { return lenp, nil } +// moveFiles moves files and/or directories from src to dst. +// +// See section 9.9.4 for when various HTTP status codes apply. +func moveFiles(fs FileSystem, src, dst string, overwrite bool) (status int, err error) { + created := false + if _, err := fs.Stat(dst); err != nil { + if !os.IsNotExist(err) { + return http.StatusForbidden, err + } + created = true + } else if overwrite { + // Section 9.9.3 says that "If a resource exists at the destination + // and the Overwrite header is "T", then prior to performing the move, + // the server must perform a DELETE with "Depth: infinity" on the + // destination resource. + if err := fs.RemoveAll(dst); err != nil { + return http.StatusForbidden, err + } + } else { + return http.StatusPreconditionFailed, os.ErrExist + } + if err := fs.Rename(src, dst); err != nil { + return http.StatusForbidden, err + } + if created { + return http.StatusCreated, nil + } + return http.StatusNoContent, nil +} + // copyFiles copies files and/or directories from src to dst. // // See section 9.8.5 for when various HTTP status codes apply. diff --git a/webdav/file_test.go b/webdav/file_test.go index 03bcb8d..0d06b3f 100644 --- a/webdav/file_test.go +++ b/webdav/file_test.go @@ -321,33 +321,30 @@ func testFS(t *testing.T, fs FileSystem) { " stat /d want dir", " stat /d/m want dir", " find / /a /b /d /d/m", - "rename /b /b want ok", - " stat /b want 2", - " stat /c want errNotExist", - "rename /b /c want ok", + "move__ o=F /b /c want ok", " stat /b want errNotExist", " stat /c want 2", " stat /d/m want dir", " stat /d/n want errNotExist", " find / /a /c /d /d/m", - "rename /d/m /d/n want ok", + "move__ o=F /d/m /d/n want ok", "create /d/n/q QQQQ want ok", " stat /d/m want errNotExist", " stat /d/n want dir", " stat /d/n/q want 4", - "rename /d /d/n/z want err", - "rename /c /d/n/q want ok", + "move__ o=F /d /d/n/z want err", + "move__ o=T /c /d/n/q want ok", " stat /c want errNotExist", " stat /d/n/q want 2", " find / /a /d /d/n /d/n/q", "create /d/n/r RRRRR want ok", "mk-dir /u want ok", "mk-dir /u/v want ok", - "rename /d/n /u want err", + "move__ o=F /d/n /u want errExist", "create /t TTTTTT want ok", - "rename /d/n /t want err", + "move__ o=F /d/n /t want errExist", "rm-all /t want ok", - "rename /d/n /t want ok", + "move__ o=F /d/n /t want ok", " stat /d want dir", " stat /d/n want errNotExist", " stat /d/n/r want errNotExist", @@ -355,10 +352,10 @@ func testFS(t *testing.T, fs FileSystem) { " stat /t/q want 2", " stat /t/r want 5", " find / /a /d /t /t/q /t/r /u /u/v", - "rename /t / want err", - "rename /t /u/v want ok", + "move__ o=F /t / want errExist", + "move__ o=T /t /u/v want ok", " stat /u/v/r want 5", - "rename / /z want err", + "move__ o=F / /z want err", " find / /a /d /u /u/v /u/v/q /u/v/r", " stat /a want 1", " stat /b want errNotExist", @@ -445,13 +442,13 @@ func testFS(t *testing.T, fs FileSystem) { t.Fatalf("test case #%d %q:\ngot %s\nwant %s", i, tc, got, want) } - case "copy__", "mk-dir", "rename", "rm-all", "stat": + case "copy__", "mk-dir", "move__", "rm-all", "stat": nParts := 3 switch op { case "copy__": nParts = 6 - case "rename": - nParts = 4 + case "move__": + nParts = 5 } parts := strings.Split(arg, " ") if len(parts) != nParts { @@ -461,18 +458,15 @@ func testFS(t *testing.T, fs FileSystem) { got, opErr := "", error(nil) switch op { case "copy__": - overwrite, depth := false, 0 - if parts[0] == "o=T" { - overwrite = true - } + depth := 0 if parts[1] == "d=∞" { depth = infiniteDepth } - _, opErr = copyFiles(fs, parts[2], parts[3], overwrite, depth, 0) + _, opErr = copyFiles(fs, parts[2], parts[3], parts[0] == "o=T", depth, 0) case "mk-dir": opErr = fs.Mkdir(parts[0], 0777) - case "rename": - opErr = fs.Rename(parts[0], parts[1]) + case "move__": + _, opErr = moveFiles(fs, parts[1], parts[2], parts[0] == "o=T") case "rm-all": opErr = fs.RemoveAll(parts[0]) case "stat": diff --git a/webdav/webdav.go b/webdav/webdav.go index 7776fbf..34a872c 100644 --- a/webdav/webdav.go +++ b/webdav/webdav.go @@ -247,36 +247,7 @@ func (h *Handler) handleCopyMove(w http.ResponseWriter, r *http.Request) (status return http.StatusBadRequest, errInvalidDepth } } - - created := false - if _, err := h.FileSystem.Stat(dst); err != nil { - if !os.IsNotExist(err) { - return http.StatusForbidden, err - } - created = true - } else { - switch r.Header.Get("Overwrite") { - case "T": - // Section 9.9.3 says that "If a resource exists at the destination - // and the Overwrite header is "T", then prior to performing the move, - // the server must perform a DELETE with "Depth: infinity" on the - // destination resource. - if err := h.FileSystem.RemoveAll(dst); err != nil { - return http.StatusForbidden, err - } - case "F": - return http.StatusPreconditionFailed, os.ErrExist - default: - return http.StatusBadRequest, errInvalidOverwrite - } - } - if err := h.FileSystem.Rename(src, dst); err != nil { - return http.StatusForbidden, err - } - if created { - return http.StatusCreated, nil - } - return http.StatusNoContent, nil + return moveFiles(h.FileSystem, src, dst, r.Header.Get("Overwrite") == "T") } func (h *Handler) handleLock(w http.ResponseWriter, r *http.Request) (retStatus int, retErr error) { @@ -450,7 +421,6 @@ var ( errInvalidIfHeader = errors.New("webdav: invalid If header") errInvalidLockInfo = errors.New("webdav: invalid lock info") errInvalidLockToken = errors.New("webdav: invalid lock token") - errInvalidOverwrite = errors.New("webdav: invalid overwrite") errInvalidPropfind = errors.New("webdav: invalid propfind") errInvalidResponse = errors.New("webdav: invalid response") errInvalidTimeout = errors.New("webdav: invalid timeout") |