java-html-sanitizer icon indicating copy to clipboard operation
java-html-sanitizer copied to clipboard

Attributes allowed globally together with "style" are lost

Open corebonts opened this issue 4 years ago • 2 comments

Since the commit 020d5d0d7b8e985be32d3608612a9889135ef060 all attributes that are allowed globally are ignored, if "style" is given as the first attribute.

Problematic code:

    public HtmlPolicyBuilder globally() {
      if(attributeNames.get(0).equals("style")) {
        return allowStyling();
      } else {
        return HtmlPolicyBuilder.this.allowAttributesGlobally(
            policy, attributeNames);
      }
    }

Proof

@Test
 public static final void testStyleWithOtherAttributesGlobally() {
   PolicyFactory policyBuilder = new HtmlPolicyBuilder()
           .allowAttributes("style", "align").globally()
           .allowElements("a", "label", "h1", "h2", "h3", "h4", "h5", "h6")
           .toFactory();
   String input = "<h1 style=\"color:green ;name:user ;\" align=\"center\">This is some green text</h1>";
   String want = "<h1 style=\"color:green\" align=\"center\">This is some green text</h1>";
   assertEquals(want, policyBuilder.sanitize(input));
 }

Note that align="center" is missing from the output.

I will file a PR to fix the issue

corebonts avatar Oct 27 '21 16:10 corebonts

Another problem that we're seeing with 020d5d0 is that it assumes the attributeNames list is non-empty. The proposed change in b493617 would help prevent having to write empty list checks prior to calling globally.

sgesin avatar Dec 14 '21 18:12 sgesin

That's true. I got some comments on my pull request but I will try to adjust it today so hopefully these things will be fixed soon.

corebonts avatar Dec 15 '21 08:12 corebonts