[Webkit-unassigned] [Bug 180539] ApplicationManifestParser should strip whitespace from the raw input

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=180539

--- Comment #4 from David Quesada <david_quesada at apple.com> ---
(In reply to Joseph Pecoraro from comment #3)
> Comment on attachment 328735 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328735&action=review
> 
> r=me
> 
> > Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:60
> >      RefPtr<JSON::Value> jsonValue;
> > -    if (!JSON::Value::parseJSON(text, jsonValue)) {
> > +    if (!JSON::Value::parseJSON(text.stripWhiteSpace(), jsonValue)) {
> 
> Does this actually change behavior? parseJSON looks to ignore leading
> whitespace:
> 
> > Token parseToken(const UChar* start, const UChar* end, const UChar** tokenStart, const UChar** tokenEnd)
> > {
> >     while (start < end && isSpaceOrNewline(*start))
> >         ++start;
> >     ...
> 
> It might also be good to have a link to the spec at the top of this file
> somewhere for quick reference. I suspect that is:
> https://w3c.github.io/manifest/
> 
> This part appears to be:
> https://w3c.github.io/manifest/#dfn-obtaining-the-manifest
> 
> 1. Let json be the result of parsing text. If parsing throws an error: ...
> 
> And here parseJSON(text) should satisfy this without the stripWhiteSpace.

The issue is with trailing whitespace. JSON::Value::parseJSON() (JSONValues.cpp:531) appears to fail if there are any characters remaining after parsing the value:

    const UChar* start = characters;
    const UChar* end = start + jsonInput.length();
    const UChar* tokenEnd;
    auto result = buildValue(start, end, &tokenEnd, 0);
    if (!result || tokenEnd != end)
        return false;

where actual JSON parsing seems to handle it just fine:

    > JSON.parse("\t[1,2,3] \n")
    < [1, 2, 3]

Also the fact that some manifests that apparently have trailing whitespace (e.g. https://www.progressivewebflap.com) parsed prior to switching to JSON::Value::parseJSON(), but not after, (but did again after this patch), means this *is* a change in behavior.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171208/a3a12e83/attachment-0001.html>