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

CSS parsing should not use regexes. #91

Closed
flavorjones opened this issue Aug 17, 2015 · 13 comments
Closed

CSS parsing should not use regexes. #91

flavorjones opened this issue Aug 17, 2015 · 13 comments

Comments

@flavorjones
Copy link
Owner

Related to #90, regexes are the wrong thing to be using for CSS property parsing, and are present only as part of this library's html5lib heritage.

Find an alternative and reimplement Loofah::HTML5::Scrub.scrub_css.

Crass has been suggested. Are there other alternatives we should evaluate?

@flavorjones
Copy link
Owner Author

I've released v2.1.0.rc1 with CSS parsing and sanitization re-implemented on top of Crass.

/cc @rgrove for a heads-up that this may be happening. Crass will become an implicit dependency of Rails if this change makes it into an actual Loofah release.

Please provide feedback on the Crass implementation on this issue.

@rgrove
Copy link

rgrove commented Aug 17, 2015

Nice! This is both scary and exciting. 😳

@flavorjones
Copy link
Owner Author

welcome-to-the-party

@flavorjones
Copy link
Owner Author

Sent an announcement about this release:

@kaspth
Copy link
Contributor

kaspth commented Aug 19, 2015

Just tried this out on Rails Html Sanitizer and it worked great! I'm a little bummed it doesn't keep the single space between key and value ¯_(ツ)_/¯

@flavorjones
Copy link
Owner Author

@kaspth Great! Thanks for the feedback, and sorry about the whitespace. ;)

If I don't hear of any blockers by 28 August 2015, I'll release 2.1.0 final based on this implementation.

@kaspth
Copy link
Contributor

kaspth commented Aug 20, 2015

Sounds good 👍

@flavorjones
Copy link
Owner Author

No objections having been received, and no blockers raised, I'm going to ship it. :shipit:

@flavorjones
Copy link
Owner Author

Just a note that I'd like to fix the failing tests on JRuby before shipping. Looks like it's unrelated to the CSS changes.

@olivierlacan
Copy link
Contributor

Looks like this should be closed.

@flavorjones
Copy link
Owner Author

@olivierlacan Not yet. Haven't made a final release with this code in it yet. Still waiting on bugfixes to JRuby Nokogiri in order to prevent JRuby failures with Loofah.

This is in v2.1.0.rc2 if anyone wants to play with it.

@flavorjones
Copy link
Owner Author

v2.1.0 has been released using crass as its css parser. w00t. Thanks, all. /cc @rgrove

@kaspth
Copy link
Contributor

kaspth commented Sep 24, 2017

💪❤️

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