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>
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 @@
//
// 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 @@
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 @@
" 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 @@
" 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 @@
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 @@
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 @@
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 @@
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")
