webdav: improve support for url prefixes

StripPrefix in the webdav package strips prefixes from requests
(including the Destination headers) but cannot handle the paths
in the xml entities responses which are confusing some clients
(e.g. cadaver).

This patch replaces StripPrefix with Prefix in the Handler to
handle prefixes both in the requests and in the xml entities
responses.

Change-Id: I67062e30337b2ae422c82a2f927454f5a8a00e34
Reviewed-on: https://go-review.googlesource.com/13857
Reviewed-by: Nigel Tao <nigeltao@golang.org>

webdav/webdav.go

@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path"
 	"runtime"
 	"strings"
 	"time"
@@ -38,6 +39,8 @@ func init() {
 }
 
 type Handler struct {
+	// Prefix is the URL path prefix to strip from WebDAV resource paths.
+	Prefix string
 	// FileSystem is the virtual file system.
 	FileSystem FileSystem
 	// LockSystem is the lock management system.
@@ -47,6 +50,16 @@ type Handler struct {
 	Logger func(*http.Request, error)
 }
 
+func (h *Handler) stripPrefix(p string) (string, int, error) {
+	if h.Prefix == "" {
+		return p, http.StatusOK, nil
+	}
+	if r := strings.TrimPrefix(p, h.Prefix); len(r) < len(p) {
+		return r, http.StatusOK, nil
+	}
+	return p, http.StatusNotFound, errPrefixMismatch
+}
+
 func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	status, err := http.StatusBadRequest, errUnsupportedMethod
 	if h.FileSystem == nil {
@@ -175,8 +188,12 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func()
 }
 
 func (h *Handler) handleOptions(w http.ResponseWriter, r *http.Request) (status int, err error) {
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
 	allow := "OPTIONS, LOCK, PUT, MKCOL"
-	if fi, err := h.FileSystem.Stat(r.URL.Path); err == nil {
+	if fi, err := h.FileSystem.Stat(reqPath); err == nil {
 		if fi.IsDir() {
 			allow = "OPTIONS, LOCK, GET, HEAD, POST, DELETE, PROPPATCH, COPY, MOVE, UNLOCK, PROPFIND"
 		} else {
@@ -192,8 +209,12 @@ func (h *Handler) handleOptions(w http.ResponseWriter, r *http.Request) (status
 }
 
 func (h *Handler) handleGetHeadPost(w http.ResponseWriter, r *http.Request) (status int, err error) {
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
 	// TODO: check locks for read-only access??
-	f, err := h.FileSystem.OpenFile(r.URL.Path, os.O_RDONLY, 0)
+	f, err := h.FileSystem.OpenFile(reqPath, os.O_RDONLY, 0)
 	if err != nil {
 		return http.StatusNotFound, err
 	}
@@ -203,19 +224,23 @@ func (h *Handler) handleGetHeadPost(w http.ResponseWriter, r *http.Request) (sta
 		return http.StatusNotFound, err
 	}
 	if !fi.IsDir() {
-		etag, err := findETag(h.FileSystem, h.LockSystem, r.URL.Path, fi)
+		etag, err := findETag(h.FileSystem, h.LockSystem, reqPath, fi)
 		if err != nil {
 			return http.StatusInternalServerError, err
 		}
 		w.Header().Set("ETag", etag)
 	}
 	// Let ServeContent determine the Content-Type header.
-	http.ServeContent(w, r, r.URL.Path, fi.ModTime(), f)
+	http.ServeContent(w, r, reqPath, fi.ModTime(), f)
 	return 0, nil
 }
 
 func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) (status int, err error) {
-	release, status, err := h.confirmLocks(r, r.URL.Path, "")
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+	release, status, err := h.confirmLocks(r, reqPath, "")
 	if err != nil {
 		return status, err
 	}
@@ -226,20 +251,24 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) (status i
 	// "godoc os RemoveAll" says that "If the path does not exist, RemoveAll
 	// returns nil (no error)." WebDAV semantics are that it should return a
 	// "404 Not Found". We therefore have to Stat before we RemoveAll.
-	if _, err := h.FileSystem.Stat(r.URL.Path); err != nil {
+	if _, err := h.FileSystem.Stat(reqPath); err != nil {
 		if os.IsNotExist(err) {
 			return http.StatusNotFound, err
 		}
 		return http.StatusMethodNotAllowed, err
 	}
-	if err := h.FileSystem.RemoveAll(r.URL.Path); err != nil {
+	if err := h.FileSystem.RemoveAll(reqPath); err != nil {
 		return http.StatusMethodNotAllowed, err
 	}
 	return http.StatusNoContent, nil
 }
 
 func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int, err error) {
-	release, status, err := h.confirmLocks(r, r.URL.Path, "")
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+	release, status, err := h.confirmLocks(r, reqPath, "")
 	if err != nil {
 		return status, err
 	}
@@ -247,7 +276,7 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int,
 	// TODO(rost): Support the If-Match, If-None-Match headers? See bradfitz'
 	// comments in http.checkEtag.
 
-	f, err := h.FileSystem.OpenFile(r.URL.Path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+	f, err := h.FileSystem.OpenFile(reqPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
 	if err != nil {
 		return http.StatusNotFound, err
 	}
@@ -264,7 +293,7 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int,
 	if closeErr != nil {
 		return http.StatusMethodNotAllowed, closeErr
 	}
-	etag, err := findETag(h.FileSystem, h.LockSystem, r.URL.Path, fi)
+	etag, err := findETag(h.FileSystem, h.LockSystem, reqPath, fi)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -273,7 +302,11 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int,
 }
 
 func (h *Handler) handleMkcol(w http.ResponseWriter, r *http.Request) (status int, err error) {
-	release, status, err := h.confirmLocks(r, r.URL.Path, "")
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+	release, status, err := h.confirmLocks(r, reqPath, "")
 	if err != nil {
 		return status, err
 	}
@@ -282,7 +315,7 @@ func (h *Handler) handleMkcol(w http.ResponseWriter, r *http.Request) (status in
 	if r.ContentLength > 0 {
 		return http.StatusUnsupportedMediaType, nil
 	}
-	if err := h.FileSystem.Mkdir(r.URL.Path, 0777); err != nil {
+	if err := h.FileSystem.Mkdir(reqPath, 0777); err != nil {
 		if os.IsNotExist(err) {
 			return http.StatusConflict, err
 		}
@@ -304,7 +337,16 @@ func (h *Handler) handleCopyMove(w http.ResponseWriter, r *http.Request) (status
 		return http.StatusBadGateway, errInvalidDestination
 	}
 
-	dst, src := u.Path, r.URL.Path
+	src, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+
+	dst, status, err := h.stripPrefix(u.Path)
+	if err != nil {
+		return status, err
+	}
+
 	if dst == "" {
 		return http.StatusBadGateway, errInvalidDestination
 	}
@@ -398,8 +440,12 @@ func (h *Handler) handleLock(w http.ResponseWriter, r *http.Request) (retStatus
 				return http.StatusBadRequest, errInvalidDepth
 			}
 		}
+		reqPath, status, err := h.stripPrefix(r.URL.Path)
+		if err != nil {
+			return status, err
+		}
 		ld = LockDetails{
-			Root:      r.URL.Path,
+			Root:      reqPath,
 			Duration:  duration,
 			OwnerXML:  li.Owner.InnerXML,
 			ZeroDepth: depth == 0,
@@ -418,8 +464,8 @@ func (h *Handler) handleLock(w http.ResponseWriter, r *http.Request) (retStatus
 		}()
 
 		// Create the resource if it didn't previously exist.
-		if _, err := h.FileSystem.Stat(r.URL.Path); err != nil {
-			f, err := h.FileSystem.OpenFile(r.URL.Path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+		if _, err := h.FileSystem.Stat(reqPath); err != nil {
+			f, err := h.FileSystem.OpenFile(reqPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
 			if err != nil {
 				// TODO: detect missing intermediate dirs and return http.StatusConflict?
 				return http.StatusInternalServerError, err
@@ -468,7 +514,11 @@ func (h *Handler) handleUnlock(w http.ResponseWriter, r *http.Request) (status i
 }
 
 func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status int, err error) {
-	fi, err := h.FileSystem.Stat(r.URL.Path)
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+	fi, err := h.FileSystem.Stat(reqPath)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return http.StatusNotFound, err
@@ -489,13 +539,13 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status
 
 	mw := multistatusWriter{w: w}
 
-	walkFn := func(path string, info os.FileInfo, err error) error {
+	walkFn := func(reqPath string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
 		var pstats []Propstat
 		if pf.Propname != nil {
-			pnames, err := propnames(h.FileSystem, h.LockSystem, path)
+			pnames, err := propnames(h.FileSystem, h.LockSystem, reqPath)
 			if err != nil {
 				return err
 			}
@@ -505,17 +555,17 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status
 			}
 			pstats = append(pstats, pstat)
 		} else if pf.Allprop != nil {
-			pstats, err = allprop(h.FileSystem, h.LockSystem, path, pf.Prop)
+			pstats, err = allprop(h.FileSystem, h.LockSystem, reqPath, pf.Prop)
 		} else {
-			pstats, err = props(h.FileSystem, h.LockSystem, path, pf.Prop)
+			pstats, err = props(h.FileSystem, h.LockSystem, reqPath, pf.Prop)
 		}
 		if err != nil {
 			return err
 		}
-		return mw.write(makePropstatResponse(path, pstats))
+		return mw.write(makePropstatResponse(path.Join(h.Prefix, reqPath), pstats))
 	}
 
-	walkErr := walkFS(h.FileSystem, depth, r.URL.Path, fi, walkFn)
+	walkErr := walkFS(h.FileSystem, depth, reqPath, fi, walkFn)
 	closeErr := mw.close()
 	if walkErr != nil {
 		return http.StatusInternalServerError, walkErr
@@ -527,13 +577,17 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status
 }
 
 func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (status int, err error) {
-	release, status, err := h.confirmLocks(r, r.URL.Path, "")
+	reqPath, status, err := h.stripPrefix(r.URL.Path)
+	if err != nil {
+		return status, err
+	}
+	release, status, err := h.confirmLocks(r, reqPath, "")
 	if err != nil {
 		return status, err
 	}
 	defer release()
 
-	if _, err := h.FileSystem.Stat(r.URL.Path); err != nil {
+	if _, err := h.FileSystem.Stat(reqPath); err != nil {
 		if os.IsNotExist(err) {
 			return http.StatusNotFound, err
 		}
@@ -543,7 +597,7 @@ func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (statu
 	if err != nil {
 		return status, err
 	}
-	pstats, err := patch(h.FileSystem, h.LockSystem, r.URL.Path, patches)
+	pstats, err := patch(h.FileSystem, h.LockSystem, reqPath, patches)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -605,29 +659,6 @@ func parseDepth(s string) int {
 	return invalidDepth
 }
 
-// StripPrefix is like http.StripPrefix but it also strips the prefix from any
-// Destination headers, so that COPY and MOVE requests also see stripped paths.
-func StripPrefix(prefix string, h http.Handler) http.Handler {
-	if prefix == "" {
-		return h
-	}
-	h = http.StripPrefix(prefix, h)
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		dsts := r.Header["Destination"]
-		for i, dst := range dsts {
-			u, err := url.Parse(dst)
-			if err != nil {
-				continue
-			}
-			if p := strings.TrimPrefix(u.Path, prefix); len(p) < len(u.Path) {
-				u.Path = p
-				dsts[i] = u.String()
-			}
-		}
-		h.ServeHTTP(w, r)
-	})
-}
-
 // http://www.webdav.org/specs/rfc4918.html#status.code.extensions.to.http11
 const (
 	StatusMulti               = 207
@@ -668,6 +699,7 @@ var (
 	errNoFileSystem            = errors.New("webdav: no file system")
 	errNoLockSystem            = errors.New("webdav: no lock system")
 	errNotADirectory           = errors.New("webdav: not a directory")
+	errPrefixMismatch          = errors.New("webdav: prefix mismatch")
 	errRecursionTooDeep        = errors.New("webdav: recursion too deep")
 	errUnsupportedLockInfo     = errors.New("webdav: unsupported lock info")
 	errUnsupportedMethod       = errors.New("webdav: unsupported method")

webdav/webdav_test.go

@@ -15,13 +15,8 @@ import (
 	"testing"
 )
 
-// TestStripPrefix tests the StripPrefix function. We can't test the
-// StripPrefix function with the litmus test, even though all of the litmus
-// test paths start with "/litmus/", because one of the first things that the
-// litmus test does is "MKCOL /litmus/". That request succeeds without a
-// StripPrefix, but fails with a StripPrefix because you cannot MKCOL the root
-// directory of a FileSystem.
-func TestStripPrefix(t *testing.T) {
+// TODO: add tests to check XML responses with the expected prefix path
+func TestPrefix(t *testing.T) {
 	const dst, blah = "Destination", "blah blah blah"
 
 	do := func(method, urlStr string, body io.Reader, wantStatusCode int, headers ...string) error {
@@ -52,14 +47,13 @@ func TestStripPrefix(t *testing.T) {
 	}
 	for _, prefix := range prefixes {
 		fs := NewMemFS()
-		h := http.Handler(&Handler{
+		h := &Handler{
 			FileSystem: fs,
 			LockSystem: NewMemLS(),
-		})
+		}
 		mux := http.NewServeMux()
 		if prefix != "/" {
-			// Note that this is webdav.StripPrefix, not http.StripPrefix.
-			h = StripPrefix(prefix, h)
+			h.Prefix = prefix
 		}
 		mux.Handle(prefix, h)
 		srv := httptest.NewServer(mux)