webdav: Support empty multistatus responses and simplify error handling.

Currently, the package-internal multistatus writer does not allow to
write empty XML multistatus elements. This conflicts with RFC 4918
(see [1]), and prevents to reuse the writer to implement REPORTs and
other WebDAV extensions. This CL adds support to write empty multi-
status responses. Based on this, it simplifies the error handling of
PROPFIND and PROPPATCH request handling.

[1] http://webdav.org/specs/rfc4918.html#ELEMENT_multistatus

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

webdav/webdav.go

@@ -500,18 +500,15 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status
 		return mw.write(makePropstatResponse(path, pstats))
 	}
 
-	err = walkFS(h.FileSystem, depth, r.URL.Path, fi, walkFn)
-	if mw.enc == nil {
-		if err == nil {
-			err = errEmptyMultistatus
-		}
-		// Not a single response has been written.
-		return http.StatusInternalServerError, err
+	walkErr := walkFS(h.FileSystem, depth, r.URL.Path, fi, walkFn)
+	closeErr := mw.close()
+	if walkErr != nil {
+		return http.StatusInternalServerError, walkErr
 	}
-	if err != nil {
-		return 0, err
+	if closeErr != nil {
+		return http.StatusInternalServerError, closeErr
 	}
-	return 0, mw.close()
+	return 0, nil
 }
 
 func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (status int, err error) {
@@ -541,7 +538,10 @@ func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (statu
 	if writeErr != nil {
 		return http.StatusInternalServerError, writeErr
 	}
-	return 0, closeErr
+	if closeErr != nil {
+		return http.StatusInternalServerError, closeErr
+	}
+	return 0, nil
 }
 
 // davHeaderNames maps the names of DAV properties to their corresponding
@@ -638,7 +638,6 @@ func StatusText(code int) string {
 var (
 	errDestinationEqualsSource = errors.New("webdav: destination equals source")
 	errDirectoryNotEmpty       = errors.New("webdav: directory not empty")
-	errEmptyMultistatus        = errors.New("webdav: empty multistatus response")
 	errInvalidDepth            = errors.New("webdav: invalid depth")
 	errInvalidDestination      = errors.New("webdav: invalid destination")
 	errInvalidIfHeader         = errors.New("webdav: invalid If header")

webdav/xml.go

@@ -264,31 +264,39 @@ func (w *multistatusWriter) write(r *response) error {
 			return errInvalidResponse
 		}
 	}
-	if w.enc == nil {
-		w.w.Header().Add("Content-Type", "text/xml; charset=utf-8")
-		w.w.WriteHeader(StatusMulti)
-		_, err := fmt.Fprintf(w.w, `<?xml version="1.0" encoding="UTF-8"?>`)
-		if err != nil {
-			return err
-		}
-		w.enc = xml.NewEncoder(w.w)
-		err = w.enc.EncodeToken(xml.StartElement{
-			Name: xml.Name{
-				Space: "DAV:",
-				Local: "multistatus",
-			},
-			Attr: []xml.Attr{{
-				Name:  xml.Name{Local: "xmlns"},
-				Value: "DAV:",
-			}},
-		})
-		if err != nil {
-			return err
-		}
+	err := w.writeHeader()
+	if err != nil {
+		return err
 	}
 	return w.enc.Encode(r)
 }
 
+// writeHeader writes a XML multistatus start element on w's underlying
+// http.ResponseWriter and returns the result of the write operation.
+// After the first write attempt, writeHeader becomes a no-op.
+func (w *multistatusWriter) writeHeader() error {
+	if w.enc != nil {
+		return nil
+	}
+	w.w.Header().Add("Content-Type", "text/xml; charset=utf-8")
+	w.w.WriteHeader(StatusMulti)
+	_, err := fmt.Fprintf(w.w, `<?xml version="1.0" encoding="UTF-8"?>`)
+	if err != nil {
+		return err
+	}
+	w.enc = xml.NewEncoder(w.w)
+	return w.enc.EncodeToken(xml.StartElement{
+		Name: xml.Name{
+			Space: "DAV:",
+			Local: "multistatus",
+		},
+		Attr: []xml.Attr{{
+			Name:  xml.Name{Local: "xmlns"},
+			Value: "DAV:",
+		}},
+	})
+}
+
 // Close completes the marshalling of the multistatus response. It returns
 // an error if the multistatus response could not be completed. If both the
 // return value and field enc of w are nil, then no multistatus response has

webdav/xml_test.go

@@ -348,12 +348,13 @@ func TestMultistatusWriter(t *testing.T) {
 	///The "section x.y.z" test cases come from section x.y.z of the spec at
 	// http://www.webdav.org/specs/rfc4918.html
 	testCases := []struct {
-		desc      string
-		responses []response
-		respdesc  string
-		wantXML   string
-		wantCode  int
-		wantErr   error
+		desc        string
+		responses   []response
+		respdesc    string
+		writeHeader bool
+		wantXML     string
+		wantCode    int
+		wantErr     error
 	}{{
 		desc: "section 9.2.2 (failed dependency)",
 		responses: []response{{
@@ -472,14 +473,19 @@ func TestMultistatusWriter(t *testing.T) {
 			`</multistatus>`,
 		wantCode: StatusMulti,
 	}, {
-		desc: "bad: no response written",
+		desc: "no response written",
 		// default of http.responseWriter
 		wantCode: http.StatusOK,
 	}, {
-		desc:     "bad: no response written (with description)",
+		desc:     "no response written (with description)",
 		respdesc: "too bad",
 		// default of http.responseWriter
 		wantCode: http.StatusOK,
+	}, {
+		desc:        "empty multistatus with header",
+		writeHeader: true,
+		wantXML:     `<multistatus xmlns="DAV:"></multistatus>`,
+		wantCode:    StatusMulti,
 	}, {
 		desc: "bad: no href",
 		responses: []response{{
@@ -556,6 +562,12 @@ loop:
 	for _, tc := range testCases {
 		rec := httptest.NewRecorder()
 		w := multistatusWriter{w: rec, responseDescription: tc.respdesc}
+		if tc.writeHeader {
+			if err := w.writeHeader(); err != nil {
+				t.Errorf("%s: got writeHeader error %v, want nil", tc.desc, err)
+				continue
+			}
+		}
 		for _, r := range tc.responses {
 			if err := w.write(&r); err != nil {
 				if err != tc.wantErr {