mirror of
https://github.com/rclone/rclone.git
synced 2026-07-11 16:25:42 -04:00
serve webdav: fix MOVE overwrite failing without Overwrite header
Per RFC 4918 section 10.6, when the Overwrite header is omitted from a
COPY or MOVE request the resource MUST treat the request as if
Overwrite: T had been sent.
The upstream golang.org/x/net/webdav library mishandles this for MOVE
by checking == "T" instead of != "F", so an absent header is treated
as Overwrite: F and the request fails with 412 Precondition Failed.
Normalise the header to T in the rclone WebDAV server before
delegating to the upstream handler when the client did not send one.
This restores RFC-compliant default behaviour and can be removed once
the upstream fix in golang/go#66059 lands and the golang.org/x/net
dependency is bumped.
Fixes #9496
(cherry picked from commit cfb9a10a3d)
This commit is contained in:
committed by
Nick Craig-Wood
parent
eb09655949
commit
39487c5ebf
@@ -391,6 +391,12 @@ func (w *WebDAV) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
|
||||
w.serveDir(rw, r, remote)
|
||||
return
|
||||
}
|
||||
// Work around bug in x/net/webdav which handles Overwrite incorrectly
|
||||
// See: https://github.com/golang/go/issues/66059
|
||||
// Remove when the above bug is fixed.
|
||||
if (r.Method == "COPY" || r.Method == "MOVE") && r.Header.Get("Overwrite") == "" {
|
||||
r.Header.Set("Overwrite", "T")
|
||||
}
|
||||
// Add URL Prefix back to path since webdavhandler needs to
|
||||
// return absolute references.
|
||||
r.URL.Path = w.opt.HTTP.BaseURL + r.URL.Path
|
||||
|
||||
@@ -271,3 +271,89 @@ func TestRc(t *testing.T) {
|
||||
"vfs_cache_mode": "off",
|
||||
})
|
||||
}
|
||||
|
||||
// startWritableServer starts a webdav server backed by a fresh temp
|
||||
// directory and returns the server URL. It is used by the Overwrite tests
|
||||
// which need to exercise mutating verbs such as MKCOL and MOVE.
|
||||
func startWritableServer(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
f, err := fs.NewFs(context.Background(), t.TempDir())
|
||||
require.NoError(t, err)
|
||||
|
||||
opt := Opt
|
||||
opt.HTTP.ListenAddr = []string{testBindAddress}
|
||||
|
||||
w, err := newWebDAV(context.Background(), f, &opt, &vfscommon.Opt, &proxy.Opt)
|
||||
require.NoError(t, err)
|
||||
go func() {
|
||||
require.NoError(t, w.Serve())
|
||||
}()
|
||||
t.Cleanup(func() {
|
||||
assert.NoError(t, w.Shutdown())
|
||||
})
|
||||
|
||||
return w.server.URLs()[0]
|
||||
}
|
||||
|
||||
func mkcol(t *testing.T, baseURL, path string) {
|
||||
t.Helper()
|
||||
req, err := http.NewRequest("MKCOL", baseURL+path, nil)
|
||||
require.NoError(t, err)
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
_ = resp.Body.Close()
|
||||
require.Equal(t, http.StatusCreated, resp.StatusCode, "MKCOL %s", path)
|
||||
}
|
||||
|
||||
// TestMoveDefaultsToOverwrite is a regression test for
|
||||
// https://github.com/rclone/rclone/issues/9496
|
||||
//
|
||||
// RFC 4918 section 10.6 requires that when the Overwrite header is omitted
|
||||
// from a COPY or MOVE request, the resource MUST behave as if Overwrite: T
|
||||
// had been sent. The upstream golang.org/x/net/webdav library mis-handles
|
||||
// the MOVE case (see https://github.com/golang/go/issues/66059), so rclone
|
||||
// normalises the header before delegating so the default matches the RFC.
|
||||
func TestMoveDefaultsToOverwrite(t *testing.T) {
|
||||
testURL := startWritableServer(t)
|
||||
|
||||
mkcol(t, testURL, "dir1")
|
||||
mkcol(t, testURL, "dir2")
|
||||
|
||||
// MOVE without Overwrite header: per RFC 4918 the default is T, so the
|
||||
// existing destination must be replaced and the server must return 2xx.
|
||||
req, err := http.NewRequest("MOVE", testURL+"dir2", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Destination", testURL+"dir1")
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
assert.NotEqual(t, http.StatusPreconditionFailed, resp.StatusCode,
|
||||
"MOVE without Overwrite header must not return 412; RFC 4918 default is Overwrite: T")
|
||||
assert.True(t, resp.StatusCode >= 200 && resp.StatusCode < 300,
|
||||
"expected 2xx, got %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
// TestMoveOverwriteFalseStillRejects ensures the rclone normalisation only
|
||||
// fills in a missing Overwrite header and never overrides an explicit
|
||||
// Overwrite: F sent by the client.
|
||||
func TestMoveOverwriteFalseStillRejects(t *testing.T) {
|
||||
testURL := startWritableServer(t)
|
||||
|
||||
mkcol(t, testURL, "dir1")
|
||||
mkcol(t, testURL, "dir2")
|
||||
|
||||
req, err := http.NewRequest("MOVE", testURL+"dir2", nil)
|
||||
require.NoError(t, err)
|
||||
req.Header.Set("Destination", testURL+"dir1")
|
||||
req.Header.Set("Overwrite", "F")
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
assert.Equal(t, http.StatusPreconditionFailed, resp.StatusCode,
|
||||
"MOVE with explicit Overwrite: F must still return 412 when destination exists")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user