Skip to content

Commit 4e49712

Browse files
jeremiahjstaceyjeremiahjstacey
authored
HTMValidationRule & Test Updates (#607)
* HTMLValidationRule static updates Splitting the logic of the static block into multiple smaller blocks. Doing this appears to have make the tests more consistent between the full build and running the ClasspathTest in isolation. Both now fail due to bad configuration in the esapi-antisamy-CP.xml file. * HTML Validation CP Tests Correcting test file reference to point to the antisamy copy file. Removing invalid characters from the antisamy copy file so that it may be parsed by the runtime. * HTMLValidationRule Tests Adding a known invalid antisamy configuration file. Updating the static initialization blocks from private to package-protected for targeted testing. Adding a test that asserts that when we attempt to use an invalid Antisamy configuration file, that a PolicyException is emitted. * Adding Test for Antisamy Schema Validation TEST IS FAILING Adding test to verify that the known bad configuration is accepted when the system property for Antisamy is set appropriately. * Updating Antisamy Version Update to 1.6.1 * Antisamy Stream WorkAround Creating a ByteArrayInputStream in the HTMLValidationRule to circumvent the undesired stream close event when processing a malformed xml configuraiton while antisamy schema validation is disabled. Moving the property test to an isolated file. * Antisamy Schema Validation Tests Switching to configuring validation behavior through static methods rather than system properties. Properties are read once on construction in the Policy, and without forcing test execution order, the validation would be based on execution order * Stream to ByteArrayStream Fix Updates to the process to convert the resource stream to a ByteArrayInputStream through the use of a Reader implementation. This allows the java7 try-with-resources block when converting the InputStream Object, which should ensure that the original stream is cleaned up once read into memory. * Cleanup Removing dead method. Adding newline at end of file * Antisamy 1.6.2 Update Prepping content for the Antisamy 1.6.2 artifact that is expected to be available in the near future. Updated pom dependency to anticipated version Updated HTMLValidationRule to remove pre-wrapping of ByteArrayInputStream for Policy Object creation * HTMLValidationRule Cleanup Removing unused imports.
1 parent b7e9a55 commit 4e49712

File tree

6 files changed

+208
-427
lines changed

6 files changed

+208
-427
lines changed

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@
241241
<dependency>
242242
<groupId>org.owasp.antisamy</groupId>
243243
<artifactId>antisamy</artifactId>
244-
<version>1.6.0</version>
244+
<version>1.6.2</version>
245245
</dependency>
246246
<dependency>
247247
<groupId>org.slf4j</groupId>

‎src/main/java/org/owasp/esapi/reference/validation/HTMLValidationRule.java‎

Lines changed: 99 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,27 @@
1515
*/
1616
package org.owasp.esapi.reference.validation;
1717

18+
19+
20+
import java.io.File;
1821
import java.io.IOException;
1922
import java.io.InputStream;
23+
import java.util.Arrays;
2024
import java.util.List;
2125

22-
import org.owasp.esapi.errors.ConfigurationException;
2326
import org.owasp.esapi.ESAPI;
2427
import org.owasp.esapi.Encoder;
2528
import org.owasp.esapi.Logger;
29+
import org.owasp.esapi.SecurityConfiguration;
2630
import org.owasp.esapi.StringUtilities;
31+
import org.owasp.esapi.errors.ConfigurationException;
2732
import org.owasp.esapi.errors.ValidationException;
33+
import org.owasp.esapi.reference.DefaultSecurityConfiguration.DefaultSearchPath;
2834
import org.owasp.validator.html.AntiSamy;
2935
import org.owasp.validator.html.CleanResults;
3036
import org.owasp.validator.html.Policy;
3137
import org.owasp.validator.html.PolicyException;
3238
import org.owasp.validator.html.ScanException;
33-
import org.owasp.esapi.reference.DefaultSecurityConfiguration;
3439

3540

3641
/**
@@ -49,110 +54,112 @@ public class HTMLValidationRule extends StringValidationRule {
4954
private static final Logger LOGGER = ESAPI.getLogger( "HTMLValidationRule" );
5055
private static final String ANTISAMYPOLICY_FILENAME = "antisamy-esapi.xml";
5156

52-
/**
53-
* Used to load antisamy-esapi.xml from a variety of different classpath locations.
54-
* The classpath locations are the same classpath locations as used to load esapi.properties.
55-
* See DefaultSecurityConfiguration.DefaultSearchPath.
56-
*
57-
* @param fileName The resource file filename.
58-
*/
59-
private static InputStream getResourceStreamFromClasspath(String fileName) {
60-
InputStream resourceStream = null;
57+
//TESTING -- Mock Classloaders, verify that the classloader is called as desired with the searchpath and filename concat
58+
// Verify that when no match is found, null is returned
59+
// Verify that when match is found, remaining classloaders are not invoked and expected InputStream is returned.
60+
/*package */ static InputStream getResourceStreamFromClassLoader(String contextDescription, ClassLoader classLoader, String fileName, List<String> searchPaths) {
61+
InputStream result = null;
62+
63+
for (String searchPath: searchPaths) {
64+
result = classLoader.getResourceAsStream(searchPath + fileName);
65+
66+
if (result != null) {
67+
LOGGER.info(Logger.EVENT_SUCCESS, "SUCCESSFULLY LOADED " + fileName + " via the CLASSPATH from '" +
68+
searchPath + "' using " + contextDescription + "!");
69+
break;
70+
}
71+
}
72+
73+
return result;
74+
}
75+
76+
//TESTING
77+
// Harder to test... Use Junit to place files in each of the DefaultSearchPathLocations and verify that the file can be found.
78+
// Not sure how to test that the classpaths are iterated.
79+
/*package */ static InputStream getResourceStreamFromClasspath(String fileName) {
80+
LOGGER.info(Logger.EVENT_FAILURE, "Loading " + fileName + " from classpaths");
6181

62-
ClassLoader[] loaders = new ClassLoader[] {
63-
Thread.currentThread().getContextClassLoader(),
64-
ClassLoader.getSystemClassLoader(),
65-
ESAPI.securityConfiguration().getClass().getClassLoader()
66-
/* can't use just getClass.getClassLoader() in a static context, so using the DefaultSecurityConfiguration class. */
67-
};
82+
InputStream resourceStream = null;
6883

69-
String[] classLoaderNames = {
70-
"current thread context class loader",
71-
"system class loader",
72-
"class loader for DefaultSecurityConfiguration class"
73-
};
84+
List<String> orderedSearchPaths = Arrays.asList(DefaultSearchPath.ROOT.value(),
85+
DefaultSearchPath.RESOURCE_DIRECTORY.value(),
86+
DefaultSearchPath.DOT_ESAPI.value(),
87+
DefaultSearchPath.ESAPI.value(),
88+
DefaultSearchPath.RESOURCES.value(),
89+
DefaultSearchPath.SRC_MAIN_RESOURCES.value());
7490

75-
int i = 0;
76-
for (ClassLoader loader : loaders) {
77-
// try root
78-
String currentClasspathSearchLocation = "/ (root)";
79-
resourceStream = loader.getResourceAsStream(DefaultSecurityConfiguration.DefaultSearchPath.ROOT.value() + fileName);
80-
81-
// try resourceDirectory folder
82-
if (resourceStream == null){
83-
currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.RESOURCE_DIRECTORY.value();
84-
resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName);
85-
}
86-
87-
// try .esapi folder. Look here first for backward compatibility.
88-
if (resourceStream == null){
89-
currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.DOT_ESAPI.value();
90-
resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName);
91-
}
92-
93-
// try esapi folder (new directory)
94-
if (resourceStream == null){
95-
currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.ESAPI.value();
96-
resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName);
97-
}
98-
99-
// try resources folder
100-
if (resourceStream == null){
101-
currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.RESOURCES.value();
102-
resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName);
103-
}
104-
105-
// try src/main/resources folder
106-
if (resourceStream == null){
107-
currentClasspathSearchLocation = DefaultSecurityConfiguration.DefaultSearchPath.SRC_MAIN_RESOURCES.value();
108-
resourceStream = loader.getResourceAsStream(currentClasspathSearchLocation + fileName);
109-
}
110-
111-
if (resourceStream != null) {
112-
LOGGER.info(Logger.EVENT_FAILURE, "SUCCESSFULLY LOADED " + fileName + " via the CLASSPATH from '" +
113-
currentClasspathSearchLocation + "' using " + classLoaderNames[i] + "!");
114-
break; // Outta here since we've found and loaded it.
115-
}
116-
117-
i++;
118-
}
91+
resourceStream = getResourceStreamFromClassLoader("current thread context class loader", Thread.currentThread().getContextClassLoader(), fileName, orderedSearchPaths);
92+
93+
//I'm lazy. Using ternary for shorthand "if null then do next thing" Harder to read, sorry
94+
resourceStream = resourceStream != null ? resourceStream : getResourceStreamFromClassLoader("system class loader", ClassLoader.getSystemClassLoader(), fileName, orderedSearchPaths);
95+
resourceStream = resourceStream != null ? resourceStream : getResourceStreamFromClassLoader("class loader for DefaultSecurityConfiguration class", ESAPI.securityConfiguration().getClass().getClassLoader(), fileName, orderedSearchPaths);
11996

12097
return resourceStream;
12198
}
99+
100+
//TESTING
101+
// Mock SecurityConfiguration - Return file check (true) - return resourceStream - expect Policy object
102+
// Mock SecurityConfiguration - Return file check (false) - use junit to place file in any of the DefaultSearchPathLocations - verify Policy Object
103+
// Mock SecurityConfiguration - return file check (true) - throw IOException on resource stream - Verify IOException
104+
// Mock SecurityConfiguration - return file Check (true) - use Junit to place a BAD FILE - verify PolicyException
105+
// HOW TO TEST NULL RETURN.....
106+
/*package */ static Policy loadAntisamyPolicy(String antisamyPolicyFilename) throws IOException, PolicyException {
107+
InputStream resourceStream = null;
108+
SecurityConfiguration secCfg = ESAPI.securityConfiguration();
109+
110+
//Rather than catching the IOException from the resource stream, let's ask if the file exists to give this a best-case resolution.
111+
//This helps with the IOException handling too. If the file is there and we get an IOException from the SecurityConfiguration, then the file is there and something else is wrong (FAIL -- don't try the other path)
112+
File file = secCfg.getResourceFile(antisamyPolicyFilename);
113+
114+
resourceStream = file == null ? getResourceStreamFromClasspath(antisamyPolicyFilename) : secCfg.getResourceStream(antisamyPolicyFilename);
115+
return resourceStream == null ? null : Policy.getInstance(resourceStream);
116+
}
122117

123-
static {
124-
InputStream resourceStream = null;
125-
String antisamyPolicyFilename = null;
126-
127-
try {
128-
antisamyPolicyFilename = ESAPI.securityConfiguration().getStringProp(
118+
//TESTING
119+
// Mock SecurityConfiguration - return a valid string on property request - verify String is returned from call
120+
// Mock SecurityConfiguration -- throw ConfigurationException on property request -- Verify Default Filename is returned from call
121+
/*package */ static String resolveAntisamyFilename() {
122+
String antisamyPolicyFilename = ANTISAMYPOLICY_FILENAME;
123+
try {
124+
antisamyPolicyFilename = ESAPI.securityConfiguration().getStringProp(
129125
// Future: This will be moved to a new PropNames class
130126
org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE );
131-
} catch (ConfigurationException cex) {
132-
127+
} catch (ConfigurationException cex) {
128+
133129
LOGGER.info(Logger.EVENT_FAILURE, "ESAPI property " +
134130
org.owasp.esapi.reference.DefaultSecurityConfiguration.VALIDATOR_HTML_VALIDATION_CONFIGURATION_FILE +
135131
" not set, using default value: " + ANTISAMYPOLICY_FILENAME);
136-
antisamyPolicyFilename = ANTISAMYPOLICY_FILENAME;
137-
}
138-
try {
139-
resourceStream = ESAPI.securityConfiguration().getResourceStream(antisamyPolicyFilename);
140-
} catch (IOException e) {
141-
142-
LOGGER.info(Logger.EVENT_FAILURE, "Loading " + antisamyPolicyFilename + " from classpaths");
132+
}
133+
return antisamyPolicyFilename;
134+
}
135+
136+
//TESTING
137+
// Mock SecurityConfiguration - return file check (true) - throw IOException on resource stream - Verify ConfigurationException from IOException
138+
// Mock SecurityConfiguration - return file Check (true) - use Junit to place a BAD FILE - verify ConfigurationException from PolicyException
139+
// Force NULL return from loadAntisamyPolicy call -- Verify ConfigurationException from null value
140+
/*package */ static void configureInstance() {
141+
String antisamyPolicyFilename = resolveAntisamyFilename();
143142

144-
resourceStream = getResourceStreamFromClasspath(antisamyPolicyFilename);
145-
}
146-
if (resourceStream != null) {
147-
try {
148-
antiSamyPolicy = Policy.getInstance(resourceStream);
149-
} catch (PolicyException e) {
150-
throw new ConfigurationException("Couldn't parse " + antisamyPolicyFilename, e);
151-
}
152-
}
153-
else {
143+
try {
144+
antiSamyPolicy = loadAntisamyPolicy(antisamyPolicyFilename);
145+
} catch (IOException ioe) {
146+
//Thrown if file is found by SecurityConfiguration, but a stream cannot be generated.
147+
throw new ConfigurationException("Failed to load file from SecurityConfiguration context: " + antisamyPolicyFilename, ioe);
148+
} catch (PolicyException e) {
149+
//Thrown if the resource stream was created, but the contents of the file are not compatible with antisamy expectations.
150+
throw new ConfigurationException("Couldn't parse " + antisamyPolicyFilename, e);
151+
}
152+
153+
if (antiSamyPolicy == null) {
154154
throw new ConfigurationException("Couldn't find " + antisamyPolicyFilename);
155-
}
155+
}
156+
157+
}
158+
159+
//TESTING
160+
// None.
161+
static {
162+
configureInstance();
156163
}
157164

158165
public HTMLValidationRule( String typeName ) {

‎src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleAntisamyPropertyTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* OWASP Enterprise Security API (ESAPI)
3+
*
4+
* This file is part of the Open Web Application Security Project (OWASP)
5+
* Enterprise Security API (ESAPI) project. For details, please see
6+
* <a href="http://www.owasp.org/index.php/ESAPI">http://www.owasp.org/index.php/ESAPI</a>.
7+
*
8+
* Copyright (c) 2019 - The OWASP Foundation
9+
*
10+
* The ESAPI is published by OWASP under the BSD license. You should read and accept the
11+
* LICENSE before you use, modify, and/or redistribute this software.
12+
*
13+
* @author [email protected]
14+
* @since 2019
15+
*/
16+
package org.owasp.esapi.reference.validation;
17+
18+
import org.junit.After;
19+
import org.junit.AfterClass;
20+
import org.junit.Before;
21+
import org.junit.BeforeClass;
22+
import org.junit.Test;
23+
import org.owasp.validator.html.Policy;
24+
25+
/**
26+
* Isolate scope test to assert the behavior of the HTMLValidationRule
27+
* when schema validation is disabled in the Antisamy Project.
28+
*/
29+
public class HTMLValidationRuleAntisamyPropertyTest {
30+
/** The intentionally non-compliant AntiSamy policy file. We don't intend to
31+
* actually <i>use</i> it for anything.
32+
*/
33+
private static final String INVALID_ANTISAMY_POLICY_FILE = "antisamy-InvalidPolicy.xml";
34+
35+
@AfterClass
36+
public static void enableAntisamySchemaValidation() {
37+
Policy.setSchemaValidation(true);
38+
}
39+
40+
@BeforeClass
41+
public static void disableAntisamySchemaValidation() {
42+
Policy.setSchemaValidation(false);
43+
//System property is read once, so we're preferring the static method for testing.
44+
//System.setProperty( "owasp.validator.validateschema", "false" );
45+
}
46+
47+
@Test
48+
public void checkAntisamySystemPropertyWorksAsAdvertised() throws Exception {
49+
HTMLValidationRule.loadAntisamyPolicy(INVALID_ANTISAMY_POLICY_FILE);
50+
}
51+
52+
}

‎src/test/java/org/owasp/esapi/reference/validation/HTMLValidationRuleClasspathTest.java‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,23 @@
1515
*/
1616
package org.owasp.esapi.reference.validation;
1717

18+
import static org.junit.Assert.assertEquals;
19+
import static org.junit.Assert.assertFalse;
20+
import static org.junit.Assert.assertTrue;
21+
import static org.junit.Assert.fail;
22+
23+
import org.junit.After;
24+
import org.junit.Before;
25+
import org.junit.Rule;
26+
import org.junit.Test;
27+
import org.junit.rules.ExpectedException;
1828
import org.owasp.esapi.ESAPI;
1929
import org.owasp.esapi.SecurityConfiguration;
2030
import org.owasp.esapi.SecurityConfigurationWrapper;
2131
import org.owasp.esapi.ValidationErrorList;
22-
import org.owasp.esapi.ValidationRule;
2332
import org.owasp.esapi.Validator;
2433
import org.owasp.esapi.errors.ValidationException;
25-
import org.owasp.esapi.reference.validation.HTMLValidationRule;
26-
27-
import org.junit.Test;
28-
import org.junit.Before;
29-
import org.junit.After;
30-
import org.junit.Rule;
31-
import org.junit.rules.ExpectedException;
32-
import static org.junit.Assert.*;
34+
import org.owasp.validator.html.PolicyException;
3335

3436
/**
3537
* The Class HTMLValidationRuleThrowsTest.
@@ -48,10 +50,19 @@
4850
* the cleansed (sanitizied) output when certain unsafe input is encountered.
4951
*/
5052
public class HTMLValidationRuleClasspathTest {
53+
/** The intentionally non-compliant AntiSamy policy file. We don't intend to
54+
* actually <i>use</i> it for anything.
55+
*/
56+
private static final String INVALID_ANTISAMY_POLICY_FILE = "antisamy-InvalidPolicy.xml";
57+
/** The intentionally non-compliant AntiSamy policy file. We don't intend to
58+
* actually <i>use</i> it for anything.
59+
*/
60+
private static final String ANTISAMY_POLICY_FILE_NONSTANDARD_LOCATION = "antisamy-esapi-CP.xml";
61+
5162
private static class ConfOverride extends SecurityConfigurationWrapper {
52-
private String desiredReturnAction = "clean";
53-
private String desiredReturnConfigurationFile = "antisamy-esapi.xml";
54-
63+
private String desiredReturnAction = "clean";
64+
private String desiredReturnConfigurationFile = null;
65+
5566
ConfOverride(SecurityConfiguration orig, String desiredReturnAction, String desiredReturnConfigurationFile) {
5667
super(orig);
5768
this.desiredReturnAction = desiredReturnAction;
@@ -78,17 +89,23 @@ public String getStringProp(String propName) {
7889
@After
7990
public void tearDown() throws Exception {
8091
ESAPI.override(null);
81-
thrownEx = ExpectedException.none();
8292
}
8393

8494
@Before
8595
public void setUp() throws Exception {
8696
ESAPI.override(
87-
new ConfOverride( ESAPI.securityConfiguration(), "throw", "antisamy-esapi-CP.xml" )
97+
new ConfOverride( ESAPI.securityConfiguration(), "throw", ANTISAMY_POLICY_FILE_NONSTANDARD_LOCATION )
8898
);
89-
9099
}
91100

101+
102+
@Test
103+
public void checkPolicyExceptionWithBadConfig() throws Exception {
104+
ESAPI.override(null);
105+
thrownEx.expect(PolicyException.class);
106+
HTMLValidationRule.loadAntisamyPolicy(INVALID_ANTISAMY_POLICY_FILE);
107+
}
108+
92109
@Test
93110
public void testGetValid() throws Exception {
94111
System.out.println("getValidCP");

0 commit comments

Comments
 (0)