Repository: shapesecurity/salvation (java)

Issue:
Warn about unquoted keyword-source values in ancestor-source-list
Warn about unquoted `keyword-source` values in `ancestor-source-list` as we do for `source-expression`s.

Example: `frame-ancestors self;`


Pull request:
fixes #105: warn on unquoted keywords

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Constants.java b/src/main/java/com/shapesecurity/salvation/Constants.java
index c9d6e79..36df416 100644
--- a/src/main/java/com/shapesecurity/salvation/Constants.java
+++ b/src/main/java/com/shapesecurity/salvation/Constants.java
@@ -12,6 +12,7 @@ import java.util.regex.Pattern;
         "^allow-(?:forms|modals|pointer-lock" + "|popups|popups-to-escape-sandbox|same-origin"
             + "|scripts|top-navigation)$");
     public static final Pattern mediaTypePattern = Pattern.compile("^(?<type>[^/]+)/(?<subtype>[^/]+)$");
+    public static final Pattern unquotedKeywordPattern = Pattern.compile("^(?:self|unsafe-inline|unsafe-eval|unsafe-redirect|none)$");
     // port-part constants
     public static final int WILDCARD_PORT = -200;
     public static final int EMPTY_PORT = -1;
diff --git a/src/main/java/com/shapesecurity/salvation/Parser.java b/src/main/java/com/shapesecurity/salvation/Parser.java
index 529c73d..53ed5b6 100644
--- a/src/main/java/com/shapesecurity/salvation/Parser.java
+++ b/src/main/java/com/shapesecurity/salvation/Parser.java
@@ -388,15 +388,8 @@ public class Parser {
             case "'unsafe-redirect'":
                 this.warn(token, "'unsafe-redirect' has been removed from CSP as of version 2.0.");
                 return KeywordSource.UnsafeRedirect;
-            case "self":
-            case "unsafe-inline":
-            case "unsafe-eval":
-            case "unsafe-redirect":
-            case "none":
-                this.warn(token,
-                    "This host name is unusual, and likely meant to be a keyword that is missing the required quotes: \'"
-                        + token.value.toLowerCase() + "\'.");
             default:
+                checkForUnquotedKeyword(token);
                 if (token.value.startsWith("'nonce-")) {
                     String nonce = token.value.substring(7, token.value.length() - 1);
                     NonceSource nonceSource = new NonceSource(nonce);
@@ -452,13 +445,9 @@ public class Parser {
                         String portString = matcher.group("port");
                         int port;
                         if (portString == null) {
-                            port = scheme == null ?
-                                Constants.EMPTY_PORT :
-                                SchemeHostPortTriple.defaultPortForProtocol(scheme);
+                            port = scheme == null ? Constants.EMPTY_PORT : SchemeHostPortTriple.defaultPortForProtocol(scheme);
                         } else {
-                            port = portString.equals(":*") ?
-                                Constants.WILDCARD_PORT :
-                                Integer.parseInt(portString.substring(1));
+                            port = portString.equals(":*") ? Constants.WILDCARD_PORT : Integer.parseInt(portString.substring(1));
                         }
                         String host = matcher.group("host");
                         String path = matcher.group("path");
@@ -504,6 +493,7 @@ public class Parser {
         if (token.value.equalsIgnoreCase("'self'")) {
             return KeywordSource.Self;
         }
+        checkForUnquotedKeyword(token);
         if (token.value.matches("^" + Constants.schemePart + ":$")) {
             return new SchemeSource(token.value.substring(0, token.value.length() - 1));
         } else {
@@ -517,8 +507,7 @@ public class Parser {
                 if (portString == null) {
                     port = scheme == null ? Constants.EMPTY_PORT : SchemeHostPortTriple.defaultPortForProtocol(scheme);
                 } else {
-                    port =
-                        portString.equals(":*") ? Constants.WILDCARD_PORT : Integer.parseInt(portString.substring(1));
+                    port = portString.equals(":*") ? Constants.WILDCARD_PORT : Integer.parseInt(portString.substring(1));
                 }
                 String host = matcher.group("host");
                 String path = matcher.group("path");
@@ -529,6 +518,14 @@ public class Parser {
         throw INVALID_ANCESTOR_SOURCE;
     }
 
+    private void checkForUnquotedKeyword(@Nonnull Token token) {
+        if (Constants.unquotedKeywordPattern.matcher(token.value).find()) {
+            this.warn(token,
+                "This host name is unusual, and likely meant to be a keyword that is missing the required quotes: \'"
+                    + token.value + "\'.");
+        }
+    }
+
     @Nonnull private ReferrerValue parseReferrerToken(@Nonnull Token directiveNameToken) throws DirectiveParseException {
         if (this.hasNext(DirectiveValueToken.class)) {
             Token token = this.advance();


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/LocationTest.java b/src/test/java/com/shapesecurity/salvation/LocationTest.java
index c2e18be..95772bf 100644
--- a/src/test/java/com/shapesecurity/salvation/LocationTest.java
+++ b/src/test/java/com/shapesecurity/salvation/LocationTest.java
@@ -399,6 +399,16 @@ public class LocationTest extends CSPTest {
         assertEquals(
             "Warning: This host name is unusual, and likely meant to be a keyword that is missing the required quotes: 'unsafe-redirect'.",
             notice.toString());
+
+        notices.clear();
+        ParserWithLocation.parse("frame-ancestors abc none self", URI.parse("https://origin"), notices);
+        assertEquals(2, notices.size());
+        assertEquals(
+            "1:21: This host name is unusual, and likely meant to be a keyword that is missing the required quotes: 'none'.",
+            notices.get(0).show());
+        assertEquals(
+            "Warning: This host name is unusual, and likely meant to be a keyword that is missing the required quotes: 'self'.",
+            notices.get(1).toString());
     }
 
     @Test public void testNoticeHelpers() {