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: encoding a start token with a namespace set in Name and Attr results in invalid XML #42807

Open
SamWhited opened this issue Nov 24, 2020 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@SamWhited
Copy link
Member

What version of Go are you using (go version)?

$ go version
go version devel +6965b01ea248 20201124070706 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

https://play.golang.org/p/ZK8yhFoVwW9

	e := xml.NewEncoder(os.Stdout)
	e.EncodeToken(xml.StartElement{
		Name: xml.Name{Space: "space", Local: "local"},
		Attr: []xml.Attr{{Name: xml.Name{Local: "xmlns"}, Value: "spaceAttr"}},
	})
	e.Flush()
	// <local xmlns="space" xmlns="spaceAttr">

What did you expect to see?

One "xmlns" attribute.

What did you see instead?

Two "xmlns" attributes.

While the provided example is obviously very contrived this can happen in the real world (without a user doing something silly like manually adding a second xmlns attribute) in the situation where the user decodes some XML, makes modifications to the tokens, then encodes those tokens somewhere else (probably a fairly common use case). The decoder will add the "xmlns" attribute and fill out Name, then the encoder will duplicate it. The user would have to manually remove xmlns before encoding, or use RawToken (but this is not an option if the decoder is an arbitrary xml.TokenReader so this isn't always possible). Skipping xmlns attributes on marshal if start.Name is set is a simple patch that would fix this issue.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/272806 mentions this issue: encoding/xml: prevent multiple XMLNS attributes

MelliumBot pushed a commit to mellium/xmpp that referenced this issue Nov 25, 2020
Encoding the output of TokenReader can create multiple xmlns attributes
due to a bug in encoding/xml.
This patch uses RawToken in the decode stage to work around this and can
be reverted after the upstream issue is fixed and in all supported
versions of Go.

Upstream issue: https://golang.org/issue/42807 (golang/go#42807)
Upstream CL: https://golang.org/cl/272806 (golang/go#42808)

Fixes #74

Signed-off-by: Sam Whited <[email protected]>
@cagedmantis cagedmantis added this to the Backlog milestone Dec 2, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 2, 2020
@cagedmantis
Copy link
Contributor

/cc @rsc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants