Changeset 225699 in webkit

Timestamp:

Dec 8, 2017 2:05:07 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

ApplicationManifestParser should strip whitespace from the raw input
​https://bugs.webkit.org/show_bug.cgi?id=180539
rdar://problem/35915075

Patch by David Quesada <​david_quesada@apple.com> on 2017-12-08
Reviewed by Joseph Pecoraro.

Source/WebCore:

• Modules/applicationmanifest/ApplicationManifestParser.cpp:

(WebCore::ApplicationManifestParser::parseManifest):

Tools:

Added an API test for parsing manifests with surrounding whitespace.

Also drive-by fix ApplicationManifestParserTest.Display. Earlier versions of the
manifest spec explicitly stated that the "display" value should be treated as if
it were run through String.prototype.trim(), which allowed for the really weird
edge case of an array containing one string. This behavior was lost when I changed
ApplicationManifestParser's JSON parsing from using JavaScriptCore to WTF's JSON
parsing. Update the unit test accordingly.

• TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp:

(TEST_F):

Location:

trunk

Files:

• Source/WTF/wtf/JSONValues.cpp (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WTF/JSONValue.cpp (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp (modified) (2 diffs)

trunk/Source/WTF/wtf/JSONValues.cpp

@@ -537 +537 @@
     const UChar* tokenEnd;
     auto result = buildValue(start, end, &tokenEnd, 0);
-    if (!result || tokenEnd != end)
-        return false;
+    if (!result)
+        return false;
+
+    for (const UChar* valueEnd = tokenEnd; valueEnd < end; ++valueEnd) {
+        if (!isSpaceOrNewline(*valueEnd))
+            return false;
+    }
 
     output = WTFMove(result);

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-12-08  David Quesada  <david_quesada@apple.com>
+
+        ApplicationManifestParser should strip whitespace from the raw input
+        https://bugs.webkit.org/show_bug.cgi?id=180539
+        rdar://problem/35915075
+
+        Reviewed by Joseph Pecoraro.
+
+        * Modules/applicationmanifest/ApplicationManifestParser.cpp:
+        (WebCore::ApplicationManifestParser::parseManifest):
+
 2017-12-08  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Tools/ChangeLog

@@ +1 @@
+2017-12-08  David Quesada  <david_quesada@apple.com>
+
+        ApplicationManifestParser should strip whitespace from the raw input
+        https://bugs.webkit.org/show_bug.cgi?id=180539
+        rdar://problem/35915075
+
+        Reviewed by Joseph Pecoraro.
+
+        Added an API test for parsing manifests with surrounding whitespace.
+
+        Also drive-by fix ApplicationManifestParserTest.Display. Earlier versions of the
+        manifest spec explicitly stated that the "display" value should be treated as if
+        it were run through String.prototype.trim(), which allowed for the really weird
+        edge case of an array containing one string. This behavior was lost when I changed
+        ApplicationManifestParser's JSON parsing from using JavaScriptCore to WTF's JSON
+        parsing. Update the unit test accordingly.
+
+        * TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp:
+        (TEST_F):
+
 2017-12-08  Konstantin Tokarev  <annulen@yandex.ru>
 

trunk/Tools/TestWebKitAPI/Tests/WTF/JSONValue.cpp

@@ -636 +636 @@
         EXPECT_FALSE(JSON::Value::parseJSON("[{\"foo\":{\"baz\":false}]", value));
     }
+
+    {
+        RefPtr<JSON::Value> value;
+        EXPECT_TRUE(JSON::Value::parseJSON(" \"foo\" \n", value));
+        String stringValue;
+        EXPECT_TRUE(value->asString(stringValue));
+        EXPECT_EQ("foo", stringValue);
+    }
+
+    {
+        RefPtr<JSON::Value> value;
+        EXPECT_TRUE(JSON::Value::parseJSON(" 1", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("\t1", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("\n1", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("1 ", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("1\t", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("1\n", value));
+        EXPECT_TRUE(JSON::Value::parseJSON(" 1 ", value));
+        EXPECT_TRUE(JSON::Value::parseJSON(" {} ", value));
+        EXPECT_TRUE(JSON::Value::parseJSON(" [] ", value));
+
+        EXPECT_FALSE(JSON::Value::parseJSON("1 1", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("{} {}", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("[] []", value));
+    }
 }
 

trunk/Tools/TestWebKitAPI/Tests/WebCore/ApplicationManifestParser.cpp

@@ -207 +207 @@
     testDisplay("\"minimal-ui\"", ApplicationManifest::Display::MinimalUI);
     testDisplay("\"fullscreen\"", ApplicationManifest::Display::Fullscreen);
-
-    testDisplay("[\"\\tMINIMAL-ui\\t\"]", ApplicationManifest::Display::MinimalUI);
+    testDisplay("\"\t\nMINIMAL-UI \"", ApplicationManifest::Display::MinimalUI);
 }
 
@@ -287 +286 @@
 }
 
+TEST_F(ApplicationManifestParserTest, Whitespace)
+{
+    auto manifest = parseString(ASCIILiteral("  { \"name\": \"PASS\" }\n"));
+
+    EXPECT_STREQ("PASS", manifest.name.utf8().data());
+}
+
 #endif