Escaped ampersands in query params within src/href attributes

[pgoldrbx]

Ampersands in query params within src/href attributes are being escaped

Currently, all attributes are sanitized for the browser using escapeTextContentForBrowser. However, in the case of src or href attributes, this will escape & within query parameters.

Example

ReactDOMServer.renderToStaticMarkup(<a href="http://foo.com?a=1&b=2" />);
// => <a href="http://foo.com?a=1&amp;b=2"></a>

The attribute name is available until calling quoteAttributeValueForBrowser. There would need to a be a degree of refactor to create conditional behavior, with consideration given to attributes that might contain urls.

Possible attributes to alter

• src
• href
• data- variations of the above

I don't know if this is a bug or if it exists by design, for non-obvious reasons.

Versions 0.14.x, 15.x

[syranide]

Not a bug, this is all according to spec. Not escaping would be a bug; consider if the URL you provide is http://foobar/foo&bar/, the browser would then translate that to http://foobar/foo&bar/ unless it wasn't first escaped, that is not what you intended.

Why are you bringing this up, are you seeing any problems?

[poscar]

Ah, I see. So whoever is in the receiving end of the markup (e.g. browser, but could be something else) should be sticking to the spec and parsing escaped ampersands correctly.

[pgoldrbx]

@syranide Correct, I would not expect ampersands within a uri to be unescaped. However it's the query string that I'm concerned with. e.g.

http://foobar/foo&bar/?foo=1&bar=2

[syranide]

@pgoldrbx It should still be escaped, render and click it and you'll see.

[pgoldrbx]

@syranide — confirmed. will close.

• anchor => renders escaped => clicking through behaves as desired
• iframe => passes params to the server => both hapi and express receive params as expected

thanks for your feedback.

[alexmarchant]

I actually have an issue here... using react on wordpress and my logout link is being rendered as such:

<a href="http://localhost:8888/wp-login.php?action=logout&amp;amp;_wpnonce=fdc432e270&amp;amp;redirect_to=http%3A%2F%2Flocalhost%3A8888">Logout</a>

Wordpress is not parsing this correctly and is not picking up the _wpnonce and redirect_to params.

[pgoldrbx]

@alexmarchant — this appears doubly escaped — is the url escaped already prior to rendering it with react?

[alexmarchant]

@pgoldrbx oh ya you're right, didn't even notice that until pasting it here, it actually shows normally in chrome's url bar:

Weird. Looking more into it.

[alexmarchant]

@pgoldrbx Thought maybe it was ReactDOMServer.renderToString, but just threw together a jsfiddle, https://jsfiddle.net/alex_marchant/pyadp4eg/4/, and it looks like it's working fine. Must be something on my end server side. Thanks for the helpful comment.

[depoulo]

Unfortunately, Facebook's in-app browser, or whatever sends x-fb-http-engine: Liger and has

user agents like this

Mozilla/5.0 (iPhone; CPU iPhone OS 11_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E216 [FBAN/FBIOS;FBAV/174.0.0.48.98;FBBV/110921384;FBDV/iPhone9,3;FBMD/iPhone;FBSN/iOS;FBSV/11.3;FBSS/2;FBCR/TelekomHU;FBID/phone;FBLC/hu_HU;FBOP/5;FBRV/112241032]

doesn't seem to stick to the spec, giving us thousands of access log entries per day with non-unescaped query parameters in image URLs (ironically, the escaping is done by React on our servers). In our case it's CSS background-images in inline styles.