513549 - Rhino's new JSON.parse breaks on trailing whitespace

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Daniel Friesen]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.13) Gecko/2009080315 Ubuntu/9.04 (jaunty) Firefox/3.0.13 FirePHP/0.3
Build Identifier: Rhino 1.7 release 3 PRERELEASE 2009 08 05

The JSON.parse in CVS breaks when a JSON string has trailing whitespace.

Reproducible: Always

Steps to Reproduce:
1. Open up the javascript shell
2. Type in `JSON.parse("{} ");
3. Type in `JSON.parse("{}\n");
Actual Results:  
Rhino throws a "SyntaxError: Expected end of stream at char 2" indicating JSON.parse considers the whitespace after the last } to be invalid JSON.

Expected Results:  
Both JSON.parse calls should return a new empty object.

SpiderMonkey's JSON.parse handles trailing whitespace fine. Also I don't believe trailing whitespace is supposed to be considered illegal. After all whitespace doesn't break things anywhere else in a JSON string, why should trailing whitespace be any different?

This causes issues when reading JSON from files. On POSIX systems files normally end in a newline and most reading methods end up reading in a string with a trailing newline, thus if you try to read a file then JSON.parse it using Rhino's new JSON system you'll get an unexpected (and hard to understand) error.

Comment 1

[Hannes Wallnoefer]

Thanks for the report, JSON source is now trimmed before parsing.

Checking in src/org/mozilla/javascript/json/JsonParser.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/json/JsonParser.java,v  <--  JsonParser.java
new revision: 1.4; previous revision: 1.3

Comment 2

[Daniel Friesen]

Great, I'll update rhino in MonkeyScript and pull out my workaround (trimming it with .trim() first) once the commit makes it's way into the git-mirror.

Comment 3

[Daniel Friesen]

This bug looks like it has cropped up again and json ending with a \n is throwing errors again.

Comment 4

[Hannes Wallnoefer]

Norris converted it back with the following commit message:

Fix regression in unit test.
Patch from Raphael Speyer. 

I don't think it makes much sense, but I trust Raphael on ES5 issues (and haven't actually made the effort to read the spec).

Comment 5

[rspeyer]

Attachment: Ignore trailing whitespace when parsing JSON

Sorry, only just saw this bug now. As far as I can tell, whitespace at the beginning and end should be ignored. Paragraph 2 of 5.1.2 seems to imply that, and that's what other implementations do. 

I didn't trim so that the character offset of errors could still be reported correctly.

Comment 6

[rspeyer]

Attachment: made consumption of whitespace a little more robust

Comment 7

[Norris Boyd]

Thanks!

Checking in testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java;
/cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java,v  <--  JsonParserTest.java
new revision: 1.3; previous revision: 1.2
done
Checking in src/org/mozilla/javascript/json/JsonParser.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/json/JsonParser.java,v  <--  JsonParser.java
new revision: 1.6; previous revision: 1.5
done

Comment 8

[Hannes Wallnoefer]

Thanks for setting this straight, Raphael and Norris! After you undid my simple trim() patch, I wrongly assumed that ES5 would not allow leading/trailing whitespace. I never actually checked with the spec, although I definitely should have.