Bug 51671 - <VT> and <FF> are not valid JSON whitespace characters
Summary: <VT> and <FF> are not valid JSON whitespace characters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 41876
  Show dependency treegraph
 
Reported: 2010-12-28 01:05 PST by Helder Correia
Modified: 2010-12-29 23:14 PST (History)
3 users (show)

See Also:


Attachments
Patch (3.73 KB, patch)
2010-12-28 01:10 PST, Helder Correia
no flags Details | Formatted Diff | Diff
Add suggestions from comment #3 (link to JSON spec) (3.93 KB, patch)
2010-12-28 14:12 PST, Helder Correia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helder Correia 2010-12-28 01:05:35 PST
Vertical Tab and Form Feed are not considered allowed white spaces by the JSON spec (section 2: JSON Grammar): http://www.ietf.org/rfc/rfc4627.txt
Comment 1 Helder Correia 2010-12-28 01:10:10 PST
Created attachment 77543 [details]
Patch
Comment 2 Alexey Proskuryakov 2010-12-28 11:40:35 PST
At least part of this was already being tracked as bug 41102.
Comment 3 Eric Seidel (no email) 2010-12-28 13:50:51 PST
Comment on attachment 77543 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77543&action=review

The fix looks fine.  But making isAllowedSpace clearer will help make this code easier to understand for future peopel hacking here.

> JavaScriptCore/runtime/LiteralParser.cpp:38
> +static inline bool isAllowedSpace(const UChar c)

Seems this name should more clearly state what type of allowed space this is.  is this a JSON Whitespace?  is this a SomeSpecificSpecNameWhitespace?  It would be best to provide a link next to this function where the list of whitespace chars are defined.  How do I validate that this is the right list when I'm looking at this code a month from now?
Comment 4 Helder Correia 2010-12-28 14:12:38 PST
Created attachment 77576 [details]
Add suggestions from comment #3 (link to JSON spec)
Comment 5 Eric Seidel (no email) 2010-12-29 01:00:41 PST
Comment on attachment 77576 [details]
Add suggestions from comment #3 (link to JSON spec)

Perfect!  Thanks!
Comment 6 WebKit Commit Bot 2010-12-29 01:31:07 PST
The commit-queue encountered the following flaky tests while processing attachment 77576 [details]:

http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2010-12-29 01:32:25 PST
Comment on attachment 77576 [details]
Add suggestions from comment #3 (link to JSON spec)

Clearing flags on attachment: 77576

Committed r74737: <http://trac.webkit.org/changeset/74737>
Comment 8 WebKit Commit Bot 2010-12-29 01:32:31 PST
All reviewed patches have been landed.  Closing bug.