React: Unable to render backgroundImage with single quote in URL.

Created on 8 May 2016  路  8Comments  路  Source: facebook/react

If there is a single quote in the background image url, React will not even flush the changes to the DOM. I got around this by doing...

string = string.replace(/'/g, "%27");

The same url works fine (without the manipulation) if set as the src of an img. Not exactly sure why React is rejecting even putting this into the DOM, and if there is some sort of inconsistency with how the browser handles single quotes in a background image could react handle that simple string manipulation itself?

Most helpful comment

The bigger issue is XSS if you ask me, I assume lack of encoding is the most likely reason people would bump into this and it's unlikely that any such warning would ever help you there before it's too late.

That seems like even more reason to warn. Increase the chances that someone sees the problem and digs into it.

cherry-picking some easy to detect cases and not being able to test all the others seems like a can of worms.

Idk, I have a more pragmatic view. Create helpful warnings whenever we can, and don't get hung up on the fact that we won't/can't catch every possible conceivable bug.

All 8 comments

React is not rejecting anything, the browser is rejecting invalid CSS. Single and double quotes are special characters in CSS and you cannot use them as part of a value without encoding them one way or another.

@actionnick Perhaps we could warn for such situations. Can you give the exact style object you're setting?

@jimfb That would require parsing CSS though and that's not something we want to do?

@syranide Not something we'd want to do on the production code path, but it might be worth doing in dev mode if it allows us to give good warnings for style failures that might otherwise be difficult to debug.

I'm also a little curious to see exactly what went wrong, to better understand what was going through the user's mind and why it didn't work as he expected.

@jimfb url(foo'bar.jpg).

Not something we'd want to do on the production code path, but it might be worth doing in dev mode if it allows us to give good warnings for style failures that might otherwise be difficult to debug.

The bigger issue is XSS if you ask me, I assume lack of encoding is the most likely reason people would bump into this and it's unlikely that any such warning would ever help you there before it's too late. So really, IMHO this is about teaching/lack of understanding and something you need to be proactive about for it to really have an impact.

Not saying it isn't worth doing, but for it to make sense to me we should validate all CSS. One way or another. That would be beneficial (it not a perf issue), but cherry-picking some easy to detect cases and not being able to test all the others seems like a can of worms.

The bigger issue is XSS if you ask me, I assume lack of encoding is the most likely reason people would bump into this and it's unlikely that any such warning would ever help you there before it's too late.

That seems like even more reason to warn. Increase the chances that someone sees the problem and digs into it.

cherry-picking some easy to detect cases and not being able to test all the others seems like a can of worms.

Idk, I have a more pragmatic view. Create helpful warnings whenever we can, and don't get hung up on the fact that we won't/can't catch every possible conceivable bug.

I believe this deserves a mention here: https://facebook.github.io/react/docs/tags-and-attributes.html

I have accidentally discovered this issue several months into using React, but was it mentioned in the documentation I would've been aware of it and avoided it.

Was this page helpful?
0 / 5 - 0 ratings