[modules][api] Implement HostGetSupportedImportAssertions

Implement the HostGetSupportedImportAssertions, whose purpose
is to filter the list of import assertions exposed to the embedder to
only those assertion with keys that the embedder recognizes. See
https://tc39.es/proposal-import-assertions/#sec-hostgetsupportedimportassertions.

This change doesn't actually implement it as a callback, but instead
passes the supported assertions during creation of the Isolate via
CreateParams. This expresses clearly the requirement that the supported
assertions must never change for the lifetime of the Isolate.

Note that we still need to maintain all assertions in a map
while parsing the import assertions clause, because duplicate keys for
an unsupported assertion still needs to be detected as a parse error. So,
the filtering is done later during
SourceTextModuleDescriptor::AstModuleRequest::Serialize.

The actual filtering algorithm simply iterates the assertions and the
supported assertion keys in a nested loop. There's currently only one
assertion in use ("type"), so there should be no reason to get too
clever here unless at least several more assertions are generally
supported.

Bug: v8:10958
Change-Id: I9a2d965e9d452718d0ddfe9dca55b7b4ed963019
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2572173Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#71776}

include/v8.h

@@ -8422,6 +8422,20 @@ class V8_EXPORT Isolate {
      * This is an experimental feature and may still change significantly.
      */
     std::shared_ptr<CppHeapCreateParams> cpp_heap_params;
+
+    /**
+     * This list is provided by the embedder to indicate which import assertions
+     * they want to handle. Only import assertions whose keys are present in
+     * supported_import_assertions will be included in the import assertions
+     * lists of ModuleRequests that will be passed to the embedder. If
+     * supported_import_assertions is left empty, then the embedder will not
+     * receive any import assertions.
+     *
+     * This corresponds to the list returned by the HostGetSupportedAssertions
+     * host-defined abstract operation:
+     * https://tc39.es/proposal-import-assertions/#sec-hostgetsupportedimportassertions
+     */
+    std::vector<std::string> supported_import_assertions;
   };
 
   /**

src/api/api.cc

@@ -8557,6 +8557,9 @@ void Isolate::Initialize(Isolate* isolate,
   i_isolate->set_api_external_references(params.external_references);
   i_isolate->set_allow_atomics_wait(params.allow_atomics_wait);
 
+  i_isolate->set_supported_import_assertions(
+      params.supported_import_assertions);
+
   i_isolate->heap()->ConfigureHeap(params.constraints);
   if (params.cpp_heap_params) {
     i_isolate->heap()->ConfigureCppHeap(params.cpp_heap_params);

src/ast/modules.cc

@@ -127,23 +127,62 @@ Handle<PrimitiveHeapObject> ToStringOrUndefined(LocalIsolate* isolate,
 template <typename LocalIsolate>
 Handle<ModuleRequest> SourceTextModuleDescriptor::AstModuleRequest::Serialize(
     LocalIsolate* isolate) const {
-  // The import assertions will be stored in this array in the form:
-  // [key1, value1, location1, key2, value2, location2, ...]
+  // Copy the assertions to a FixedArray, filtering out as we go the import
+  // assertions that are not supported by the embedder.
+  //
+  // This is O(m * n) where m is the number of import assertions in this
+  // request and n is the number of supported assertions.  Both m and n are
+  // expected to be quite small, with m being 0 in the common case and n
+  // currently expected to be at most 1 since "type" is the only import
+  // assertion in use at the time of this writing. So for now we go with
+  // this simple nested loop approach, which should not result in a larger
+  // than necessary performance cost unless more than a few additional
+  // import assertions are standardized.
+  //
+  // The import assertions will be stored in the new array in the form: [key1,
+  // value1, location1, key2, value2, location2, ...].
+  const std::vector<std::string>& supported_assertions =
+      isolate->supported_import_assertions();
   Handle<FixedArray> import_assertions_array =
       isolate->factory()->NewFixedArray(static_cast<int>(
           import_assertions()->size() * ModuleRequest::kAssertionEntrySize));
-
-  int i = 0;
+  int filtered_assertions_index = 0;
   for (auto iter = import_assertions()->cbegin();
-       iter != import_assertions()->cend();
-       ++iter, i += ModuleRequest::kAssertionEntrySize) {
-    import_assertions_array->set(i, *iter->first->string());
-    import_assertions_array->set(i + 1, *iter->second.first->string());
-    import_assertions_array->set(i + 2,
-                                 Smi::FromInt(iter->second.second.beg_pos));
+       iter != import_assertions()->cend(); ++iter) {
+    for (const std::string& supported_assertion : supported_assertions) {
+      Handle<String> assertion_key = iter->first->string();
+      if (assertion_key->IsEqualTo(Vector<const char>(
+              supported_assertion.c_str(), supported_assertion.length()))) {
+        import_assertions_array->set(filtered_assertions_index,
+                                     *iter->first->string());
+        import_assertions_array->set(filtered_assertions_index + 1,
+                                     *iter->second.first->string());
+        import_assertions_array->set(filtered_assertions_index + 2,
+                                     Smi::FromInt(iter->second.second.beg_pos));
+        filtered_assertions_index += ModuleRequest::kAssertionEntrySize;
+        break;
+      }
+    }
   }
+
+  Handle<FixedArray> shortened_import_assertions_array;
+  if (filtered_assertions_index < import_assertions_array->length()) {
+    // If we did filter out any assertions, create a FixedArray with the correct
+    // length. This should be rare, since it would be unexpected for developers
+    // to commonly use unsupported assertions.  Note, we don't use
+    // FixedArray::ShrinkOrEmpty here since FixedArray::Shrink isn't available
+    // on a LocalIsolate/LocalHeap.
+    shortened_import_assertions_array =
+        isolate->factory()->NewFixedArray(filtered_assertions_index);
+    import_assertions_array->CopyTo(0, *shortened_import_assertions_array, 0,
+                                    filtered_assertions_index);
+  } else {
+    shortened_import_assertions_array = import_assertions_array;
+  }
+
   return v8::internal::ModuleRequest::New(isolate, specifier()->string(),
-                                          import_assertions_array, position());
+                                          shortened_import_assertions_array,
+                                          position());
 }
 template Handle<ModuleRequest>
 SourceTextModuleDescriptor::AstModuleRequest::Serialize(Isolate* isolate) const;

src/execution/isolate.h

@@ -1590,6 +1590,14 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void set_allow_atomics_wait(bool set) { allow_atomics_wait_ = set; }
   bool allow_atomics_wait() { return allow_atomics_wait_; }
 
+  void set_supported_import_assertions(
+      const std::vector<std::string>& supported_import_assertions) {
+    supported_import_assertions_ = supported_import_assertions;
+  }
+  const std::vector<std::string>& supported_import_assertions() const {
+    return supported_import_assertions_;
+  }
+
   // Register a finalizer to be called at isolate teardown.
   void RegisterManagedPtrDestructor(ManagedPtrDestructor* finalizer);
 
@@ -1823,6 +1831,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   PromiseHook promise_hook_ = nullptr;
   HostImportModuleDynamicallyCallback host_import_module_dynamically_callback_ =
       nullptr;
+  std::vector<std::string> supported_import_assertions_;
   HostInitializeImportMetaObjectCallback
       host_initialize_import_meta_object_callback_ = nullptr;
   base::Mutex rail_mutex_;

src/execution/local-isolate.cc

@@ -20,7 +20,8 @@ LocalIsolate::LocalIsolate(Isolate* isolate, ThreadKind kind)
       thread_id_(ThreadId::Current()),
       stack_limit_(kind == ThreadKind::kMain
                        ? isolate->stack_guard()->real_climit()
-                       : GetCurrentStackPosition() - FLAG_stack_size * KB) {}
+                       : GetCurrentStackPosition() - FLAG_stack_size * KB),
+      supported_import_assertions_(isolate->supported_import_assertions()) {}
 
 LocalIsolate::~LocalIsolate() = default;
 

src/execution/local-isolate.h

@@ -84,6 +84,10 @@ class V8_EXPORT_PRIVATE LocalIsolate final : private HiddenLocalFactory {
 
   bool is_main_thread() const { return heap_.is_main_thread(); }
 
+  const std::vector<std::string>& supported_import_assertions() const {
+    return supported_import_assertions_;
+  }
+
  private:
   friend class v8::internal::LocalFactory;
 
@@ -96,6 +100,7 @@ class V8_EXPORT_PRIVATE LocalIsolate final : private HiddenLocalFactory {
   std::unique_ptr<LocalLogger> logger_;
   ThreadId const thread_id_;
   Address const stack_limit_;
+  std::vector<std::string> supported_import_assertions_;
 };
 
 }  // namespace internal

test/cctest/test-modules.cc

@@ -148,13 +148,13 @@ MaybeLocal<Module> ResolveCallbackWithImportAssertions(
   } else if (specifier->StrictEquals(v8_str("./bar.js"))) {
     CHECK_EQ(3, import_assertions->Length());
     Local<String> assertion_key =
-        import_assertions->Get(env.local(), 0).As<Value>().As<String>();
+        import_assertions->Get(context, 0).As<Value>().As<String>();
     CHECK(v8_str("a")->StrictEquals(assertion_key));
     Local<String> assertion_value =
-        import_assertions->Get(env.local(), 1).As<Value>().As<String>();
+        import_assertions->Get(context, 1).As<Value>().As<String>();
     CHECK(v8_str("b")->StrictEquals(assertion_value));
     Local<Data> assertion_source_offset_object =
-        import_assertions->Get(env.local(), 2);
+        import_assertions->Get(context, 2);
     Local<Int32> assertion_source_offset_int32 =
         assertion_source_offset_object.As<Value>()
             ->ToInt32(context)
@@ -176,18 +176,25 @@ TEST(ModuleInstantiationWithImportAssertions) {
   bool prev_top_level_await = i::FLAG_harmony_top_level_await;
   bool prev_import_assertions = i::FLAG_harmony_import_assertions;
   i::FLAG_harmony_import_assertions = true;
+
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  create_params.supported_import_assertions = {"extra0", "a", "extra1"};
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+
   for (auto top_level_await : {true, false}) {
     i::FLAG_harmony_top_level_await = top_level_await;
-    Isolate* isolate = CcTest::isolate();
+    v8::Isolate::Scope isolate_scope(isolate);
     HandleScope scope(isolate);
-    LocalContext env;
+    Local<v8::Context> context = v8::Context::New(isolate);
+    v8::Context::Scope context_scope(context);
     v8::TryCatch try_catch(isolate);
 
     Local<Module> module;
     {
       Local<String> source_text = v8_str(
           "import './foo.js' assert { };\n"
-          "export {} from './bar.js' assert { a: 'b' };");
+          "export {} from './bar.js' assert { a: 'b', c: 'd' };");
       ScriptOrigin origin = ModuleOrigin(v8_str("file.js"), CcTest::isolate());
       ScriptCompiler::Source source(source_text, origin);
       module = ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
@@ -195,7 +202,7 @@ TEST(ModuleInstantiationWithImportAssertions) {
       Local<FixedArray> module_requests = module->GetModuleRequests();
       CHECK_EQ(2, module_requests->Length());
       Local<ModuleRequest> module_request_0 =
-          module_requests->Get(env.local(), 0).As<ModuleRequest>();
+          module_requests->Get(context, 0).As<ModuleRequest>();
       CHECK(v8_str("./foo.js")->StrictEquals(module_request_0->GetSpecifier()));
       int offset = module_request_0->GetSourceOffset();
       CHECK_EQ(7, offset);
@@ -205,7 +212,7 @@ TEST(ModuleInstantiationWithImportAssertions) {
       CHECK_EQ(0, module_request_0->GetImportAssertions()->Length());
 
       Local<ModuleRequest> module_request_1 =
-          module_requests->Get(env.local(), 1).As<ModuleRequest>();
+          module_requests->Get(context, 1).As<ModuleRequest>();
       CHECK(v8_str("./bar.js")->StrictEquals(module_request_1->GetSpecifier()));
       offset = module_request_1->GetSourceOffset();
       CHECK_EQ(45, offset);
@@ -217,13 +224,13 @@ TEST(ModuleInstantiationWithImportAssertions) {
           module_request_1->GetImportAssertions();
       CHECK_EQ(3, import_assertions_1->Length());
       Local<String> assertion_key =
-          import_assertions_1->Get(env.local(), 0).As<String>();
+          import_assertions_1->Get(context, 0).As<String>();
       CHECK(v8_str("a")->StrictEquals(assertion_key));
       Local<String> assertion_value =
-          import_assertions_1->Get(env.local(), 1).As<String>();
+          import_assertions_1->Get(context, 1).As<String>();
       CHECK(v8_str("b")->StrictEquals(assertion_value));
       int32_t assertion_source_offset =
-          import_assertions_1->Get(env.local(), 2).As<Int32>()->Value();
+          import_assertions_1->Get(context, 2).As<Int32>()->Value();
       CHECK_EQ(65, assertion_source_offset);
       loc = module->SourceOffsetToLocation(assertion_source_offset);
       CHECK_EQ(1, loc.GetLineNumber());
@@ -248,6 +255,72 @@ TEST(ModuleInstantiationWithImportAssertions) {
           ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
     }
 
+    CHECK(
+        module->InstantiateModule(context, ResolveCallbackWithImportAssertions)
+            .FromJust());
+    CHECK_EQ(Module::kInstantiated, module->GetStatus());
+
+    MaybeLocal<Value> result = module->Evaluate(context);
+    CHECK_EQ(Module::kEvaluated, module->GetStatus());
+    if (i::FLAG_harmony_top_level_await) {
+      Local<Promise> promise = Local<Promise>::Cast(result.ToLocalChecked());
+      CHECK_EQ(promise->State(), v8::Promise::kFulfilled);
+      CHECK(promise->Result()->IsUndefined());
+    } else {
+      CHECK(!result.IsEmpty());
+      ExpectInt32("Object.expando", 42);
+    }
+    CHECK(!try_catch.HasCaught());
+  }
+
+  isolate->Dispose();
+  i::FLAG_harmony_top_level_await = prev_top_level_await;
+  i::FLAG_harmony_import_assertions = prev_import_assertions;
+}
+
+TEST(ModuleInstantiationWithImportAssertionsWithoutSupportedAssertions) {
+  bool prev_top_level_await = i::FLAG_harmony_top_level_await;
+  bool prev_import_assertions = i::FLAG_harmony_import_assertions;
+  i::FLAG_harmony_import_assertions = true;
+  for (auto top_level_await : {true, false}) {
+    i::FLAG_harmony_top_level_await = top_level_await;
+    Isolate* isolate = CcTest::isolate();
+    HandleScope scope(isolate);
+    LocalContext env;
+    v8::TryCatch try_catch(isolate);
+
+    Local<Module> module;
+    {
+      Local<String> source_text =
+          v8_str("import './foo.js' assert { a: 'b' };");
+      ScriptOrigin origin = ModuleOrigin(v8_str("file.js"), CcTest::isolate());
+      ScriptCompiler::Source source(source_text, origin);
+      module = ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+      CHECK_EQ(Module::kUninstantiated, module->GetStatus());
+      Local<FixedArray> module_requests = module->GetModuleRequests();
+      CHECK_EQ(1, module_requests->Length());
+      Local<ModuleRequest> module_request_0 =
+          module_requests->Get(env.local(), 0).As<ModuleRequest>();
+      CHECK(v8_str("./foo.js")->StrictEquals(module_request_0->GetSpecifier()));
+      int offset = module_request_0->GetSourceOffset();
+      CHECK_EQ(7, offset);
+      Location loc = module->SourceOffsetToLocation(offset);
+      CHECK_EQ(0, loc.GetLineNumber());
+      CHECK_EQ(7, loc.GetColumnNumber());
+      // No supported assertions were provided in the Isolate's CreateParams, so
+      // no import assertions should be visible on the API surface.
+      CHECK_EQ(0, module_request_0->GetImportAssertions()->Length());
+    }
+
+    // foo.js
+    {
+      Local<String> source_text = v8_str("Object.expando = 40");
+      ScriptOrigin origin = ModuleOrigin(v8_str("foo.js"), CcTest::isolate());
+      ScriptCompiler::Source source(source_text, origin);
+      fooModule =
+          ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+    }
+
     CHECK(module
               ->InstantiateModule(env.local(),
                                   ResolveCallbackWithImportAssertions)
@@ -262,7 +335,7 @@ TEST(ModuleInstantiationWithImportAssertions) {
       CHECK(promise->Result()->IsUndefined());
     } else {
       CHECK(!result.IsEmpty());
-      ExpectInt32("Object.expando", 42);
+      ExpectInt32("Object.expando", 40);
     }
     CHECK(!try_catch.HasCaught());
   }