You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2018/07/06 15:52:15 UTC

[sling-org-apache-sling-xss] branch master updated: SLING-7766 - Optimise the way the AntiSamy configuration is read

This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-xss.git


The following commit(s) were added to refs/heads/master by this push:
     new 61dd6a9  SLING-7766 - Optimise the way the AntiSamy configuration is read
61dd6a9 is described below

commit 61dd6a99958c4999bb5ff288f144100fd7c91c4a
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Fri Jul 6 17:52:05 2018 +0200

    SLING-7766 - Optimise the way the AntiSamy configuration is read
    
    * added OSGi configuration to define which policy file will be loaded
---
 pom.xml                                            |   2 +-
 .../org/apache/sling/xss/impl/XSSFilterImpl.java   | 176 ++++++++++-----------
 .../org/apache/sling/xss/impl/XSSAPIImplTest.java  | 141 +++++++++--------
 3 files changed, 156 insertions(+), 163 deletions(-)

diff --git a/pom.xml b/pom.xml
index 07844fb..7145772 100644
--- a/pom.xml
+++ b/pom.xml
@@ -290,7 +290,7 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.testing.sling-mock</artifactId>
-            <version>2.2.14</version>
+            <version>2.2.18</version>
             <scope>test</scope>
         </dependency>
     </dependencies>
diff --git a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
index 49349da..4396dc9 100644
--- a/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
+++ b/src/main/java/org/apache/sling/xss/impl/XSSFilterImpl.java
@@ -22,9 +22,9 @@ import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Dictionary;
+import java.util.Hashtable;
 import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
 
 import javax.annotation.Nonnull;
@@ -41,10 +41,16 @@ import org.apache.sling.api.resource.observation.ResourceChangeListener;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.apache.sling.xss.ProtectionContext;
 import org.apache.sling.xss.XSSFilter;
-import org.osgi.framework.Constants;
+import org.osgi.framework.ServiceRegistration;
+import org.osgi.service.component.ComponentContext;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Modified;
 import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.metatype.annotations.AttributeDefinition;
+import org.osgi.service.metatype.annotations.Designate;
+import org.osgi.service.metatype.annotations.ObjectClassDefinition;
 import org.owasp.validator.html.model.Attribute;
 import org.owasp.validator.html.model.Tag;
 import org.slf4j.Logger;
@@ -55,20 +61,28 @@ import org.slf4j.LoggerFactory;
  * <a href="http://code.google.com/p/owaspantisamy/">http://code.google.com/p/owaspantisamy/</a>.
  */
 @Component(
-        service = {ResourceChangeListener.class, XSSFilter.class},
-        property = {
-                Constants.SERVICE_VENDOR + "=The Apache Software Foundation",
-                ResourceChangeListener.CHANGES + "=ADDED",
-                ResourceChangeListener.CHANGES + "=CHANGED",
-                ResourceChangeListener.CHANGES + "=REMOVED",
-                ResourceChangeListener.PATHS + "=" + XSSFilterImpl.DEFAULT_POLICY_PATH
-        }
+        service = {XSSFilter.class}
 )
-public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, ExternalResourceChangeListener {
+@Designate(ocd = XSSFilterImpl.Configuration.class)
+public class XSSFilterImpl implements XSSFilter {
+
+    @ObjectClassDefinition(
+            name = "Apache Sling XSS Filter",
+            description = "XSS filtering utility based on AntiSamy."
+    )
+    @interface Configuration {
+
+        @AttributeDefinition(
+                name = "AntiSamy Policy Path",
+                description = "The path to the AntiSamy policy file (absolute or relative to the configured search paths)."
+
+        )
+        String policyPath() default XSSFilterImpl.DEFAULT_POLICY_PATH;
+
+    }
 
     private final Logger logger = LoggerFactory.getLogger(XSSFilterImpl.class);
 
-    public static final String GRAPHEME = "(?>\\P{M}\\p{M}*)";
     public static final String ALPHA = "(?:\\p{L}\\p{M}*)";
     public static final String HEX_DIGIT = "\\p{XDigit}";
     public static final String PCT_ENCODED = "%" + HEX_DIGIT + HEX_DIGIT;
@@ -130,17 +144,16 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
 
     static final String DEFAULT_POLICY_PATH = "sling/xss/config.xml";
     private static final String EMBEDDED_POLICY_PATH = "SLING-INF/content/config.xml";
-    private static final int DEFAULT_POLICY_CACHE_SIZE = 128;
-    private PolicyHandler defaultHandler;
+    private PolicyHandler policyHandler;
     private Attribute hrefAttribute;
+    private String policyPath;
+    private boolean policyLoadedFromFile;
+    private ServiceRegistration<ResourceChangeListener> serviceRegistration;
 
     // available contexts
     private final XSSFilterRule htmlHtmlContext = new HtmlToHtmlContentContext();
     private final XSSFilterRule plainHtmlContext = new PlainTextToHtmlContentContext();
 
-    // policies cache
-    private final Map<String, PolicyHandler> policies = new ConcurrentHashMap<>();
-
     @Reference
     private ResourceResolverFactory resourceResolverFactory;
 
@@ -148,18 +161,9 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
     private ServiceUserMapped serviceUserMapped;
 
     @Override
-    public void onChange(@Nonnull List<ResourceChange> resourceChanges) {
-        for (ResourceChange change : resourceChanges) {
-            if (change.getPath().endsWith(DEFAULT_POLICY_PATH)) {
-                logger.info("Detected policy file change ({}) at {}. Updating default handler.", change.getType().name(), change.getPath());
-                updateDefaultHandler();
-            }
-        }
-    }
-
-    @Override
     public boolean check(final ProtectionContext context, final String src) {
-        return this.check(context, src, null);
+        final XSSFilterRule ctx = this.getFilterRule(context);
+        return ctx.check(policyHandler, src);
     }
 
     @Override
@@ -169,7 +173,8 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
 
     @Override
     public String filter(final ProtectionContext context, final String src) {
-        return this.filter(context, src, null);
+        final XSSFilterRule ctx = this.getFilterRule(context);
+        return ctx.filter(policyHandler, src);
     }
 
     @Override
@@ -205,72 +210,41 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
     }
 
     @Activate
-    protected void activate() {
+    @Modified
+    protected void activate(ComponentContext componentContext, Configuration configuration) {
         // load default handler
-        updateDefaultHandler();
-    }
-
-    /*
-        The following methods are not part of the API. Client-code dependency to these methods is risky as they can be removed at any
-        point in time from the implementation.
-     */
-
-    public boolean check(final ProtectionContext context, final String src, final String policy) {
-        final XSSFilterRule ctx = this.getFilterRule(context);
-        PolicyHandler handler = null;
-        if (ctx.supportsPolicy()) {
-            if (policy == null || (handler = policies.get(policy)) == null) {
-                handler = defaultHandler;
-            }
+        policyLoadedFromFile = false;
+        policyPath = configuration.policyPath();
+        updatePolicy();
+        if (serviceRegistration != null) {
+            serviceRegistration.unregister();
         }
-        return ctx.check(handler, src);
-    }
-
-    public String filter(final ProtectionContext context, final String src, final String policy) {
-        if (src == null) {
-            return "";
-        }
-        final XSSFilterRule ctx = this.getFilterRule(context);
-        PolicyHandler handler = null;
-        if (ctx.supportsPolicy()) {
-            if (policy == null || (handler = policies.get(policy)) == null) {
-                handler = defaultHandler;
-            }
+        Dictionary<String, Object> rclProperties = new Hashtable<>();
+        rclProperties.put(ResourceChangeListener.CHANGES, new String[]{"ADDED", "CHANGED", "REMOVED"});
+        rclProperties.put(ResourceChangeListener.PATHS, policyPath);
+        if (policyLoadedFromFile) {
+            serviceRegistration = componentContext.getBundleContext()
+                    .registerService(ResourceChangeListener.class, new PolicyChangeListener(), rclProperties);
+            logger.info("Registered a resource change listener for file {}.", policyPath);
         }
-        return ctx.filter(handler, src);
-    }
-
-    public void setDefaultPolicy(InputStream policyStream) throws Exception {
-        setDefaultHandler(new PolicyHandler(policyStream));
-    }
-
-    public void resetDefaultPolicy() {
-        updateDefaultHandler();
     }
 
-    public void loadPolicy(String policyName, InputStream policyStream) throws Exception {
-        if (policies.size() < DEFAULT_POLICY_CACHE_SIZE) {
-            PolicyHandler policyHandler = new PolicyHandler(policyStream);
-            policies.put(policyName, policyHandler);
+    @Deactivate
+    protected void deactivate() {
+        if (serviceRegistration != null) {
+            serviceRegistration.unregister();;
         }
     }
 
-    public void unloadPolicy(String policyName) {
-        policies.remove(policyName);
-    }
-
-    public boolean hasPolicy(String policyName) {
-        return policies.containsKey(policyName);
-    }
-
-    private synchronized void updateDefaultHandler() {
-        this.defaultHandler = null;
+    private synchronized void updatePolicy() {
+        this.policyHandler = null;
         try (final ResourceResolver xssResourceResolver = resourceResolverFactory.getServiceResourceResolver(null)) {
-            Resource policyResource = xssResourceResolver.getResource(DEFAULT_POLICY_PATH);
+            Resource policyResource = xssResourceResolver.getResource(policyPath);
             if (policyResource != null) {
                 try (InputStream policyStream = policyResource.adaptTo(InputStream.class)) {
-                    setDefaultHandler(new PolicyHandler(policyStream));
-                    logger.info("Installed default policy from {}.", policyResource.getPath());
+                    setPolicyHandler(new PolicyHandler(policyStream));
+                    logger.info("Installed policy from {}.", policyResource.getPath());
+                    policyLoadedFromFile = true;
                 } catch (Exception e) {
                     Throwable[] suppressed = e.getSuppressed();
                     if (suppressed.length > 0) {
@@ -284,13 +258,13 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
         } catch (final LoginException e) {
             logger.error("Unable to load the default policy file.", e);
         }
-        if (defaultHandler == null) {
+        if (policyHandler == null) {
             // the content was not installed but the service is active; let's use the embedded file for the default handler
-            logger.info("Could not find a policy file at the default location {}. Attempting to use the default resource embedded in" +
-                    " the bundle.", DEFAULT_POLICY_PATH);
+            logger.info("Could not find a policy file at the configured location {}. Attempting to use the default resource embedded in" +
+                    " the bundle.", policyPath);
             try (InputStream policyStream = this.getClass().getClassLoader().getResourceAsStream(EMBEDDED_POLICY_PATH)) {
-                setDefaultHandler(new PolicyHandler(policyStream));
-                logger.info("Installed default policy from the embedded {} file from the bundle.", EMBEDDED_POLICY_PATH);
+                setPolicyHandler(new PolicyHandler(policyStream));
+                logger.info("Installed policy from the embedded {} file from the bundle.", EMBEDDED_POLICY_PATH);
             } catch (Exception e) {
                 Throwable[] suppressed = e.getSuppressed();
                 if (suppressed.length > 0) {
@@ -301,8 +275,8 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
                 logger.error("Unable to load policy from embedded policy file.", e);
             }
         }
-        if (defaultHandler == null) {
-            throw new IllegalStateException("Cannot load a default policy handler.");
+        if (policyHandler == null) {
+            throw new IllegalStateException("Cannot load a policy handler.");
         }
     }
 
@@ -320,15 +294,27 @@ public class XSSFilterImpl implements XSSFilter, ResourceChangeListener, Externa
         return this.plainHtmlContext;
     }
 
-    private void setDefaultHandler(PolicyHandler defaultHandler) {
-        Tag linkTag = defaultHandler.getPolicy().getTagByLowercaseName("a");
+    private void setPolicyHandler(PolicyHandler policyHandler) {
+        Tag linkTag = policyHandler.getPolicy().getTagByLowercaseName("a");
         Attribute hrefAttribute = (linkTag != null) ? linkTag.getAttributeByName("href") : null;
         if (hrefAttribute == null) {
             // Fallback to default configuration
             hrefAttribute = DEFAULT_HREF_ATTRIBUTE;
         }
 
-        this.defaultHandler = defaultHandler;
+        this.policyHandler = policyHandler;
         this.hrefAttribute = hrefAttribute;
     }
+
+    private class PolicyChangeListener implements ResourceChangeListener, ExternalResourceChangeListener {
+        @Override
+        public void onChange(@Nonnull List<ResourceChange> resourceChanges) {
+            for (ResourceChange change : resourceChanges) {
+                if (change.getPath().endsWith(policyPath)) {
+                    logger.info("Detected policy file change ({}) at {}. Updating policy handler.", change.getType().name(), change.getPath());
+                    updatePolicy();
+                }
+            }
+        }
+    }
 }
diff --git a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
index 004e839..9eaef54 100644
--- a/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
+++ b/src/test/java/org/apache/sling/xss/impl/XSSAPIImplTest.java
@@ -16,82 +16,60 @@
  ******************************************************************************/
 package org.apache.sling.xss.impl;
 
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.lang.reflect.Field;
+import java.util.HashMap;
 import java.util.regex.Pattern;
 
-import org.apache.commons.lang.StringEscapeUtils;
-import org.apache.commons.lang3.StringUtils;
-import org.apache.sling.api.SlingHttpServletRequest;
-import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.observation.ResourceChangeListener;
+import org.apache.sling.serviceusermapping.ServiceUserMapped;
+import org.apache.sling.testing.mock.sling.junit.SlingContext;
 import org.apache.sling.xss.XSSAPI;
-import org.apache.sling.xss.XSSFilter;
+import org.junit.After;
 import org.junit.Assert;
-import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
-import org.owasp.validator.html.AntiSamy;
-import org.owasp.validator.html.Policy;
+import org.osgi.framework.ServiceReference;
 import org.owasp.validator.html.model.Attribute;
 import org.powermock.reflect.Whitebox;
 
 import junit.framework.TestCase;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 public class XSSAPIImplTest {
 
-    public static final String RUBBISH = "rubbish";
-    public static final String RUBBISH_JSON = "[\"rubbish\"]";
-    public static final String RUBBISH_XML = "<rubbish/>";
+    private static final String RUBBISH = "rubbish";
+    private static final String RUBBISH_JSON = "[\"rubbish\"]";
+    private static final String RUBBISH_XML = "<rubbish/>";
+
+    @Rule
+    public SlingContext context = new SlingContext();
 
     private XSSAPI xssAPI;
 
-    @Before
-    public void setup() {
-        try {
-            XSSFilterImpl xssFilter = new XSSFilterImpl();
-            setDefaultHandler(xssFilter, "./src/main/resources/SLING-INF/content/config.xml");
-
-            xssAPI = new XSSAPIImpl();
-            Whitebox.invokeMethod(xssAPI, "activate");
-            Field filterField = XSSAPIImpl.class.getDeclaredField("xssFilter");
-            filterField.setAccessible(true);
-            filterField.set(xssAPI, xssFilter);
-
-            ResourceResolver mockResolver = mock(ResourceResolver.class);
-            when(mockResolver.map(anyString())).thenAnswer(new Answer() {
-                public Object answer(InvocationOnMock invocation) {
-                    Object[] args = invocation.getArguments();
-                    String url = (String) args[0];
-                    return url.replaceAll("jcr:", "_jcr_");
-                }
-            });
-        } catch (Exception e) {
-            e.printStackTrace();
-        }
+    /**
+     * Due to how OSGi mocks are currently designed, it's impossible to unregister services. Therefore this method has to be explicitly
+     * called by each method that needs the default setup.
+     *
+     * The only exception currently is {@link #testGetValidHrefWithoutHrefConfig()}.
+     */
+    private void setUp() {
+        context.registerService(ServiceUserMapped.class, mock(ServiceUserMapped.class));
+        context.registerInjectActivateService(new XSSFilterImpl());
+        context.registerInjectActivateService(new XSSAPIImpl());
+        xssAPI = context.getService(XSSAPI.class);
     }
 
-    private static void setDefaultHandler(XSSFilterImpl xssFilter, String filename) throws Exception {
-        InputStream policyStream = new FileInputStream(filename);
-        Policy policy = Policy.getInstance(policyStream);
-        AntiSamy antiSamy = new AntiSamy(policy);
-
-        PolicyHandler mockPolicyHandler = mock(PolicyHandler.class);
-        when(mockPolicyHandler.getPolicy()).thenReturn(policy);
-        when(mockPolicyHandler.getAntiSamy()).thenReturn(antiSamy);
-
-        Whitebox.invokeMethod(xssFilter, "setDefaultHandler", mockPolicyHandler);
+    @After
+    public void tearDown() {
+        xssAPI = null;
     }
 
     @Test
     public void testEncodeForHTML() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -115,6 +93,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testEncodeForHTMLAttr() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -137,6 +116,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testEncodeForXML() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -159,6 +139,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testEncodeForXMLAttr() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -182,6 +163,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testFilterHTML() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 {null, ""},
@@ -221,8 +203,7 @@ public class XSSAPIImplTest {
         }
     }
 
-    @Test
-    public void testGetValidHref() {
+    private void testHref() {
         String[][] testData = {
                 //         Href                                        Expected Result
                 //
@@ -231,16 +212,16 @@ public class XSSAPIImplTest {
                         "test?discount=25%25"
                 },
                 {
-                    "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
-                    "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29"
+                        "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
+                        "/base?backHref=%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29"
                 },
                 {
-                    "%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
-                    ""
+                        "%26%23x6a%3b%26%23x61%3b%26%23x76%3b%26%23x61%3b%26%23x73%3b%26%23x63%3b%26%23x72%3b%26%23x69%3b%26%23x70%3b%26%23x74%3b%26%23x3a%3balert%281%29",
+                        ""
                 },
                 {
-                    "&#x6a;&#x61;&#x76;&#x61;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3a;alert(1)",
-                    ""
+                        "&#x6a;&#x61;&#x76;&#x61;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3a;alert(1)",
+                        ""
                 },
                 {"%2Fscripts%2Ftest.js", "%2Fscripts%2Ftest.js"},
                 {"/etc/commerce/collections/中文", "/etc/commerce/collections/中文"},
@@ -326,20 +307,35 @@ public class XSSAPIImplTest {
     }
 
     @Test
+    public void testGetValidHref() {
+        setUp();
+        testHref();
+    }
+
+    @Test
     public void testGetValidHrefWithoutHrefConfig() throws Exception {
+        context.registerService(ServiceUserMapped.class, mock(ServiceUserMapped.class));
+        context.load().binaryFile("/configWithoutHref.xml", "/apps/sling/xss/configWithoutHref.xml");
+        context.registerInjectActivateService(new XSSFilterImpl(), new HashMap<String, Object>(){{
+            put("policyPath", "/apps/sling/xss/configWithoutHref.xml");
+        }});
+        context.registerInjectActivateService(new XSSAPIImpl());
+        xssAPI = context.getService(XSSAPI.class);
+        ServiceReference<ResourceChangeListener> xssFilterRCL = context.bundleContext().getServiceReference(ResourceChangeListener.class);
+        assertEquals("/apps/sling/xss/configWithoutHref.xml", xssFilterRCL.getProperty(ResourceChangeListener.PATHS));
         // Load AntiSamy configuration without href filter
         XSSFilterImpl xssFilter = Whitebox.getInternalState(xssAPI, "xssFilter");
-        setDefaultHandler(xssFilter, "./src/test/resources/configWithoutHref.xml");
 
         Attribute hrefAttribute = Whitebox.getInternalState(xssFilter, "hrefAttribute");
-        Assert.assertEquals(hrefAttribute, XSSFilterImpl.DEFAULT_HREF_ATTRIBUTE);
+        assertEquals(hrefAttribute, XSSFilterImpl.DEFAULT_HREF_ATTRIBUTE);
 
         // Run same tests again to check default configuration
-        testGetValidHref();
+        testHref();
     }
 
     @Test
     public void testGetValidInteger() {
+        setUp();
         String[][] testData = {
                 //         Source                                        Expected Result
                 //
@@ -355,7 +351,7 @@ public class XSSAPIImplTest {
 
         for (String[] aTestData : testData) {
             String source = aTestData[0];
-            Integer expected = (aTestData[1] != null) ? new Integer(aTestData[1]) : null;
+            Integer expected = (aTestData[1] != null) ? Integer.parseInt(aTestData[1]) : null;
 
             TestCase.assertEquals("Validating integer '" + source + "'", expected, xssAPI.getValidInteger(source, 123));
         }
@@ -363,6 +359,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidLong() {
+        setUp();
         String[][] testData = {
                 //         Source                                        Expected Result
                 //
@@ -378,7 +375,7 @@ public class XSSAPIImplTest {
 
         for (String[] aTestData : testData) {
             String source = aTestData[0];
-            Long expected = (aTestData[1] != null) ? new Long(aTestData[1]) : null;
+            Long expected = (aTestData[1] != null) ? Long.parseLong(aTestData[1]) : null;
 
             TestCase.assertEquals("Validating long '" + source + "'", expected, xssAPI.getValidLong(source, 123));
         }
@@ -386,6 +383,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidDouble() {
+        setUp();
         String[][] testData = {
                 //         Source                                        Expected Result
                 //
@@ -400,7 +398,7 @@ public class XSSAPIImplTest {
 
         for (String[] aTestData : testData) {
             String source = aTestData[0];
-            Double expected = (aTestData[1] != null) ? new Double(aTestData[1]) : null;
+            Double expected = (aTestData[1] != null) ? Double.parseDouble(aTestData[1]) : null;
 
             TestCase.assertEquals("Validating double '" + source + "'", expected, xssAPI.getValidDouble(source, 123));
         }
@@ -408,6 +406,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidDimension() {
+        setUp();
         String[][] testData = {
                 //         Source                                        Expected Result
                 //
@@ -438,6 +437,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testEncodeForJSString() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -463,6 +463,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidJSToken() {
+        setUp();
         String[][] testData = {
                 //         Source                            Expected Result
                 //
@@ -495,6 +496,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testEncodeForCSSString() {
+        setUp();
         String[][] testData = {
                 // Source   Expected result
                 {null, null},
@@ -515,6 +517,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidStyleToken() {
+        setUp();
         String[][] testData = {
                 // Source                           Expected result
                 {null                               , RUBBISH},
@@ -579,7 +582,7 @@ public class XSSAPIImplTest {
             String expected = aTestData[1];
 
             String result = xssAPI.getValidStyleToken(source, RUBBISH);
-            if (!result.equals(expected)) {
+            if (result == null || !result.equals(expected)) {
                 fail("Validating style token '" + source + "', expecting '" + expected + "', but got '" + result + "'");
             }
         }
@@ -587,6 +590,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidCSSColor() {
+        setUp();
         String[][] testData = {
                 //      Source                          Expected Result
                 //
@@ -615,7 +619,7 @@ public class XSSAPIImplTest {
             String expected = aTestData[1];
 
             String result = xssAPI.getValidCSSColor(source, RUBBISH);
-            if (!result.equals(expected)) {
+            if (result == null || !result.equals(expected)) {
                 fail("Validating CSS Color '" + source + "', expecting '" + expected + "', but got '" + result + "'");
             }
         }
@@ -623,6 +627,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidMultiLineComment() {
+        setUp();
         String[][] testData = {
                 //Source            Expected Result
 
@@ -644,6 +649,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidJSON() {
+        setUp();
         String[][] testData = {
                 {null,      RUBBISH_JSON},
                 {"",        ""},
@@ -683,6 +689,7 @@ public class XSSAPIImplTest {
 
     @Test
     public void testGetValidXML() {
+        setUp();
         String[][] testData = {
                 {null,      RUBBISH_XML},
                 {"",        ""},