From e45385e9b226f570b1f086bf287b25d3d4117776 Mon Sep 17 00:00:00 2001 From: Nigel Tao Date: Thu, 7 Apr 2016 12:17:48 +1000 Subject: [PATCH] webdav: have the exported API use the standard library's xml.Name type. In particular, the Property and DeadPropsHolder types need to refer to the standard xml package, not the internal fork, to be usable by other packages. Inside the package, the XML marshaling and unmarshaling is still done by the etc/internal/xml package, and will remain that way until https://github.com/golang/go/issues/13400 is resolved. Fixes golang/go#15128. Change-Id: Ie7e7927d8b30d97d10b1a4a654d774fdf3e7a1e3 Reviewed-on: https://go-review.googlesource.com/21635 Reviewed-by: Andrew Gerrand --- webdav/file.go | 3 +- webdav/file_test.go | 3 +- webdav/prop.go | 3 +- webdav/prop_test.go | 3 +- webdav/xml.go | 70 ++++++++++++++++++++++++++++++++++++++------- webdav/xml_test.go | 37 ++++++++++++------------ 6 files changed, 83 insertions(+), 36 deletions(-) diff --git a/webdav/file.go b/webdav/file.go index 9ba1ca16e..3d95c6cba 100644 --- a/webdav/file.go +++ b/webdav/file.go @@ -5,6 +5,7 @@ package webdav import ( + "encoding/xml" "io" "net/http" "os" @@ -13,8 +14,6 @@ import ( "strings" "sync" "time" - - "golang.org/x/net/webdav/internal/xml" ) // slashClean is equivalent to but slightly more efficient than diff --git a/webdav/file_test.go b/webdav/file_test.go index 99547e16b..7ce6c1274 100644 --- a/webdav/file_test.go +++ b/webdav/file_test.go @@ -5,6 +5,7 @@ package webdav import ( + "encoding/xml" "fmt" "io" "io/ioutil" @@ -17,8 +18,6 @@ import ( "strconv" "strings" "testing" - - "golang.org/x/net/webdav/internal/xml" ) func TestSlashClean(t *testing.T) { diff --git a/webdav/prop.go b/webdav/prop.go index 88b9a3a35..ac6c65a18 100644 --- a/webdav/prop.go +++ b/webdav/prop.go @@ -5,6 +5,7 @@ package webdav import ( + "encoding/xml" "fmt" "io" "mime" @@ -12,8 +13,6 @@ import ( "os" "path/filepath" "strconv" - - "golang.org/x/net/webdav/internal/xml" ) // Proppatch describes a property update instruction as defined in RFC 4918. diff --git a/webdav/prop_test.go b/webdav/prop_test.go index c55a7ba7a..ee188acdf 100644 --- a/webdav/prop_test.go +++ b/webdav/prop_test.go @@ -5,14 +5,13 @@ package webdav import ( + "encoding/xml" "fmt" "net/http" "os" "reflect" "sort" "testing" - - "golang.org/x/net/webdav/internal/xml" ) func TestMemPS(t *testing.T) { diff --git a/webdav/xml.go b/webdav/xml.go index 7fb114758..790dc8169 100644 --- a/webdav/xml.go +++ b/webdav/xml.go @@ -9,11 +9,29 @@ package webdav import ( "bytes" + "encoding/xml" "fmt" "io" "net/http" "time" + // As of https://go-review.googlesource.com/#/c/12772/ which was submitted + // in July 2015, this package uses an internal fork of the standard + // library's encoding/xml package, due to changes in the way namespaces + // were encoded. Such changes were introduced in the Go 1.5 cycle, but were + // rolled back in response to https://github.com/golang/go/issues/11841 + // + // However, this package's exported API, specifically the Property and + // DeadPropsHolder types, need to refer to the standard library's version + // of the xml.Name type, as code that imports this package cannot refer to + // the internal version. + // + // This file therefore imports both the internal and external versions, as + // ixml and xml, and converts between them. + // + // In the long term, this package should use the standard library's version + // only, and the internal fork deleted, once + // https://github.com/golang/go/issues/13400 is resolved. ixml "golang.org/x/net/webdav/internal/xml" ) @@ -116,7 +134,7 @@ func next(d *ixml.Decoder) (ixml.Token, error) { } // http://www.webdav.org/specs/rfc4918.html#ELEMENT_prop (for propfind) -type propfindProps []ixml.Name +type propfindProps []xml.Name // UnmarshalXML appends the property names enclosed within start to pn. // @@ -143,7 +161,7 @@ func (pn *propfindProps) UnmarshalXML(d *ixml.Decoder, start ixml.StartElement) if _, ok := t.(ixml.EndElement); !ok { return fmt.Errorf("unexpected token %T", t) } - *pn = append(*pn, name) + *pn = append(*pn, xml.Name(name)) } } } @@ -190,7 +208,7 @@ func readPropfind(r io.Reader) (pf propfind, status int, err error) { // See http://www.webdav.org/specs/rfc4918.html#data.model.for.resource.properties type Property struct { // XMLName is the fully qualified name that identifies this property. - XMLName ixml.Name + XMLName xml.Name // Lang is an optional xml:lang attribute. Lang string `xml:"xml:lang,attr,omitempty"` @@ -206,6 +224,14 @@ type Property struct { InnerXML []byte `xml:",innerxml"` } +// ixmlProperty is the same as the Property type except it holds an ixml.Name +// instead of an xml.Name. +type ixmlProperty struct { + XMLName ixml.Name + Lang string `xml:"xml:lang,attr,omitempty"` + InnerXML []byte `xml:",innerxml"` +} + // http://www.webdav.org/specs/rfc4918.html#ELEMENT_error // See multistatusWriter for the "D:" namespace prefix. type xmlError struct { @@ -222,18 +248,42 @@ type propstat struct { ResponseDescription string `xml:"D:responsedescription,omitempty"` } +// ixmlPropstat is the same as the propstat type except it holds an ixml.Name +// instead of an xml.Name. +type ixmlPropstat struct { + Prop []ixmlProperty `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 *ixml.Encoder, start ixml.StartElement) error { + // Convert from a propstat to an ixmlPropstat. + ixmlPs := ixmlPropstat{ + Prop: make([]ixmlProperty, len(ps.Prop)), + Status: ps.Status, + Error: ps.Error, + ResponseDescription: ps.ResponseDescription, + } for k, prop := range ps.Prop { + ixmlPs.Prop[k] = ixmlProperty{ + XMLName: ixml.Name(prop.XMLName), + Lang: prop.Lang, + InnerXML: prop.InnerXML, + } + } + + for k, prop := range ixmlPs.Prop { if prop.XMLName.Space == "DAV:" { prop.XMLName = ixml.Name{Space: "", Local: "D:" + prop.XMLName.Local} - ps.Prop[k] = prop + ixmlPs.Prop[k] = prop } } // Distinct type to avoid infinite recursion of MarshalXML. - type newpropstat propstat - return e.EncodeElement(newpropstat(ps), start) + type newpropstat ixmlPropstat + return e.EncodeElement(newpropstat(ixmlPs), start) } // http://www.webdav.org/specs/rfc4918.html#ELEMENT_response @@ -350,9 +400,6 @@ func (w *multistatusWriter) close() error { return w.enc.Flush() } -// http://www.webdav.org/specs/rfc4918.html#ELEMENT_prop (for proppatch) -type proppatchProps []Property - var xmlLangName = ixml.Name{Space: "http://www.w3.org/XML/1998/namespace", Local: "lang"} func xmlLang(s ixml.StartElement, d string) string { @@ -393,6 +440,9 @@ func (v *xmlValue) UnmarshalXML(d *ixml.Decoder, start ixml.StartElement) error return nil } +// http://www.webdav.org/specs/rfc4918.html#ELEMENT_prop (for proppatch) +type proppatchProps []Property + // UnmarshalXML appends the property names and values enclosed within start // to ps. // @@ -416,7 +466,7 @@ func (ps *proppatchProps) UnmarshalXML(d *ixml.Decoder, start ixml.StartElement) return nil case ixml.StartElement: p := Property{ - XMLName: t.(ixml.StartElement).Name, + XMLName: xml.Name(t.(ixml.StartElement).Name), Lang: xmlLang(t.(ixml.StartElement), lang), } err = d.DecodeElement(((*xmlValue)(&p.InnerXML)), &elem) diff --git a/webdav/xml_test.go b/webdav/xml_test.go index 7cd14b6c6..a3d9e1ed8 100644 --- a/webdav/xml_test.go +++ b/webdav/xml_test.go @@ -6,6 +6,7 @@ package webdav import ( "bytes" + "encoding/xml" "fmt" "io" "net/http" @@ -176,7 +177,7 @@ func TestReadPropfind(t *testing.T) { wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, Allprop: new(struct{}), - Include: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Include: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: include followed by allprop", @@ -188,7 +189,7 @@ func TestReadPropfind(t *testing.T) { wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, Allprop: new(struct{}), - Include: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Include: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: propfind", @@ -198,7 +199,7 @@ func TestReadPropfind(t *testing.T) { "", wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, - Prop: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Prop: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: prop with ignored comments", @@ -211,7 +212,7 @@ func TestReadPropfind(t *testing.T) { "", wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, - Prop: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Prop: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: propfind with ignored whitespace", @@ -221,7 +222,7 @@ func TestReadPropfind(t *testing.T) { "", wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, - Prop: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Prop: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: propfind with ignored mixed-content", @@ -231,7 +232,7 @@ func TestReadPropfind(t *testing.T) { "", wantPF: propfind{ XMLName: ixml.Name{Space: "DAV:", Local: "propfind"}, - Prop: propfindProps{ixml.Name{Space: "DAV:", Local: "displayname"}}, + Prop: propfindProps{xml.Name{Space: "DAV:", Local: "displayname"}}, }, }, { desc: "propfind: propname with ignored element (section A.4)", @@ -364,7 +365,7 @@ func TestMultistatusWriter(t *testing.T) { Href: []string{"http://example.com/foo"}, Propstat: []propstat{{ Prop: []Property{{ - XMLName: ixml.Name{ + XMLName: xml.Name{ Space: "http://ns.example.com/", Local: "Authors", }, @@ -372,7 +373,7 @@ func TestMultistatusWriter(t *testing.T) { Status: "HTTP/1.1 424 Failed Dependency", }, { Prop: []Property{{ - XMLName: ixml.Name{ + XMLName: xml.Name{ Space: "http://ns.example.com/", Local: "Copyright-Owner", }, @@ -427,13 +428,13 @@ func TestMultistatusWriter(t *testing.T) { Href: []string{"http://example.com/foo"}, Propstat: []propstat{{ Prop: []Property{{ - XMLName: ixml.Name{Space: "http://ns.example.com/boxschema/", Local: "bigbox"}, + XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "bigbox"}, InnerXML: []byte(`` + `` + `Box type A` + ``), }, { - XMLName: ixml.Name{Space: "http://ns.example.com/boxschema/", Local: "author"}, + XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "author"}, InnerXML: []byte(`` + `` + `J.J. Johnson` + @@ -442,9 +443,9 @@ func TestMultistatusWriter(t *testing.T) { Status: "HTTP/1.1 200 OK", }, { Prop: []Property{{ - XMLName: ixml.Name{Space: "http://ns.example.com/boxschema/", Local: "DingALing"}, + XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "DingALing"}, }, { - XMLName: ixml.Name{Space: "http://ns.example.com/boxschema/", Local: "Random"}, + XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "Random"}, }}, Status: "HTTP/1.1 403 Forbidden", ResponseDescription: "The user does not have access to the DingALing property.", @@ -494,7 +495,7 @@ func TestMultistatusWriter(t *testing.T) { responses: []response{{ Propstat: []propstat{{ Prop: []Property{{ - XMLName: ixml.Name{ + XMLName: xml.Name{ Space: "http://example.com/", Local: "foo", }, @@ -527,7 +528,7 @@ func TestMultistatusWriter(t *testing.T) { Href: []string{"http://example.com/foo"}, Propstat: []propstat{{ Prop: []Property{{ - XMLName: ixml.Name{ + XMLName: xml.Name{ Space: "http://example.com/", Local: "foo", }, @@ -548,7 +549,7 @@ func TestMultistatusWriter(t *testing.T) { }, Propstat: []propstat{{ Prop: []Property{{ - XMLName: ixml.Name{ + XMLName: xml.Name{ Space: "http://example.com/", Local: "foo", }, @@ -638,14 +639,14 @@ func TestReadProppatch(t *testing.T) { ``, wantPP: []Proppatch{{ Props: []Property{{ - ixml.Name{Space: "http://ns.example.com/z/", Local: "Authors"}, + xml.Name{Space: "http://ns.example.com/z/", Local: "Authors"}, "", []byte(`somevalue`), }}, }, { Remove: true, Props: []Property{{ - ixml.Name{Space: "http://ns.example.com/z/", Local: "Copyright-Owner"}, + xml.Name{Space: "http://ns.example.com/z/", Local: "Copyright-Owner"}, "", nil, }}, @@ -663,7 +664,7 @@ func TestReadProppatch(t *testing.T) { ``, wantPP: []Proppatch{{ Props: []Property{{ - ixml.Name{Space: "http://example.com/ns", Local: "foo"}, + xml.Name{Space: "http://example.com/ns", Local: "foo"}, "en", nil, }},