[webkit-reviews] review granted: [Bug 180539] ApplicationManifestParser should strip whitespace from the raw input : [Attachment 328735] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted David Quesada
<david_quesada at apple.com>'s request for review:
Bug 180539: ApplicationManifestParser should strip whitespace from the raw
input
https://bugs.webkit.org/show_bug.cgi?id=180539

Attachment 328735: Patch

https://bugs.webkit.org/attachment.cgi?id=328735&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 328735
  --> https://bugs.webkit.org/attachment.cgi?id=328735
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.