!important CSS rule is removed

[mrpeny]

We started to use AntiSamy for CSS validation in our WEB project and realized that it removes !important CSS rules from the styles.

Eg. <p style=\"color: red !important\">Some Text</p> resolves to <p style=\"color: red\">Some Text</p>

The following test added to AntiSamyTest fails.

    @Test
    public void givenImportantRuleWhenScanThenPreserved() throws ScanException, PolicyException {
        String s = as.scan("<p style=\"color: red !important\">Some Text</p>", policy, AntiSamy.DOM).getCleanHTML();
        assertTrue(s.contains("!important"));

        s = as.scan("<p style=\"color: red !important\">Some Text</p>", policy, AntiSamy.SAX).getCleanHTML();
        assertTrue(s.contains("!important"));
    }

I see it from the method parameters of org.owasp.validator.css.CssHandler#property that we are aware of the fact if a property is important or not but it looks like the code ignores this information as the argument is not used anywhere.

...
public void property(String name, LexicalUnit value, boolean important)
			throws CSSException {
		// only bother validating and building if we are either inline or within
		// a selector tag

		if (!selectorOpen && !isInline) {
...

Is there a way to get it working or am I missing something? Let me know if you need further information!

Thank you in advance!

[davewichers]

Thanks for raising this issue. @spassarop - can you confirm the problem (with the provided test case), and see if you can come up with a fix?

[spassarop]

I can confirm. The important boolean is not used and the code that actually rebuilds the CSS is the following:

if (!isInline) { styleSheet.append('\t'); }
styleSheet.append(name);
styleSheet.append(':');

// append all values
while (value != null) {
styleSheet.append(' ');
styleSheet.append(validator.lexicalValueToString(value));
value = value.getNextLexicalUnit();
}
styleSheet.append(';');
if (!isInline) { styleSheet.append('\n'); }

I guess we could just add "!important" before the ';' append with an if. That for sure make the tests pass and I don't see the inconvenient with just adding the text before ending the CSS property.

[davewichers]

OK. Can you add a test case with and without !important, so we can verify this change only includes it when it is specified, and doesn't add it when it is not specified?

[spassarop]

Yes. I’ll do that now.

…

On Tue, 6 Apr 2021 at 19:17 Dave Wichers ***@***.***> wrote: OK. Can you add a test case with and without !important, so we can verify this change only includes it when it is specified, and doesn't add it when it is not specified? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHL3BMORNKSCVDWUWVQD6PDTHOCBBANCNFSM42O5HYCQ> .

[davewichers]

@spassarop - Thanks for jumping on this so quick! @mrpeny - We have implemented a quick fix in the 1.6.3 branch. Can you test the fix out to verify it fixes the problem as you'd expect. I want to make sure that the !important added back in is in exactly the right place and works as you expect.

[mrpeny]

@davewichers I have checked the implementation in the PR and it looks good to me. I have also included 1.6.3-SNAPSHOT into our web project and it works as expected. Thank you for your prompt reply. 👍🏼

When do you plan to release 1.6.3?

[davewichers]

@mrpeny - Thanks for reviewing. I'd like to get the release out tonight. I just have to update some JavaDocs related to the other fix going out with 1.6.3. Then it should be ready to ship.

[mrpeny]

Thank you.

[davewichers]

Fixed in release 1.6.3 that just went out.

[mrpeny]

@davewichers when is it expected to appear on Maven Central?

[davewichers]

It's already there I believe.

[mrpeny]

It's there now. Thank you. Have a nice weekend!