Constantin Konstantinidis has uploaded this change for review.
encoding/xml fix overriding by empty namespace
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.
A typo was also fixed as the attribute was writing the tag space
and not the attribute space. If no namespace is active,
nothing is written.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go
index d393d06..916fac2 100644
--- a/src/encoding/xml/marshal.go
+++ b/src/encoding/xml/marshal.go
@@ -478,8 +478,10 @@
} else if v, ok := xmlname.value(val).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
start.Name.Space, start.Name.Local = finfo.xmlns, finfo.name
}
if start.Name.Local == "" {
@@ -512,6 +514,12 @@
}
}
+ // 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
}
@@ -687,11 +695,11 @@
p.tags = append(p.tags, start.Name)
p.markPrefix()
- p.writeIndent(1)
+ p.writeIndent(1) // Handling relative depth of a tag
p.WriteByte('<')
p.WriteString(start.Name.Local)
- if start.Name.Space != "" {
+ if start.Name.Space != "" { // Prefix is never written in tag.
p.WriteString(` xmlns="`)
p.EscapeString(start.Name.Space)
p.WriteByte('"')
@@ -708,7 +716,8 @@
p.WriteString(p.createAttrPrefix(name.Space))
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('"')
@@ -949,9 +958,6 @@
}
func (p *printer) writeIndent(depthDelta int) {
- if len(p.prefix) == 0 && len(p.indent) == 0 {
- return
- }
if depthDelta < 0 {
p.depth--
if p.indentedIn {
@@ -960,7 +966,7 @@
}
p.indentedIn = false
}
- if p.putNewline {
+ if p.putNewline && (len(p.indent) > 0 || len(p.prefix) > 0) {
p.WriteByte('\n')
} else {
p.putNewline = true
diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index 7a3511d..50c2f73 100644
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -798,6 +798,57 @@
}
}
+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 := ""
+ xmlTest := `<A xmlns="` + structSpace + `"><C xmlns="` + fieldSpace + `"></C></A>`
+ 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{
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis uploaded patch set #2 to this change.
encoding/xml: fix overriding by empty namespace
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.
A typo was also fixed as the attribute was writing the tag space
and not the attribute space. If no namespace is active,
nothing is written.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 65 insertions(+), 8 deletions(-)
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis uploaded patch set #3 to this change.
encoding/xml: fix overriding by empty namespace
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.
A typo was also fixed as the attribute was writing the tag space
and not the attribute space. If no namespace is active,
nothing is written.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 65 insertions(+), 8 deletions(-)
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis uploaded patch set #4 to this change.
encoding/xml: fix overriding by empty namespace
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.
A typo was also fixed as the attribute was writing the tag space
and not the attribute space. If no namespace is active,
nothing is written.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 64 insertions(+), 4 deletions(-)
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
I discussed these XML changes with Russ and others and it sounds like we want to prioritize these (and JSON) for after Go 1.11 and after vgo. We just didn't have the review bandwidth in this area this cycle and can't accept the risk of regressions this late in Go 1.11. I'm going to mark this for Go 1.12 for now. Sorry.
Constantin Konstantinidis uploaded patch set #6 to this change.
encoding/xml: allow overriding by empty namespace
The namespace defined by xmlns="value" can be overridden in every included tag
by the empty namespace xmlns="" without a prefix.
Method to calculate indent of XML handles depth of tag and its associated namespace is
still 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 override of any non-empty namespace of its outer tag.
An attribute is added to have the required namespace display.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 76 insertions(+), 0 deletions(-)
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Heschi Kreinick, Ian Lance Taylor.
1 comment:
Patchset:
Adding recent reviewers.
CL is rebased and clutter is removed to focus on the issue independently other problems in #13400.
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Constantin Konstantinidis.
Patch set 6:Run-TryBot +1
Attention is currently required from: Constantin Konstantinidis.
Patch set 6:Run-TryBot +1Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Thanks.
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Constantin Konstantinidis.
Patch set 6:Code-Review +1
Gopher Robot submitted this change.
encoding/xml: allow overriding by empty namespace
The namespace defined by xmlns="value" can be overridden in every included tag
by the empty namespace xmlns="" without a prefix.
Method to calculate indent of XML handles depth of tag and its associated namespace is
still 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 override of any non-empty namespace of its outer tag.
An attribute is added to have the required namespace display.
Fixes #7113
Change-Id: I57f2308e98c66f04108ab136d350bdc3a6091e98
Reviewed-on: https://go-review.googlesource.com/c/go/+/108796
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Michael Knyszek <mkny...@google.com>
Run-TryBot: Ian Lance Taylor <ia...@golang.org>
Run-TryBot: Ian Lance Taylor <ia...@google.com>
Reviewed-by: Ian Lance Taylor <ia...@google.com>
Auto-Submit: Ian Lance Taylor <ia...@google.com>
---
M src/encoding/xml/marshal.go
M src/encoding/xml/xml_test.go
2 files changed, 83 insertions(+), 0 deletions(-)
diff --git a/src/encoding/xml/marshal.go b/src/encoding/xml/marshal.go
index 07b6042..0c3cc0d 100644
--- a/src/encoding/xml/marshal.go
+++ b/src/encoding/xml/marshal.go
@@ -543,6 +543,11 @@
}
}
+ // If a name was found, namespace is overridden with an empty space
+ if tinfo.xmlname != nil && start.Name.Space == "" &&
+ len(p.tags) != 0 && p.tags[len(p.tags)-1].Space != "" {
+ start.Attr = append(start.Attr, Attr{Name{"", xmlnsPrefix}, ""})
+ }
if err := p.writeStart(&start); err != nil {
return err
}
diff --git a/src/encoding/xml/xml_test.go b/src/encoding/xml/xml_test.go
index 30fb94d..8205ac3 100644
--- a/src/encoding/xml/xml_test.go
+++ b/src/encoding/xml/xml_test.go
@@ -1059,6 +1059,56 @@
}
}
+func TestIssue7113(t *testing.T) {
+ type C struct {
+ XMLName Name `xml:""` // Sets empty namespace
+ }
+
+ type A struct {
+ XMLName Name `xml:""`
+ C C `xml:""`
+ }
+
+ var a A
+ structSpace := "b"
+ xmlTest := `<A xmlns="` + structSpace + `"><C xmlns=""></C></A>`
+ t.Log(xmlTest)
+ err := Unmarshal([]byte(xmlTest), &a)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if a.XMLName.Space != structSpace {
+ t.Errorf("overidding with empty namespace: unmarshalling, got %s, want %s\n", a.XMLName.Space, structSpace)
+ }
+ if len(a.C.XMLName.Space) != 0 {
+ t.Fatalf("overidding with empty namespace: unmarshalling, got %s, want empty\n", a.C.XMLName.Space)
+ }
+
+ var b []byte
+ b, err = Marshal(&a)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(a.C.XMLName.Space) != 0 {
+ t.Errorf("overidding with empty namespace: marshaling, got %s in C tag which should be empty\n", a.C.XMLName.Space)
+ }
+ if string(b) != xmlTest {
+ t.Fatalf("overidding with empty namespace: marshalling, got %s, want %s\n", b, xmlTest)
+ }
+ var c A
+ err = Unmarshal(b, &c)
+ if err != nil {
+ t.Fatalf("second Unmarshal failed: %s", err)
+ }
+ if c.XMLName.Space != "b" {
+ t.Errorf("overidding with empty namespace: after marshaling & unmarshaling, XML name space: got %s, want %s\n", a.XMLName.Space, structSpace)
+ }
+ if len(c.C.XMLName.Space) != 0 {
+ t.Errorf("overidding with empty namespace: after marshaling & unmarshaling, got %s, want empty\n", a.C.XMLName.Space)
+ }
+}
+
func TestIssue20396(t *testing.T) {
var attrError = UnmarshalError("XML syntax error on line 1: expected attribute name in element")
To view, visit change 108796. To unsubscribe, or for help writing mail filters, visit settings.