Skip to content

Commit

Permalink
encoding/xml : Fixes to enforce XML namespace standard
Browse files Browse the repository at this point in the history
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 : golang#7113, golang#7535, golang#8068,  golang#8535, golang#10538, golang#11431, golang#13185, golang#16497, golang#20396, golang#20614, golang#20685

Change-Id: Ib4a60347a47d23ff59b63307cebb83b71c7c9165
  • Loading branch information
iwdgo authored and Randy Reddig committed Sep 27, 2021
1 parent aeea5ba commit 2544535
Show file tree
Hide file tree
Showing 6 changed files with 947 additions and 153 deletions.
96 changes: 65 additions & 31 deletions src/encoding/xml/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ func NewEncoder(w io.Writer) *Encoder {
func (enc *Encoder) Indent(prefix, indent string) {
enc.p.prefix = prefix
enc.p.indent = indent
enc.p.minimalIndent = prefix == "" && indent == "" //Empty values are still indented
}

// Encode writes the XML encoding of v to the stream.
Expand Down Expand Up @@ -302,17 +303,18 @@ func (enc *Encoder) Flush() error {

type printer struct {
*bufio.Writer
encoder *Encoder
seq int
indent string
prefix string
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
encoder *Encoder
seq int
indent string
prefix string
depth int
indentedIn bool
putNewline bool
minimalIndent bool // new line even with empty prefix and indent
attrNS map[string]string // map prefix -> name space
attrPrefix map[string]string // map name space -> prefix
prefixes []string
tags []Name
}

// createAttrPrefix finds the name space prefix attribute to use for the given name space,
Expand Down Expand Up @@ -382,7 +384,7 @@ func (p *printer) deleteAttrPrefix(prefix string) {
delete(p.attrNS, prefix)
}

func (p *printer) markPrefix() {
func (p *printer) markPrefix() { // This is why prefix are never showing
p.prefixes = append(p.prefixes, "")
}

Expand Down Expand Up @@ -482,17 +484,23 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
xmlname := tinfo.xmlname
if xmlname.name != "" {
start.Name.Space, start.Name.Local = xmlname.xmlns, xmlname.name
// .Space is equivalent to xmlns=".Space" so adding the attribute
if start.Name.Space != "" {
start.Attr = append(start.Attr, Attr{Name{"", xmlnsPrefix}, start.Name.Space})
}
} 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 {
if start.Name.Local == "" && finfo != nil { // XMLName overrides tag name - anonymous struct
start.Name.Space, start.Name.Local = finfo.xmlns, 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.
Expand Down Expand Up @@ -526,6 +534,13 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
}
}

/* If an xmlname was found, namespace must be overridden */
if tinfo.xmlname != nil && start.Name.Space == "" &&
len(p.tags) != 0 && p.tags[len(p.tags)-1].Space != "" {
//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
}
Expand Down Expand Up @@ -698,17 +713,35 @@ func (p *printer) writeStart(start *StartElement) error {
return fmt.Errorf("xml: start tag with no name")
}

// pushes the value of the namespace and not the eventual prefix
p.tags = append(p.tags, start.Name)
p.markPrefix()
p.markPrefix() // pushes an empty prefix

p.writeIndent(1)
p.writeIndent(1) // Handling relative depth of a tag
p.WriteByte('<')
p.WriteString(start.Name.Local)

if start.Name.Space != "" {
p.WriteString(` xmlns="`)
p.EscapeString(start.Name.Space)
p.WriteByte('"')
p.WriteString(start.Name.Local) // if prefix exists, it is not printed
/* The attribute was not added if no XMLName field existed. */
if start.Name.Space != "" { // tag starts with <.Space:.Local
// The tag prefix is not the default name space and it is a mistake to print it if not bound by an attribute.
// But this is used in some cases which are unrelated to XML standards. Invalid XML can then be produced.
dontPrintTagSpace := false
for _, attr := range start.Attr {
// Name.Space only contains a namespace xmlns=".Space" or a .Value xmlns:(unavailable)=.Space
// Attributes values which are namespaces are searched to avoid reprinting the domain
dontPrintTagSpace = (start.Name.Space == attr.Value && attr.Name.Space == xmlnsPrefix && attr.Name.Local != "") ||
(attr.Name.Space == "" && attr.Name.Local == xmlnsPrefix && attr.Value == start.Name.Space)
if dontPrintTagSpace {
if attr.Name.Space == xmlnsPrefix {
p.tags[len(p.tags)-1].Space = attr.Name.Local // Overriding with prefix
}
break
}
}
if !dontPrintTagSpace {
p.WriteString(` xmlns="`)
p.EscapeString(start.Name.Space)
p.WriteByte('"')
}
}

// Attributes
Expand All @@ -718,11 +751,15 @@ func (p *printer) writeStart(start *StartElement) error {
continue
}
p.WriteByte(' ')
if name.Space != "" {
p.WriteString(p.createAttrPrefix(name.Space))
if name.Space == xmlnsPrefix { // printing prefix name.Local xmlns:{.Local}={.Value}
p.WriteString(xmlnsPrefix)
p.WriteByte(':')
} else if name.Space != "" { // not a name space {.Space}:{.Local}={.Value}
p.WriteString(p.createAttrPrefix(name.Space)) // name.Space is not a prefix
p.WriteByte(':')
}
p.WriteString(name.Local)
// When space is empty, only writing .Local=.Value
p.WriteString(attr.Name.Local)
p.WriteString(`="`)
p.EscapeString(attr.Value)
p.WriteByte('"')
Expand All @@ -739,9 +776,9 @@ func (p *printer) writeEnd(name Name) error {
return fmt.Errorf("xml: end tag </%s> without start tag", name.Local)
}
if top := p.tags[len(p.tags)-1]; top != name {
if top.Local != name.Local {
if top.Local != name.Local { // Tag names do not match
return fmt.Errorf("xml: end tag </%s> does not match start tag <%s>", name.Local, top.Local)
}
} // Namespaces do not match
return fmt.Errorf("xml: end tag </%s> in namespace %s does not match start tag <%s> in namespace %s", name.Local, name.Space, top.Local, top.Space)
}
p.tags = p.tags[:len(p.tags)-1]
Expand Down Expand Up @@ -968,9 +1005,6 @@ func (p *printer) cachedWriteError() error {
}

func (p *printer) writeIndent(depthDelta int) {
if len(p.prefix) == 0 && len(p.indent) == 0 {
return
}
if depthDelta < 0 {
p.depth--
if p.indentedIn {
Expand All @@ -979,7 +1013,7 @@ func (p *printer) writeIndent(depthDelta int) {
}
p.indentedIn = false
}
if p.putNewline {
if p.putNewline && (len(p.indent) > 0 || len(p.prefix) > 0 || p.minimalIndent) {
p.WriteByte('\n')
} else {
p.putNewline = true
Expand Down
Loading

0 comments on commit 2544535

Please sign in to comment.