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

<core-style> should use an element that parses in plain text mode #637

Closed
esprehn opened this issue Jul 16, 2014 · 7 comments
Closed

<core-style> should use an element that parses in plain text mode #637

esprehn opened this issue Jul 16, 2014 · 7 comments

Comments

@esprehn
Copy link

esprehn commented Jul 16, 2014

ex.

<style is="core-style" type="polymer/style">
    .span::before { content: "<script>alert(1)</script>"; }
</style>

Using today to do that will alert since it just has random HTML inside it which is both slower for the parser, and will interpret stuff inside it as tags. Instead it should work as above.

@esprehn esprehn changed the title <core-style> should use a nested element that parses in plain text mode <core-style> should use an element that parses in plain text mode Jul 16, 2014
@esprehn
Copy link
Author

esprehn commented Jul 16, 2014

An alternative is to use a nested element like:

<core-style>
   <style type="polymer">
   </style>
</core-style>

@sjmiles
Copy link
Contributor

sjmiles commented Jul 16, 2014

Can you explain why the nested element is different from the first example?

@esprehn
Copy link
Author

esprehn commented Jul 16, 2014

It's not really different, depends on what polymer prefers. Doing is@ syntax also means that when using the style the custom element can just change itself directly and not appendChild a sibling.

ex.

<style is="core-style" ref="..."></style>

@sjmiles
Copy link
Contributor

sjmiles commented Jul 16, 2014

So, both of your examples are 'the good way'? Reading your first post, it sounded like you were saying if we did it as in the example you provided, there would be an alert. Sorry if I'm being dense.

I understand the notion of using a special type to avoid the parser becoming confused. It seemed like you were pointing to a derivative problem, but I likely read too much in.

@esprehn
Copy link
Author

esprehn commented Jul 16, 2014

Oh sorry, I wrote this bug in the most confusing way possible. Currently does this:

<core-style>
    .span::before { content: "<script>alert(1)</script>"; }
</core-style>

which will alert, since the content of the core-style is parsed as tags.

So polymer should either use <style is="core-style" type="polymer"> (or <script>) or use a nested <style> inside the <core-style>.

It's also bad to use the text content since the parser is not in the fast text mode for large style blocks.

@sorvell
Copy link
Contributor

sorvell commented Jul 16, 2014

Thanks for highlighting this Elliot.

We actually may want to allow elements inside core-style because it let's nest <template>'s. In that case, I guess we need to wrap the content in a template or extend template.

@tjsavage
Copy link
Contributor

auto-moving this issue to googlearchive/core-style#5 and closing this one

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

No branches or pull requests

4 participants