Repository: shapesecurity/salvation (java)

Issue:
Incorrect behavior when merging policies that don't contain fetch directives
Merging empty policy with `script-src a` should produce empty policy ` `.

Similarly, merging `frame-ancestors a` with `script-src a` should produce `frame-ancestors a`, 
as policy that does not contain fetch directives yields `script-src` to be not governed by CSP at all, rather than yielding `script-src *`.


Pull request:
Fixes #210

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index 8876aca..13fb42d 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -254,6 +254,10 @@ public class Policy implements Show {
 		return true;
 	}
 
+	public boolean containsFetchDirective() {
+		return directives.values().stream().anyMatch(x -> x instanceof FetchDirective);
+	}
+
 	public void postProcessOptimisation() {
 
 		DefaultSrcDirective defaultSrcDirective;
@@ -335,7 +339,9 @@ public class Policy implements Show {
 		if (oldDirective != null) {
 			oldDirective.union(directive);
 		} else {
-			this.directives.put(directive.getClass(), directive);
+			if (!(directive instanceof FetchDirective) || this.containsFetchDirective()) {
+				this.directives.put(directive.getClass(), directive);
+			}
 		}
 	}
 


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index 40e57a1..659090f 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -264,6 +264,14 @@ public class ParserTest extends CSPTest {
 		p = parse("script-src http://example.org/cpproducts/index.htm%2Cjsessionid=0C159CF2854B7519C5379634FAC36577");
 		assertEquals("script-src http://example.org/cpproducts/index.htm%2Cjsessionid=0C159CF2854B7519C5379634FAC36577", p.show());
 
+		p = parse("script-src 'none' 'none'");
+		assertEquals("", p.show());
+
+		p = parse("script-src 'self' 'self'");
+		assertEquals("script-src 'self'", p.show());
+
+		p = parse("script-src 'self' 'none'");
+		assertEquals("", p.show());
 	}
 
 	@Test
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
index f974a41..68fd342 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
@@ -54,6 +54,21 @@ public class PolicyMergeTest extends CSPTest {
 		assertEquals(
 				"default-src a; form-action a; frame-ancestors b; navigate-to c",
 				p1.show());
+
+		p1 = Parser.parse("form-action aaa; frame-ancestors bbb; navigate-to ccc", "https://origin1.com");
+		p2 = Parser.parse("script-src a", "https://origin1.com");
+		p1.union(p2);
+		assertEquals("form-action aaa; frame-ancestors bbb; navigate-to ccc", p1.show());
+
+		p1 = Parser.parse("frame-ancestors bbb;", "https://origin1.com");
+		p2 = Parser.parse("script-src a", "https://origin1.com");
+		p1.union(p2);
+		assertEquals("frame-ancestors bbb", p1.show());
+
+		p1 = Parser.parse("", "https://origin1.com");
+		p2 = Parser.parse("script-src a", "https://origin1.com");
+		p1.union(p2);
+		assertEquals("", p1.show());
 	}
 
 	@Test
@@ -288,6 +303,16 @@ public class PolicyMergeTest extends CSPTest {
 		p2 = parse("navigate-to a;");
 		p1.intersect(p2);
 		assertEquals("navigate-to a", p1.show());
+
+		p1 = parse("navigate-to a b c;");
+		p2 = parse("script-src zzz");
+		p1.intersect(p2);
+		assertEquals("navigate-to a b c; script-src zzz", p1.show());
+
+		p1 = Parser.parse("", "https://origin1.com");
+		p2 = Parser.parse("script-src a", "https://origin1.com");
+		p1.intersect(p2);
+		assertEquals("script-src a", p1.show());
 	}
 
 	@Test
@@ -425,7 +450,7 @@ public class PolicyMergeTest extends CSPTest {
 
 		p.unionDirective(scriptSrcDirective);
 		p.unionDirective(styleSrcDirective);
-		assertEquals("script-src 'nonce-Q-ecAIccSGatv6lJrCBVARPr'; style-src 'nonce-Q-ecAIccSGatv6lJrCBVARPr'",
+		assertEquals("",
 				p.show());
 	}
 
@@ -647,10 +672,10 @@ public class PolicyMergeTest extends CSPTest {
 		p.union(q);
 		assertEquals("script-src 'self'", p.show());
 
-		p = parse("script-src 'none' 'none'");
+		p = parse("");
 		q = parse("script-src 'self'");
 		p.union(q);
-		assertEquals("script-src 'self'", p.show());
+		assertEquals("", p.show());
 
 		p = parse("script-src 'self'");
 		q = parse("script-src 'none'");