From 787f8c08ec6a7b760ddc9ef09fe6aaf8ba196362 Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Thu, 18 Apr 2024 12:11:20 -0500 Subject: [PATCH] drive: rewrite Location headers This ensures that MOVE, LOCK and any other verbs that use the Location header work correctly. Fixes #11758 Signed-off-by: Percy Wegmann --- drive/driveimpl/compositedav/compositedav.go | 27 ++++++++++++++++---- drive/driveimpl/compositedav/propfind.go | 2 +- drive/driveimpl/drive_test.go | 20 +++++++++++++-- drive/driveimpl/shared/pathutil.go | 11 ++++++++ 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/drive/driveimpl/compositedav/compositedav.go b/drive/driveimpl/compositedav/compositedav.go index 9bcba158e..024f8be3b 100644 --- a/drive/driveimpl/compositedav/compositedav.go +++ b/drive/driveimpl/compositedav/compositedav.go @@ -100,7 +100,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { pathComponents := shared.CleanAndSplit(r.URL.Path) if len(pathComponents) >= mpl { - h.delegate(pathComponents[mpl-1:], w, r) + h.delegate(mpl, pathComponents[mpl-1:], w, r) return } h.handle(w, r) @@ -129,24 +129,41 @@ func (h *Handler) handle(w http.ResponseWriter, r *http.Request) { } // delegate sends the request to the Child WebDAV server. -func (h *Handler) delegate(pathComponents []string, w http.ResponseWriter, r *http.Request) string { +func (h *Handler) delegate(mpl int, pathComponents []string, w http.ResponseWriter, r *http.Request) { + dest := r.Header.Get("Destination") + if dest != "" { + // Rewrite destination header + destURL, err := url.Parse(dest) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + destinationComponents := shared.CleanAndSplit(destURL.Path) + if len(destinationComponents) < mpl || destinationComponents[mpl-1] != pathComponents[0] { + http.Error(w, "Destination across shares is not supported", http.StatusBadRequest) + return + } + updatedDest := shared.JoinEscaped(destinationComponents[mpl:]...) + r.Header.Set("Destination", updatedDest) + } + childName := pathComponents[0] child := h.GetChild(childName) if child == nil { w.WriteHeader(http.StatusNotFound) - return childName + return } + u, err := url.Parse(child.BaseURL) if err != nil { h.logf("warning: parse base URL %s failed: %s", child.BaseURL, err) http.Error(w, err.Error(), http.StatusInternalServerError) - return childName + return } u.Path = path.Join(u.Path, shared.Join(pathComponents[1:]...)) r.URL = u r.Host = u.Host child.rp.ServeHTTP(w, r) - return childName } // SetChildren replaces the entire existing set of children with the given diff --git a/drive/driveimpl/compositedav/propfind.go b/drive/driveimpl/compositedav/propfind.go index 868a1d2b2..60b90edb2 100644 --- a/drive/driveimpl/compositedav/propfind.go +++ b/drive/driveimpl/compositedav/propfind.go @@ -37,7 +37,7 @@ func (h *Handler) handlePROPFIND(w http.ResponseWriter, r *http.Request) { bw := &bufferingResponseWriter{ResponseWriter: w} mpl := h.maxPathLength(r) - h.delegate(pathComponents[mpl-1:], bw, r) + h.delegate(mpl, pathComponents[mpl-1:], bw, r) // Fixup paths to add the requested path as a prefix. pathPrefix := shared.Join(pathComponents[0:mpl]...) diff --git a/drive/driveimpl/drive_test.go b/drive/driveimpl/drive_test.go index 2f25cf50b..6dc4c7ce5 100644 --- a/drive/driveimpl/drive_test.go +++ b/drive/driveimpl/drive_test.go @@ -33,6 +33,7 @@ const ( share11 = `sha re$%11` share12 = `_sha re$%12` file111 = `fi le$%111.txt` + file112 = `file112.txt` ) func init() { @@ -81,9 +82,13 @@ func TestFileManipulation(t *testing.T) { s.checkFileStatus(remote1, share11, file111) s.checkFileContents(remote1, share11, file111) + s.renameFile("renaming file across shares should fail", remote1, share11, file111, share12, file112, false) + + s.renameFile("renaming file in same share should succeed", remote1, share11, file111, share11, file112, true) + s.checkFileContents(remote1, share11, file112) + s.addShare(remote1, share12, drive.PermissionReadOnly) s.writeFile("writing file to read-only remote should fail", remote1, share12, file111, "hello world", false) - s.writeFile("writing file to non-existent remote should fail", "non-existent", share11, file111, "hello world", false) s.writeFile("writing file to non-existent share should fail", remote1, "non-existent", file111, "hello world", false) } @@ -241,7 +246,18 @@ func (s *system) writeFile(label, remoteName, shareName, name, contents string, if expectSuccess && err != nil { s.t.Fatalf("%v: expected success writing file %q, but got error %v", label, path, err) } else if !expectSuccess && err == nil { - s.t.Fatalf("%v: expected error writing file %q", label, path) + s.t.Fatalf("%v: expected error writing file %q, but got no error", label, path) + } +} + +func (s *system) renameFile(label, remoteName, fromShare, fromFile, toShare, toFile string, expectSuccess bool) { + fromPath := pathTo(remoteName, fromShare, fromFile) + toPath := pathTo(remoteName, toShare, toFile) + err := s.client.Rename(fromPath, toPath, true) + if expectSuccess && err != nil { + s.t.Fatalf("%v: expected success moving file %q to %q, but got error %v", label, fromPath, toPath, err) + } else if !expectSuccess && err == nil { + s.t.Fatalf("%v: expected error moving file %q to %q, but got no error", label, fromPath, toPath) } } diff --git a/drive/driveimpl/shared/pathutil.go b/drive/driveimpl/shared/pathutil.go index 49f8ba849..748f75701 100644 --- a/drive/driveimpl/shared/pathutil.go +++ b/drive/driveimpl/shared/pathutil.go @@ -4,6 +4,7 @@ package shared import ( + "net/url" "path" "strings" ) @@ -35,6 +36,16 @@ func Join(parts ...string) string { return path.Join(fullParts...) } +// JoinEscaped is like Join but path escapes each part. +func JoinEscaped(parts ...string) string { + fullParts := make([]string, 0, len(parts)) + fullParts = append(fullParts, sepString) + for _, part := range parts { + fullParts = append(fullParts, url.PathEscape(part)) + } + return path.Join(fullParts...) +} + // IsRoot determines whether a given path p is the root path, defined as either // empty or "/". func IsRoot(p string) bool {