webdav: Simplify handling of Etag and Content-Type headers for GET, HEAD,

POST and PUT requests.

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

Author: rsto

webdav/prop.go

@@ -359,13 +359,8 @@ func findContentType(fs FileSystem, ls LockSystem, name string, fi os.FileInfo)
 }
 
 func findETag(fs FileSystem, ls LockSystem, name string, fi os.FileInfo) (string, error) {
-	return detectETag(fi), nil
-}
-
-// detectETag determines the ETag for the file described by fi.
-func detectETag(fi os.FileInfo) string {
 	// The Apache http 2.4 web server by default concatenates the
 	// modification time and size of a file. We replicate the heuristic
 	// with nanosecond granularity.
-	return fmt.Sprintf(`"%x%x"`, fi.ModTime().UnixNano(), fi.Size())
+	return fmt.Sprintf(`"%x%x"`, fi.ModTime().UnixNano(), fi.Size()), nil
 }

webdav/prop_test.go

@@ -17,7 +17,7 @@ import (
 func TestMemPS(t *testing.T) {
 	// calcProps calculates the getlastmodified and getetag DAV: property
 	// values in pstats for resource name in file-system fs.
-	calcProps := func(name string, fs FileSystem, pstats []Propstat) error {
+	calcProps := func(name string, fs FileSystem, ls LockSystem, pstats []Propstat) error {
 		fi, err := fs.Stat(name)
 		if err != nil {
 			return err
@@ -32,7 +32,11 @@ func TestMemPS(t *testing.T) {
 					if fi.IsDir() {
 						continue
 					}
-					p.InnerXML = []byte(detectETag(fi))
+					etag, err := findETag(fs, ls, name, fi)
+					if err != nil {
+						return err
+					}
+					p.InnerXML = []byte(etag)
 					pst.Props[i] = p
 				}
 			}
@@ -494,7 +498,7 @@ func TestMemPS(t *testing.T) {
 		ls := NewMemLS()
 		for _, op := range tc.propOp {
 			desc := fmt.Sprintf("%s: %s %s", tc.desc, op.op, op.name)
-			if err = calcProps(op.name, fs, op.wantPropstats); err != nil {
+			if err = calcProps(op.name, fs, ls, op.wantPropstats); err != nil {
 				t.Fatalf("%s: calcProps: %v", desc, err)
 			}
 

webdav/webdav.go

@@ -6,7 +6,6 @@
 package webdav // import "golang.org/x/net/webdav"
 
 import (
-	"encoding/xml"
 	"errors"
 	"fmt"
 	"io"
@@ -203,14 +202,14 @@ func (h *Handler) handleGetHeadPost(w http.ResponseWriter, r *http.Request) (sta
 	if err != nil {
 		return http.StatusNotFound, err
 	}
-	pstats, err := props(h.FileSystem, h.LockSystem, r.URL.Path, []xml.Name{
-		{Space: "DAV:", Local: "getetag"},
-		{Space: "DAV:", Local: "getcontenttype"},
-	})
-	if err != nil {
-		return http.StatusInternalServerError, err
+	if !fi.IsDir() {
+		etag, err := findETag(h.FileSystem, h.LockSystem, r.URL.Path, fi)
+		if err != nil {
+			return http.StatusInternalServerError, err
+		}
+		w.Header().Set("ETag", etag)
 	}
-	writeDAVHeaders(w, pstats)
+	// Let ServeContent determine the Content-Type header.
 	http.ServeContent(w, r, r.URL.Path, fi.ModTime(), f)
 	return 0, nil
 }
@@ -245,26 +244,31 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int,
 		return status, err
 	}
 	defer release()
+	// 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)
 	if err != nil {
 		return http.StatusNotFound, err
 	}
 	_, copyErr := io.Copy(f, r.Body)
+	fi, statErr := f.Stat()
 	closeErr := f.Close()
+	// TODO(rost): Returning 405 Method Not Allowed might not be appropriate.
 	if copyErr != nil {
 		return http.StatusMethodNotAllowed, copyErr
 	}
+	if statErr != nil {
+		return http.StatusMethodNotAllowed, statErr
+	}
 	if closeErr != nil {
 		return http.StatusMethodNotAllowed, closeErr
 	}
-	pstats, err := props(h.FileSystem, h.LockSystem, r.URL.Path, []xml.Name{
-		{Space: "DAV:", Local: "getetag"},
-	})
+	etag, err := findETag(h.FileSystem, h.LockSystem, r.URL.Path, fi)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
-	writeDAVHeaders(w, pstats)
+	w.Header().Set("ETag", etag)
 	return http.StatusCreated, nil
 }
 
@@ -557,26 +561,6 @@ func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (statu
 	return 0, nil
 }
 
-// davHeaderNames maps the names of DAV properties to their corresponding
-// HTTP response headers.
-var davHeaderNames = map[xml.Name]string{
-	xml.Name{Space: "DAV:", Local: "getetag"}:        "ETag",
-	xml.Name{Space: "DAV:", Local: "getcontenttype"}: "Content-Type",
-}
-
-func writeDAVHeaders(w http.ResponseWriter, pstats []Propstat) {
-	for _, pst := range pstats {
-		if pst.Status == http.StatusOK {
-			for _, p := range pst.Props {
-				if n, ok := davHeaderNames[p.XMLName]; ok {
-					w.Header().Set(n, string(p.InnerXML))
-				}
-			}
-			break
-		}
-	}
-}
-
 func makePropstatResponse(href string, pstats []Propstat) *response {
 	resp := response{
 		Href:     []string{href},