From 96514a8b9fdbd19e4bf7171d4ab935c4c9c7f973 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Sun, 19 Nov 2023 22:22:49 +0100
Subject: [PATCH] src: iterate on import attributes array correctly

The array's length is supposed to be a multiple of two for dynamic
import callbacks.

Fixes: https://github.com/nodejs/node/issues/50700
PR-URL: https://github.com/nodejs/node/pull/50703
Backport-PR-URL: https://github.com/nodejs/node/pull/51136
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/module_wrap.cc                                   | 12 ++++++++----
 test/es-module/test-esm-import-attributes-errors.js  |  5 +++++
 test/es-module/test-esm-import-attributes-errors.mjs |  5 +++++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index cf9425f4786144..eae79b0fc765db 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -250,10 +250,14 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
 }
 
 static Local<Object> createImportAttributesContainer(
-    Environment* env, Isolate* isolate, Local<FixedArray> raw_attributes) {
+    Environment* env,
+    Isolate* isolate,
+    Local<FixedArray> raw_attributes,
+    const int elements_per_attribute) {
+  CHECK_EQ(raw_attributes->Length() % elements_per_attribute, 0);
   Local<Object> attributes =
       Object::New(isolate, v8::Null(env->isolate()), nullptr, nullptr, 0);
-  for (int i = 0; i < raw_attributes->Length(); i += 3) {
+  for (int i = 0; i < raw_attributes->Length(); i += elements_per_attribute) {
     attributes
         ->Set(env->context(),
               raw_attributes->Get(env->context(), i).As<String>(),
@@ -299,7 +303,7 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
 
     Local<FixedArray> raw_attributes = module_request->GetImportAssertions();
     Local<Object> attributes =
-        createImportAttributesContainer(env, isolate, raw_attributes);
+        createImportAttributesContainer(env, isolate, raw_attributes, 3);
 
     Local<Value> argv[] = {
         specifier,
@@ -583,7 +587,7 @@ static MaybeLocal<Promise> ImportModuleDynamically(
       options->Get(context, HostDefinedOptions::kID).As<Symbol>();
 
   Local<Object> attributes =
-      createImportAttributesContainer(env, isolate, import_attributes);
+      createImportAttributesContainer(env, isolate, import_attributes, 2);
 
   Local<Value> import_args[] = {
       id,
diff --git a/test/es-module/test-esm-import-attributes-errors.js b/test/es-module/test-esm-import-attributes-errors.js
index 4521248db176e5..976d4a3f67f778 100644
--- a/test/es-module/test-esm-import-attributes-errors.js
+++ b/test/es-module/test-esm-import-attributes-errors.js
@@ -26,6 +26,11 @@ async function test() {
     { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
   );
 
+  await rejects(
+    import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }),
+    { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
+  );
+
   await rejects(
     import(jsModuleDataUrl, { with: { type: 'unsupported' } }),
     { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }
diff --git a/test/es-module/test-esm-import-attributes-errors.mjs b/test/es-module/test-esm-import-attributes-errors.mjs
index ff932636e39a5f..072b94b1b0fc55 100644
--- a/test/es-module/test-esm-import-attributes-errors.mjs
+++ b/test/es-module/test-esm-import-attributes-errors.mjs
@@ -21,6 +21,11 @@ await rejects(
   { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
 );
 
+await rejects(
+  import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }),
+  { code: 'ERR_IMPORT_ASSERTION_TYPE_FAILED' }
+);
+
 await rejects(
   import(import.meta.url, { with: { type: 'unsupported' } }),
   { code: 'ERR_IMPORT_ASSERTION_TYPE_UNSUPPORTED' }