diff --git a/releases/releases.yaml b/releases/releases.yaml index 2d3530d8..10143f29 100644 --- a/releases/releases.yaml +++ b/releases/releases.yaml @@ -1374,6 +1374,7 @@ duktape_releases: - "Accept unlabelled function statements outside of top level for strict functions (using hoist semantics), previously they were rejected with a SyntaxError (GH-2213)" - "Accept unescaped U+2028 and U+2029 in string literals so that all JSON.stringify() output parses with eval() (ES2019) (GH-2235)" - "Don't treat U+180E as whitespace e.g. for String trim() purposes, (ES2016, requires Unicode 8.0.0 or higher) (GH-2236)" + - "Align JSON.stringify() Array 'replacer' setup with the latest specification (GH-2379)" - "Use wasm for dukweb.js compilation (including duktape.org site), fix async loading of emcc-compiled code in dukweb.html (GH-2244)" - - "Fix duk_opcodes.yaml metadata for TRYCATCH and CALLn (GH-2277)" - "Improve DUK_USE_OS_STRING for macOS, iOS, watchOS, and tvOS (GH-2288)" + - "Fix JSON.stringify() handling of Array 'replacer' duplicates (e.g. JSON.stringify({foo: 123}, [\"foo\", \"foo\"])); previously incorrectly serialized multiple times, now only once (GH-2379)" diff --git a/src-input/duk_bi_json.c b/src-input/duk_bi_json.c index e118c26e..3e6c5e05 100644 --- a/src-input/duk_bi_json.c +++ b/src-input/duk_bi_json.c @@ -2903,6 +2903,50 @@ void duk_bi_json_parse_helper(duk_hthread *thr, DUK_ASSERT(duk_get_top(thr) == entry_top + 1); } +DUK_LOCAL void duk__json_setup_plist_from_array(duk_hthread *thr, duk_json_enc_ctx *js_ctx, duk_idx_t idx_replacer) { + /* ES5.1 required enumeration, later specification versions use an + * explicit index loop (and makes it clear inheritance is required). + */ + + duk_uarridx_t repl_len; + duk_uarridx_t repl_idx; + duk_uarridx_t plist_idx = 0; + + js_ctx->idx_proplist = duk_push_bare_array(thr); + (void) duk_push_bare_object(thr); + + /* [ ... proplist found ] */ + + repl_len = (duk_uarridx_t) duk_get_length(thr, idx_replacer); + for (repl_idx = 0; repl_idx < repl_len; repl_idx++) { + (void) duk_get_prop_index(thr, idx_replacer, repl_idx); + /* Accept strings, numbers, Strings, and Numbers, and ignore + * anything else. Reject duplicates. + */ + if (duk__json_enc_allow_into_proplist(DUK_GET_TVAL_NEGIDX(thr, -1))) { + (void) duk_to_string(thr, -1); /* extra coercion of strings is OK */ + duk_dup_top(thr); /* -> [ ... proplist found key key ] */ + (void) duk_get_prop(thr, -3); /* -> [ ... proplist found key found[key] ] */ + if (duk_to_boolean(thr, -1)) { + duk_pop_2_unsafe(thr); + } else { + duk_pop_unsafe(thr); + duk_dup_top(thr); + duk_push_true(thr); /* -> [ ... proplist found key key true ] */ + (void) duk_put_prop(thr, -4); /* -> [ ... proplist found key ] */ + (void) duk_put_prop_index(thr, -3, plist_idx); /* -> [ ... proplist found ] */ + plist_idx++; + } + } else { + duk_pop_unsafe(thr); + } + } + + duk_pop_unsafe(thr); + + /* [ ... proplist ] */ +} + DUK_INTERNAL void duk_bi_json_stringify_helper(duk_hthread *thr, duk_idx_t idx_value, @@ -3023,41 +3067,7 @@ void duk_bi_json_stringify_helper(duk_hthread *thr, if (DUK_HOBJECT_IS_CALLABLE(h)) { js_ctx->h_replacer = h; } else if (duk_js_isarray_hobject(h)) { - /* Here the specification requires correct array index enumeration - * which is a bit tricky for sparse arrays (it is handled by the - * enum setup code). We now enumerate ancestors too, although the - * specification is not very clear on whether that is required. - */ - - duk_uarridx_t plist_idx = 0; - duk_small_uint_t enum_flags; - - js_ctx->idx_proplist = duk_push_bare_array(thr); - - enum_flags = DUK_ENUM_ARRAY_INDICES_ONLY | - DUK_ENUM_SORT_ARRAY_INDICES; /* expensive flag */ - duk_enum(thr, idx_replacer, enum_flags); - while (duk_next(thr, -1 /*enum_index*/, 1 /*get_value*/)) { - /* [ ... proplist enum_obj key val ] */ - if (duk__json_enc_allow_into_proplist(duk_get_tval(thr, -1))) { - /* XXX: duplicates should be eliminated here */ - DUK_DDD(DUK_DDDPRINT("proplist enum: key=%!T, val=%!T --> accept", - (duk_tval *) duk_get_tval(thr, -2), - (duk_tval *) duk_get_tval(thr, -1))); - duk_to_string(thr, -1); /* extra coercion of strings is OK */ - duk_put_prop_index(thr, -4, plist_idx); /* -> [ ... proplist enum_obj key ] */ - plist_idx++; - duk_pop(thr); - } else { - DUK_DDD(DUK_DDDPRINT("proplist enum: key=%!T, val=%!T --> reject", - (duk_tval *) duk_get_tval(thr, -2), - (duk_tval *) duk_get_tval(thr, -1))); - duk_pop_2(thr); - } - } - duk_pop(thr); /* pop enum */ - - /* [ ... proplist ] */ + duk__json_setup_plist_from_array(thr, js_ctx, idx_replacer); } } diff --git a/tests/ecmascript/test-bi-json-enc-proplist-ancestor.js b/tests/ecmascript/test-bi-json-enc-proplist-ancestor.js index 7ca948af..3fd02e6a 100644 --- a/tests/ecmascript/test-bi-json-enc-proplist-ancestor.js +++ b/tests/ecmascript/test-bi-json-enc-proplist-ancestor.js @@ -3,18 +3,19 @@ 0 quux 1 foo 2 baz -{"quux":3,"foo":1,"baz":4} +{"quux":3,"foo":1} +1 foo +0 baz +2 baz +{"baz":4,"foo":1} ===*/ /* Should ancestor properties be enumerated when creating PropertyList? * Specification is a bit unclear on this point. See E5.1 Section 15.12.3, * main algorithm, step 4.b.ii. * - * We currently iterate ancestor array indexed properties too. - * - * Creating an array instance with a chosen internal prototype is apparently - * not possible. However, Array.prototype is itself an array and we can add - * a value to it. + * Newer specifications use an explicit index loop (to .length - 1) and make + * it clear ancestors are enumerated (but not beyond .length - 1). */ function propertyListAncestorTest() { @@ -22,23 +23,25 @@ function propertyListAncestorTest() { var k; var plist; - // baseline + // Baseline. print(JSON.stringify(obj, [ 'quux', 'foo' ])); - // this is not very common + // Plist will contain "0" and "1"; it will inherit "2" but it's >= .length so it's ignored + // by JSON.stringify(). Array.prototype[2] = 'baz'; - - // plist will contain "0" and "1", and will inherit "2" plist = [ 'quux', 'foo' ]; for (var k in plist) { print(k, plist[k]); } + print(JSON.stringify(obj, plist)); - // Now E5.1 Section 15.12.3, main algorithm, step 4.b.ii should presumably - // iterate indices "0" and "1" from the array and "2" from the ancestor. - // Note that 'length' plays no role in this. - // - // (Both Rhino and V8 fail this test.) + // If the gap is at < .length, it gets looked up from an ancestor. + Array.prototype[0] = 'baz'; + plist = []; + plist[1] = 'foo'; + for (var k in plist) { + print(k, plist[k]); + } print(JSON.stringify(obj, plist)); } diff --git a/tests/ecmascript/test-bi-json-enc-proplist-dups.js b/tests/ecmascript/test-bi-json-enc-proplist-dups.js index 7cb3d0bf..13f6cc4b 100644 --- a/tests/ecmascript/test-bi-json-enc-proplist-dups.js +++ b/tests/ecmascript/test-bi-json-enc-proplist-dups.js @@ -6,8 +6,6 @@ /* E5.1 Section 15.12.13, main algorithm, step 4.b.ii.5 requires that an * implementation detect duplicates in the property list and refuse to * serialize the same property name twice. - * - * Note: both V8 and Rhino fail this test (but in different ways). */ function dupTest1() { diff --git a/tests/ecmascript/test-bi-json-enc-proplist-large.js b/tests/ecmascript/test-bi-json-enc-proplist-large.js new file mode 100644 index 00000000..72cc2dba --- /dev/null +++ b/tests/ecmascript/test-bi-json-enc-proplist-large.js @@ -0,0 +1,27 @@ +/*=== +{"0":0,"prop0":0,"1":1,"prop1":1,"2":2,"prop2":2,"3":3,"prop3":3,"4":4,"prop4":4,"5":5,"prop5":5,"6":6,"prop6":6,"7":7,"prop7":7,"8":8,"prop8":8,"9":9,"prop9":9,"10":10,"prop10":10,"11":11,"prop11":11,"12":12,"prop12":12,"13":13,"prop13":13,"14":14,"prop14":14,"15":15,"prop15":15,"16":16,"prop16":16,"17":17,"prop17":17,"18":18,"prop18":18,"19":19,"prop19":19,"20":20,"prop20":20,"21":21,"prop21":21,"22":22,"prop22":22,"23":23,"prop23":23,"24":24,"prop24":24,"25":25,"prop25":25,"26":26,"prop26":26,"27":27,"prop27":27,"28":28,"prop28":28,"29":29,"prop29":29,"30":30,"prop30":30,"31":31,"prop31":31,"32":32,"prop32":32,"33":33,"prop33":33,"34":34,"prop34":34,"35":35,"prop35":35,"36":36,"prop36":36,"37":37,"prop37":37,"38":38,"prop38":38,"39":39,"prop39":39,"40":40,"prop40":40,"41":41,"prop41":41,"42":42,"prop42":42,"43":43,"prop43":43,"44":44,"prop44":44,"45":45,"prop45":45,"46":46,"prop46":46,"47":47,"prop47":47,"48":48,"prop48":48,"49":49,"prop49":49,"50":50,"prop50":50,"51":51,"prop51":51,"52":52,"prop52":52,"53":53,"prop53":53,"54":54,"prop54":54,"55":55,"prop55":55,"56":56,"prop56":56,"57":57,"prop57":57,"58":58,"prop58":58,"59":59,"prop59":59,"60":60,"prop60":60,"61":61,"prop61":61,"62":62,"prop62":62,"63":63,"prop63":63,"64":64,"prop64":64,"65":65,"prop65":65,"66":66,"prop66":66,"67":67,"prop67":67,"68":68,"prop68":68,"69":69,"prop69":69,"70":70,"prop70":70,"71":71,"prop71":71,"72":72,"prop72":72,"73":73,"prop73":73,"74":74,"prop74":74,"75":75,"prop75":75,"76":76,"prop76":76,"77":77,"prop77":77,"78":78,"prop78":78,"79":79,"prop79":79,"80":80,"prop80":80,"81":81,"prop81":81,"82":82,"prop82":82,"83":83,"prop83":83,"84":84,"prop84":84,"85":85,"prop85":85,"86":86,"prop86":86,"87":87,"prop87":87,"88":88,"prop88":88,"89":89,"prop89":89,"90":90,"prop90":90,"91":91,"prop91":91,"92":92,"prop92":92,"93":93,"prop93":93,"94":94,"prop94":94,"95":95,"prop95":95,"96":96,"prop96":96,"97":97,"prop97":97,"98":98,"prop98":98,"99":99,"prop99":99} +===*/ + +function test() { + var obj = { foo: 1, bar: 2, quux: 3, baz: 4 }; + var i; + + for (i = 0; i < 100; i++) { + obj[i] = i; + obj['prop' + i] = i; + } + + var plist = []; + for (i = 0; i < 1000000; i++) { + plist.push((i % 1000)); + plist.push('prop' + (i % 900)); + } + + print(JSON.stringify(obj, plist)); +} + +try { + test(); +} catch (e) { + print(e); +} diff --git a/tests/knownissues/test-bi-json-enc-proplist-dups-1.txt b/tests/knownissues/test-bi-json-enc-proplist-dups-1.txt deleted file mode 100644 index c1aee3ac..00000000 --- a/tests/knownissues/test-bi-json-enc-proplist-dups-1.txt +++ /dev/null @@ -1,4 +0,0 @@ -summary: dups in proplist not handled correctly ---- -{"bar":2,"foo":1,"baz":4,"baz":4,"baz":4,"quux":3} -{"bar":2,"1":5,"1":5,"1":5,"foo":1}