Problem with JSON.parse()

1,182 views
Skip to first unread message

Christian Grigis

unread,
Apr 21, 2011, 5:28:14 AM4/21/11
to mozilla-rhino
Dear Rhino developers and users,

While doing some tests using the JSON object in the CVS version of Rhino
1.7R3, I am running into some problems (similar to what I ran into some
time ago) when using "empty" strings as hash keys.

More specifically:

js> s = JSON.stringify({'a':1,'b':2,'':3,' ':4,' ':5})
{"a":1,"b":2,"":3," ":4," ":5}
js> o = JSON.parse(s)
[object Object]
js> o['a']
1
js> o['b']
2
js> o['']
js> for (var k in o) {print("'" + k + "'");}
'a'
'b'
'0'
js> o['0']
5
js> JSON.stringify(o)
{"a":1,"b":2,"0":5}

It appears that all empty-ish (whitespace) hash keys are parsed back in
the "0" key.

Doing the same thing using the Prototype JSON facility works as expected:

js> s = Object.toJSON({'a':1,'b':2,'':3,' ':4,' ':5})
{"a": 1, "b": 2, "": 3, " ": 4, " ": 5}
js> o = s.evalJSON()
[object Object]
js> o['a']
1
js> o['b']
2
js> o['']
3
js> for (var k in o) {print("'" + k + "'");}
'a'
'b'
''
' '
' '
js> Object.toJSON(o)
{"a": 1, "b": 2, "": 3, " ": 4, " ": 5}


I fixed it with the following patch:

Index: src/org/mozilla/javascript/json/JsonParser.java
===================================================================
RCS file:
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/json/JsonParser.java,v
retrieving revision 1.9
diff -u -r1.9 JsonParser.java
--- src/org/mozilla/javascript/json/JsonParser.java 18 Jan 2010
09:44:06 -0000 1.9
+++ src/org/mozilla/javascript/json/JsonParser.java 21 Apr 2011 09:11:41
-0000
@@ -146,7 +146,7 @@

double d = ScriptRuntime.toNumber(id);
int index = (int) d;
- if (d != index) {
+ if ((id.trim().length() == 0) || (d != index)) {
object.put(id, object, value);
} else {
object.put(index, object, value);


What do you think?

Thanks and best regards,

-Christian

--
Christian Grigis
Senior Software Engineer
NEXThink S.A. -- http://www.nexthink.com/

Hannes Wallnoefer

unread,
Apr 21, 2011, 7:44:58 AM4/21/11
to mozill...@googlegroups.com
Thanks for the report, Christian.

I just committed a patch that makes JSON.parse() use the same name ->
index conversion logic that is used for object literals.

This is stricter than ScriptRuntime.toNumber() in that it only
converts to an index if the string actually looks like a number, and
it will also leave "00" or " 0" as strings.

Hannes

2011/4/21 Christian Grigis <christia...@nexthink.com>:

Christian Grigis

unread,
Apr 21, 2011, 8:36:32 AM4/21/11
to mozill...@googlegroups.com
Hello Hannes,

On 04/21/2011 01:44 PM, Hannes Wallnoefer wrote:
> I just committed a patch that makes JSON.parse() use the same name ->
> index conversion logic that is used for object literals.
>
> This is stricter than ScriptRuntime.toNumber() in that it only
> converts to an index if the string actually looks like a number, and
> it will also leave "00" or " 0" as strings.

Thank you for the fix, that is much better indeed. :)

I now have another weirdness however:

js> JSON.parse(JSON.stringify({"a": 100}))["a"]
100
js> JSON.parse(JSON.stringify({"1": 100}))["1"]
100
js> JSON.parse(JSON.stringify({"-1": 100}))["-1"]
js>

Even though:
js> JSON.stringify(JSON.parse(JSON.stringify({"-1": 100})))
{"-1":100}

Negative keys are not handled correctly, in a way that looks similar to
the problem I had in
http://groups.google.com/group/mozilla.dev.tech.js-engine.rhino/browse_thread/thread/efbe638cabd8cfda/598e07fdc8f33027

Same issue?

Thank you and best regards,

Hannes Wallnoefer

unread,
Apr 21, 2011, 9:15:28 AM4/21/11
to mozill...@googlegroups.com
Turns out I hadn't fully understood how ScriptRuntime.indexToString
works. It actually does converts negative index keys (which current
Spidermonkey doesn't), so you have to check for >= 0 before casting to
int, not afterwards.

This should be fixed in CVS now. Thanks for checking.

Hannes

2011/4/21 Christian Grigis <christia...@nexthink.com>:

Christian Grigis

unread,
Apr 21, 2011, 11:17:26 AM4/21/11
to mozill...@googlegroups.com
Hello Hannes,

On 04/21/2011 03:15 PM, Hannes Wallnoefer wrote:
> Turns out I hadn't fully understood how ScriptRuntime.indexToString
> works. It actually does converts negative index keys (which current
> Spidermonkey doesn't), so you have to check for>= 0 before casting to
> int, not afterwards.
>
> This should be fixed in CVS now. Thanks for checking.

It looks like it is working beautifully.

Thank you, and have great holidays!

Reply all
Reply to author
Forward
0 new messages