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: disallow attributes named xmlns:* #8167

Open
gopherbot opened this issue Jun 7, 2014 · 8 comments

Comments

Projects
None yet
5 participants
@gopherbot
Copy link

commented Jun 7, 2014

by opennota:

The Encoder will add namespaced to the elements, even if there are the same namespaces
in the parent element:
http://play.golang.org/p/LDRJCxUJHX

// Output:
// <p xmlns="http://www.gribuser.ru/xml/fictionbook/2.0";
xmlns:l="http://www.w3.org/1999/xlink";><a
xmlns="http://www.gribuser.ru/xml/fictionbook/2.0";
xmlns:xlink="http://www.w3.org/1999/xlink";
xlink:href="http://google.com";>google.com</a></p>

I expect it to not repeat the namespaces:

// <p xmlns="http://www.gribuser.ru/xml/fictionbook/2.0";
xmlns:l="http://www.w3.org/1999/xlink";><a
l:href="http://google.com";>google.com</a></p>
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2014

Comment 1:

Labels changed: added repo-main, release-go1.4.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Aug 21, 2014

Comment 2 by glen.newton:

Output XML is correct and equivalent to input XML (thus idempotent transformation).
And - unfortunately - larger.
That said, as this issue does not result in incorrect and/or non-equivalent XML, I would
suggest first fixing higher priority bugs that either produce incorrect and/or
non-equivalent XML (or just break), such as #7535, #6800, #8068, #8535, #7113
@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2014

Comment 3:

It's not intended that you can even emit xmlns: attributes that way. The encoder does
not know about them and may step on you (or vice versa). If you want to put the name
space definition in an outer element, the easiest thing to do is to use it in an
attribute on that element: http://play.golang.org/p/2-Y-CrhaP7, which will let the
Encoder define it instead.
The test program in the initial report may be just invalid for trying to define an
attribute named xmlns:l. Perhaps we should disallow all attributes named xmlns:* in a
future version of Go. Not for 1.4 though.

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

Status changed to Accepted.

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014

@bradfitz bradfitz removed the release-go1.5 label Dec 16, 2014

@rsc rsc removed accepted labels Apr 14, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 21, 2015

CL is here 4 days ago: https://go-review.googlesource.com/#/c/12351/ but lacked the magic syntax, so this bug wasn't updated.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Jul 21, 2015

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

@rsc rsc modified the milestones: Go1.6, Go1.5 Jul 23, 2015

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2015

See #11841.

@gopherbot

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

Blocked on #13400.

@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.9Early, Go1.10Early 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

@ianlancetaylor ianlancetaylor removed the Blocked label May 17, 2018

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 May 18, 2018

@gopherbot gopherbot modified the milestones: Go1.12, Unplanned May 23, 2018

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.