Skip to content

Commit 8ac9e1a

Browse files
iwdgoydnar
authored andcommitted
encoding/xml : Fixes to enforce XML namespace standard
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
1 parent ff8a7e5 commit 8ac9e1a

File tree

6 files changed

+947
-153
lines changed

6 files changed

+947
-153
lines changed

src/encoding/xml/marshal.go

+65-31
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ func NewEncoder(w io.Writer) *Encoder {
150150
func (enc *Encoder) Indent(prefix, indent string) {
151151
enc.p.prefix = prefix
152152
enc.p.indent = indent
153+
enc.p.minimalIndent = prefix == "" && indent == "" //Empty values are still indented
153154
}
154155

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

303304
type printer struct {
304305
*bufio.Writer
305-
encoder *Encoder
306-
seq int
307-
indent string
308-
prefix string
309-
depth int
310-
indentedIn bool
311-
putNewline bool
312-
attrNS map[string]string // map prefix -> name space
313-
attrPrefix map[string]string // map name space -> prefix
314-
prefixes []string
315-
tags []Name
306+
encoder *Encoder
307+
seq int
308+
indent string
309+
prefix string
310+
depth int
311+
indentedIn bool
312+
putNewline bool
313+
minimalIndent bool // new line even with empty prefix and indent
314+
attrNS map[string]string // map prefix -> name space
315+
attrPrefix map[string]string // map name space -> prefix
316+
prefixes []string
317+
tags []Name
316318
}
317319

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

385-
func (p *printer) markPrefix() {
387+
func (p *printer) markPrefix() { // This is why prefix are never showing
386388
p.prefixes = append(p.prefixes, "")
387389
}
388390

@@ -482,17 +484,23 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
482484
xmlname := tinfo.xmlname
483485
if xmlname.name != "" {
484486
start.Name.Space, start.Name.Local = xmlname.xmlns, xmlname.name
487+
// .Space is equivalent to xmlns=".Space" so adding the attribute
488+
if start.Name.Space != "" {
489+
start.Attr = append(start.Attr, Attr{Name{"", xmlnsPrefix}, start.Name.Space})
490+
}
485491
} else {
486492
fv := xmlname.value(val, dontInitNilPointers)
487493
if v, ok := fv.Interface().(Name); ok && v.Local != "" {
488494
start.Name = v
489495
}
490496
}
497+
} else {
498+
// No enforced namespace, i.e. the outer tag namespace remains valid
491499
}
492-
if start.Name.Local == "" && finfo != nil {
500+
if start.Name.Local == "" && finfo != nil { // XMLName overrides tag name - anonymous struct
493501
start.Name.Space, start.Name.Local = finfo.xmlns, finfo.name
494502
}
495-
if start.Name.Local == "" {
503+
if start.Name.Local == "" { // No or empty XMLName and still no tag name
496504
name := typ.Name()
497505
if i := strings.IndexByte(name, '['); i >= 0 {
498506
// Truncate generic instantiation name. See issue 48318.
@@ -526,6 +534,13 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo, startTemplat
526534
}
527535
}
528536

537+
/* If an xmlname was found, namespace must be overridden */
538+
if tinfo.xmlname != nil && start.Name.Space == "" &&
539+
len(p.tags) != 0 && p.tags[len(p.tags)-1].Space != "" {
540+
//add attr xmlns="" to override the outer tag namespace
541+
start.Attr = append(start.Attr, Attr{Name{"", xmlnsPrefix}, ""})
542+
}
543+
/* */
529544
if err := p.writeStart(&start); err != nil {
530545
return err
531546
}
@@ -698,17 +713,35 @@ func (p *printer) writeStart(start *StartElement) error {
698713
return fmt.Errorf("xml: start tag with no name")
699714
}
700715

716+
// pushes the value of the namespace and not the eventual prefix
701717
p.tags = append(p.tags, start.Name)
702-
p.markPrefix()
718+
p.markPrefix() // pushes an empty prefix
703719

704-
p.writeIndent(1)
720+
p.writeIndent(1) // Handling relative depth of a tag
705721
p.WriteByte('<')
706-
p.WriteString(start.Name.Local)
707-
708-
if start.Name.Space != "" {
709-
p.WriteString(` xmlns="`)
710-
p.EscapeString(start.Name.Space)
711-
p.WriteByte('"')
722+
p.WriteString(start.Name.Local) // if prefix exists, it is not printed
723+
/* The attribute was not added if no XMLName field existed. */
724+
if start.Name.Space != "" { // tag starts with <.Space:.Local
725+
// The tag prefix is not the default name space and it is a mistake to print it if not bound by an attribute.
726+
// But this is used in some cases which are unrelated to XML standards. Invalid XML can then be produced.
727+
dontPrintTagSpace := false
728+
for _, attr := range start.Attr {
729+
// Name.Space only contains a namespace xmlns=".Space" or a .Value xmlns:(unavailable)=.Space
730+
// Attributes values which are namespaces are searched to avoid reprinting the domain
731+
dontPrintTagSpace = (start.Name.Space == attr.Value && attr.Name.Space == xmlnsPrefix && attr.Name.Local != "") ||
732+
(attr.Name.Space == "" && attr.Name.Local == xmlnsPrefix && attr.Value == start.Name.Space)
733+
if dontPrintTagSpace {
734+
if attr.Name.Space == xmlnsPrefix {
735+
p.tags[len(p.tags)-1].Space = attr.Name.Local // Overriding with prefix
736+
}
737+
break
738+
}
739+
}
740+
if !dontPrintTagSpace {
741+
p.WriteString(` xmlns="`)
742+
p.EscapeString(start.Name.Space)
743+
p.WriteByte('"')
744+
}
712745
}
713746

714747
// Attributes
@@ -718,11 +751,15 @@ func (p *printer) writeStart(start *StartElement) error {
718751
continue
719752
}
720753
p.WriteByte(' ')
721-
if name.Space != "" {
722-
p.WriteString(p.createAttrPrefix(name.Space))
754+
if name.Space == xmlnsPrefix { // printing prefix name.Local xmlns:{.Local}={.Value}
755+
p.WriteString(xmlnsPrefix)
756+
p.WriteByte(':')
757+
} else if name.Space != "" { // not a name space {.Space}:{.Local}={.Value}
758+
p.WriteString(p.createAttrPrefix(name.Space)) // name.Space is not a prefix
723759
p.WriteByte(':')
724760
}
725-
p.WriteString(name.Local)
761+
// When space is empty, only writing .Local=.Value
762+
p.WriteString(attr.Name.Local)
726763
p.WriteString(`="`)
727764
p.EscapeString(attr.Value)
728765
p.WriteByte('"')
@@ -739,9 +776,9 @@ func (p *printer) writeEnd(name Name) error {
739776
return fmt.Errorf("xml: end tag </%s> without start tag", name.Local)
740777
}
741778
if top := p.tags[len(p.tags)-1]; top != name {
742-
if top.Local != name.Local {
779+
if top.Local != name.Local { // Tag names do not match
743780
return fmt.Errorf("xml: end tag </%s> does not match start tag <%s>", name.Local, top.Local)
744-
}
781+
} // Namespaces do not match
745782
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)
746783
}
747784
p.tags = p.tags[:len(p.tags)-1]
@@ -968,9 +1005,6 @@ func (p *printer) cachedWriteError() error {
9681005
}
9691006

9701007
func (p *printer) writeIndent(depthDelta int) {
971-
if len(p.prefix) == 0 && len(p.indent) == 0 {
972-
return
973-
}
9741008
if depthDelta < 0 {
9751009
p.depth--
9761010
if p.indentedIn {
@@ -979,7 +1013,7 @@ func (p *printer) writeIndent(depthDelta int) {
9791013
}
9801014
p.indentedIn = false
9811015
}
982-
if p.putNewline {
1016+
if p.putNewline && (len(p.indent) > 0 || len(p.prefix) > 0 || p.minimalIndent) {
9831017
p.WriteByte('\n')
9841018
} else {
9851019
p.putNewline = true

0 commit comments

Comments
 (0)