512447 - JSON.stringify does not correct handle replacer functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Oliver Hunt]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6; en-us) AppleWebKit/531.5 (KHTML, like Gecko) Version/4.0.3 Safari/531.7
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6; en-us) AppleWebKit/531.5 (KHTML, like Gecko) Version/4.0.3 Safari/531.7 Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20090824 Minefield/3.7a1pre

The linked test case shows that spidermonkey's JSON object is not detecting the cyclic object being returned by the reviver function

Reproducible: Always

Steps to Reproduce:
1. Run the testcase at http://trac.webkit.org/export/47743/trunk/LayoutTests/fast/js/JSON-stringify-replacer.html
2.  Note the last 3 failures
Actual Results:  
The JSON object should be throwing an exception due to the infinite recursion, but is not.

Expected Results:  
All the tests should pass.

Comment 1

[Saint Wesonga]

Attachment: Patch v1

Am I correct in assuming that a test for this should be added to /dom/src/json/test/unit/test_replacer.js

Comment 2

[Saint Wesonga]

Attachment: Patch

Comment 3

[Robert Sayre]

I'll get to these JSON patches soon. thanks for them!

Comment 4

[Saint Wesonga]

Attachment: Patch

Applies cleanly

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 417298 [details] [diff] [review]
Patch

Does this regress bug 493586?  My mind is still grappling a little with the streamingness of SpiderMonkey's implementation versus the spec's from-building-blocks method, but I think it does.

Comment 6

[John J. Barton]

Requesting blocking as way of checking to see that this is on some one's todo list.

Comment 7

[Saint Wesonga]

(In reply to comment #5)
> (From update of attachment 417298 [details] [diff] [review])
> Does this regress bug 493586?  My mind is still grappling a little with the
> streamingness of SpiderMonkey's implementation versus the spec's
> from-building-blocks method, but I think it does.

I just verified that all JSON xpcshell tests pass with this patch (including the one added in bug 493586). Not sure if there's something else I'm missing.

Comment 8

[Damon Sicore (:damons)]

Sayre, can we just knock this one out?

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Or push it off to beta2? Not sure what the consequence of this not being in b1 is ...

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/42371f85bbfb