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.

Sign up for GitHub

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

Jump to bottom

x/net/html: unescape doesn't handle double quotes in attributes #60864

Closed
viralgarg7 opened this issue Jun 18, 2023 · 8 comments
Closed

x/net/html: unescape doesn't handle double quotes in attributes #60864

viralgarg7 opened this issue Jun 18, 2023 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@viralgarg7
Copy link

viralgarg7 commented Jun 18, 2023 •
edited

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

$ go version = 1.20

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

MACOS M1 chip

Issue-

<div style='font-family:arial, "helvetica neue", helvetica, sans-serif;'>Mehr Informationen</div>

while tokenizing the above code with net/html package, and converting the token html to string using
tokenizer.Token().String()
we get - <div style="font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif;">Mehr Informationen</div>
which on unescaping is - <div style="font-family:arial, "helvetica neue", helvetica, sans-serif;">Mehr Informationen</div>

which is incorrect. As its containing nested double quotes.

This is causing an issue while previewing the HTML on the Outlook app.

link to code- "golang.org/x/net/html"

What did you expect to see?

<div style='font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif;'>Mehr Informationen</div>

What did you see instead?

<div style="font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif;">Mehr Informationen</div>
@viralgarg7
Copy link
Author

@Nasfame shouldn't the expected output be -
<div style='font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif;'>Mehr Informationen</div> ?

as our input was - <div style='font-family:arial, "helvetica neue", helvetica, sans-serif;'>Mehr Informationen</div>

@seankhliao seankhliao changed the title affected/package: net/http x/net/html: unescape doesn't handle double quotes in attributes Jun 19, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jun 19, 2023
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 19, 2023
@seankhliao
Copy link
Member

see #52911 (comment)
I believe this is an incorrect use of UnescapeString, the Token.String output is already correctly escaped.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2023
@crisman
Copy link
Contributor

crisman commented Jun 19, 2023

What did you see instead?

<div style="font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif;">Mehr Informationen</div>

I think that is normal legal encoding and is correct.

In general, unescaping html and expecting valid html out the other side is not correct.

@crisman
Copy link
Contributor

crisman commented Jun 20, 2023

@Nasfame Once the HTML in message has been tokenized there is no concept of what attribute value syntax was used.

https://html.spec.whatwg.org/multipage/syntax.html#attributes-2

At the logical level a tag's attribute has only two strings for state (name and value), nothing about what quoting was used. Any of the following lines will result in the same token object:

<div style=font-family:arial>
<div style='font-family:arial'>
<div style="font-family:arial">

Notice in the Serializing HTML fragments section:

https://html.spec.whatwg.org/multipage/parsing.html#serialising-html-fragments

Under 13.3 4. 2. says:

For each attribute that the element has, append a U+0020 SPACE character, the attribute's serialized name as described below, a U+003D EQUALS SIGN character (=), a U+0022 QUOTATION MARK character ("), the attribute's value, escaped as described below in attribute mode, and a second U+0022 QUOTATION MARK character (").

note in particular that attributes always serialize with a double quote:

a U+0022 QUOTATION MARK character ("),

34 vs 39

This is also why the String() can not change &#34; to &#39; as those are different characters.

As the attribute value of font-family:arial, "helvetica neue", helvetica, sans-serif; output as font-family:arial, &#34;helvetica neue&#34;, helvetica, sans-serif; is valid as that is normal encoding, but changing that to font-family:arial, &#39;helvetica neue&#39;, helvetica, sans-serif; while fine at the CSS level (it means the same thing) is not a valid HTML parse and serialize as those are two different strings for the attribute value.

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

No branches or pull requests
5 participants
@crisman @gopherbot @seankhliao @viralgarg7 and others