From edd54ccda7eea3e243814f38fcda1d3fa3dc80f3 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Fri, 27 Apr 2018 08:49:27 +0200 Subject: [PATCH] encoding/xml: fixes to enforce XML namespace standard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All issues of #13400 which are not new functionalities have fixes. There are minor incompatibilities between them due to the handling of prefixes. Duplicating a prefix or an namespace is invalid XML. This is now avoided. XML produced is always valid. Tests have been added for each fix and example and previous tests fixed as output is already more compact is some cases. encoding/xml: fix duplication of namespace tags by encoder (#7535) A tag prefix identifies the name space of the tag and not the default name space like xmlns="...". Writing the prefix is incorrect when it is bound to a name space using the standard xmlns:prefix="..." attribute. This fix skips this print. Consequences are - duplication is avoided in line with name space standard in reference. - the _xmlns declaration does not appear anymore. To keep the previous behaviour, the prefix is printed in all other cases. Token now always produces well-formed XML except when the explicit name space collides with the prefix. Made prefix handling "xmlns="space" xmlns:_xmlns="xmlns" _..." has be removed in all wants of tests. In some cases, useless declarations like xmlns:x="x" are still added in line with previous behavior. encoding/xml: fix unexpected behavior of encoder.Indent("", "") (#13185, #11431) MarshalIndent and Marshal share code. When prefix and indent are empty, the behavior is like Marshal when it should have a minimal indent, i.e. new line as in documentation. A boolean is added to the local printer struct which defaults to false. It is set to true only when MarshalIndent is used and prefix and indent are empty. encoding/xml: fix overriding by empty namespace (#7113) The namespace defined by xmlns="value" can be overridden in every included tag including by the empty namespace xmlns="". Empty namespace is not authorized with a prefix (xmlns:ns=""). Method to calculate indent of XML handles depth of tag and its associated namespace. The fix leaves the method active even when no indent is required. An XMLName field in a struct means that namespace must be enforced even if empty. This occurs only on an inner tag as an overwrite of any non-empty namespace of its outer tag. To obtain the xmlns="" required, an attribute is added. encoding/xml: fix panic on embedded unexported XMLName (#10538) By laws of reflection, unexported fields are unreachable by .Value. XMLName are allowed at any level of an inner struct but the struct may not be reachable. If XMLName field is found without name, it cannot be exported and the value is discarded like other fields. Some comments have been to underline where the issue arises. Another XMLName test was incorrectly set up and is fixed. Various cases added in a specific test. encoding/xml: fix panic of unmarshaling of anonymous structs (#16497) Encoding/xml is using type "typinfo" which provides its own "value" method using the reflection package. It fails to check for the documented possible panics of the reflection methods. It is impossible to fix the method as the parameter is also using reflection. The fix is to discard anonymous structs which have no value anyway. Encoder/xml documentation already mentions that fields are accessed using the reflection package. A relevant test is added. encoding/xml: fix closing tag failure (#20685) Push/pop of elements name must be done using the eventual prefix together with the tag name. The operation is moved before the translation of prefix into its URI. One end element of a test is fixed as expecting the last used namespace is incorrect. After closing a tag using a namespace, the valid namespace must be taken from the opening tag. encoding/xml: add check of namespaces to detect field names conflicts (#8535, #11724) Comparing namespaces of fields was missing and false conflicts were detected. encoding/xml: fix invalid empty namespace without prefix (#8068) Empty namespace is allowed only if it has no prefix. An error message is now returned if a prefix exists. A similar case when no prefix is provided, thus with syntax xmlns:="..." is also rejected. encoding/xml: fix normalization of attributes values (#20614) The attribute value was read as text. The existing attribute reader logic is fixed as an attribute may have a namespace or only a prefix. Other possibilities have been removed. To keep the behavior of raw token which allows many faults in attributes list, error handling is heavily using the Strict parameter of the decoder. Specific tests have been added including list of attributes. To keep the behavior or unmarshal, escaped characters handling has been added but it is not symmetrical to Marshal for quotes but follows XML specification. encoding/xml: fix absence of detection of another : in qualified names (#20396) The occurrence of second : in space and tag name is rejected. Fixes: #7113, #7535, #8068, #8535, #10538, #11431, #13185, #16497, #20396, #20614, #20685 Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165 Commit originally authored by: Constantin Konstantinidis encoding/xml: fix printing of namespace prefix in tag names Prefix displays in XML when defined in tag names. The URL of the name space is also returned for an End Token as documentation requires to improve idempotency of Marshal/Unmarshal. Translating the prefix is popping the NS which was unavailable for translation. Translate for an End Token has been moved inside the pop element part. Fixes #9519 Change-Id: Id7a14d07c106a76a487b5c4e28d9d563fe061c60 encoding/xml: restore changes lost in merge See #43168. encoding/xml: gofmt encoding/xml: minimalIndent -> minIndent to shrink diff encoding/xml: gofmt encoding/xml: revert changes to (*Decoder).attrval from master encoding/xml: restore (*printer).writeIndent to master encoding/xml: remove comments that don’t change functionality of tests encoding/xml: use t.Errorf instead of fmt.Errorf encoding/xml: fix test case for #11496 encoding/xml: revert unrelated change in (*Decoder).readName encoding/xml: remove comments encoding/xml: remove comments encoding/xml: edit comments encoding/xml: add test to ensure '@' is legal in an attr val encoding/xml: improve test error quoting for readability encoding/xml: (*Decoder).nsname now strictly parses namespaces encoding/xml: disable 3/4 tests for #43168 I’m not sure what the right fix is for this. Malformed namespaces (leading or trailing colons, more than 1 colon) should result in an error. encoding/xml: allow callers to specify namespace prefix A preferred namespace prefix can now be specified by setting the local name space to "prefix:space". In a struct tag: struct Message { XMLName struct{} `xml:"urn:ietf:foo foo:message"` Value string `xml:"urn:ietf:foo foo:value"` } encoding/xml: use tag prefixes if previously defined Instead of having redundant xmlns= attrs. encoding/xml: rename (*printer).popPrefix to popPrefixes since it pops 1 or more prefixes encoding/xml: rename printer struct fields To reflect the fact that prefixes apply to tag names, not just attrs. encoding/xml: url->uri; reorg comments encoding/xml: fix comments encoding/xml: elide redundant xmlns= attrs for default namespace (no prefix) encoding/xml: move namespace prefix tests encoding/xml: handle namespace prefixes at the root element encoding/xml: handle namespaced prefixed attr names encoding/xml: split out prefixes during reflection, not parsing encoding/xml: rename tagName to xmlTag; document it encoding/xml: separate printing xmlns:prefix= attr from creating prefix encoding/xml: clean up comments encoding/xml: don’t use xmlTag in attr serialization encoding/xml: test nested prefixed namespaced tags encoding/xml: add test for prefixed XML tags without xmlns encoding/xml: update doc comments encoding/xml: update comments encoding/xml: preemptively declare and record all namespace prefixes Immediate children of XML tags that share a namespace and prefix will now have that namespace prefix declared on the parent tag. Explicitly declared namespace prefixes via xmlns:prefix="uri" attributes will now have their values recorded and reused for marshalling child tags. This commit reorganizes how namespace prefixes are recorded and detected, along with how start tags are recorded via a slice of element structs. Some tests are changed, because the value of the marshalled XML has changed, but the resulting XML should be equivalent (and correct). encoding/xml: restore tests with xmlns:prefix= attrs to existing behavior Use xmlnsURL instead of xmlnsPrefix to specify namespace prefixes. encoding/xml: prefer unprefixed names where possible encoding/xml: add EncodeToken tests for explicit prefixes encoding/xml: add test cases for prefixed local names encoding/xml: revert change in 8f143f5277; do not preemptively declare XML namespaces --- src/encoding/xml/marshal.go | 301 +++++++++---- src/encoding/xml/marshal_test.go | 330 ++++++++++++-- src/encoding/xml/read.go | 20 +- src/encoding/xml/typeinfo.go | 39 +- src/encoding/xml/xml.go | 55 ++- src/encoding/xml/xml_test.go | 716 ++++++++++++++++++++++++++++++- 6 files changed, 1304 insertions(+), 157 deletions(-) diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go index a8c8f659caca59..d52a31bce03190 100644 --- a/src/encoding/xml/marshal.go +++ b/src/encoding/xml/marshal.go @@ -300,6 +300,41 @@ func (enc *Encoder) Flush() error { return enc.p.Flush() } +// element represents a serialized XML element, in the form of either: +// <$name> +// <$name xmlns="$xmlns"> +// <$prefix:$name> +// <$prefix:$name xmlns:$prefix="$xmlns"> +type element struct { + xmlns string + prefix string + name string + nsToPrefix map[string]string + prefixToNS map[string]string +} + +func newElement(name Name) element { + var e element + e.xmlns = name.Space + e.prefix, e.name = splitPrefixed(name.Local) + return e +} + +func splitPrefixed(prefixed string) (prefix, name string) { + i := strings.Index(prefixed, ":") + if i < 1 || i > len(prefixed)-2 { + return "", prefixed + } + return prefixed[:i], prefixed[i+1:] +} + +func joinPrefixed(prefix, name string) string { + if prefix == "" { + return name + } + return prefix + ":" + name +} + type printer struct { *bufio.Writer encoder *Encoder @@ -309,36 +344,60 @@ type printer struct { depth int indentedIn bool putNewline bool - attrNS map[string]string // map prefix -> name space - attrPrefix map[string]string // map name space -> prefix - prefixes []string - tags []Name + elements []element +} + +// getPrefix finds the prefix to use for the given namespace URI, but does not create it. +func (p *printer) getPrefix(uri string) string { + switch uri { + case xmlURL: + // The "http://www.w3.org/XML/1998/namespace" name space is predefined as "xml" + // and must be referred to that way. + return xmlPrefix + case xmlnsURL: + // (The "http://www.w3.org/2000/xmlns/" name space is also predefined as "xmlns", + // but users should not be trying to use that one directly - that's our job.) + return xmlnsPrefix + } + for i := len(p.elements) - 1; i >= 0; i-- { + prefix := p.elements[i].nsToPrefix[uri] + if prefix != "" { + // Look for prefix collision deeper in the tree + for i++; i < len(p.elements); i++ { + if p.elements[i].prefixToNS[prefix] != "" { + return "" + } + } + return prefix + } + } + return "" } -// createAttrPrefix finds the name space prefix attribute to use for the given name space, +// createPrefix finds a prefix to use for the given namespace URI, // defining a new prefix if necessary. It returns the prefix. -func (p *printer) createAttrPrefix(url string) string { - if prefix := p.attrPrefix[url]; prefix != "" { - return prefix +// If set, it will attempt to use preferred as the prefix. +func (p *printer) createPrefix(uri, preferred string) (string, bool) { + if prefix := p.getPrefix(uri); prefix != "" && (prefix == preferred || preferred == "") { + return prefix, false } - // The "http://www.w3.org/XML/1998/namespace" name space is predefined as "xml" - // and must be referred to that way. - // (The "http://www.w3.org/2000/xmlns/" name space is also predefined as "xmlns", - // but users should not be trying to use that one directly - that's our job.) - if url == xmlURL { - return xmlPrefix + if len(p.elements) == 0 { + return "", false } - // Need to define a new name space. - if p.attrPrefix == nil { - p.attrPrefix = make(map[string]string) - p.attrNS = make(map[string]string) + // Need to define a new namespace prefix + e := &p.elements[len(p.elements)-1] + if e.nsToPrefix == nil { + e.nsToPrefix = make(map[string]string) + e.prefixToNS = make(map[string]string) } - // Pick a name. We try to use the final element of the path - // but fall back to _. - prefix := strings.TrimRight(url, "/") + // Pick a name. Try to use the final element of the path but fall back to _. + prefix := preferred + if prefix == "" || e.prefixToNS[prefix] != "" { + prefix = strings.TrimRight(uri, "/") + } if i := strings.LastIndex(prefix, "/"); i >= 0 { prefix = prefix[i+1:] } @@ -352,49 +411,30 @@ func (p *printer) createAttrPrefix(url string) string { if len(prefix) >= 3 && strings.EqualFold(prefix[:3], "xml") { prefix = "_" + prefix } - if p.attrNS[prefix] != "" { + if e.prefixToNS[prefix] != "" { // Name is taken. Find a better one. for p.seq++; ; p.seq++ { - if id := prefix + "_" + strconv.Itoa(p.seq); p.attrNS[id] == "" { + if id := prefix + "_" + strconv.Itoa(p.seq); e.prefixToNS[id] == "" { prefix = id break } } } - p.attrPrefix[url] = prefix - p.attrNS[prefix] = url + if e.nsToPrefix[uri] == "" { + e.nsToPrefix[uri] = prefix + } + e.prefixToNS[prefix] = uri + return prefix, true +} + +func (p *printer) writePrefixAttr(prefix, uri string) { p.WriteString(`xmlns:`) p.WriteString(prefix) p.WriteString(`="`) - EscapeText(p, []byte(url)) - p.WriteString(`" `) - - p.prefixes = append(p.prefixes, prefix) - - return prefix -} - -// deleteAttrPrefix removes an attribute name space prefix. -func (p *printer) deleteAttrPrefix(prefix string) { - delete(p.attrPrefix, p.attrNS[prefix]) - delete(p.attrNS, prefix) -} - -func (p *printer) markPrefix() { - p.prefixes = append(p.prefixes, "") -} - -func (p *printer) popPrefix() { - for len(p.prefixes) > 0 { - prefix := p.prefixes[len(p.prefixes)-1] - p.prefixes = p.prefixes[:len(p.prefixes)-1] - if prefix == "" { - break - } - p.deleteAttrPrefix(prefix) - } + p.EscapeString(uri) + p.WriteByte('"') } var ( @@ -481,18 +521,20 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat } else if tinfo.xmlname != nil { xmlname := tinfo.xmlname if xmlname.name != "" { - start.Name.Space, start.Name.Local = xmlname.xmlns, xmlname.name + start.Name.Space, start.Name.Local = xmlname.xmlns, joinPrefixed(xmlname.prefix, xmlname.name) } else { fv := xmlname.value(val, dontInitNilPointers) if v, ok := fv.Interface().(Name); ok && v.Local != "" { start.Name = v } } + } else { + // No enforced namespace, i.e. the outer tag namespace remains valid } - if start.Name.Local == "" && finfo != nil { - start.Name.Space, start.Name.Local = finfo.xmlns, finfo.name + if start.Name.Local == "" && finfo != nil { // XMLName overrides tag name - anonymous struct + start.Name.Space, start.Name.Local = finfo.xmlns, joinPrefixed(finfo.prefix, finfo.name) } - if start.Name.Local == "" { + if start.Name.Local == "" { // No or empty XMLName and still no tag name name := typ.Name() if i := strings.IndexByte(name, '['); i >= 0 { // Truncate generic instantiation name. See issue 48318. @@ -510,6 +552,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat if finfo.flags&fAttr == 0 { continue } + fv := finfo.value(val, dontInitNilPointers) if finfo.flags&fOmitEmpty != 0 && isEmptyValue(fv) { @@ -520,12 +563,19 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat continue } - name := Name{Space: finfo.xmlns, Local: finfo.name} + name := Name{finfo.xmlns, joinPrefixed(finfo.prefix, finfo.name)} if err := p.marshalAttr(&start, name, fv); err != nil { return err } } + // If an xmlname was found, namespace must be overridden. + if tinfo.xmlname != nil && start.Name.Space == "" && + len(p.elements) != 0 && p.elements[len(p.elements)-1].xmlns != "" { + // Add attr xmlns="" to override the outer tag namespace + start.Attr = append(start.Attr, Attr{Name{"", xmlnsPrefix}, ""}) + } + if err := p.writeStart(&start); err != nil { return err } @@ -647,7 +697,7 @@ func defaultStart(typ reflect.Type, finfo *fieldInfo, startTemplate *StartElemen start.Name = startTemplate.Name start.Attr = append(start.Attr, startTemplate.Attr...) } else if finfo != nil && finfo.name != "" { - start.Name.Local = finfo.name + start.Name.Local = joinPrefixed(finfo.prefix, finfo.name) start.Name.Space = finfo.xmlns } else if typ.Name() != "" { start.Name.Local = typ.Name() @@ -661,10 +711,10 @@ func defaultStart(typ reflect.Type, finfo *fieldInfo, startTemplate *StartElemen // marshalInterface marshals a Marshaler interface value. func (p *printer) marshalInterface(val Marshaler, start StartElement) error { - // Push a marker onto the tag stack so that MarshalXML + // Push a marker onto the element stack so that MarshalXML // cannot close the XML tags that it did not open. - p.tags = append(p.tags, Name{}) - n := len(p.tags) + p.elements = append(p.elements, element{}) + n := len(p.elements) err := val.MarshalXML(p.encoder, start) if err != nil { @@ -672,10 +722,10 @@ func (p *printer) marshalInterface(val Marshaler, start StartElement) error { } // Make sure MarshalXML closed all its tags. p.tags[n-1] is the mark. - if len(p.tags) > n { - return fmt.Errorf("xml: %s.MarshalXML wrote invalid XML: <%s> not closed", receiverType(val), p.tags[len(p.tags)-1].Local) + if len(p.elements) > n { + return fmt.Errorf("xml: %s.MarshalXML wrote invalid XML: <%s> not closed", receiverType(val), p.elements[len(p.elements)-1].name) } - p.tags = p.tags[:n-1] + p.elements = p.elements[:n-1] return nil } @@ -698,31 +748,95 @@ func (p *printer) writeStart(start *StartElement) error { return fmt.Errorf("xml: start tag with no name") } - p.tags = append(p.tags, start.Name) - p.markPrefix() + // Push a new XML element + p.elements = append(p.elements, newElement(start.Name)) + e := &p.elements[len(p.elements)-1] - p.writeIndent(1) - p.WriteByte('<') - p.WriteString(start.Name.Local) + // Set the namespace prefix, if necessary + var spaceDefined bool + if e.xmlns != "" { + // Try to avoid needing a prefix first. + for _, attr := range start.Attr { + if attr.Name.Space == "" && attr.Name.Local == xmlnsPrefix && attr.Value == e.xmlns { + spaceDefined = true + break + } + } + // Walk up the tree to see if a default namespace is already defined without a prefix. + if !spaceDefined && e.prefix == "" { + for i := len(p.elements) - 2; i >= 0; i-- { + if p.elements[i].prefix == "" && p.elements[i].xmlns == e.xmlns { + spaceDefined = true + break + } + } + } + // Locate an eventual prefix. If none, the XML is without start tag", name.Local) } - if top := p.tags[len(p.tags)-1]; top != name { - if top.Local != name.Local { - return fmt.Errorf("xml: end tag does not match start tag <%s>", name.Local, top.Local) - } - return fmt.Errorf("xml: end tag in namespace %s does not match start tag <%s> in namespace %s", name.Local, name.Space, top.Local, top.Space) + e := p.elements[len(p.elements)-1] + prefix, local := splitPrefixed(name.Local) + // Tag prefixes do not match + if prefix != "" && e.prefix != "" && prefix != e.prefix { + return fmt.Errorf("xml: end prefix does not match start prefix <%s:%s>", prefix, local, e.prefix, e.name) + } + // Tag names do not match + if local != e.name { + return fmt.Errorf("xml: end tag does not match start tag <%s>", local, e.name) + } + // Namespaces do not match + if name.Space != e.xmlns { + return fmt.Errorf("xml: end namespace %q does not match start namespace %q", name.Space, e.xmlns) } - p.tags = p.tags[:len(p.tags)-1] - p.writeIndent(-1) p.WriteByte('<') p.WriteByte('/') - p.WriteString(name.Local) + if e.prefix != "" { + p.WriteString(e.prefix) + p.WriteByte(':') + } + p.WriteString(e.name) p.WriteByte('>') - p.popPrefix() + // Pop elements stack + p.elements = p.elements[:len(p.elements)-1] return nil } diff --git a/src/encoding/xml/marshal_test.go b/src/encoding/xml/marshal_test.go index d2e5137afd7c05..8f143f52778b43 100644 --- a/src/encoding/xml/marshal_test.go +++ b/src/encoding/xml/marshal_test.go @@ -524,6 +524,38 @@ type IfaceAny struct { T2 T2 } +type EPP struct { + XMLName struct{} `xml:"urn:ietf:params:xml:ns:epp-1.0 epp"` + Command *Command `xml:"command,omitempty"` +} + +type Command struct { + Check *Check `xml:"urn:ietf:params:xml:ns:epp-1.0 check,omitempty"` +} + +type Check struct { + DomainCheck *DomainCheck `xml:"urn:ietf:params:xml:ns:domain-1.0 domain:check,omitempty"` +} + +type DomainCheck struct { + DomainNames []string `xml:"urn:ietf:params:xml:ns:domain-1.0 domain:name,omitempty"` +} + +type SecureEnvelope struct { + XMLName struct{} `xml:"urn:test:secure-1.0 sec:envelope"` + Message *SecureMessage `xml:"urn:test:message-1.0 msg,omitempty"` +} + +type SecureMessage struct { + Body string `xml:"urn:test:message-1.0 body,omitempty"` + Signer string `xml:"urn:test:secure-1.0 sec:signer,attr,omitempty"` +} + +type NamespacedNested struct { + XMLName struct{} `xml:"urn:test:nested-1.0 nested"` + Value string `xml:"urn:test:nested-1.0 nested:wrapper>nested:value"` +} + var ( nameAttr = "Sarah" ageAttr = uint(12) @@ -629,6 +661,11 @@ var marshalTests = []struct { {Value: &Port{Type: "ssl", Number: "443"}, ExpectXML: `443`}, {Value: &Port{Number: "443"}, ExpectXML: `443`}, {Value: &Port{Type: ""}, ExpectXML: ``}, + // Marshal is not symetric to Unmarshal for these oddities because &apos is written as ' + {Value: &Port{Type: ""}, ExpectXML: ``, UnmarshalOnly: true}, + {Value: &Port{Type: ""}, ExpectXML: ``, UnmarshalOnly: true}, + {Value: &Port{Type: ""}, ExpectXML: ``}, + {Value: &Port{Type: ""}, ExpectXML: ``, UnmarshalOnly: true}, {Value: &Port{Number: "443", Comment: "https"}, ExpectXML: `443`}, {Value: &Port{Number: "443", Comment: "add space-"}, ExpectXML: `443`, MarshalOnly: true}, {Value: &Domain{Name: []byte("google.com&friends")}, ExpectXML: `google.com&friends`}, @@ -807,7 +844,7 @@ var marshalTests = []struct { D1: "d1", }, ExpectXML: `` + - `abc` + + `abc` + `c1` + `d1` + `` + @@ -1047,6 +1084,10 @@ var marshalTests = []struct { Value: &OmitAttrTest{}, ExpectXML: ``, }, + { + Value: &OmitAttrTest{Str: "gopher@golang.org"}, + ExpectXML: ``, + }, // pointer fields { @@ -1107,10 +1148,10 @@ var marshalTests = []struct { Value: &AnyTest{Nested: "known", AnyField: AnyHolder{ XML: "", - XMLName: Name{Local: "AnyField"}, + XMLName: Name{Local: "other"}, // Overriding the field name is the purpose of the test }, }, - ExpectXML: `known`, + ExpectXML: `known`, }, { ExpectXML: `b`, @@ -1651,6 +1692,59 @@ var marshalTests = []struct { Value: &DirectAny{Any: string("")}, UnmarshalOnly: true, }, + + // Test namespace prefixes + { + ExpectXML: ``, + Value: &EPP{}, + }, + { + ExpectXML: ``, + Value: &EPP{Command: &Command{}}, + }, + { + ExpectXML: ``, + Value: &EPP{Command: &Command{Check: &Check{}}}, + }, + { + ExpectXML: ``, + Value: &EPP{Command: &Command{Check: &Check{DomainCheck: &DomainCheck{}}}}, + }, + { + ExpectXML: `golang.org`, + Value: &EPP{Command: &Command{Check: &Check{DomainCheck: &DomainCheck{DomainNames: []string{"golang.org"}}}}}, + }, + { + ExpectXML: `golang.orggo.dev`, + Value: &EPP{Command: &Command{Check: &Check{DomainCheck: &DomainCheck{DomainNames: []string{"golang.org", "go.dev"}}}}}, + }, + { + ExpectXML: ``, + Value: &SecureEnvelope{}, + }, + { + ExpectXML: ``, + Value: &SecureEnvelope{Message: &SecureMessage{}}, + }, + { + ExpectXML: `Hello, world.`, + Value: &SecureEnvelope{Message: &SecureMessage{Body: "Hello, world."}}, + }, + { + ExpectXML: `Thanks`, + Value: &SecureEnvelope{Message: &SecureMessage{Body: "Thanks", Signer: "gopher@golang.org"}}, + }, + { + ExpectXML: `You’re welcome!`, + Value: &NamespacedNested{Value: "You’re welcome!"}, + }, + { + ExpectXML: `value`, + Value: &struct { + XMLName struct{} `xml:"space:name"` + Value string `xml:"space:value"` + }{Value: "value"}, + }, } func TestMarshal(t *testing.T) { @@ -1801,16 +1895,16 @@ func TestUnmarshal(t *testing.T) { if err != nil { if test.UnmarshalError == "" { - t.Errorf("unmarshal(%#v): %s", test.ExpectXML, err) + t.Errorf("unmarshal(%s): %s", test.ExpectXML, err) return } if !strings.Contains(err.Error(), test.UnmarshalError) { - t.Errorf("unmarshal(%#v): %s, want %q", test.ExpectXML, err, test.UnmarshalError) + t.Errorf("unmarshal(%s): %s, want %q", test.ExpectXML, err, test.UnmarshalError) } return } if got, want := dest, test.Value; !reflect.DeepEqual(got, want) { - t.Errorf("unmarshal(%q):\nhave %#v\nwant %#v", test.ExpectXML, got, want) + t.Errorf("unmarshal(%s):\nhave %#v\nwant %#v", test.ExpectXML, got, want) } }) } @@ -1961,7 +2055,7 @@ var encodeTokenTests = []struct { want string err string }{{ - desc: "start element with name space", + desc: "start element with namespace", toks: []Token{ StartElement{Name{"space", "local"}, nil}, }, @@ -2058,7 +2152,7 @@ var encodeTokenTests = []struct { StartElement{Name{"space", "foo"}, nil}, EndElement{Name{"another", "foo"}}, }, - err: "xml: end tag in namespace another does not match start tag in namespace space", + err: `xml: end namespace "another" does not match start namespace "space"`, want: ``, }, { desc: "start element with explicit namespace", @@ -2069,6 +2163,23 @@ var encodeTokenTests = []struct { }}, }, want: ``, +}, { + desc: "start element with explicit namespace prefix", + toks: []Token{ + StartElement{Name{"space", "local"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + {Name{"space", "foo"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "start element with prefixed local name", + toks: []Token{ + StartElement{Name{"space", "x:local"}, []Attr{ + {Name{"space", "foo"}, "value"}, + }}, + }, + want: ``, }, { desc: "start element with explicit namespace and colliding prefix", toks: []Token{ @@ -2079,6 +2190,25 @@ var encodeTokenTests = []struct { }}, }, want: ``, +}, { + desc: "start element with explicit namespace prefix and colliding prefix", + toks: []Token{ + StartElement{Name{"space", "local"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + {Name{"space", "foo"}, "value"}, + {Name{"x", "bar"}, "other"}, + }}, + }, + want: ``, +}, { + desc: "start element with prefixed local name and colliding prefix", + toks: []Token{ + StartElement{Name{"space", "x:local"}, []Attr{ + {Name{"space", "foo"}, "value"}, + {Name{"x", "bar"}, "other"}, + }}, + }, + want: ``, }, { desc: "start element using previously defined namespace", toks: []Token{ @@ -2091,7 +2221,27 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "nested name space with same prefix", + desc: "start element using previously defined namespace prefix", + toks: []Token{ + StartElement{Name{"", "local"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + }}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"space", "x"}, "y"}, + }}, + }, + want: ``, +}, { + desc: "start element using prefixed local name", + toks: []Token{ + StartElement{Name: Name{"space", "x:local"}}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"space", "x"}, "y"}, + }}, + }, + want: ``, +}, { + desc: "nested namespace with same prefix", toks: []Token{ StartElement{Name{"", "foo"}, []Attr{ {Name{"xmlns", "x"}, "space1"}, @@ -2112,7 +2262,28 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "start element defining several prefixes for the same name space", + desc: "nested namespace with same prefix", + toks: []Token{ + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space1"}, + }}, + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space2"}, + }}, + StartElement{Name{"", "foo"}, []Attr{ + {Name{"space1", "a"}, "space1 value"}, + {Name{"space2", "b"}, "space2 value"}, + }}, + EndElement{Name{"", "foo"}}, + EndElement{Name{"", "foo"}}, + StartElement{Name{"", "foo"}, []Attr{ + {Name{"space1", "a"}, "space1 value"}, + {Name{"space2", "b"}, "space2 value"}, + }}, + }, + want: ``, +}, { + desc: "start element defining several prefixes for the same namespace", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"xmlns", "a"}, "space"}, @@ -2122,7 +2293,17 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "nested element redefines name space", + desc: "start element explicitly defining several prefixes for the same namespace", + toks: []Token{ + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "a"}, "space"}, + {Name{"http://www.w3.org/2000/xmlns/", "b"}, "space"}, + {Name{"space", "x"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element redefines namespace", toks: []Token{ StartElement{Name{"", "foo"}, []Attr{ {Name{"xmlns", "x"}, "space"}, @@ -2134,7 +2315,30 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "nested element creates alias for default name space", + desc: "nested element redefines namespace prefix", + toks: []Token{ + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + }}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "y"}, "space"}, + {Name{"space", "a"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element explicitly redefines namespace prefix", + toks: []Token{ + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + }}, + StartElement{Name{"space", "y:foo"}, []Attr{ + {Name{"space", "a"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element creates alias for default namespace", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"", "xmlns"}, "space"}, @@ -2144,9 +2348,32 @@ var encodeTokenTests = []struct { {Name{"space", "a"}, "value"}, }}, }, - want: ``, + want: ``, +}, { + desc: "nested element creates alias prefix for default namespace", + toks: []Token{ + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"", "xmlns"}, "space"}, + }}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "y"}, "space"}, + {Name{"space", "a"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element explicitly creates alias prefix for default namespace", + toks: []Token{ + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"", "xmlns"}, "space"}, + }}, + StartElement{Name{"space", "y:foo"}, []Attr{ + {Name{"space", "a"}, "value"}, + }}, + }, + want: ``, }, { - desc: "nested element defines default name space with existing prefix", + desc: "nested element defines default namespace with existing prefix", toks: []Token{ StartElement{Name{"", "foo"}, []Attr{ {Name{"xmlns", "x"}, "space"}, @@ -2156,9 +2383,31 @@ var encodeTokenTests = []struct { {Name{"space", "a"}, "value"}, }}, }, - want: ``, + want: ``, }, { - desc: "nested element uses empty attribute name space when default ns defined", + desc: "nested element defines default namespace prefix with existing prefix", + toks: []Token{ + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "x"}, "space"}, + }}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"", "xmlns"}, "space"}, + {Name{"space", "a"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element defines explicit attribute namespace prefix with existing prefix", + toks: []Token{ + StartElement{Name: Name{"", "foo"}}, + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"", "xmlns"}, "space"}, + {Name{"space", "x:a"}, "value"}, + }}, + }, + want: ``, +}, { + desc: "nested element uses empty attribute namespace when default ns defined", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"", "xmlns"}, "space"}, @@ -2167,7 +2416,7 @@ var encodeTokenTests = []struct { {Name{"", "attr"}, "value"}, }}, }, - want: ``, + want: ``, }, { desc: "redefine xmlns", toks: []Token{ @@ -2177,7 +2426,7 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "xmlns with explicit name space #1", + desc: "xmlns with explicit namespace #1", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"xml", "xmlns"}, "space"}, @@ -2185,7 +2434,7 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "xmlns with explicit name space #2", + desc: "xmlns with explicit namespace #2", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{xmlURL, "xmlns"}, "space"}, @@ -2193,13 +2442,21 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "empty name space declaration is ignored", + desc: "empty namespace declaration is ignored", toks: []Token{ StartElement{Name{"", "foo"}, []Attr{ {Name{"xmlns", "foo"}, ""}, }}, }, want: ``, +}, { + desc: "empty namespace prefix declaration is ignored", + toks: []Token{ + StartElement{Name{"", "foo"}, []Attr{ + {Name{"http://www.w3.org/2000/xmlns/", "foo"}, ""}, + }}, + }, + want: ``, }, { desc: "attribute with no name is ignored", toks: []Token{ @@ -2228,18 +2485,18 @@ var encodeTokenTests = []struct { {Name{"space", "x"}, "value"}, }}, }, - want: ``, + want: ``, }, { - desc: "nested element requires empty default name space", + desc: "nested element requires empty default namespace", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"", "xmlns"}, "space"}, }}, StartElement{Name{"", "foo"}, nil}, }, - want: ``, + want: ``, }, { - desc: "attribute uses name space from xmlns", + desc: "attribute uses namespace from xmlns", toks: []Token{ StartElement{Name{"some/space", "foo"}, []Attr{ {Name{"", "attr"}, "value"}, @@ -2248,7 +2505,7 @@ var encodeTokenTests = []struct { }, want: ``, }, { - desc: "default name space should not be used by attributes", + desc: "default namespace should not be used by attributes", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"", "xmlns"}, "space"}, @@ -2259,9 +2516,22 @@ var encodeTokenTests = []struct { EndElement{Name{"space", "baz"}}, EndElement{Name{"space", "foo"}}, }, - want: ``, + want: ``, +}, { + desc: "default namespace prefix should not be used by attributes", + toks: []Token{ + StartElement{Name{"space", "foo"}, []Attr{ + {Name{"", "xmlns"}, "space"}, + {Name{"http://www.w3.org/2000/xmlns/", "bar"}, "space"}, + {Name{"space", "baz"}, "foo"}, + }}, + StartElement{Name{"space", "baz"}, nil}, + EndElement{Name{"space", "baz"}}, + EndElement{Name{"space", "foo"}}, + }, + want: ``, }, { - desc: "default name space not used by attributes, not explicitly defined", + desc: "default namespace not used by attributes, not explicitly defined", toks: []Token{ StartElement{Name{"space", "foo"}, []Attr{ {Name{"", "xmlns"}, "space"}, @@ -2271,7 +2541,7 @@ var encodeTokenTests = []struct { EndElement{Name{"space", "baz"}}, EndElement{Name{"space", "foo"}}, }, - want: ``, + want: ``, }, { desc: "impossible xmlns declaration", toks: []Token{ @@ -2318,12 +2588,12 @@ loop: for j, tok := range tt.toks { err = enc.EncodeToken(tok) if err != nil && j < len(tt.toks)-1 { - t.Errorf("#%d %s token #%d: %v", i, tt.desc, j, err) + t.Errorf("#%d %s; token #%d: %v", i, tt.desc, j, err) continue loop } } errorf := func(f string, a ...interface{}) { - t.Errorf("#%d %s token #%d:%s", i, tt.desc, len(tt.toks)-1, fmt.Sprintf(f, a...)) + t.Errorf("#%d %s; token #%d:%s", i, tt.desc, len(tt.toks)-1, fmt.Sprintf(f, a...)) } switch { case tt.err != "" && err == nil: diff --git a/src/encoding/xml/read.go b/src/encoding/xml/read.go index ef5df3f7f6aecc..0eaf94805d2414 100644 --- a/src/encoding/xml/read.go +++ b/src/encoding/xml/read.go @@ -435,9 +435,23 @@ func (d *Decoder) unmarshal(val reflect.Value, start *StartElement) error { } return UnmarshalError(e) } - fv := finfo.value(sv, initNilPointers) - if _, ok := fv.Interface().(Name); ok { - fv.Set(reflect.ValueOf(start.Name)) + // Anonymous struct with no field or anonymous fields cannot get a value using the reflection + // package and must be discarded. + noValue := true + if sv.Type().Name() == "" && sv.Type().Kind() == reflect.Struct { + i := 0 + for i < sv.Type().NumField() && noValue { + noValue = noValue && sv.Type().Field(i).Anonymous + i++ + } + } else { + noValue = false + } + if !noValue { + fv := finfo.value(sv, initNilPointers) + if _, ok := fv.Interface().(Name); ok { + fv.Set(reflect.ValueOf(start.Name)) + } } } diff --git a/src/encoding/xml/typeinfo.go b/src/encoding/xml/typeinfo.go index 162724ef1a58b0..3b58ac920af7ab 100644 --- a/src/encoding/xml/typeinfo.go +++ b/src/encoding/xml/typeinfo.go @@ -20,6 +20,7 @@ type typeInfo struct { // fieldInfo holds details for the xml representation of a single field. type fieldInfo struct { idx []int + prefix string name string xmlns string flags fieldFlags @@ -65,7 +66,7 @@ func getTypeInfo(typ reflect.Type) (*typeInfo, error) { } // For embedded structs, embed its fields. - if f.Anonymous { + if f.Anonymous { // i.e. reflect package will panic to get a Value t := f.Type if t.Kind() == reflect.Ptr { t = t.Elem() @@ -75,13 +76,14 @@ func getTypeInfo(typ reflect.Type) (*typeInfo, error) { if err != nil { return nil, err } - if tinfo.xmlname == nil { + if tinfo.xmlname == nil && inner.xmlname != nil && inner.xmlname.name != "" { + // to leave xmlname to nil and to avoid assigning unexported xmlname field tinfo.xmlname = inner.xmlname } for _, finfo := range inner.fields { finfo.idx = append([]int{i}, finfo.idx...) if err := addFieldInfo(typ, tinfo, &finfo); err != nil { - return nil, err + return nil, err // Any detected conflict returns an error } } continue @@ -93,19 +95,16 @@ func getTypeInfo(typ reflect.Type) (*typeInfo, error) { return nil, err } - if f.Name == xmlName { + if f.Name == xmlName { // copying the field for "easier" access when .FieldByName() is appropriate tinfo.xmlname = finfo - continue - } - - // Add the field if it doesn't conflict with other fields. - if err := addFieldInfo(typ, tinfo, finfo); err != nil { + } else if err := addFieldInfo(typ, tinfo, finfo); err != nil { + // Add the field if it doesn't conflict with other fields. return nil, err } } } - ti, _ := tinfoMap.LoadOrStore(typ, tinfo) + ti, _ := tinfoMap.LoadOrStore(typ, tinfo) // Returned bool is always false return ti.(*typeInfo), nil } @@ -122,7 +121,7 @@ func structFieldInfo(typ reflect.Type, f *reflect.StructField) (*fieldInfo, erro // Parse flags. tokens := strings.Split(tag, ",") if len(tokens) == 1 { - finfo.flags = fElement + finfo.flags = fElement // Nothing but the name of field } else { tag = tokens[0] for _, flag := range tokens[1:] { @@ -179,7 +178,7 @@ func structFieldInfo(typ reflect.Type, f *reflect.StructField) (*fieldInfo, erro // The XMLName field records the XML element name. Don't // process it as usual because its name should default to // empty rather than to the field name. - finfo.name = tag + finfo.prefix, finfo.name = splitPrefixed(tag) return finfo, nil } @@ -187,8 +186,9 @@ func structFieldInfo(typ reflect.Type, f *reflect.StructField) (*fieldInfo, erro // If the name part of the tag is completely empty, get // default from XMLName of underlying struct if feasible, // or field name otherwise. + // This is how an anonymous struct gets a value if xmlname := lookupXMLName(f.Type); xmlname != nil { - finfo.xmlns, finfo.name = xmlname.xmlns, xmlname.name + finfo.xmlns, finfo.prefix, finfo.name = xmlname.xmlns, xmlname.prefix, xmlname.name } else { finfo.name = f.Name } @@ -203,7 +203,11 @@ func structFieldInfo(typ reflect.Type, f *reflect.StructField) (*fieldInfo, erro if parents[len(parents)-1] == "" { return nil, fmt.Errorf("xml: trailing '>' in field %s of type %s", f.Name, typ) } - finfo.name = parents[len(parents)-1] + finfo.prefix, finfo.name = splitPrefixed(parents[len(parents)-1]) + // TODO(ydnar): should we allow prefixed parents, e.g.: space:a>space>b? + for i := range parents { + _, parents[i] = splitPrefixed(parents[i]) + } if len(parents) > 1 { if (finfo.flags & fElement) == 0 { return nil, fmt.Errorf("xml: %s chain not valid with %s flag", tag, strings.Join(tokens[1:], ",")) @@ -240,7 +244,7 @@ func lookupXMLName(typ reflect.Type) (xmlname *fieldInfo) { if f.Name != xmlName { continue } - finfo, err := structFieldInfo(typ, &f) + finfo, err := structFieldInfo(typ, &f) // Recursive call if err == nil && finfo.name != "" { return finfo } @@ -292,7 +296,8 @@ Loop: conflicts = append(conflicts, i) } } else { - if newf.name == oldf.name { + if newf.name == oldf.name && newf.xmlns == oldf.xmlns { + // A conflict is only if the names are identical in the same namespace which might be "" conflicts = append(conflicts, i) } } @@ -314,7 +319,7 @@ Loop: // Otherwise, if any of them is at the same depth level, it's an error. for _, i := range conflicts { oldf := &tinfo.fields[i] - if len(oldf.idx) == len(newf.idx) { + if len(oldf.idx) == len(newf.idx) { // Same depth f1 := typ.FieldByIndex(oldf.idx) f2 := typ.FieldByIndex(newf.idx) return &TagPathError{typ, f1.Name, f1.Tag.Get("xml"), f2.Name, f2.Tag.Get("xml")} diff --git a/src/encoding/xml/xml.go b/src/encoding/xml/xml.go index c14954df155a64..e3cb9139334f0d 100644 --- a/src/encoding/xml/xml.go +++ b/src/encoding/xml/xml.go @@ -216,7 +216,7 @@ type Decoder struct { toClose Name nextToken Token nextByte int - ns map[string]string + ns map[string]string // url for a prefix ns[.Space]=.Value err error line int offset int64 @@ -309,29 +309,38 @@ func (d *Decoder) Token() (Token, error) { // to the other attribute names, so process // the translations first. for _, a := range t1.Attr { - if a.Name.Space == xmlnsPrefix { - v, ok := d.ns[a.Name.Local] - d.pushNs(a.Name.Local, v, ok) + if a.Name.Space == xmlnsPrefix { // name space attribute {.Space}xmlns:{.Local}={.Value} + if a.Value == "" { + d.err = d.syntaxError("empty namespace without prefix") + return nil, d.err + } + if a.Name.Local == "" { + d.err = d.syntaxError("empty prefix") + return nil, d.err + } + v, ok := d.ns[a.Name.Local] // Checking existence + // Recording the level of the name space by recording tag name + d.pushNs(a.Name.Local, v, ok) // Pushing tag, eventual value, and existence of namespace d.ns[a.Name.Local] = a.Value } - if a.Name.Space == "" && a.Name.Local == xmlnsPrefix { - // Default space for untagged names + if a.Name.Space == "" && a.Name.Local == xmlnsPrefix { // xmlns=".Value" + // Default space for non-prefixed names v, ok := d.ns[""] d.pushNs("", v, ok) d.ns[""] = a.Value } } + d.pushElement(t1.Name) // Pushing the element with its eventual prefix + // Assigning value to Space of the attributed using the NS bindings d.translate(&t1.Name, true) for i := range t1.Attr { d.translate(&t1.Attr[i].Name, false) } - d.pushElement(t1.Name) t = t1 case EndElement: - d.translate(&t1.Name, true) - if !d.popElement(&t1) { + if !d.popElement(&t1) { // Popping the element with its eventual prefix for appropriate comparison return nil, d.err } t = t1 @@ -341,8 +350,9 @@ func (d *Decoder) Token() (Token, error) { const ( xmlURL = "http://www.w3.org/XML/1998/namespace" - xmlnsPrefix = "xmlns" xmlPrefix = "xml" + xmlnsURL = "http://www.w3.org/2000/xmlns/" + xmlnsPrefix = "xmlns" ) // Apply name space translation to name n. @@ -351,6 +361,7 @@ const ( func (d *Decoder) translate(n *Name, isElementName bool) { switch { case n.Space == xmlnsPrefix: + n.Space = xmlnsURL return case n.Space == "" && !isElementName: return @@ -499,10 +510,13 @@ func (d *Decoder) popElement(t *EndElement) bool { return false case s.name.Space != name.Space: d.err = d.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space + - "closed by in space " + name.Space) + " closed by in space " + name.Space) return false } + // translating before removal of ns + d.translate(&t.Name, true) // returning a namespace and not its prefix as doc says + // Pop stack until a Start or EOF is on the top, undoing the // translations that were associated with the element we just closed. for d.stk != nil && d.stk.kind != stkStart && d.stk.kind != stkEOF { @@ -798,6 +812,9 @@ func (d *Decoder) rawToken() (Token, error) { for { d.space() if b, ok = d.mustgetc(); !ok { + if len(attr) > 0 && !d.Strict { + break // When not strict, an attribute might end with EOF + } return nil, d.err } if b == '/' { @@ -832,7 +849,6 @@ func (d *Decoder) rawToken() (Token, error) { d.err = d.syntaxError("attribute name without = in element") return nil, d.err } - d.ungetc(b) a.Value = a.Name.Local } else { d.space() @@ -1016,6 +1032,11 @@ Input: d.ungetc('<') break Input } + // This occurs only for an unquoted attr name. + if b == '>' && !cdata && quote < 0 { // Possible end of tag reached + d.ungetc('>') // Leaving end of tag available + break // returning text + } if quote >= 0 && b == byte(quote) { break Input } @@ -1112,6 +1133,7 @@ Input: } // We must rewrite unescaped \r and \r\n into \n. + // End of line handling https://www.w3.org/TR/xml/#sec-line-ends if b == '\r' { d.buf.WriteByte('\n') } else if b1 == '\r' && b == '\n' { @@ -1162,10 +1184,15 @@ func (d *Decoder) nsname() (name Name, ok bool) { if !ok { return } - if strings.Count(s, ":") > 1 { + n := strings.Count(s, ":") + if n == 0 { // No colons, no namespace. OK. + name.Local = s + } else if n > 1 { // More than one colon, not OK. name.Local = s - } else if i := strings.Index(s, ":"); i < 1 || i > len(s)-2 { + return name, false + } else if i := strings.Index(s, ":"); i < 1 || i > len(s)-2 { // Leading or trailing colon, not OK. name.Local = s + return name, false } else { name.Space = s[0:i] name.Local = s[i+1:] diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go index 19152dbdb68951..9e6fdd8f550add 100644 --- a/src/encoding/xml/xml_test.go +++ b/src/encoding/xml/xml_test.go @@ -182,7 +182,7 @@ var cookedTokens = []Token{ Directive(`DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"`), CharData("\n"), - StartElement{Name{"ns2", "body"}, []Attr{{Name{"xmlns", "foo"}, "ns1"}, {Name{"", "xmlns"}, "ns2"}, {Name{"xmlns", "tag"}, "ns3"}}}, + StartElement{Name{"ns2", "body"}, []Attr{{Name{"http://www.w3.org/2000/xmlns/", "foo"}, "ns1"}, {Name{"", "xmlns"}, "ns2"}, {Name{"http://www.w3.org/2000/xmlns/", "tag"}, "ns3"}}}, CharData("\n "), StartElement{Name{"ns2", "hello"}, []Attr{{Name{"", "lang"}, "en"}}}, CharData("World <>'\" 白鵬翔"), @@ -195,7 +195,7 @@ var cookedTokens = []Token{ StartElement{Name{"ns2", "goodbye"}, []Attr{}}, EndElement{Name{"ns2", "goodbye"}}, CharData("\n "), - StartElement{Name{"ns2", "outer"}, []Attr{{Name{"ns1", "attr"}, "value"}, {Name{"xmlns", "tag"}, "ns4"}}}, + StartElement{Name{"ns2", "outer"}, []Attr{{Name{"ns1", "attr"}, "value"}, {Name{"http://www.w3.org/2000/xmlns/", "tag"}, "ns4"}}}, CharData("\n "), StartElement{Name{"ns2", "inner"}, []Attr{}}, EndElement{Name{"ns2", "inner"}}, @@ -864,6 +864,345 @@ func TestIssue5880(t *testing.T) { } } +func TestIssue8535(t *testing.T) { + + type ExampleConflict struct { + XMLName Name `xml:"example"` + Link string `xml:"link"` + AtomLink string `xml:"http://www.w3.org/2005/Atom link"` // No conflict but no assignment + } + testCases := []string{ + ` + Example + http://example.com/default + http://example.com/home + http://example.com/ns + `, + } + + var dest ExampleConflict + d := NewDecoder(strings.NewReader(testCases[0])) + if err := d.Decode(&dest); err != nil { + t.Errorf("%s: Field conflicts : got error %v, want no fail", testCases[0], err) + } +} + +func TestIssue11431(t *testing.T) { // + + type Test struct { + XMLName Name `xml:"Test"` + Ns string `xml:"xmlns,attr"` + Body string + } + + s := &Test{Ns: "http://example.com/ns", Body: "hello world"} + b, err := Marshal(s) + if err != nil { + t.Errorf("namespace handling: expected no error, got %s", err) + } + + want := `hello world` + if string(b) != want { + t.Errorf("namespace handling: got %s, want %s \n", string(b), want) + } +} + +func TestIssue11431NsWoAttr(t *testing.T) { + + type Test struct { + Body string `xml:"http://example.com/ns body"` + } + + s := &Test{Body: "hello world"} + b, err := Marshal(s) + if err != nil { + t.Errorf("namespace handling: expected no error, got %s", err) + } + + want := `hello world` + if string(b) != want { + t.Errorf("namespace handling: got %s, want %s \n", string(b), want) + } +} + +func TestIssue11431XMLName(t *testing.T) { // + + type Test struct { + XMLName Name `xml:"http://example.com/ns Test"` + Body string + } + + //s := &Test{XMLName: Name{"http://example.com/ns",""}, Body: "hello world"} is unusable as the "-" is missing + // as documentation states + s := &Test{Body: "hello world"} + b, err := Marshal(s) + if err != nil { + t.Errorf("namespace handling: expected no error, got %s", err) + } + + want := `hello world` + if string(b) != want { + t.Errorf("namespace handling: got %s, want %s \n", string(b), want) + } +} + +func TestIssue11431UsingAttr(t *testing.T) { // + + type T struct { + Ns string `xml:"xmlns,attr"` + Body string + } + + //s := &Test{XMLName: Name{"http://example.com/ns",""}, Body: "hello world"} is unusable as the "-" is missing + // as documentation states + s := &T{Ns: "http://example.com/ns", Body: "hello world"} + b, err := Marshal(s) + if err != nil { + t.Errorf("namespace handling: expected no error, got %s", err) + } + + want := `hello world` + if string(b) != want { + t.Errorf("namespace handling: got %s, want %s \n", string(b), want) + } +} + +func TestIssue11496(t *testing.T) { // Issue answered + + type Person struct { + XMLName Name `xml:"ns1 person"` + Name string `xml:"name"` + Phone string `xml:"ns2 phone,omitempty"` + } + + p := &Person{ + Name: "Oliver", + Phone: "110", + } + + got, err := Marshal(p) + if err != nil { + t.Errorf("namespace assignment: marshal error returned is %s", err) + } + + want := `Oliver110` + if string(got) != want { + t.Errorf("namespace assignment:\ngot: %s\nwant: %s", string(got), want) + } + + // Output: + // + // Oliver + // 110 + // + // + // Want: + // + // Oliver + // 110 + // + +} + +func TestIssue8068(t *testing.T) { + + testCases := []struct { + s string + ok bool + }{ // Empty prefixed namespace is not allowed + {``, true}, + {``, false}, + {``, false}, + {``, false}, + {``, false}, + } + + var dest string // type does not matter as tested tags are empty + var err error + for _, tc := range testCases { + err = Unmarshal([]byte(tc.s), &dest) + + if err != nil && tc.ok { + t.Errorf("%s: Empty prefixed namespace : expected no error, got %s", tc.s, err) + continue + } + if err == nil && !tc.ok { + t.Errorf("%s: Empty prefixed namespace : expected error, got nil", tc.s) + } + } + +} + +func TestIssue10538(t *testing.T) { + // There is no restriction of the placement of XMLName in embedded structs + // If the field is unexported, reflect package will panic in the documented cases + // Purpose of the test is to show that no panic occurs with multiple set ups of embedded structs using XMLName + type elementNoXMLName struct { + Children []interface{} + } + + type element struct { + XMLName Name + Children []interface{} + } + + type Element struct { + XMLName Name + Children []interface{} + } + + type svgstrEmptyStruct struct { + elementNoXMLName //is not exported and empty + Height string `xml:"height,attr,omitempty"` + Width string `xml:"width,attr,omitempty"` + } + + type svgstr struct { + element // not exported and .Value panics + Height string `xml:"height,attr,omitempty"` + Width string `xml:"width,attr,omitempty"` + } + + type svgstrExp struct { + Element element // exported and .Value does not panic + Height string `xml:"height,attr,omitempty"` + Width string `xml:"width,attr,omitempty"` + } + + type svgstrExpType struct { + Element // exported and .Value does not panic + Height string `xml:"height,attr,omitempty"` + Width string `xml:"width,attr,omitempty"` + } + + type svgstr2 struct { + XMLName Name + Children []interface{} + Height string `xml:"height,attr,omitempty"` + Width string `xml:"width,attr,omitempty"` + } + + /* No embedded XMLName */ + result := `` + sE := svgstrEmptyStruct{ + Width: "400", + Height: "200", + } + a, err := Marshal(sE) + if err != nil { + t.Errorf("xmlname handling : marshaling failed with %s \n", err) + } + if string(a) != result { + t.Errorf("xmlname handling : got %s, want %s \n", string(a), result) + } + /* XMLName in a unexported field is not assigned */ + result = `` + s := svgstr{ + element: element{XMLName: Name{Local: "svg", Space: "www.etc"}, Children: nil}, + Width: "400", + Height: "200", + } + + f, err := Marshal(s) + if err != nil { + t.Errorf("xmlname handling : marshaling failed with %s \n", err) + } + if string(f) != result { + t.Errorf("xmlname handling : got %s, want %s \n", string(f), result) + } + /* Embedding the XMLName gets it assigned to the inner struct */ + result = `` + sExp := svgstrExp{ + Element: element{XMLName: Name{Local: "svg", Space: "www.etc"}, Children: nil}, + Width: "400", + Height: "200", + } + + b, err := Marshal(sExp) + if err != nil { + t.Errorf("xmlname handling : marshaling failed with %s \n", err) + } + if string(b) != result { + t.Errorf("xmlname handling : got %s, want %s \n", string(b), result) + } + /* XMLName is not assigned to outer tag but to inner tag. Not working due to other issues */ + result = `` + sExpType := svgstrExpType{ + Element: Element{XMLName: Name{Local: "svg", Space: "www.etc"}, Children: []interface{}{""}}, + Width: "400", + Height: "200", + } + + d, err := Marshal(sExpType) + if err != nil { + t.Errorf("xmlname handling : marshaling failed with %s \n", err) + } + if string(d) != result { + t.Errorf("xmlname handling : got %s, want %s \n", string(d), result) + } + /* No inner struct. XMLName is assigned as usual */ + result = `` + s2 := svgstr2{ + XMLName: Name{Local: "svg", Space: "www.etc"}, + Width: "400", + Height: "200", + } + + c, err := Marshal(s2) + if err != nil { + t.Errorf("xmlname handling : marshaling failed with %s \n", err) + } + if string(c) != result { + t.Errorf("xmlname handling : got %s, want %s \n", string(c), result) + } +} + +func TestIssue7535(t *testing.T) { + source := `` + result := `` + // A prefix is the namespace known from the tag where it is declared and not the default namespace. + // But in a well-formed xml, it is useless as the prefix is bound and recorded as an attribute + in := strings.NewReader(source) + var errl, err error + var token Token + + for i := 0; i < 4; i++ { + out := &bytes.Buffer{} + d := NewDecoder(in) + e := NewEncoder(out) + errl = nil + for errl == nil { + token, err = d.Token() + if err != nil { + if err == io.EOF { + errl = err + } else { + t.Errorf("read token failed:%s", err) + return + } + } else { // err is nil + // end token contains now the URL which can be encoded only if the NS if available + // from the start token + err = e.EncodeToken(token) + if err != nil { + t.Errorf("encode token failed : %s", err) + return + } + } + } + e.Flush() + if out.String() != result { + t.Errorf("duplicating namespace : got %s, want %s \n", out.String(), result) + return + } + in.Reset(out.String()) + } + + if errl != nil && errl != io.EOF { + t.Errorf("%s \n: duplicating namespace : got error %v, want no fail \n", source, errl) + } +} + func TestIssue11405(t *testing.T) { testCases := []string{ "", @@ -917,6 +1256,371 @@ func TestIssue12417(t *testing.T) { } } +func TestIssue20396(t *testing.T) { + testCases := []struct { + s string + ok bool + }{ // Should not allow to change namespace of opening tag + {``, false}, + {``, false}, + {``, false}, + {``, true}, + } + for _, tc := range testCases { + d := NewDecoder(strings.NewReader(tc.s)) + var err error + for { + _, err = d.Token() + if err != nil { + if err == io.EOF { //EOF indicates that process is complete + err = nil + } + break + } + } + if err != nil && tc.ok { + err = d.err + t.Errorf("%q: Multiple colons in tag : expected no error, got %s", tc.s, err) + continue + } + if err == nil && !tc.ok { + t.Errorf("%q: Multiple colons in tag : expected error, got nil", tc.s) + } + } +} + +func TestIssue20685(t *testing.T) { + testCases := []struct { + s string + ok bool + }{ + {`one`, false}, + {`one`, true}, + {`one`, false}, + {`one`, false}, + {`one`, false}, + {`one`, false}, + {`one`, false}, + } + for _, tc := range testCases { + d := NewDecoder(strings.NewReader(tc.s)) + var err error + for { + _, err = d.Token() + if err != nil { + if err == io.EOF { + err = nil + } + break + } + } + if err != nil && tc.ok { + t.Errorf("%q: Closing tag with namespace : expected no error, got %s", tc.s, err) + continue + } + if err == nil && !tc.ok { + t.Errorf("%q: Closing tag with namespace : expected error, got nil", tc.s) + } + } +} + +func TestIssue16497(t *testing.T) { + + type IQ struct { + Type string `xml:"type,attr"` + XMLName Name `xml:"iq"` + } + + type embedIQ struct { + IQ IQ + } + + /* Anonymous struct */ + resp := struct { + IQ + }{} /* */ + + var err error + err = Unmarshal([]byte(``), &resp) + if err != nil { + t.Errorf("unmarshal anonymous struct failed with %s", err) + return + } + // assigning values or not does not change anything + var respEmbed embedIQ + err = Unmarshal([]byte(``), &respEmbed) + if err != nil { + t.Errorf("unmarshal anonymous struct failed with %s", err) + return + } +} + +func TestIssue9519(t *testing.T) { + // Expects prefixed notation prefix:tag name iso xmlns: + type HouseType struct { + XMLName Name `xml:"prefix11 House"` + MessageId string `xml:"message_id,attr"` + } + + var tm HouseType + var err error + + tm.MessageId = "test1234" + + var data1 []byte + data1, err = Marshal(tm) + if err != nil { + t.Errorf("%s : handling namespace : got error %v, want no fail \n", data1, err) + } + + result := `` + if string(data1) != result { + t.Errorf("handling namespace : got %v, want %s \n", string(data1), result) + } + + var tm2 HouseType + err = Unmarshal([]byte(data1), &tm2) + if err != nil { + t.Errorf("%s : handling namespace : got error %v, want no fail \n", data1, err) + } + + if tm.MessageId != tm2.MessageId { + t.Errorf("handling namespace : got %s, want %s \n", tm.MessageId, tm2.MessageId) + } +} + +func TestUnmarshalXMLName(t *testing.T) { + + type InnerStruct struct { + XMLName Name `xml:"testns outer"` + } + + type OuterStruct struct { + InnerStruct + IntAttr int `xml:"int,attr"` + } + + type OuterNamedStruct struct { + InnerStruct + IntAttr int `xml:"int,attr"` + XMLName Name `xml:"outerns test"` + } + + type OuterNamedOrderedStruct struct { + XMLName Name `xml:"outerns test"` + InnerStruct + IntAttr int `xml:"int,attr"` + } + + var unMarshalTestsXMLName = []struct { + Value interface{} + ExpectXML string + MarshalOnly bool + MarshalError string + UnmarshalOnly bool + UnmarshalError string + }{ + { + ExpectXML: ``, + Value: &OuterStruct{IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterStruct{IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterNamedStruct{XMLName: Name{Space: "outerns", Local: "test"}, IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterNamedOrderedStruct{XMLName: Name{Space: "outerns", Local: "test"}, IntAttr: 10}, + }, + } + for i, test := range unMarshalTestsXMLName { + if test.MarshalOnly { + continue + } + if _, ok := test.Value.(*Plain); ok { + continue + } + if test.ExpectXML == ``+ + `b`+ + `b1`+ + `` { + // TODO(rogpeppe): re-enable this test in + // https://go-review.googlesource.com/#/c/5910/ + continue + } + + vt := reflect.TypeOf(test.Value) + dest := reflect.New(vt.Elem()).Interface() + err := Unmarshal([]byte(test.ExpectXML), dest) + + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + switch fix := dest.(type) { + case *Feed: + fix.Author.InnerXML = "" + for i := range fix.Entry { + fix.Entry[i].Author.InnerXML = "" + } + } + + if err != nil { + if test.UnmarshalError == "" { + t.Errorf("unmarshal(%#v): %s", test.ExpectXML, err) + return + } + if !strings.Contains(err.Error(), test.UnmarshalError) { + t.Errorf("unmarshal(%#v): %s, want %q", test.ExpectXML, err, test.UnmarshalError) + } + return + } + if got, want := dest, test.Value; !reflect.DeepEqual(got, want) { + t.Errorf("unmarshal(%q):\nhave %#v\nwant %#v", test.ExpectXML, got, want) + } + }) + } +} + +func TestMarshalXMLName(t *testing.T) { + + type InnerStruct struct { + XMLName Name `xml:"testns outer"` + } + + type OuterStruct struct { + InnerStruct + IntAttr int `xml:"int,attr"` + } + + type OuterNamedStruct struct { + InnerStruct + IntAttr int `xml:"int,attr"` + XMLName Name `xml:"outerns test"` + } + + type OuterNamedOrderedStruct struct { + XMLName Name `xml:"outerns test"` + InnerStruct + IntAttr int `xml:"int,attr"` + } + + var marshalTestsXMLName = []struct { + Value interface{} + ExpectXML string + MarshalOnly bool + MarshalError string + UnmarshalOnly bool + UnmarshalError string + }{ + { + ExpectXML: ``, + Value: &OuterStruct{IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterStruct{IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterNamedStruct{XMLName: Name{Space: "outerns", Local: "test"}, IntAttr: 10}, + }, + { + ExpectXML: ``, + Value: &OuterNamedOrderedStruct{XMLName: Name{Space: "outerns", Local: "test"}, IntAttr: 10}, + }, + } + + for idx, test := range marshalTestsXMLName { + if test.UnmarshalOnly { + continue + } + + t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { + data, err := Marshal(test.Value) + if err != nil { + if test.MarshalError == "" { + t.Errorf("marshal(%#v): %s", test.Value, err) + return + } + if !strings.Contains(err.Error(), test.MarshalError) { + t.Errorf("marshal(%#v): %s, want %q", test.Value, err, test.MarshalError) + } + return + } + if test.MarshalError != "" { + t.Errorf("Marshal succeeded, want error %q", test.MarshalError) + return + } + if got, want := string(data), test.ExpectXML; got != want { + if strings.Contains(want, "\n") { + t.Errorf("marshal(%#v):\nHAVE:\n%s\nWANT:\n%s", test.Value, got, want) + } else { + t.Errorf("marshal(%#v):\nhave %#q\nwant %#q", test.Value, got, want) + } + } + }) + } +} + +func TestIssue7113(t *testing.T) { + type C struct { + XMLName Name `xml:""` // To reset namespace to "" + } + + type A struct { + XMLName Name `xml:""` + C C `xml:""` + } + + var a A + structSpace := "b" + fieldSpace := "" + xmlTests := []string{ + ``, + // ``, + } + for _, xmltest := range xmlTests { + + err := Unmarshal([]byte(xmltest), &a) + if err != nil { + t.Errorf("overidding with empty namespace: expected no error, got %s", err) + } + if a.XMLName.Space != structSpace { + t.Errorf("overidding with empty namespace: before marshaling, got %s != %s, want == \n", a.XMLName.Space, structSpace) + } + if a.C.XMLName.Space != fieldSpace { + t.Errorf("overidding with empty namespace: before marshaling, got %s != %s, want == \n", a.C.XMLName.Space, fieldSpace) + } + + var b []byte + /* Because of unmarshaling, namespaces are already assigned */ + b, err = Marshal(&a) + if string(b) != xmltest { + t.Errorf("overidding with empty namespace: after marshaling, got %s != %s, want == \n", string(b), xmltest) + return + } + // Unmarshaling has no interest if the previous test succeed as the structs are initially empty unless + if a.C.XMLName.Local != "C" { + t.Errorf("overidding with empty namespace: after marshaling, unmarshaling will fail, got %s as C tag space which should be tag name C \n", a.C.XMLName.Local) + } + if a.C.XMLName.Space != "" { + t.Errorf("overidding with empty namespace: after marshaling, unmarshaling will fail, got %s in C tag which should be empty \n", a.C.XMLName.Space) + } + err = Unmarshal(b, &a) + if err != nil { + t.Errorf("overidding with empty namespace: expected no error, got %s", err) + } + if a.XMLName.Space != "b" { + t.Errorf("overidding with empty namespace: after marshaling & unmarshaling, got %s in XMLName != %s, want == \n", a.XMLName.Space, "b") + } + if a.C.XMLName.Space != "" { + t.Errorf("overidding with empty namespace: after marshaling & unmarshaling, got %q in C tag != %q, want == \n", a.C.XMLName.Space, "") + } + } +} + func tokenMap(mapping func(t Token) Token) func(TokenReader) TokenReader { return func(src TokenReader) TokenReader { return mapper{ @@ -1051,9 +1755,11 @@ func testRoundTrip(t *testing.T, input string) { func TestRoundTrip(t *testing.T) { tests := map[string]string{ - "leading colon": `<::Test ::foo="bar"><:::Hello>`, - "trailing colon": ``, - "double colon": ``, + // Disabling these tests because the parser now treats malformed namespaces as an error. + // See https://github.com/golang/go/issues/43168. + // "leading colon": `<::Test ::foo="bar"><:::Hello>`, + // "trailing colon": ``, + // "double colon": ``, "comments in directives": `--x --> > --x ]>`, } for name, input := range tests {