search

OWASP / java-html-sanitizer

Takes third-party HTML and produces HTML that is safe to embed in your web application. Fast and easy to configure.
Other
843 stars 213 forks source link

Do not ignore attributes allowed globally together with 'style' (#237) #238

Open corebonts opened 2 years ago

corebonts commented 2 years ago

Also, allowStyling() internally allows the 'style' attribute, so it is not necessary to ignore it.

mikesamuel commented 2 years ago

Thanks for adding a testcase. The check for style as the zero-th element seems good to change, but what prompted this?

Instead of using null for the policy, and checking == null, can we check for the identity policy? My vague recollection was that .join was pretty good about just returning x when joining x with the identity policy.

corebonts commented 2 years ago

First, thanks for reviewing it. But sorry, it's not clear, what prompted what? The change that calls allowStyling() when "style" property is allowed or the change that uses now the contains check instead of the zero-th element check?

For the first, I don't know, it's someone else's change and I don't know the reason behind it. For me it also feels a bit magical. For the latter one, we had failing tests in our product when we updated to the latest sanitizer.

And for your comment about the nullcheck, you're right, I will change that.