If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Rhino's new JSON.parse breaks on trailing whitespace

RESOLVED FIXED

Status

Rhino
Core
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Daniel Friesen, Unassigned)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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

8 years ago
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
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 2

8 years ago
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.
(Reporter)

Comment 3

8 years ago
This bug looks like it has cropped up again and json ending with a \n is throwing errors again.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---

Comment 4

8 years ago
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

8 years ago
Created attachment 415249 [details] [diff] [review]
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

8 years ago
Created attachment 415299 [details] [diff] [review]
made consumption of whitespace a little more robust
Attachment #415249 - Attachment is obsolete: true

Comment 7

8 years ago
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
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED

Comment 8

8 years ago
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.
You need to log in before you can comment on or make changes to this bug.