Dynamic import attributes broken with multiple attributes

I was trying to adapt a V8 test to run it in Node.js

The following version fails in a C++ CHECK (it doesn't matter if the json file exists or not):

import assert from 'assert';

var life;
import('./modules-skip-1.json', { with: { type: 'json', notARealAssertion: 'value' } }).then(
    namespace => life = namespace.default.life).then(() => {
      assert.strictEqual(life, 42);
    });

var life2;
import('./modules-skip-1.json', { with: { 0: 'value', type: 'json' } }).then(
    namespace => life2 = namespace.default.life).then(() => {
      assert.strictEqual(life2, 42);
    });

#
# Fatal error in , line 0
# Check failed: i < self->length().
#
#
#
#FailureMessage Object: 0x16d934a68
 1: 0x1025fad74 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 2: 0x1036dbcc4 V8_Fatal(char const*, ...) [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 3: 0x10273d8bc v8::FixedArray::Get(v8::Local<v8::Context>, int) const [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 4: 0x10255ad78 node::loader::createImportAttributesContainer(node::Environment*, v8::Isolate*, v8::Local<v8::FixedArray>) [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 5: 0x10255c3d8 node::loader::ImportModuleDynamically(v8::Local<v8::Context>, v8::Local<v8::Data>, v8::Local<v8::Value>, v8::Local<v8::String>, v8::Local<v8::FixedArray>) [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 6: 0x102895ab0 v8::internal::Isolate::RunHostImportModuleDynamicallyCallback(v8::internal::MaybeHandle<v8::internal::Script>, v8::internal::Handle<v8::internal::Object>, v8::internal::MaybeHandle<v8::internal::Object>) [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 7: 0x102cc3670 v8::internal::Runtime_DynamicImportCall(int, unsigned long*, v8::internal::Isolate*) [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]
 8: 0x103057768 Builtins_CEntry_Return1_ArgvInRegister_NoBuiltinExit [/Users/mzasso/.volta/tools/image/node/21.1.0/bin/node]

This happens in Node.js 21 and v20.x-staging.

I thought this would be easy to fix when I saw

node/src/module_wrap.cc

Line 256 in 0304da2

for (int i = 0; i < raw_attributes->Length(); i += 3) {

But this is not just an off-by-one issue.

For import(jsModuleDataUrl, { with: { type: 'json', other: 'unsupported' } }) (test I added), the raw_attributes array contains 4 elements.

For import(`data:text/javascript,import${JSON.stringify(jsModuleDataUrl)}with{type:"json"}`) (existing test), it contains 3 elements.

I don't understand how it's possible. The array is guaranteed to have a size that's a multiple of two:

node/deps/v8/src/execution/isolate.cc

Lines 5446 to 5448 in 0304da2

    
           constexpr size_t kAssertionEntrySizeForDynamicImport = 2; 
        
           import_assertions_array = factory()->NewFixedArray(static_cast<int>( 
        
               assertion_keys->length() * kAssertionEntrySizeForDynamicImport));

Debug build makes it a bit more understandable:

$ ./node_g test/es-module/test-esm-import-attributes-errors.mjs
FATAL ERROR: v8::String::Cast Value is not a String
----- Native stack trace -----

 1: 0x100645944 node::DumpNativeBacktrace(__sFILE*) [/Users/mzasso/git/nodejs/node/out/Debug/node]
 2: 0x100784638 node::Abort() [/Users/mzasso/git/nodejs/node/out/Debug/node]
 3: 0x100784798 node::OOMErrorHandler(char const*, v8::OOMDetails const&) [/Users/mzasso/git/nodejs/node/out/Debug/node]
 4: 0x100b344fc v8::String::CheckCast(v8::Data*) [/Users/mzasso/git/nodejs/node/out/Debug/node]
 5: 0x1005fd248 v8::String::Cast(v8::Data*) [/Users/mzasso/git/nodejs/node/out/Debug/node]
 6: 0x1006ed5e8 v8::Local<v8::String> v8::Local<v8::String>::Cast<v8::Data>(v8::Local<v8::Data>) [/Users/mzasso/git/nodejs/node/out/Debug/node]
 7: 0x1006ed590 v8::Local<v8::String> v8::Local<v8::Data>::As<v8::String>() const [/Users/mzasso/git/nodejs/node/out/Debug/node]
 8: 0x1006ea2dc node::loader::createImportAttributesContainer(node::Environment*, v8::Isolate*, v8::Local<v8::FixedArray>) [/Users/mzasso/git/nodejs/node/out/Debug/node]

Found the issue: #50703

src: iterate on import attributes array correctly

Navigation Menu

Repository navigation

Dynamic `import` attributes broken with multiple attributes #50700

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	constexpr size_t kAssertionEntrySizeForDynamicImport = 2;
	import_assertions_array = factory()->NewFixedArray(static_cast<int>(
	assertion_keys->length() * kAssertionEntrySizeForDynamicImport));

Dynamic import attributes broken with multiple attributes #50700

Description

Activity

targos commented on Nov 13, 2023

targos commented on Nov 13, 2023

targos commented on Nov 13, 2023

targos commented on Nov 13, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Dynamic `import` attributes broken with multiple attributes #50700