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: prevent multiple XMLNS attributes #42808

Closed
wants to merge 1 commit into from
Closed

encoding/xml: prevent multiple XMLNS attributes #42808

wants to merge 1 commit into from

Conversation

SamWhited
Copy link
Member

Having multiple attributes of the same name is a violation of the XML
well-formedness rules. This patch does not fix every possible way this
could happen, but it does prevent marshaling from introducing such an
issue when a start token is encoded that contains both a Name attribute
with a namespace set and an xmlns attribute.

Fixes #42807

Having multiple attributes of the same name is a violation of the XML
well-formedness rules. This patch does not fix every possible way this
could happen, but it does prevent marshaling from introducing such an
issue when a start token is encoded that contains both a Name attribute
with a namespace set and an xmlns attribute.

Fixes #42807
@google-cla google-cla bot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Nov 24, 2020
@@ -2144,7 +2162,7 @@ var encodeTokenTests = []struct {
{Name{"space", "a"}, "value"},
}},
},
want: `<foo xmlns="space" xmlns="space"><foo xmlns="space" xmlns:_xmlns="xmlns" _xmlns:y="space" xmlns:space="space" space:a="value">`,
Copy link
Member Author

@SamWhited SamWhited Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a little concerned that this was intentional behavior when I saw that existing tests had this, but couldn't see why this would be the case so I went ahead and "fixed" them hoping this was just an oversight while copy/pasting some output for these individual tests. If this is not so, please advise.

@gopherbot
Copy link

This PR (HEAD: a9b76c1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/272806 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Sam Whited:

Patch Set 1: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/272806.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=cfb219a5


Please don’t reply on this GitHub thread. Visit golang.org/cl/272806.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 1: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/272806.
After addressing review feedback, remember to publish your drafts!

MelliumBot pushed a commit to mellium/xmpp that referenced this pull request 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 <sam@samwhited.com>
@gopherbot
Copy link

Message from Sam Whited:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/272806.
After addressing review feedback, remember to publish your drafts!

@heschi heschi closed this Dec 15, 2021
@SamWhited
Copy link
Member Author

Hi, could I get an explanation for why this was closed without review? Thank you.

@SamWhited SamWhited reopened this Dec 16, 2021
@heschi
Copy link
Contributor

heschi commented Dec 16, 2021

I closed old PRs to reduce load on the Gerrit importer (#50197), sorry for the trouble. I'll reopen the CL.

@SamWhited SamWhited closed this by deleting the head repository Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

encoding/xml: encoding a start token with a namespace set in Name and Attr results in invalid XML
3 participants