Skip to content

Commit

Permalink
webdav: factor out a moveFiles function, and make the tests call that
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
  • Loading branch information
@nigeltao
nigeltao committed Feb 10, 2015
1 parent f090b05 commit 5b4754d
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 54 deletions.
34 changes: 34 additions & 0 deletions webdav/file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
40 changes: 17 additions & 23 deletions webdav/file_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,44 +321,41 @@ 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",
" stat /t want dir",
" 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",
Expand Down Expand Up @@ -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 {
Expand All @@ -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":
Expand Down
32 changes: 1 addition & 31 deletions webdav/webdav.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 5b4754d

Please sign in to comment.