Repository: shapesecurity/salvation (java)

Issue:
directives followed by keyword-source without a WSP between them should fail parsing
directives followed by keyword-source without a WSP between them should fail parsing
Example: `script-src'self'`. 

Pull request:
fixes #111 and #113: correct whitespace handling in tokeniser
Fixes #111 and #113.

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Tokeniser.java b/src/main/java/com/shapesecurity/salvation/Tokeniser.java
index 9e64d36..71a688a 100644
--- a/src/main/java/com/shapesecurity/salvation/Tokeniser.java
+++ b/src/main/java/com/shapesecurity/salvation/Tokeniser.java
@@ -28,7 +28,6 @@ public class Tokeniser {
         this.tokens = new ArrayList<>();
         this.sourceText = sourceText;
         this.length = sourceText.length();
-        this.eatWhitespace();
     }
 
     @Nonnull public static Token[] tokenise(@Nonnull String sourceText) {
@@ -48,7 +47,6 @@ public class Tokeniser {
         int start = this.index;
         this.index = matcher.end();
         this.tokens.add(ctor.apply(this.sourceText.substring(start, this.index)));
-        this.eatWhitespace();
         return true;
     }
 
@@ -69,6 +67,14 @@ public class Tokeniser {
         return this.eat(UnknownToken::new, Tokeniser.notSeparator);
     }
 
+    private boolean eatSingleWhitespace() {
+        if (this.hasNext() && Tokeniser.isWhitespace(this.sourceText.charAt(this.index))) {
+            ++this.index;
+            return true;
+        }
+        return false;
+    }
+
     private void eatWhitespace() {
         while (this.hasNext() && Tokeniser.isWhitespace(this.sourceText.charAt(this.index))) {
             ++this.index;
@@ -81,12 +87,18 @@ public class Tokeniser {
 
     @Nonnull protected Token[] tokenise() {
         while (this.hasNext()) {
-            if (this.eatSeparator())
+            this.eatWhitespace();
+            if (this.eatSeparator()) {
                 continue;
+            }
             if (!this.eatDirectiveName()) {
                 this.eatUntilSeparator();
                 continue;
             }
+            if (!this.eatSingleWhitespace()) {
+                this.eatUntilSeparator();
+                continue;
+            }
             if (this.eatSeparator())
                 continue;
             this.eatDirectiveValue();
@@ -151,22 +163,18 @@ public class Tokeniser {
     @Nonnull private static List<SubDirectiveValueToken> splitByWSP(@Nonnull Token token) {
         List<SubDirectiveValueToken> tokens = new ArrayList<>();
         @Nullable Location startLocation = token.startLocation;
-        if (startLocation == null) {
-            for (String s : WSP.split(trimRHSWS(token.value))) {
-                tokens.add(new SubDirectiveValueToken(s));
-            }
-        } else {
-            Matcher m = NotWSP.matcher(token.value);
-            int offset = 0;
-            while (m.find(offset)) {
-                SubDirectiveValueToken dv = new SubDirectiveValueToken(token.value.substring(m.start(), m.end()));
+        Matcher m = NotWSP.matcher(token.value);
+        int offset = 0;
+        while (m.find(offset)) {
+            SubDirectiveValueToken dv = new SubDirectiveValueToken(token.value.substring(m.start(), m.end()));
+            if (startLocation != null) {
                 dv.startLocation = new Location(startLocation.line, startLocation.column + m.start(),
                     startLocation.offset + m.start());
                 dv.endLocation =
                     new Location(startLocation.line, startLocation.column + m.end(), startLocation.offset + m.end());
-                offset = m.end();
-                tokens.add(dv);
             }
+            offset = m.end();
+            tokens.add(dv);
         }
         return tokens;
     }


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index 72e2d77..52e8c59 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -6,6 +6,9 @@ import com.shapesecurity.salvation.data.Policy;
 import com.shapesecurity.salvation.data.URI;
 import com.shapesecurity.salvation.directiveValues.HashSource;
 import com.shapesecurity.salvation.directives.*;
+import com.shapesecurity.salvation.tokens.DirectiveNameToken;
+import com.shapesecurity.salvation.tokens.DirectiveValueToken;
+import com.shapesecurity.salvation.tokens.Token;
 import org.junit.Test;
 
 import java.io.FileNotFoundException;
@@ -364,9 +367,7 @@ public class ParserTest extends CSPTest {
 
         p = parse("report-to        a");
         q = parse("report-to a; ");
-        assertEquals("report-to hashcode match", p.hashCode(), q.hashCode());
-        assertTrue("report-to equals", p.equals(q));
-
+        assertFalse("report-to equals", p.equals(q));
     }
 
     @Test public void testMediaTypeUnion() {
@@ -543,6 +544,27 @@ public class ParserTest extends CSPTest {
             parse("img-src example.com 'unsafe-redirect'").getDirectiveByType(ImgSrcDirective.class).show());
     }
 
+    @Test public void testDirectNameSpacing() {
+        ArrayList<Notice> notices = new ArrayList<>();
+        Policy p = parseWithNotices("script-src'self'", notices);
+
+        assertEquals("", p.show());
+        assertEquals(1, notices.size());
+        assertEquals(Notice.Type.ERROR, notices.get(0).type);
+        assertEquals("Expecting directive-value but found U+0027 ('). Non-ASCII and non-printable characters must be percent-encoded.", notices.get(0).message);
+    }
+
+    @Test public void testDirectValueSpacing() {
+        Token[] tokens = Tokeniser.tokenise("some-directive-name   a  ");
+
+        assertEquals(2, tokens.length);
+        assertTrue(tokens[0] instanceof DirectiveNameToken);
+        assertEquals("some-directive-name", tokens[0].value);
+        assertEquals(DirectiveNameToken.DirectiveNameSubtype.Unrecognised, ((DirectiveNameToken) tokens[0]).subtype);
+        assertTrue(tokens[1] instanceof DirectiveValueToken);
+        assertEquals("  a  ", tokens[1].value);
+    }
+
     @Test public void testUnknownTokens() {
         ArrayList<Notice> notices = new ArrayList();
         Policy p = parseWithNotices("img-src √", notices);
@@ -638,7 +660,7 @@ public class ParserTest extends CSPTest {
     @Test public void testNewDirectives() {
         Policy p;
         ArrayList<Notice> notices = new ArrayList<>();
-        p = parseWithNotices("referrer      no-referrer   ", notices);
+        p = parseWithNotices("referrer no-referrer", notices);
         assertEquals(1, p.getDirectives().size());
         assertEquals(1, notices.size());
 
@@ -649,6 +671,12 @@ public class ParserTest extends CSPTest {
         assertEquals("The referrer directive is an experimental directive that will be likely added to the CSP specification.", notices.get(0).message);
         assertEquals("The referrer directive must contain exactly one referrer directive value.", notices.get(1).message);
 
+        notices.clear();
+        p = parseWithNotices("referrer   no-referrer  ", notices);
+        assertEquals(0, p.getDirectives().size());
+        assertEquals(2, notices.size());
+        assertEquals("Expecting referrer directive value but found \"  no-referrer  \".", notices.get(1).message);
+
         notices.clear();
         p = parseWithNotices("referrer aaa", notices);
         assertEquals(0, p.getDirectives().size());