Skip to content

Commit

Permalink
webdav: have the exported API use the standard library's xml.Name type.
Browse files Browse the repository at this point in the history 
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
golang/go#13400 is resolved.

Fixes golang/go#15128.

Change-Id: Ie7e7927d8b30d97d10b1a4a654d774fdf3e7a1e3
Reviewed-on: https://go-review.googlesource.com/21635
Reviewed-by: Andrew Gerrand <[email protected]>
  • Loading branch information
@nigeltao
nigeltao committed Apr 7, 2016
1 parent 8f3641d commit e45385e
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 36 deletions.
3 changes: 1 addition & 2 deletions webdav/file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package webdav

import (
"encoding/xml"
"io"
"net/http"
"os"
Expand All @@ -13,8 +14,6 @@ import (
"strings"
"sync"
"time"

"golang.org/x/net/webdav/internal/xml"
)

// slashClean is equivalent to but slightly more efficient than
Expand Down
3 changes: 1 addition & 2 deletions webdav/file_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package webdav

import (
"encoding/xml"
"fmt"
"io"
"io/ioutil"
Expand All @@ -17,8 +18,6 @@ import (
"strconv"
"strings"
"testing"

"golang.org/x/net/webdav/internal/xml"
)

func TestSlashClean(t *testing.T) {
Expand Down
3 changes: 1 addition & 2 deletions webdav/prop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@
package webdav

import (
"encoding/xml"
"fmt"
"io"
"mime"
"net/http"
"os"
"path/filepath"
"strconv"

"golang.org/x/net/webdav/internal/xml"
)

// Proppatch describes a property update instruction as defined in RFC 4918.
Expand Down
3 changes: 1 addition & 2 deletions webdav/prop_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
70 changes: 60 additions & 10 deletions webdav/xml.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
//
Expand All @@ -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))
}
}
}
Expand Down Expand Up @@ -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"`
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
//
Expand All @@ -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)
Expand Down
37 changes: 19 additions & 18 deletions webdav/xml_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package webdav

import (
"bytes"
"encoding/xml"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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)",
Expand Down Expand Up @@ -364,15 +365,15 @@ 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",
},
}},
Status: "HTTP/1.1 424 Failed Dependency",
}, {
Prop: []Property{{
XMLName: ixml.Name{
XMLName: xml.Name{
Space: "http://ns.example.com/",
Local: "Copyright-Owner",
},
Expand Down Expand Up @@ -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` +
Expand All @@ -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.",
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand Down Expand Up @@ -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,
}},
Expand All @@ -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,
}},
Expand Down

0 comments on commit e45385e

Please sign in to comment.