Browse Source

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 <adg@golang.org>
Nigel Tao 9 years ago
parent
commit
e45385e9b2
6 changed files with 83 additions and 36 deletions
  1. 1 2
      webdav/file.go
  2. 1 2
      webdav/file_test.go
  3. 1 2
      webdav/prop.go
  4. 1 2
      webdav/prop_test.go
  5. 60 10
      webdav/xml.go
  6. 19 18
      webdav/xml_test.go

+ 1 - 2
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

+ 1 - 2
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) {

+ 1 - 2
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.

+ 1 - 2
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) {

+ 60 - 10
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)

+ 19 - 18
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) {
 			"</A:propfind>",
 		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) {
 			"</A:propfind>",
 		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) {
 			"</A:propfind>",
 		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) {
 			"</A:propfind>",
 		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(`` +
 						`<BoxType xmlns="http://ns.example.com/boxschema/">` +
 						`Box type A` +
 						`</BoxType>`),
 				}, {
-					XMLName: ixml.Name{Space: "http://ns.example.com/boxschema/", Local: "author"},
+					XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "author"},
 					InnerXML: []byte(`` +
 						`<Name xmlns="http://ns.example.com/boxschema/">` +
 						`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) {
 			`</D:propertyupdate>`,
 		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) {
 			`</D:propertyupdate>`,
 		wantPP: []Proppatch{{
 			Props: []Property{{
-				ixml.Name{Space: "http://example.com/ns", Local: "foo"},
+				xml.Name{Space: "http://example.com/ns", Local: "foo"},
 				"en",
 				nil,
 			}},