Bug 187724

Summary:

JSON.stringify should emit non own properties if second array argument includes

Product:

WebKit

Reporter:

zac spitzer <zac.spitzer>

Component:

JavaScriptCore

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, sam, webkit-bug-importer, youennf, ysuzuki

Priority:

Keywords:

InRadar

Version:

Safari 11

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=187807

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews206 for win-future	none

Description zac spitzer 2018-07-16 19:43:52 PDT

The following test case produces an empty object in Webkit

https://jsbin.com/qiluqen/edit?js,console

Firefox, Edge and Chrome all output an object with three properties

function createSyntaxException() {
    try {
        // trigger a DOMException
        document.querySelectorAll("div:foo");
    } catch(e) {
        // this produces an empty object in webkit
        var err = JSON.parse(
          JSON.stringify(e, ['code','name','message'] )
        );      
        console.log(err);
        console.log(JSON.stringify(err));        
        var err2 = {
          code: e.code,
          name: e.name,
          message: e.message
        };
        console.log(err2);
    }
}
throw createSyntaxException();

Comment 1 Radar WebKit Bug Importer 2018-07-17 16:56:03 PDT

<rdar://problem/42310909>

Comment 2 Yusuke Suzuki 2018-07-17 17:22:14 PDT

It seems that WebKit behavior is correct, but I would like to hear opinions from binding folks. My understanding is the following.

According to DOMException's IDL, defined in WebIDL.

https://heycam.github.io/webidl/#idl-DOMException

interface DOMException { // but see below note about ECMAScript binding
  readonly attribute DOMString name;
  readonly attribute DOMString message;
  readonly attribute unsigned short code;

  const unsigned short INDEX_SIZE_ERR = 1;
...

This definition says that "name", "message", and "code" are readonly attributes. So they should be defined as getters in DOMException.prototype.

From the point of view of JSC, this makes `JSON.stringify(error)` empty since "name", "message", and "code" are not own properties.

Comment 3 zac spitzer 2018-07-17 20:22:30 PDT

but I'm passing in an array of ['code','name','message'] as a replacer to stringify which should pluck those values out?

Comment 4 Yusuke Suzuki 2018-07-17 23:33:54 PDT

Ah, I missed the point. The second argument array forces us to emit properties even if it is not an own property.

Comment 5 Yusuke Suzuki 2018-07-18 04:01:28 PDT

Created attachment 345235 [details]
Patch

Comment 6 EWS Watchlist 2018-07-18 05:37:24 PDT

Comment on attachment 345235 [details]
Patch

Attachment 345235 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8573642

New failing tests:
http/tests/security/local-video-source-from-remote.html

Comment 7 EWS Watchlist 2018-07-18 05:37:37 PDT

Created attachment 345237 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Comment 8 Mark Lam 2018-07-18 11:42:05 PDT

Comment on attachment 345235 [details]
Patch

r=me

Comment 9 Yusuke Suzuki 2018-07-18 11:45:09 PDT

Comment on attachment 345235 [details]
Patch

Thank you!

Comment 10 WebKit Commit Bot 2018-07-18 12:01:41 PDT

Comment on attachment 345235 [details]
Patch

Clearing flags on attachment: 345235

Committed r233924: <https://trac.webkit.org/changeset/233924>

Comment 11 WebKit Commit Bot 2018-07-18 12:01:43 PDT

All reviewed patches have been landed.  Closing bug.