Make SAC Parser replaceable

src/main/java/org/owasp/validator/css/CssScanner.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007-2022, Arshan Dabirsiaghi, Jason Li
+ * Copyright (c) 2007-2026, Arshan Dabirsiaghi, Jason Li
  *
  * All rights reserved.
  *
@@ -40,8 +40,6 @@
 import java.util.ResourceBundle;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import org.apache.batik.css.parser.ParseException;
-import org.apache.batik.css.parser.Parser;
 import org.apache.hc.client5.http.ClientProtocolException;
 import org.apache.hc.client5.http.classic.HttpClient;
 import org.apache.hc.client5.http.classic.methods.HttpGet;
@@ -61,7 +59,9 @@
 import org.owasp.validator.html.util.HTMLEntityEncoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.w3c.css.sac.CSSException;
 import org.w3c.css.sac.InputSource;
+import org.w3c.css.sac.Parser;
 
 /**
  * Encapsulates the parsing and validation of a CSS stylesheet or inline declaration. To make use of
@@ -81,7 +81,7 @@ public class CssScanner {
   private static final String CDATA = "^\\s*<!\\[CDATA\\[(.*)\\]\\]>\\s*$";
 
   /** The parser to be used in any scanning */
-  private final Parser parser = new CssParser();
+  private final Parser parser = ParserFactory.makeParser();
 
   /** The policy file to be used in any scanning */
   private final InternalPolicy policy;
@@ -168,10 +168,7 @@ public CleanResults scanStyleSheet(String taintedCss, int sizeLimit) throws Scan
       // should already have been counted by the caller since it was
       // embedded in the HTML
       parser.parseStyleSheet(new InputSource(new StringReader(taintedCss)));
-    } catch (IOException | ParseException e) {
-      /*
-       * ParseException, from batik, is unfortunately a RuntimeException.
-       */
+    } catch (CSSException | IOException | RuntimeException e) {
       throw new ScanException(e);
     }
 
@@ -211,7 +208,7 @@ public CleanResults scanInlineStyle(String taintedCss, String tagName, int sizeL
       // note this does not count against the size limit because it
       // should already have been counted by the caller since it was
       // embedded in the HTML
-      parser.parseStyleDeclaration(taintedCss);
+      parseStyleDeclaration(parser, taintedCss);
     } catch (IOException ioe) {
       throw new ScanException(ioe);
     }
@@ -221,6 +218,12 @@ public CleanResults scanInlineStyle(String taintedCss, String tagName, int sizeL
     return new CleanResults(startOfScan, cleaned, null, errorMessages);
   }
 
+  private static void parseStyleDeclaration(final Parser parser, final String cssDeclaration)
+      throws CSSException, IOException {
+    InputSource source = new InputSource(new StringReader(cssDeclaration));
+    parser.parseStyleDeclaration(source);
+  }
+
   private String getCleanStylesheetWithImports(
       int sizeLimit, List<String> errorMessages, CssHandler handler) throws ScanException {
     String cleaned = handler.getCleanStylesheet();
@@ -313,7 +316,7 @@ public String handleResponse(final ClassicHttpResponse response) throws IOExcept
                   final HttpEntity entity = response.getEntity();
                   try {
                     return entity != null ? EntityUtils.toString(entity) : null;
-                  } catch (final ParseException | org.apache.hc.core5.http.ParseException ex) {
+                  } catch (final org.apache.hc.core5.http.ParseException ex) {
                     throw new ClientProtocolException(ex);
                   }
                 } else {

src/main/java/org/owasp/validator/css/ParserFactory.java

@@ -0,0 +1,55 @@
+package org.owasp.validator.css;
+
+import java.lang.reflect.InvocationTargetException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.css.sac.Parser;
+
+abstract class ParserFactory {
+  private static final Logger LOGGER = LoggerFactory.getLogger(ParserFactory.class);
+
+  /**
+   * Defines a system property which can be used to instantiate a different parser.
+   * The value should be the fully qualified class name of a class that implements
+   * {@link org.w3c.css.sac.Parser}. If not set or invalid, the default Batik
+   * parser will be used.
+   */
+  private static final String SAC_PARSER_SYSTEM_PROPERTY_NAME = "org.w3c.css.sac.parser";
+
+  private ParserFactory() {
+    // prevent instantiation
+  }
+
+  static Parser makeParser() {
+    final String sacParserClassName = System.getProperty(SAC_PARSER_SYSTEM_PROPERTY_NAME);
+    Parser result = null;
+    try {
+      if (sacParserClassName == null || sacParserClassName.trim().isEmpty()) {
+        result = new org.owasp.validator.css.batik.CssParser();
+        LOGGER.debug("No system property \"{}\" found. AntiSamy will use default \"{}\" CSS Parser.", 
+          SAC_PARSER_SYSTEM_PROPERTY_NAME,
+          result.getClass().getCanonicalName());
+      } else {
+        LOGGER.debug("Found system property \"{}\", trying to use it as CSS Parser in AntiSamy");
+        result = (Parser) Class.forName(sacParserClassName)
+          .getDeclaredConstructor().newInstance();
+        LOGGER.warn("AntiSamy will be run with the unsupported \"{}\" CSS Parser. Using an unsupported parser is at your own risk.", 
+          result.getClass().getCanonicalName());  
+      }
+    } catch (ClassNotFoundException
+        | NoSuchMethodException
+        | InvocationTargetException
+        | IllegalAccessException
+        | InstantiationException
+        | NullPointerException
+        | ClassCastException e) {
+      result = new org.owasp.validator.css.batik.CssParser();
+      LOGGER.error("Failed to instantiate \"{}\" as CSS Parser. AntiSamy will fall back to use default \"{}\" CSS Parser.",
+        sacParserClassName,
+        result.getClass().getCanonicalName(),
+        e
+      );
+    }
+    return result;
+  }
+}

src/main/java/org/owasp/validator/css/CssParser.java → src/main/java/org/owasp/validator/css/batik/CssParser.java

@@ -26,23 +26,97 @@
  * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-package org.owasp.validator.css;
+package org.owasp.validator.css.batik;
 
 import static org.owasp.validator.css.media.CssMediaQueryLogicalOperator.AND;
 import static org.owasp.validator.css.media.CssMediaQueryLogicalOperator.OR;
 
+import java.io.IOException;
 import java.util.Objects;
 import org.apache.batik.css.parser.CSSSACMediaList;
 import org.apache.batik.css.parser.LexicalUnits;
+import org.apache.batik.css.parser.ParseException;
+import org.apache.batik.css.parser.Parser;
 import org.owasp.validator.css.media.CssMediaFeature;
 import org.owasp.validator.css.media.CssMediaQuery;
 import org.owasp.validator.css.media.CssMediaQueryList;
 import org.owasp.validator.css.media.CssMediaQueryLogicalOperator;
 import org.w3c.css.sac.CSSException;
 import org.w3c.css.sac.CSSParseException;
+import org.w3c.css.sac.InputSource;
 import org.w3c.css.sac.LexicalUnit;
+import org.w3c.css.sac.SelectorList;
 
-public class CssParser extends org.apache.batik.css.parser.Parser {
+public class CssParser extends Parser {
+
+  /**
+   * Overwritten in order to keep all Batik References in one place.
+   *
+   * @param InputSource the input source containing the Style Sheet to parse. Not null.
+   */
+  @Override
+  public void parseStyleSheet(InputSource source) throws CSSException, IOException {
+    Objects.requireNonNull(source, "Source must not to be null.");
+    try {
+      super.parseStyleSheet(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public void parseStyleSheet(String uri) throws CSSException, IOException {
+    try {
+      super.parseStyleSheet(uri);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public void parseStyleDeclaration(InputSource source) throws CSSException, IOException {
+    try {
+      super.parseStyleDeclaration(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public void parseRule(InputSource source) throws CSSException, IOException {
+    try {
+      super.parseRule(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public SelectorList parseSelectors(InputSource source) throws CSSException, IOException {
+    try {
+      return super.parseSelectors(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public LexicalUnit parsePropertyValue(InputSource source) throws CSSException, IOException {
+    try {
+      return super.parsePropertyValue(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
+
+  @Override
+  public boolean parsePriority(InputSource source) throws CSSException, IOException {
+    try {
+      return super.parsePriority(source);
+    } catch (ParseException parserException) {
+      throw new CSSException(parserException);
+    }
+  }
 
   /**
    * This implementation is a workaround to solve leading dash errors on property names.
@@ -120,7 +194,8 @@ protected CSSSACMediaList parseMediaList() {
   }
 
   private boolean hasAnotherMediaQuery() {
-    return current == LexicalUnits.COMMA || (current == LexicalUnits.IDENTIFIER && scanner.getStringValue().equals(OR.toString()));
+    return current == LexicalUnits.COMMA
+        || (current == LexicalUnits.IDENTIFIER && scanner.getStringValue().equals(OR.toString()));
   }
 
   protected CssMediaQuery parseMediaQuery() {
@@ -165,7 +240,8 @@ protected CssMediaQuery parseMediaQuery() {
       query.addMediaFeature(parseMediaFeature());
     }
 
-    while (current == LexicalUnits.IDENTIFIER && CssMediaQueryLogicalOperator.parse(scanner.getStringValue()) == AND) {
+    while (current == LexicalUnits.IDENTIFIER
+        && CssMediaQueryLogicalOperator.parse(scanner.getStringValue()) == AND) {
       nextIgnoreSpaces();
       query.addMediaFeature(parseMediaFeature());
     }

src/test/java/org/owasp/validator/css/ParserFactoryTest.java

@@ -0,0 +1,123 @@
+package org.owasp.validator.css;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+
+import java.io.IOException;
+import java.util.Locale;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.w3c.css.sac.CSSException;
+import org.w3c.css.sac.ConditionFactory;
+import org.w3c.css.sac.DocumentHandler;
+import org.w3c.css.sac.ErrorHandler;
+import org.w3c.css.sac.InputSource;
+import org.w3c.css.sac.LexicalUnit;
+import org.w3c.css.sac.Parser;
+import org.w3c.css.sac.SelectorFactory;
+import org.w3c.css.sac.SelectorList;
+
+public class ParserFactoryTest {
+
+    private static final String PROP_NAME = "org.w3c.css.sac.parser";
+
+    @Before
+    @After
+    public void clearSystemProperty() {
+        System.clearProperty(PROP_NAME);
+    }
+
+    @Test
+    public void whenNoSystemProperty_thenDefaultParserIsReturned() {
+        Parser parser = ParserFactory.makeParser();
+        assertThat(parser, instanceOf(org.owasp.validator.css.batik.CssParser.class));
+    }
+
+    @Test
+    public void whenValidCustomParserClass_thenThatParserIsReturned() {
+        System.setProperty(PROP_NAME, DummyParser.class.getName());
+
+        Parser parser = ParserFactory.makeParser();
+        assertThat(parser, instanceOf(DummyParser.class));
+    }
+
+    @Test
+    public void whenInvalidClassName_thenDefaultParserIsReturned() {
+        System.setProperty(PROP_NAME, "non.existent.ClassName");
+
+        Parser parser = ParserFactory.makeParser();
+        assertThat(parser, instanceOf(org.owasp.validator.css.batik.CssParser.class));
+    }
+
+    @Test
+    public void whenClassDoesNotImplementParser_thenDefaultParserIsReturned() {
+        System.setProperty(PROP_NAME, NotAParser.class.getName());
+
+        Parser parser = ParserFactory.makeParser();
+        assertThat(parser, instanceOf(org.owasp.validator.css.batik.CssParser.class));
+    }
+
+    public static class DummyParser implements Parser {
+        @Override
+        public void parseStyleSheet(InputSource source) {
+        }
+
+        @Override
+        public void parseStyleDeclaration(InputSource source) {
+        }
+
+        @Override
+        public void parseRule(InputSource source) {
+        }
+
+        @Override
+        public SelectorList parseSelectors(InputSource source) {
+            throw new UnsupportedOperationException("Unimplemented method 'parseSelectors'");
+        }
+
+        @Override
+        public LexicalUnit parsePropertyValue(InputSource source) {
+            throw new UnsupportedOperationException("Unimplemented method 'parsePropertyValue'");
+        }
+
+        @Override
+        public boolean parsePriority(InputSource source) {
+            throw new UnsupportedOperationException("Unimplemented method 'parsePriority'");
+        }
+
+        @Override
+        public void setLocale(Locale locale) {
+        }
+
+        @Override
+        public void setErrorHandler(ErrorHandler handler) {
+        }
+
+        @Override
+        public void setSelectorFactory(SelectorFactory factory) {
+        }
+
+        @Override
+        public void setConditionFactory(ConditionFactory factory) {
+        }
+
+        @Override
+        public void setDocumentHandler(DocumentHandler handler) {
+        }
+
+        @Override
+        public String getParserVersion() {
+            return "dummy-parser 0.0.0";
+        }
+
+        @Override
+        public void parseStyleSheet(String uri) throws CSSException, IOException {
+            throw new UnsupportedOperationException("Unimplemented method 'parseStyleSheet'");
+        }
+    }
+
+    public static class NotAParser {
+    }
+}