From b00c11ad7c987976d8ff34a427bb5ffde4070455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Mon, 13 Nov 2023 13:39:29 +0100 Subject: [PATCH] deps: V8: cherry-pick d90d4533b053 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: Fix reading integer-indexed import assertions in dynamic import Use GetPropertyOrElement instead of GetProperty to read import assertion values from the import assertions object, to support cases in which the key is an integer index such as `"0"`. The added test case, when using GetProperty, triggers the following DCHECK in debug builds: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/objects/lookup-inl.h;l=108;drc=515f187ba067ee4a99fdf5198cca2c97abd342fd In release builds it silently fails to read the property, and thus throws about it not being a valid string. Bug: v8:14069 Change-Id: Ifd4645b7bd9bfd07f06fa33727441d27eabc4d32 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4614489 Reviewed-by: Victor Gomes Commit-Queue: Marja Hölttä Reviewed-by: Marja Hölttä Cr-Commit-Position: refs/heads/main@{#88267} Refs: https://github.com/v8/v8/commit/d90d4533b05301e2be813a5f90223f4c6c1bf63d PR-URL: https://github.com/nodejs/node/pull/50077 Reviewed-By: Ben Noordhuis Reviewed-By: Tobias Nießen Reviewed-By: Mohammed Keyvanzadeh Reviewed-By: Rafael Gonzaga Reviewed-By: Yagiz Nizipli Reviewed-By: Michael Dawson Reviewed-By: Richard Lau --- common.gypi | 2 +- deps/v8/src/execution/isolate.cc | 4 ++-- .../mjsunit/harmony/modules-import-assertions-dynamic-6.mjs | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/common.gypi b/common.gypi index 8da04166808183..4589f515178093 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.24', + 'v8_embedder_string': '-node.25', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/execution/isolate.cc b/deps/v8/src/execution/isolate.cc index 3c62ba5a5caef9..33ff1348f58989 100644 --- a/deps/v8/src/execution/isolate.cc +++ b/deps/v8/src/execution/isolate.cc @@ -5228,8 +5228,8 @@ MaybeHandle Isolate::GetImportAssertionsFromArgument( for (int i = 0; i < assertion_keys->length(); i++) { Handle assertion_key(String::cast(assertion_keys->get(i)), this); Handle assertion_value; - if (!JSReceiver::GetProperty(this, import_assertions_object_receiver, - assertion_key) + if (!Object::GetPropertyOrElement(this, import_assertions_object_receiver, + assertion_key) .ToHandle(&assertion_value)) { // This can happen if the property has a getter function that throws // an error. diff --git a/deps/v8/test/mjsunit/harmony/modules-import-assertions-dynamic-6.mjs b/deps/v8/test/mjsunit/harmony/modules-import-assertions-dynamic-6.mjs index 3388aefb5c5b13..76a0eddb0fdc34 100644 --- a/deps/v8/test/mjsunit/harmony/modules-import-assertions-dynamic-6.mjs +++ b/deps/v8/test/mjsunit/harmony/modules-import-assertions-dynamic-6.mjs @@ -8,6 +8,11 @@ var life; import('modules-skip-1.json', { assert: { type: 'json', notARealAssertion: 'value' } }).then( namespace => life = namespace.default.life); +var life2; +import('modules-skip-1.json', { assert: { 0: 'value', type: 'json' } }).then( + namespace => life2 = namespace.default.life); + %PerformMicrotaskCheckpoint(); assertEquals(42, life); +assertEquals(42, life2);