Skip to content

Commit 12daa54

Browse files
joyeecheungRafaelGSS
authored andcommitted
deps: V8: backport 93cd21e8254b
Original commit message: skip fast transition path for escaped keys JSDataObjectBuilder's fast transition path previously compared undecoded source bytes of an escaped key against existing transitions. GetKeyChars() returned undecoded source bytes sliced using the decoded length, so escaped keys could incorrectly match persisted transitions and corrupt the keys of subsequent JSON.parse() calls. This patch fixes it by skipping the fast path when the key has escape characters. Co-Authored-By: 3zrvvv@gmail.com Bug: 521080746 Change-Id: I77720cc34758180224ca8e0e5de0af48556ed013 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8412610 Commit-Queue: Joyee Cheung <joyee@igalia.com> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/heads/main@{#109925} Refs: v8/v8@93cd21e Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com> PR-URL: nodejs-private/node-private#999 Refs: https://hackerone.com/reports/3799145 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent 818e4ca commit 12daa54

4 files changed

Lines changed: 89 additions & 2 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444

4545
# Reset this number to 0 on major V8 upgrades.
4646
# Increment by one for each non-official patch applied to deps/v8.
47-
'v8_embedder_string': '-node.39',
47+
'v8_embedder_string': '-node.40',
4848

4949
##### V8 defaults for Node.js #####
5050

‎deps/v8/src/json/json-parser.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,12 @@ class JSDataObjectBuilder {
10671067
descriptor_index)),
10681068
isolate_);
10691069
target_map = expected_final_map_;
1070-
} else {
1070+
} else if (key_chars.data() != nullptr) {
1071+
// The byte-compare transition fast path is only valid when `key_chars`
1072+
// faithfully represent the decoded key. Callers that cannot provide valid
1073+
// chars (the JSON parser passes an empty vector for escaped keys, whose raw
1074+
// source bytes differ from the decoded key) fall through to the
1075+
// decode-then-FindTransitionToField path below.
10711076
TransitionsAccessor transitions(isolate_, *map_);
10721077
auto expected_transition = transitions.ExpectedTransition(key_chars);
10731078
if (!expected_transition.first.is_null()) {

‎deps/v8/src/json/json-parser.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,12 @@ class JsonParser final {
376376
JsonString ScanJsonPropertyKey(JsonContinuation* cont);
377377
base::uc32 ScanUnicodeCharacter();
378378
base::Vector<const Char> GetKeyChars(JsonString key) {
379+
// For escaped keys the source range starting at `key.start()` holds the raw
380+
// (undecoded) characters while `key.length()` is the decoded length, so the
381+
// bytes here do not represent the actual decoded key. Return an empty
382+
// vector to signal that the byte-compare transition fast path must be
383+
// skipped (see JSDataObjectBuilder::TryFastTransitionToPropertyKey).
384+
if (key.has_escape()) return base::Vector<const Char>();
379385
return base::Vector<const Char>(chars_ + key.start(), key.length());
380386
}
381387
Handle<String> MakeString(const JsonString& string,
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright 2026 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Regression test: JSON.parse must not match a hidden-class transition for an
6+
// escaped property key using the raw (undecoded) source bytes. The builder's
7+
// fast transition path compared `chars_ + key.start()` over `key.length()`
8+
// (the *decoded* length), so escaped keys (e.g. "\t", "\uXXXX") collapsed to
9+
// the byte 0x5C and matched a previously-seen "\\" key, making JSON.parse adopt
10+
// the wrong key. Transitions are isolate-global, so a prior parse corrupted a
11+
// later, unrelated parse.
12+
13+
// A single-char escaped key must keep its decoded value even after a sibling
14+
// "\\" (backslash) key has been parsed on the same prefix map.
15+
(function TestEscapedKeyNotRenamedByPriorBackslashKey() {
16+
function canaryKeys() {
17+
return Object.keys(JSON.parse('{"\\r":1,"\\f":2}'));
18+
}
19+
assertArrayEquals(['\r', '\f'], canaryKeys());
20+
// Plants a {<prefix>} -> "\" transition in the global transition tree.
21+
JSON.parse('{"\\r":3,"\\\\":4}');
22+
// Must be unaffected: "\f" stays U+000C, not U+005C.
23+
assertArrayEquals(['\r', '\f'], canaryKeys());
24+
})();
25+
26+
// Multi-character escaped key: decoded length 2 must not be compared against
27+
// the first two raw bytes ("\" + "r").
28+
(function TestMultiCharEscapedKey() {
29+
function keys() {
30+
return Object.keys(JSON.parse('{"\\t":1,"\\r\\r":2}'));
31+
}
32+
assertArrayEquals(['\t', '\r\r'], keys());
33+
JSON.parse('{"\\t":1,"\\\\r":2}'); // plants the literal "\r" (0x5C 0x72) key
34+
assertArrayEquals(['\t', '\r\r'], keys());
35+
})();
36+
37+
// \uXXXX-escaped single-character key (here decodes to plain 'A').
38+
(function TestUnicodeEscapedKey() {
39+
function keys() {
40+
return Object.keys(JSON.parse('{"x":1,"\\u0041":2}'));
41+
}
42+
assertArrayEquals(['x', 'A'], keys());
43+
JSON.parse('{"x":1,"\\\\":2}');
44+
assertArrayEquals(['x', 'A'], keys());
45+
})();
46+
47+
// Override/collide: an escaped key must not be renamed onto a real "\" sibling
48+
// and silently destroy a property (key count must be preserved).
49+
(function TestNoPropertyDestruction() {
50+
// Prime the transition state that previously triggered the collapse.
51+
JSON.parse('{"p":1,"\\\\":0}');
52+
JSON.parse('{"p":1,"\\\\":0,"\\t":0}');
53+
const o = JSON.parse('{"p":1,"\\t":"a","\\\\":"b"}');
54+
assertEquals(3, Object.keys(o).length);
55+
assertEquals('a', o['\t']);
56+
assertEquals('b', o['\\']);
57+
})();
58+
59+
// Self-contained single document: a later sibling object value must not be
60+
// corrupted by an earlier one within the same parse.
61+
(function TestNestedSiblingNotCorrupted() {
62+
const o = JSON.parse(
63+
'{"a":{"p":1,"\\\\":9},"b":{"p":1,"\\\\":"A","\\t":7},' +
64+
'"c":{"p":1,"\\t":7,"\\\\":"B"}}');
65+
assertEquals(3, Object.keys(o.c).length);
66+
assertEquals(7, o.c['\t']);
67+
assertEquals('B', o.c['\\']);
68+
})();
69+
70+
// A plain (unescaped) empty key must still work (regression guard for the
71+
// empty-key_chars path the fix relies on).
72+
(function TestEmptyKeyStillWorks() {
73+
const o = JSON.parse('{"a":1,"":2,"b":3}');
74+
assertArrayEquals(['a', '', 'b'], Object.keys(o));
75+
assertEquals(2, o['']);
76+
})();

0 commit comments

Comments
 (0)