Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/xml: missing nested namespace not handled #7113

Open
anacrolix opened this issue Jan 13, 2014 · 20 comments

Comments

Projects
None yet
@anacrolix
Copy link
Contributor

commented Jan 13, 2014

Nested XML namespaces aren't marshalled correctly. Setting an element namespace at the
default level as Go does, implicitly sets nested elements to the same namespace if they
don't override it.

<a xmlns="b"><c/></a>

The namespace of c, is b. However marshalling from Go data structures in which the
object corresponding to c above has no namespace set should give

<a xmlns="b"><c xmlns=""/></a>

The usual way to deal with this is to use namespace prefixes, but I don't think that's a
requirement.
@davecheney

This comment has been minimized.

Copy link
Contributor

commented Feb 3, 2014

Comment 1:

@anacrolix could you please include a short code sample or a test that demonstrates the
issue.

Status changed to WaitingForReply.

@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Feb 4, 2014

Comment 2:

Will do.
@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2014

Comment 3:

I've created an example: http://play.golang.org/p/5zDgqtC3ra
The output is currently:
2009/11/10 23:00:00 "b" == "b"
2009/11/10 23:00:00 "" == ""
2009/11/10 23:00:00 "<A xmlns=\"b\"><C></C></A>" != "<A
xmlns=\"b\"><C xmlns=\"\"></C></A>"
2009/11/10 23:00:00 "b" == "b"
2009/11/10 23:00:00 "b" != ""
If you read the play source, you'll observe that the namespace of "" on element C has
been lost when marshalling. When unmarshalling what Go currently produces, it no longer
has "" for C's namespace as a result.
This is incorrect, see:
http://stackoverflow.com/questions/2193220/xml-namespace-defaulting-inheritance
http://www.w3.org/TR/REC-xml-names/#scoping-defaulting
http://www.w3.org/TR/xml-names/#defaulting
The impact this has on programs using XML, is that they don't reset the default
namespace for nested elements when marshalling.
Here's some downstream bugs that result from this:
https://bitbucket.org/anacrolix/dms/issue/25/get-soap-responses-to-work-with-upnp
https://bitbucket.org/anacrolix/dms/issue/19/samsung-doesnt-like-response-to
@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2014

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2014

Comment 5:

Labels changed: added release-go1.3maybe.

Status changed to Accepted.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 9, 2014

Comment 6:

CL https://golang.org/cl/66850043 references this issue.
@rsc

This comment has been minimized.

Copy link
Contributor

commented May 13, 2014

Comment 7:

Alternate CL at https://golang.org/cl/93320043. Leaving final decision for 1.4.

Labels changed: added release-go1.4, removed release-go1.3maybe.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 21, 2014

Comment 8 by glen.newton:

Is this related to or the same bug as: 'Golang XML unmarshalling issue: local name
collisions fail'
https://stackoverflow.com/questions/24870309/golang-xml-unmarshalling-issue-local-name-collisions-fail/
I am asking as I am trying to figure out if I should submit a bug for my issue.
@anacrolix

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2014

Comment 9:

I don't believe it is the same bug. It could be related, but I recommend you create a
new issue for it.
@griesemer

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2014

Comment 10:

Labels changed: added repo-main.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2014

Comment 11:

We didn't get to this in time. My apologies.

Labels changed: added release-go1.5, removed release-go1.4.

@jordan2175

This comment has been minimized.

Copy link

commented Jan 8, 2015

Please see my example and scenario on issue #9527 as well. Also, there is another issue with prefixes that is going to be highly related to this #9519 / #6800. I would love to see both of these fixed in 1.5 if I can beg enough.

@lucasdss

This comment has been minimized.

Copy link

commented Feb 25, 2015

I'm writing an RPM manager using GO and I ended up with this problem too. The XML file primary.xml provided by any RPM repository uses "rpm:entry" and others tags with the "rpm" as namespace prefix. It took me two days until I figured out It's a bug. Well, I did some string.replace and it works, but not as good as xml.Marshal.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2015

See #11841.

@gopherbot

This comment has been minimized.

Copy link

commented Jul 23, 2015

CL https://golang.org/cl/12570 mentions this issue.

rsc added a commit that referenced this issue Jul 27, 2015

encoding/xml: restore Go 1.4 name space behavior
There is clearly work to do here with respect to xml name spaces,
but I don't believe the changes in this cycle are clearly correct.
The changes in this cycle have visible impact on the generated xml,
possibly breaking existing programs, and yet it's not clear that they
are the end of the story: there is still significant confusion about how
name spaces work or should work (see #9519, #9775, #8167, #7113).

I would like to wait to make breaking changes until we completely
understand what the behavior should be and can evaluate the benefit
of those breaking changes. My main concern here is that we will break
programs in Go 1.5 for the sake of name space adjustments and then
while trying to fix those other bugs we'll break programs in Go 1.6 too.
Let's wait until we know all the changes we want to make before we
decide whether or how to break existing programs.

This CL reverts:

5ae822b encoding/xml: minor changes
bb7e665 encoding/xml: fix xmlns= behavior
9f9d66d encoding/xml: fix default namespace of tags
b69ea01 encoding/xml: fix namespaces in a>b tags
3be158d encoding/xml: encoding name spaces correctly

and adjusts tests from

a9dddb5 encoding/xml: add more EncodeToken tests.

to expect Go 1.4 behavior.

I have confirmed that the name space parts of the test suite
as of this CL passes against the Go 1.4 encoding/xml package,
indicating that this CL successfully restores the Go 1.4 behavior.

(Other tests do not, but that's because there were some real
bug fixes in this cycle that are being kept. Specifically, the
tests that don't pass in Go 1.4 are TestMarshal's NestedAndComment
case, TestEncodeToken's encoding of newlines, and
TestSimpleUseOfEncodeToken returning an error for invalid
token types.)

I also checked that the Go 1.4 tests pass when run against
this copy of the sources.

Fixes #11841.

Change-Id: I97de06761038b40388ef6e3a55547ff43edee7cb
Reviewed-on: https://go-review.googlesource.com/12570
Reviewed-by: Nigel Tao <nigeltao@golang.org>

@rsc rsc modified the milestones: Go1.7, Go1.6 Nov 25, 2015

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016

@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 26, 2016

@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.9Early May 3, 2017

@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017

@snurmine

This comment has been minimized.

Copy link

commented Mar 9, 2018

Can I help with issue ?

@gopherbot

This comment has been minimized.

Copy link

commented Apr 23, 2018

Change https://golang.org/cl/108796 mentions this issue: encoding/xml fix overriding by empty namespace

@iWdGo

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2018

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="") and a related fix has been submitted (#8068) .

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.

A specific test is added which derives from the submitted example.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 27, 2018

Change https://golang.org/cl/109855 mentions this issue: encoding/xml : Fixes to enforce XML namespace standard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.