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

handle internal stylesheets #36

Merged
merged 3 commits into from
Oct 25, 2015
Merged

handle internal stylesheets #36

merged 3 commits into from
Oct 25, 2015

Conversation

sicks
Copy link
Contributor

@sicks sicks commented Sep 17, 2015

Fixes #35

@sicks
Copy link
Contributor Author

sicks commented Sep 17, 2015

Actually, style tags in the body are still broken if they contain quotes. Digging deeper!

@sicks
Copy link
Contributor Author

sicks commented Sep 17, 2015

Not sure what's going on here. The test passes, but as seen in this fiddle or when you test this on a real page, the quotes get turned into entities.

@ssorallen
Copy link
Contributor

Could you use dangerouslySetInnerHTML={{__html: JSON.stringify(text)}} instead of writing to its innerText? Here's a fiddle with the change: https://jsfiddle.net/ucgLqtbu/1/

This opens a security hole though if you ever let user-created content be written inside a <style> tag.

@sicks
Copy link
Contributor Author

sicks commented Sep 28, 2015

That works, but is what I had before considered safe? I would think one of the more common use cases of an internal stylesheet would be for custom end-user styles.

@Daniel15
Copy link
Member

This opens a security hole though if you ever let user-created content be written inside a <style> tag.

Do you have an example, or is it just a theoretical hole? :)

@sicks
Copy link
Contributor Author

sicks commented Oct 25, 2015

Do you have an example, or is it just a theoretical hole? :)

That's in reference to react's docs on dangerouslySetInnerHTML but I just did a quick test and <script> tags nested inside <style> tags don't get run, so I don't think it's a danger here.

Daniel15 added a commit that referenced this pull request Oct 25, 2015
handle internal stylesheets
@Daniel15 Daniel15 merged commit 0adf9a4 into reactjs:master Oct 25, 2015
@ssorallen
Copy link
Contributor

Do you have an example, or is it just a theoretical hole? :)

It's a theoretical hole that I have not proven to be real yet. Since @sickslives tested <script> tags inside <style> tags, it at least seems not blatantly bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants