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/9186
Reviewed-by: Nigel Tao <nigeltao@golang.org>
diff --git a/webdav/webdav.go b/webdav/webdav.go
index 51052fd..897acb2 100644
--- a/webdav/webdav.go
+++ b/webdav/webdav.go
@@ -500,18 +500,15 @@
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 @@
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 @@
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")
diff --git a/webdav/xml.go b/webdav/xml.go
index c7bb5c0..cfb56e8 100644
--- a/webdav/xml.go
+++ b/webdav/xml.go
@@ -264,31 +264,39 @@
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
diff --git a/webdav/xml_test.go b/webdav/xml_test.go
index 0559f86..ce40c55 100644
--- a/webdav/xml_test.go
+++ b/webdav/xml_test.go
@@ -348,12 +348,13 @@
///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,15 +473,20 @@
`</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{{
Propstat: []propstat{{
@@ -556,6 +562,12 @@
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 {
