Skip to content

Commit 6dc0abc

Browse files
rstonigeltao
rsto
authored and
nigeltao
committed
webdav: fix XML golden tests for encoding/xml xmlns change.
The change to the standard encoding/xml library was https://go-review.googlesource.com/#/c/2660/ which landed on 2015-02-14. An additional change was https://go-review.googlesource.com/#/c/5910/ which landed on 2015-03-03. Fixes #9978. Change-Id: I4f798f153e4e13b13eadc10e44b21a4b118251d3 Reviewed-on: https://go-review.googlesource.com/5714 Reviewed-by: Nigel Tao <[email protected]>
1 parent 3d87fd6 commit 6dc0abc

File tree

2 files changed

+154
-95
lines changed

2 files changed

+154
-95
lines changed

‎webdav/xml.go

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,7 @@ type xmlError struct {
212212

213213
// http://www.webdav.org/specs/rfc4918.html#ELEMENT_propstat
214214
type propstat struct {
215-
// Prop requires DAV: to be the default namespace in the enclosing
216-
// XML. This is due to the standard encoding/xml package currently
217-
// not honoring namespace declarations inside a xmltag with a
218-
// parent element for anonymous slice elements.
219-
// Use of multistatusWriter takes care of this.
220-
Prop []Property `xml:"prop>_ignored_"`
215+
Prop []Property `xml:"DAV: prop>_ignored_"`
221216
Status string `xml:"DAV: status"`
222217
Error *xmlError `xml:"DAV: error"`
223218
ResponseDescription string `xml:"DAV: responsedescription,omitempty"`
@@ -271,12 +266,24 @@ func (w *multistatusWriter) write(r *response) error {
271266
if w.enc == nil {
272267
w.w.Header().Add("Content-Type", "text/xml; charset=utf-8")
273268
w.w.WriteHeader(StatusMulti)
274-
_, err := fmt.Fprintf(w.w, `<?xml version="1.0" encoding="UTF-8"?>`+
275-
`<D:multistatus xmlns:D="DAV:">`)
269+
_, err := fmt.Fprintf(w.w, `<?xml version="1.0" encoding="UTF-8"?>`)
276270
if err != nil {
277271
return err
278272
}
279273
w.enc = xml.NewEncoder(w.w)
274+
err = w.enc.EncodeToken(xml.StartElement{
275+
Name: xml.Name{
276+
Space: "DAV:",
277+
Local: "multistatus",
278+
},
279+
Attr: []xml.Attr{{
280+
Name: xml.Name{Local: "xmlns"},
281+
Value: "DAV:",
282+
}},
283+
})
284+
if err != nil {
285+
return err
286+
}
280287
}
281288
return w.enc.Encode(r)
282289
}
@@ -289,14 +296,23 @@ func (w *multistatusWriter) close() error {
289296
if w.enc == nil {
290297
return nil
291298
}
299+
var end []xml.Token
292300
if w.responseDescription != "" {
293-
_, err := fmt.Fprintf(w.w,
294-
"<D:responsedescription>%s</D:responsedescription>",
295-
w.responseDescription)
301+
name := xml.Name{Space: "DAV:", Local: "responsedescription"}
302+
end = append(end,
303+
xml.StartElement{Name: name},
304+
xml.CharData(w.responseDescription),
305+
xml.EndElement{Name: name},
306+
)
307+
}
308+
end = append(end, xml.EndElement{
309+
Name: xml.Name{Space: "DAV:", Local: "multistatus"},
310+
})
311+
for _, t := range end {
312+
err := w.enc.EncodeToken(t)
296313
if err != nil {
297314
return err
298315
}
299316
}
300-
_, err := fmt.Fprintf(w.w, "</D:multistatus>")
301-
return err
317+
return w.enc.Flush()
302318
}

‎webdav/xml_test.go

Lines changed: 125 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
package webdav
66

77
import (
8+
"bytes"
89
"encoding/xml"
10+
"io"
911
"net/http"
1012
"net/http/httptest"
1113
"reflect"
@@ -345,13 +347,6 @@ func TestReadPropfind(t *testing.T) {
345347
func TestMultistatusWriter(t *testing.T) {
346348
///The "section x.y.z" test cases come from section x.y.z of the spec at
347349
// http://www.webdav.org/specs/rfc4918.html
348-
//
349-
// BUG:The following tests compare the actual and expected XML verbatim.
350-
// Minor tweaks in the marshalling output of either standard encoding/xml
351-
// or this package might break them. A more resilient approach could be
352-
// to normalize both actual and expected XML content before comparison.
353-
// This also would enhance readibility of the expected XML payload in the
354-
// wantXML field.
355350
testCases := []struct {
356351
desc string
357352
responses []response
@@ -365,38 +360,43 @@ func TestMultistatusWriter(t *testing.T) {
365360
Href: []string{"http://example.com/foo"},
366361
Propstat: []propstat{{
367362
Prop: []Property{{
368-
XMLName: xml.Name{Space: "http://ns.example.com/", Local: "Authors"},
363+
XMLName: xml.Name{
364+
Space: "http://ns.example.com/",
365+
Local: "Authors",
366+
},
369367
}},
370368
Status: "HTTP/1.1 424 Failed Dependency",
371369
}, {
372370
Prop: []Property{{
373-
XMLName: xml.Name{Space: "http://ns.example.com/", Local: "Copyright-Owner"},
371+
XMLName: xml.Name{
372+
Space: "http://ns.example.com/",
373+
Local: "Copyright-Owner",
374+
},
374375
}},
375376
Status: "HTTP/1.1 409 Conflict",
376377
}},
377-
ResponseDescription: " Copyright Owner cannot be deleted or altered.",
378+
ResponseDescription: "Copyright Owner cannot be deleted or altered.",
378379
}},
379-
wantXML: `<?xml version="1.0" encoding="UTF-8"?>` +
380-
`<D:multistatus xmlns:D="DAV:">` +
381-
`<response xmlns="DAV:">` +
382-
`<href xmlns="DAV:">http://example.com/foo</href>` +
383-
`<propstat xmlns="DAV:">` +
384-
`<prop>` +
385-
`<Authors xmlns="http://ns.example.com/"></Authors>` +
386-
`</prop>` +
387-
`<status xmlns="DAV:">HTTP/1.1 424 Failed Dependency</status>` +
388-
`</propstat>` +
389-
`<propstat xmlns="DAV:">` +
390-
`<prop>` +
391-
`<Copyright-Owner xmlns="http://ns.example.com/"></Copyright-Owner>` +
392-
`</prop>` +
393-
`<status xmlns="DAV:">HTTP/1.1 409 Conflict</status>` +
394-
`</propstat>` +
395-
`<responsedescription xmlns="DAV:">` +
396-
` Copyright Owner cannot be deleted or altered.` +
397-
`</responsedescription>` +
380+
wantXML: `` +
381+
`<?xml version="1.0" encoding="UTF-8"?>` +
382+
`<multistatus xmlns="DAV:">` +
383+
` <response>` +
384+
` <href>http://example.com/foo</href>` +
385+
` <propstat>` +
386+
` <prop>` +
387+
` <Authors xmlns="http://ns.example.com/"></Authors>` +
388+
` </prop>` +
389+
` <status>HTTP/1.1 424 Failed Dependency</status>` +
390+
` </propstat>` +
391+
` <propstat xmlns="DAV:">` +
392+
` <prop>` +
393+
` <Copyright-Owner xmlns="http://ns.example.com/"></Copyright-Owner>` +
394+
` </prop>` +
395+
` <status>HTTP/1.1 409 Conflict</status>` +
396+
` </propstat>` +
397+
` <responsedescription>Copyright Owner cannot be deleted or altered.</responsedescription>` +
398398
`</response>` +
399-
`</D:multistatus>`,
399+
`</multistatus>`,
400400
wantCode: StatusMulti,
401401
}, {
402402
desc: "section 9.6.2 (lock-token-submitted)",
@@ -407,14 +407,15 @@ func TestMultistatusWriter(t *testing.T) {
407407
InnerXML: []byte(`<lock-token-submitted xmlns="DAV:"/>`),
408408
},
409409
}},
410-
wantXML: `<?xml version="1.0" encoding="UTF-8"?>` +
411-
`<D:multistatus xmlns:D="DAV:">` +
412-
`<response xmlns="DAV:">` +
413-
`<href xmlns="DAV:">http://example.com/foo</href>` +
414-
`<status xmlns="DAV:">HTTP/1.1 423 Locked</status>` +
415-
`<error xmlns="DAV:"><lock-token-submitted xmlns="DAV:"/></error>` +
416-
`</response>` +
417-
`</D:multistatus>`,
410+
wantXML: `` +
411+
`<?xml version="1.0" encoding="UTF-8"?>` +
412+
`<multistatus xmlns="DAV:">` +
413+
` <response>` +
414+
` <href>http://example.com/foo</href>` +
415+
` <status>HTTP/1.1 423 Locked</status>` +
416+
` <error><lock-token-submitted xmlns="DAV:"/></error>` +
417+
` </response>` +
418+
`</multistatus>`,
418419
wantCode: StatusMulti,
419420
}, {
420421
desc: "section 9.1.3",
@@ -442,42 +443,33 @@ func TestMultistatusWriter(t *testing.T) {
442443
XMLName: xml.Name{Space: "http://ns.example.com/boxschema/", Local: "Random"},
443444
}},
444445
Status: "HTTP/1.1 403 Forbidden",
445-
ResponseDescription: " The user does not have access to the DingALing property.",
446+
ResponseDescription: "The user does not have access to the DingALing property.",
446447
}},
447448
}},
448-
respdesc: " There has been an access violation error.",
449-
wantXML: `<?xml version="1.0" encoding="UTF-8"?>` +
450-
`<D:multistatus xmlns:D="DAV:">` +
451-
`<response xmlns="DAV:">` +
452-
`<href xmlns="DAV:">http://example.com/foo</href>` +
453-
`<propstat xmlns="DAV:">` +
454-
`<prop>` +
455-
`<bigbox xmlns="http://ns.example.com/boxschema/">` +
456-
`<BoxType xmlns="http://ns.example.com/boxschema/">Box type A</BoxType>` +
457-
`</bigbox>` +
458-
`<author xmlns="http://ns.example.com/boxschema/">` +
459-
`<Name xmlns="http://ns.example.com/boxschema/">J.J. Johnson</Name>` +
460-
`</author>` +
461-
`</prop>` +
462-
`<status xmlns="DAV:">HTTP/1.1 200 OK</status>` +
463-
`</propstat>` +
464-
`<propstat xmlns="DAV:">` +
465-
`<prop>` +
466-
`<DingALing xmlns="http://ns.example.com/boxschema/">` +
467-
`</DingALing>` +
468-
`<Random xmlns="http://ns.example.com/boxschema/">` +
469-
`</Random>` +
470-
`</prop>` +
471-
`<status xmlns="DAV:">HTTP/1.1 403 Forbidden</status>` +
472-
`<responsedescription xmlns="DAV:">` +
473-
` The user does not have access to the DingALing property.` +
474-
`</responsedescription>` +
475-
`</propstat>` +
476-
`</response>` +
477-
`<D:responsedescription>` +
478-
` There has been an access violation error.` +
479-
`</D:responsedescription>` +
480-
`</D:multistatus>`,
449+
respdesc: "There has been an access violation error.",
450+
wantXML: `` +
451+
`<?xml version="1.0" encoding="UTF-8"?>` +
452+
`<multistatus xmlns="DAV:">` +
453+
` <response>` +
454+
` <href>http://example.com/foo</href>` +
455+
` <propstat>` +
456+
` <prop>` +
457+
` <bigbox xmlns="http://ns.example.com/boxschema/"><BoxType xmlns="http://ns.example.com/boxschema/">Box type A</BoxType></bigbox>` +
458+
` <author xmlns="http://ns.example.com/boxschema/"><Name xmlns="http://ns.example.com/boxschema/">J.J. Johnson</Name></author>` +
459+
` </prop>` +
460+
` <status>HTTP/1.1 200 OK</status>` +
461+
` </propstat>` +
462+
` <propstat>` +
463+
` <prop>` +
464+
` <DingALing xmlns="http://ns.example.com/boxschema/"></DingALing>` +
465+
` <Random xmlns="http://ns.example.com/boxschema/"></Random>` +
466+
` </prop>` +
467+
` <status>HTTP/1.1 403 Forbidden</status>` +
468+
` <responsedescription>The user does not have access to the DingALing property.</responsedescription>` +
469+
` </propstat>` +
470+
` </response>` +
471+
` <responsedescription>There has been an access violation error.</responsedescription>` +
472+
`</multistatus>`,
481473
wantCode: StatusMulti,
482474
}, {
483475
desc: "bad: no response written",
@@ -493,7 +485,10 @@ func TestMultistatusWriter(t *testing.T) {
493485
responses: []response{{
494486
Propstat: []propstat{{
495487
Prop: []Property{{
496-
XMLName: xml.Name{Space: "http://example.com/", Local: "foo"},
488+
XMLName: xml.Name{
489+
Space: "http://example.com/",
490+
Local: "foo",
491+
},
497492
}},
498493
Status: "HTTP/1.1 200 OK",
499494
}},
@@ -523,7 +518,10 @@ func TestMultistatusWriter(t *testing.T) {
523518
Href: []string{"http://example.com/foo"},
524519
Propstat: []propstat{{
525520
Prop: []Property{{
526-
XMLName: xml.Name{Space: "http://example.com/", Local: "foo"},
521+
XMLName: xml.Name{
522+
Space: "http://example.com/",
523+
Local: "foo",
524+
},
527525
}},
528526
Status: "HTTP/1.1 200 OK",
529527
}},
@@ -535,10 +533,16 @@ func TestMultistatusWriter(t *testing.T) {
535533
}, {
536534
desc: "bad: multiple hrefs and propstat",
537535
responses: []response{{
538-
Href: []string{"http://example.com/foo", "http://example.com/bar"},
536+
Href: []string{
537+
"http://example.com/foo",
538+
"http://example.com/bar",
539+
},
539540
Propstat: []propstat{{
540541
Prop: []Property{{
541-
XMLName: xml.Name{Space: "http://example.com/", Local: "foo"},
542+
XMLName: xml.Name{
543+
Space: "http://example.com/",
544+
Local: "foo",
545+
},
542546
}},
543547
Status: "HTTP/1.1 200 OK",
544548
}},
@@ -547,29 +551,68 @@ func TestMultistatusWriter(t *testing.T) {
547551
// default of http.responseWriter
548552
wantCode: http.StatusOK,
549553
}}
554+
550555
loop:
551556
for _, tc := range testCases {
552557
rec := httptest.NewRecorder()
553558
w := multistatusWriter{w: rec, responseDescription: tc.respdesc}
554559
for _, r := range tc.responses {
555560
if err := w.write(&r); err != nil {
556561
if err != tc.wantErr {
557-
t.Errorf("%s: got write error %v, want %v", tc.desc, err, tc.wantErr)
562+
t.Errorf("%s: got write error %v, want %v",
563+
tc.desc, err, tc.wantErr)
558564
}
559565
continue loop
560566
}
561567
}
562568
if err := w.close(); err != tc.wantErr {
563-
t.Errorf("%s: got close error %v, want %v", tc.desc, err, tc.wantErr)
569+
t.Errorf("%s: got close error %v, want %v",
570+
tc.desc, err, tc.wantErr)
564571
continue
565572
}
566573
if rec.Code != tc.wantCode {
567-
t.Errorf("%s: got HTTP status code %d, want %d\n", tc.desc, rec.Code, tc.wantCode)
574+
t.Errorf("%s: got HTTP status code %d, want %d\n",
575+
tc.desc, rec.Code, tc.wantCode)
568576
continue
569577
}
570-
if gotXML := rec.Body.String(); gotXML != tc.wantXML {
571-
t.Errorf("%s: XML body\ngot %q\nwant %q", tc.desc, gotXML, tc.wantXML)
572-
continue
578+
579+
// normalize returns the normalized XML content of s. In contrast to
580+
// the WebDAV specification, it ignores whitespace within property
581+
// values of mixed XML content.
582+
normalize := func(s string) string {
583+
d := xml.NewDecoder(strings.NewReader(s))
584+
var b bytes.Buffer
585+
e := xml.NewEncoder(&b)
586+
for {
587+
tok, err := d.Token()
588+
if err != nil {
589+
if err == io.EOF {
590+
break
591+
}
592+
t.Fatalf("%s: Token %v", tc.desc, err)
593+
}
594+
switch val := tok.(type) {
595+
case xml.Comment, xml.Directive, xml.ProcInst:
596+
continue
597+
case xml.CharData:
598+
if len(bytes.TrimSpace(val)) == 0 {
599+
continue
600+
}
601+
}
602+
if err := e.EncodeToken(tok); err != nil {
603+
t.Fatalf("%s: EncodeToken: %v", tc.desc, err)
604+
}
605+
}
606+
if err := e.Flush(); err != nil {
607+
t.Fatalf("%s: Flush: %v", tc.desc, err)
608+
}
609+
return b.String()
610+
}
611+
612+
gotXML := normalize(rec.Body.String())
613+
wantXML := normalize(tc.wantXML)
614+
if gotXML != wantXML {
615+
t.Errorf("%s: XML body\ngot %q\nwant %q", tc.desc, gotXML, wantXML)
573616
}
574617
}
575618
}

0 commit comments

Comments
 (0)