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

fix: handle CSS functions in a CSS shorthand property #200

Merged
merged 2 commits into from
Jan 14, 2021

Conversation

flavorjones
Copy link
Owner

See #199 for context. The following works as expected:

Loofah::fragment(%Q[<h1 style="background-color: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>]).scrub!(:prune)
# => <h1 style="background-color: linear-gradient(transparent 50%, #ffff66 50%);">here</h1>

Loofah::fragment(%Q[<h1 style="color: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>]).scrub!(:prune)
# => <h1 style="color: linear-gradient(transparent 50%, #ffff66 50%);">here</h1>

but this does not:

Loofah::fragment(%Q[<h1 style="background: linear-gradient(transparent 50%, #ffff66 50%); ">here</h1>]).scrub!(:prune)
# => <h1 style="background:#ffff66;">here</h1>

This fix simplifies how we handle CSS properties and their values. I'm still not totally comfortable with the fact that we check each value of a CSS shorthand property differently than the longer form, but that's for another time.

@flavorjones flavorjones merged commit 8ed486d into master Jan 14, 2021
@flavorjones flavorjones deleted the 199-css-functions-not-handled-properly branch January 14, 2021 21:29
jacobherrington added a commit to jacobherrington/rails-html-sanitizer that referenced this pull request Apr 7, 2021
One of the changes in released in Loofah 2.9.0 broke this test. I can't
tell if this was an intended or unintended side effect of the change to
support functions in shorthand CSS rule.

I'm not sure how well documented this change was, but Loofah's tests
were updated to reflect a different formatting in the commit that causes
this behavior. I think it's safe to say that stripping the whitespace
from rules like the one in this test is the expected behavior.

The test should be updated to reflect the new formatting.

Relevant links from Loofah:
https://github.com/flavorjones/loofah/releases/tag/v2.9.0
flavorjones/loofah#200
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

Successfully merging this pull request may close these issues.

1 participant