From 39f816d3f2d23ce1a19c861650a7a940aefe2a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20Gul=C3=A1csi?= Date: Tue, 23 Jun 2015 12:14:34 +0200 Subject: [PATCH] golang.org/x/net/webdav: update commit 34ff4cd5e6de00702100a0ab3bb73de8de5ab35d webdav: make it work for Mini-Redirector, emit namespace-prefixed XML. by Mathieu Lonjaret. Change-Id: Id599fc4fe5064631fe01e59117eb98abdb85ad5f --- third_party/golang.org/x/net/webdav/prop.go | 12 +- .../golang.org/x/net/webdav/prop_test.go | 44 +-- third_party/golang.org/x/net/webdav/xml.go | 99 ++++-- .../golang.org/x/net/webdav/xml_test.go | 330 +++++++++++------- 4 files changed, 304 insertions(+), 181 deletions(-) diff --git a/third_party/golang.org/x/net/webdav/prop.go b/third_party/golang.org/x/net/webdav/prop.go index 1cd068cee..9431f5e73 100644 --- a/third_party/golang.org/x/net/webdav/prop.go +++ b/third_party/golang.org/x/net/webdav/prop.go @@ -278,7 +278,7 @@ loop: if conflict { pstatForbidden := Propstat{ Status: http.StatusForbidden, - XMLError: ``, + XMLError: ``, } pstatFailedDep := Propstat{ Status: StatusFailedDependency, @@ -328,7 +328,7 @@ loop: func findResourceType(fs FileSystem, ls LockSystem, name string, fi os.FileInfo) (string, error) { if fi.IsDir() { - return ``, nil + return ``, nil } return "", nil } @@ -377,8 +377,8 @@ func findETag(fs FileSystem, ls LockSystem, name string, fi os.FileInfo) (string func findSupportedLock(fs FileSystem, ls LockSystem, name string, fi os.FileInfo) (string, error) { return `` + - `` + - `` + - `` + - ``, nil + `` + + `` + + `` + + ``, nil } diff --git a/third_party/golang.org/x/net/webdav/prop_test.go b/third_party/golang.org/x/net/webdav/prop_test.go index f588d4fcb..8bf4aee02 100644 --- a/third_party/golang.org/x/net/webdav/prop_test.go +++ b/third_party/golang.org/x/net/webdav/prop_test.go @@ -44,6 +44,15 @@ func TestMemPS(t *testing.T) { return nil } + const ( + lockEntry = `` + + `` + + `` + + `` + + `` + statForbiddenError = `` + ) + type propOp struct { op string name string @@ -95,7 +104,7 @@ func TestMemPS(t *testing.T) { Status: http.StatusOK, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "resourcetype"}, - InnerXML: []byte(``), + InnerXML: []byte(``), }, { XMLName: xml.Name{Space: "DAV:", Local: "displayname"}, InnerXML: []byte("dir"), @@ -109,13 +118,8 @@ func TestMemPS(t *testing.T) { XMLName: xml.Name{Space: "DAV:", Local: "getcontenttype"}, InnerXML: []byte("text/plain; charset=utf-8"), }, { - XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, - InnerXML: []byte(`` + - `` + - `` + - `` + - ``, - ), + XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, + InnerXML: []byte(lockEntry), }}, }}, }, { @@ -142,13 +146,8 @@ func TestMemPS(t *testing.T) { XMLName: xml.Name{Space: "DAV:", Local: "getetag"}, InnerXML: nil, // Calculated during test. }, { - XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, - InnerXML: []byte(`` + - `` + - `` + - `` + - ``, - ), + XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, + InnerXML: []byte(lockEntry), }}, }}, }, { @@ -179,13 +178,8 @@ func TestMemPS(t *testing.T) { XMLName: xml.Name{Space: "DAV:", Local: "getetag"}, InnerXML: nil, // Calculated during test. }, { - XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, - InnerXML: []byte(`` + - `` + - `` + - `` + - ``, - ), + XMLName: xml.Name{Space: "DAV:", Local: "supportedlock"}, + InnerXML: []byte(lockEntry), }}}, { Status: http.StatusNotFound, Props: []Property{{ @@ -204,7 +198,7 @@ func TestMemPS(t *testing.T) { Status: http.StatusOK, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "resourcetype"}, - InnerXML: []byte(``), + InnerXML: []byte(``), }}, }}, }, { @@ -296,7 +290,7 @@ func TestMemPS(t *testing.T) { }}, wantPropstats: []Propstat{{ Status: http.StatusForbidden, - XMLError: ``, + XMLError: statForbiddenError, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "getetag"}, }}, @@ -351,7 +345,7 @@ func TestMemPS(t *testing.T) { }}, wantPropstats: []Propstat{{ Status: http.StatusForbidden, - XMLError: ``, + XMLError: statForbiddenError, Props: []Property{{ XMLName: xml.Name{Space: "DAV:", Local: "displayname"}, }}, diff --git a/third_party/golang.org/x/net/webdav/xml.go b/third_party/golang.org/x/net/webdav/xml.go index cfb56e8c1..79e5d0762 100644 --- a/third_party/golang.org/x/net/webdav/xml.go +++ b/third_party/golang.org/x/net/webdav/xml.go @@ -206,32 +206,55 @@ type Property struct { } // http://www.webdav.org/specs/rfc4918.html#ELEMENT_error +// See multistatusWriter for the "D:" namespace prefix. type xmlError struct { - XMLName xml.Name `xml:"DAV: error"` + XMLName xml.Name `xml:"D:error"` InnerXML []byte `xml:",innerxml"` } // http://www.webdav.org/specs/rfc4918.html#ELEMENT_propstat +// See multistatusWriter for the "D:" namespace prefix. type propstat struct { - Prop []Property `xml:"DAV: prop>_ignored_"` - Status string `xml:"DAV: status"` - Error *xmlError `xml:"DAV: error"` - ResponseDescription string `xml:"DAV: responsedescription,omitempty"` + Prop []Property `xml:"D:prop>_ignored_"` + Status string `xml:"D:status"` + Error *xmlError `xml:"D:error"` + ResponseDescription string `xml:"D:responsedescription,omitempty"` +} + +// MarshalXML prepends the "D:" namespace prefix on properties in the DAV: namespace +// before encoding. See multistatusWriter. +func (ps propstat) MarshalXML(e *xml.Encoder, start xml.StartElement) error { + for k, prop := range ps.Prop { + if prop.XMLName.Space == "DAV:" { + prop.XMLName = xml.Name{Space: "", Local: "D:" + prop.XMLName.Local} + ps.Prop[k] = prop + } + } + // Distinct type to avoid infinite recursion of MarshalXML. + type newpropstat propstat + return e.EncodeElement(newpropstat(ps), start) } // http://www.webdav.org/specs/rfc4918.html#ELEMENT_response +// See multistatusWriter for the "D:" namespace prefix. type response struct { - XMLName xml.Name `xml:"DAV: response"` - Href []string `xml:"DAV: href"` - Propstat []propstat `xml:"DAV: propstat"` - Status string `xml:"DAV: status,omitempty"` - Error *xmlError `xml:"DAV: error"` - ResponseDescription string `xml:"DAV: responsedescription,omitempty"` + XMLName xml.Name `xml:"D:response"` + Href []string `xml:"D:href"` + Propstat []propstat `xml:"D:propstat"` + Status string `xml:"D:status,omitempty"` + Error *xmlError `xml:"D:error"` + ResponseDescription string `xml:"D:responsedescription,omitempty"` } // MultistatusWriter marshals one or more Responses into a XML // multistatus response. // See http://www.webdav.org/specs/rfc4918.html#ELEMENT_multistatus +// TODO(rsto, mpl): As a workaround, the "D:" namespace prefix, defined as +// "DAV:" on this element, is prepended on the nested response, as well as on all +// its nested elements. All property names in the DAV: namespace are prefixed as +// well. This is because some versions of Mini-Redirector (on windows 7) ignore +// elements with a default namespace (no prefixed namespace). A less intrusive fix +// should be possible after golang.org/cl/11074. See https://golang.org/issue/11177 type multistatusWriter struct { // ResponseDescription contains the optional responsedescription // of the multistatus XML element. Only the latest content before @@ -291,7 +314,7 @@ func (w *multistatusWriter) writeHeader() error { Local: "multistatus", }, Attr: []xml.Attr{{ - Name: xml.Name{Local: "xmlns"}, + Name: xml.Name{Space: "xmlns", Local: "D"}, Value: "DAV:", }}, }) @@ -340,6 +363,35 @@ func xmlLang(s xml.StartElement, d string) string { return d } +type xmlValue []byte + +func (v *xmlValue) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + // The XML value of a property can be arbitrary, mixed-content XML. + // To make sure that the unmarshalled value contains all required + // namespaces, we encode all the property value XML tokens into a + // buffer. This forces the encoder to redeclare any used namespaces. + var b bytes.Buffer + e := xml.NewEncoder(&b) + for { + t, err := next(d) + if err != nil { + return err + } + if e, ok := t.(xml.EndElement); ok && e.Name == start.Name { + break + } + if err = e.EncodeToken(t); err != nil { + return err + } + } + err := e.Flush() + if err != nil { + return err + } + *v = b.Bytes() + return nil +} + // UnmarshalXML appends the property names and values enclosed within start // to ps. // @@ -355,7 +407,7 @@ func (ps *proppatchProps) UnmarshalXML(d *xml.Decoder, start xml.StartElement) e if err != nil { return err } - switch t.(type) { + switch elem := t.(type) { case xml.EndElement: if len(*ps) == 0 { return fmt.Errorf("%s must not be empty", start.Name.Local) @@ -366,29 +418,10 @@ func (ps *proppatchProps) UnmarshalXML(d *xml.Decoder, start xml.StartElement) e XMLName: t.(xml.StartElement).Name, Lang: xmlLang(t.(xml.StartElement), lang), } - // The XML value of a property can be arbitrary, mixed-content XML. - // To make sure that the unmarshalled value contains all required - // namespaces, we encode all the property value XML tokens into a - // buffer. This forces the encoder to redeclare any used namespaces. - var b bytes.Buffer - e := xml.NewEncoder(&b) - for { - t, err = next(d) - if err != nil { - return err - } - if e, ok := t.(xml.EndElement); ok && e.Name == p.XMLName { - break - } - if err = e.EncodeToken(t); err != nil { - return err - } - } - err = e.Flush() + err = d.DecodeElement(((*xmlValue)(&p.InnerXML)), &elem) if err != nil { return err } - p.InnerXML = b.Bytes() *ps = append(*ps, p) } } diff --git a/third_party/golang.org/x/net/webdav/xml_test.go b/third_party/golang.org/x/net/webdav/xml_test.go index 8ab6ec49f..51fba877e 100644 --- a/third_party/golang.org/x/net/webdav/xml_test.go +++ b/third_party/golang.org/x/net/webdav/xml_test.go @@ -7,10 +7,12 @@ package webdav import ( "bytes" "encoding/xml" + "fmt" "io" "net/http" "net/http/httptest" "reflect" + "sort" "strings" "testing" ) @@ -454,20 +456,20 @@ func TestMultistatusWriter(t *testing.T) { respdesc: "There has been an access violation error.", wantXML: `` + `` + - `` + + `` + ` ` + ` http://example.com/foo` + ` ` + ` ` + - ` Box type A` + - ` J.J. Johnson` + + ` Box type A` + + ` J.J. Johnson` + ` ` + ` HTTP/1.1 200 OK` + ` ` + ` ` + ` ` + - ` ` + - ` ` + + ` ` + + ` ` + ` ` + ` HTTP/1.1 403 Forbidden` + ` The user does not have access to the DingALing property.` + @@ -562,6 +564,7 @@ func TestMultistatusWriter(t *testing.T) { wantCode: http.StatusOK, }} + n := xmlNormalizer{omitWhitespace: true} loop: for _, tc := range testCases { rec := httptest.NewRecorder() @@ -591,70 +594,46 @@ loop: tc.desc, rec.Code, tc.wantCode) continue } - - // normalize returns the normalized XML content of s. In contrast to - // the WebDAV specification, it ignores whitespace within property - // values of mixed XML content. - normalize := func(s string) string { - d := xml.NewDecoder(strings.NewReader(s)) - var b bytes.Buffer - e := xml.NewEncoder(&b) - for { - tok, err := d.Token() - if err != nil { - if err == io.EOF { - break - } - t.Fatalf("%s: Token %v", tc.desc, err) - } - switch val := tok.(type) { - case xml.Comment, xml.Directive, xml.ProcInst: - continue - case xml.CharData: - if len(bytes.TrimSpace(val)) == 0 { - continue - } - } - if err := e.EncodeToken(tok); err != nil { - t.Fatalf("%s: EncodeToken: %v", tc.desc, err) - } - } - if err := e.Flush(); err != nil { - t.Fatalf("%s: Flush: %v", tc.desc, err) - } - return b.String() + gotXML := rec.Body.String() + eq, err := n.equalXML(strings.NewReader(gotXML), strings.NewReader(tc.wantXML)) + if err != nil { + t.Errorf("%s: equalXML: %v", tc.desc, err) + continue } - - gotXML := normalize(rec.Body.String()) - wantXML := normalize(tc.wantXML) - if gotXML != wantXML { - t.Errorf("%s: XML body\ngot %q\nwant %q", tc.desc, gotXML, wantXML) + if !eq { + t.Errorf("%s: XML body\ngot %s\nwant %s", tc.desc, gotXML, tc.wantXML) } } } func TestReadProppatch(t *testing.T) { - // TODO(rost): These "golden XML" tests easily break with changes in the - // xml package. A whitespace-preserving normalizer of XML content is - // required to make these tests more robust. + ppStr := func(pps []Proppatch) string { + var outer []string + for _, pp := range pps { + var inner []string + for _, p := range pp.Props { + inner = append(inner, fmt.Sprintf("{XMLName: %q, Lang: %q, InnerXML: %q}", + p.XMLName, p.Lang, p.InnerXML)) + } + outer = append(outer, fmt.Sprintf("{Remove: %t, Props: [%s]}", + pp.Remove, strings.Join(inner, ", "))) + } + return "[" + strings.Join(outer, ", ") + "]" + } + testCases := []struct { desc string input string wantPP []Proppatch wantStatus int }{{ - desc: "proppatch: section 9.2", + desc: "proppatch: section 9.2 (with simple property value)", input: `` + `` + `` + ` ` + - ` ` + - ` ` + - ` Jim Whitehead` + - ` Roy Fielding` + - ` ` + - ` ` + + ` somevalue` + ` ` + ` ` + ` ` + @@ -664,17 +643,7 @@ func TestReadProppatch(t *testing.T) { Props: []Property{{ xml.Name{Space: "http://ns.example.com/z/", Local: "Authors"}, "", - []byte(`` + - ` ` + - `` + - `Jim Whitehead` + - `` + - ` ` + - `` + - `Roy Fielding` + - `` + - ` `, - ), + []byte(`somevalue`), }}, }, { Remove: true, @@ -684,59 +653,6 @@ func TestReadProppatch(t *testing.T) { nil, }}, }}, - }, { - desc: "proppatch: section 4.3.1 (mixed content)", - input: `` + - `` + - `` + - ` ` + - ` ` + - ` ` + - ` Jane Doe` + - ` ` + - ` mailto:jane.doe@example.com` + - ` http://www.example.com` + - ` ` + - ` Jane has been working way too long on the` + - ` long-awaited revision of ]]>.` + - ` ` + - ` ` + - ` ` + - ` ` + - ``, - wantPP: []Proppatch{{ - Props: []Property{{ - xml.Name{Space: "http://example.com/ns", Local: "author"}, - "en", - []byte(`` + - ` ` + - `Jane Doe` + - ` ` + - `` + - `mailto:jane.doe@example.com` + - `` + - ` ` + - `` + - `http://www.example.com` + - `` + - ` ` + - - `` + - ` ` + - ` Jane has been working way` + - ` too` + - ` long on the` + ` ` + - ` long-awaited revision of <RFC2518>.` + - ` ` + - `` + - ` `, - ), - }}, - }}, }, { desc: "proppatch: lang attribute on prop", input: `` + @@ -806,7 +722,187 @@ func TestReadProppatch(t *testing.T) { continue } if !reflect.DeepEqual(pp, tc.wantPP) || status != tc.wantStatus { - t.Errorf("%s: proppatch\ngot %v\nwant %v", tc.desc, pp, tc.wantPP) + t.Errorf("%s: proppatch\ngot %v\nwant %v", tc.desc, ppStr(pp), ppStr(tc.wantPP)) } } } + +func TestUnmarshalXMLValue(t *testing.T) { + testCases := []struct { + desc string + input string + wantVal string + }{{ + desc: "simple char data", + input: "foo", + wantVal: "foo", + }, { + desc: "empty element", + input: "", + wantVal: "", + }, { + desc: "preserve namespace", + input: ``, + wantVal: ``, + }, { + desc: "preserve root element namespace", + input: ``, + wantVal: ``, + }, { + desc: "preserve whitespace", + input: " \t ", + wantVal: " \t ", + }, { + desc: "preserve mixed content", + input: ` a `, + wantVal: ` a `, + }, { + desc: "section 9.2", + input: `` + + `` + + ` Jim Whitehead` + + ` Roy Fielding` + + ``, + wantVal: `` + + ` Jim Whitehead` + + ` Roy Fielding`, + }, { + desc: "section 4.3.1 (mixed content)", + input: `` + + `` + + ` Jane Doe` + + ` ` + + ` mailto:jane.doe@example.com` + + ` http://www.example.com` + + ` ` + + ` Jane has been working way too long on the` + + ` long-awaited revision of ]]>.` + + ` ` + + ``, + wantVal: `` + + ` Jane Doe` + + ` ` + + ` mailto:jane.doe@example.com` + + ` http://www.example.com` + + ` ` + + ` Jane has been working way too long on the` + + ` long-awaited revision of <RFC2518>.` + + ` `, + }} + + var n xmlNormalizer + for _, tc := range testCases { + d := xml.NewDecoder(strings.NewReader(tc.input)) + var v xmlValue + if err := d.Decode(&v); err != nil { + t.Errorf("%s: got error %v, want nil", tc.desc, err) + continue + } + eq, err := n.equalXML(bytes.NewReader(v), strings.NewReader(tc.wantVal)) + if err != nil { + t.Errorf("%s: equalXML: %v", tc.desc, err) + continue + } + if !eq { + t.Errorf("%s:\ngot %s\nwant %s", tc.desc, string(v), tc.wantVal) + } + } +} + +// xmlNormalizer normalizes XML. +type xmlNormalizer struct { + // omitWhitespace instructs to ignore whitespace between element tags. + omitWhitespace bool + // omitComments instructs to ignore XML comments. + omitComments bool +} + +// normalize writes the normalized XML content of r to w. It applies the +// following rules +// +// * Rename namespace prefixes according to an internal heuristic. +// * Remove unnecessary namespace declarations. +// * Sort attributes in XML start elements in lexical order of their +// fully qualified name. +// * Remove XML directives and processing instructions. +// * Remove CDATA between XML tags that only contains whitespace, if +// instructed to do so. +// * Remove comments, if instructed to do so. +// +func (n *xmlNormalizer) normalize(w io.Writer, r io.Reader) error { + d := xml.NewDecoder(r) + e := xml.NewEncoder(w) + for { + t, err := d.Token() + if err != nil { + if t == nil && err == io.EOF { + break + } + return err + } + switch val := t.(type) { + case xml.Directive, xml.ProcInst: + continue + case xml.Comment: + if n.omitComments { + continue + } + case xml.CharData: + if n.omitWhitespace && len(bytes.TrimSpace(val)) == 0 { + continue + } + case xml.StartElement: + start, _ := xml.CopyToken(val).(xml.StartElement) + attr := start.Attr[:0] + for _, a := range start.Attr { + if a.Name.Space == "xmlns" || a.Name.Local == "xmlns" { + continue + } + attr = append(attr, a) + } + sort.Sort(byName(attr)) + start.Attr = attr + t = start + } + err = e.EncodeToken(t) + if err != nil { + return err + } + } + return e.Flush() +} + +// equalXML tests for equality of the normalized XML contents of a and b. +func (n *xmlNormalizer) equalXML(a, b io.Reader) (bool, error) { + var buf bytes.Buffer + if err := n.normalize(&buf, a); err != nil { + return false, err + } + normA := buf.String() + buf.Reset() + if err := n.normalize(&buf, b); err != nil { + return false, err + } + normB := buf.String() + return normA == normB, nil +} + +type byName []xml.Attr + +func (a byName) Len() int { return len(a) } +func (a byName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byName) Less(i, j int) bool { + if a[i].Name.Space != a[j].Name.Space { + return a[i].Name.Space < a[j].Name.Space + } + return a[i].Name.Local < a[j].Name.Local +}