Repository: denoland/deno_core
Pull request #327 (2023-11-14 21:31:15)

feat: Import attributes validation callback
Closes https://github.com/denoland/deno_core/issues/326

This commit adds `RuntimeOptions.validate_import_attributes_cb` that can be
used to customize which import attributes are allowed.


Patch:
diff --git a/core/lib.rs b/core/lib.rs
index b147020..e856a9e 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -97,6 +97,7 @@ pub use crate::modules::ModuleType;
 pub use crate::modules::NoopModuleLoader;
 pub use crate::modules::ResolutionKind;
 pub use crate::modules::StaticModuleLoader;
+pub use crate::modules::ValidateImportAttributesCb;
 pub use crate::normalize_path::normalize_path;
 pub use crate::ops::OpError;
 pub use crate::ops::OpId;
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 1125f0a..fed8672 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -5,7 +5,6 @@ use crate::error::throw_type_error;
 use crate::fast_string::FastString;
 use crate::modules::get_asserted_module_type_from_assertions;
 use crate::modules::parse_import_assertions;
-use crate::modules::validate_import_assertions;
 use crate::modules::ImportAssertionsKind;
 use crate::modules::ModuleCode;
 use crate::modules::ModuleError;
@@ -22,6 +21,7 @@ use crate::modules::RecursiveModuleLoad;
 use crate::modules::ResolutionKind;
 use crate::runtime::JsRealm;
 use crate::runtime::SnapshottedData;
+use crate::JsRuntime;
 use anyhow::Error;
 use futures::future::FutureExt;
 use futures::stream::FuturesUnordered;
@@ -505,7 +505,12 @@ impl ModuleMap {
 
       // FIXME(bartomieju): there are no stack frames if exception
       // is thrown here
-      validate_import_assertions(tc_scope, &assertions);
+      {
+        let state_rc = JsRuntime::state_from(tc_scope);
+        let state = state_rc.borrow();
+        (state.validate_import_attributes_cb)(tc_scope, &assertions);
+      }
+
       if tc_scope.has_caught() {
         let exception = tc_scope.exception().unwrap();
         let exception = v8::Global::new(tc_scope, exception);
diff --git a/core/modules/mod.rs b/core/modules/mod.rs
index 2af22dd..64093f6 100644
--- a/core/modules/mod.rs
+++ b/core/modules/mod.rs
@@ -49,24 +49,36 @@ pub(crate) type ModuleLoadId = i32;
 pub type ModuleCode = FastString;
 pub type ModuleName = FastString;
 
+/// Callback to validate import attributes. If the validation fails and exception
+/// should be thrown using `scope.throw_exception()`.
+pub type ValidateImportAttributesCb =
+  Box<dyn Fn(&mut v8::HandleScope, &HashMap<String, String>)>;
+
 const SUPPORTED_TYPE_ASSERTIONS: &[&str] = &["json"];
 
-/// Throws V8 exception if assertions are invalid
-pub(crate) fn validate_import_assertions(
+/// Throws a `TypeError` if `type` attribute is not equal to "json". Allows
+/// all other attributes.
+pub(crate) fn validate_import_attributes(
   scope: &mut v8::HandleScope,
   assertions: &HashMap<String, String>,
 ) {
   for (key, value) in assertions {
-    if key == "type" && !SUPPORTED_TYPE_ASSERTIONS.contains(&value.as_str()) {
-      let message = v8::String::new(
-        scope,
-        &format!("\"{value}\" is not a valid module type."),
-      )
-      .unwrap();
-      let exception = v8::Exception::type_error(scope, message);
-      scope.throw_exception(exception);
-      return;
-    }
+    let msg = if key != "type" {
+      Some(format!("\"{key}\" attribute is not supported."))
+    } else if !SUPPORTED_TYPE_ASSERTIONS.contains(&value.as_str()) {
+      Some(format!("\"{value}\" is not a valid module type."))
+    } else {
+      None
+    };
+
+    let Some(msg) = msg else {
+      continue;
+    };
+
+    let message = v8::String::new(scope, &msg).unwrap();
+    let exception = v8::Exception::type_error(scope, message);
+    scope.throw_exception(exception);
+    return;
   }
 }
 
diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs
index 92835bb..81822ef 100644
--- a/core/runtime/bindings.rs
+++ b/core/runtime/bindings.rs
@@ -11,7 +11,6 @@ use crate::error::throw_type_error;
 use crate::error::JsStackFrame;
 use crate::modules::get_asserted_module_type_from_assertions;
 use crate::modules::parse_import_assertions;
-use crate::modules::validate_import_assertions;
 use crate::modules::ImportAssertionsKind;
 use crate::modules::ModuleMap;
 use crate::modules::ResolutionKind;
@@ -291,7 +290,12 @@ pub fn host_import_module_dynamically_callback<'s>(
 
   {
     let tc_scope = &mut v8::TryCatch::new(scope);
-    validate_import_assertions(tc_scope, &assertions);
+    {
+      let state_rc = JsRuntime::state_from(tc_scope);
+      let state = state_rc.borrow();
+      (state.validate_import_attributes_cb)(tc_scope, &assertions);
+    }
+
     if tc_scope.has_caught() {
       let e = tc_scope.exception().unwrap();
       resolver.reject(tc_scope, e);
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index f1b2357..6a3939d 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -20,6 +20,7 @@ use crate::modules::ModuleCode;
 use crate::modules::ModuleId;
 use crate::modules::ModuleLoader;
 use crate::modules::ModuleMap;
+use crate::modules::ValidateImportAttributesCb;
 use crate::ops::*;
 use crate::ops_metrics::dispatch_metrics_async;
 use crate::ops_metrics::OpMetricsEvent;
@@ -317,6 +318,7 @@ pub struct JsRuntimeState {
   // flimsy. Try to poll it similarly to `pending_promise_rejections`.
   pub(crate) dispatched_exception: Option<v8::Global<v8::Value>>,
   pub(crate) inspector: Option<Rc<RefCell<JsRuntimeInspector>>>,
+  pub(crate) validate_import_attributes_cb: ValidateImportAttributesCb,
 }
 
 impl JsRuntimeState {
@@ -454,6 +456,11 @@ pub struct RuntimeOptions {
   /// An optional instance of `FeatureChecker`. If one is not provided, the
   /// default instance will be created that has no features enabled.
   pub feature_checker: Option<Arc<FeatureChecker>>,
+
+  /// A callback that can be used to validate import attributes received at
+  /// the import site. If not callback is provided, a default one is used. The
+  /// default callback only allows `"type"` attribute, with a value of `"json"`.
+  pub validate_import_attributes_cb: Option<ValidateImportAttributesCb>,
 }
 
 impl RuntimeOptions {
@@ -601,6 +608,10 @@ impl JsRuntime {
       // SAFETY: we just asserted that layout has non-0 size.
       unsafe { std::alloc::alloc(layout) as *mut _ };
 
+    let validate_import_attributes_cb = options
+      .validate_import_attributes_cb
+      .unwrap_or_else(|| Box::new(crate::modules::validate_import_attributes));
+
     let state_rc = Rc::new(RefCell::new(JsRuntimeState {
       dyn_module_evaluate_idle_counter: 0,
       has_tick_scheduled: false,
@@ -613,6 +624,7 @@ impl JsRuntime {
       // Some fields are initialized later after isolate is created
       inspector: None,
       known_realms: Vec::with_capacity(1),
+      validate_import_attributes_cb,
     }));
 
     let weak = Rc::downgrade(&state_rc);


Test patch:
diff --git a/core/modules/tests.rs b/core/modules/tests.rs
index e72ce80..531f8eb 100644
--- a/core/modules/tests.rs
+++ b/core/modules/tests.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 use crate::ascii_str;
+use crate::error::exception_to_err_result;
 use crate::modules::loaders::CountingModuleLoader;
 use crate::modules::loaders::ModuleLoadEventCounts;
 use crate::resolve_import;
@@ -495,6 +496,125 @@ fn test_json_module() {
   futures::executor::block_on(receiver).unwrap().unwrap();
 }
 
+#[test]
+fn test_validate_import_attributes() {
+  fn validate_import_attrs(
+    scope: &mut v8::HandleScope,
+    _attrs: &HashMap<String, String>,
+  ) {
+    let msg = v8::String::new(scope, "boom!").unwrap();
+    let ex = v8::Exception::type_error(scope, msg);
+    scope.throw_exception(ex);
+  }
+
+  let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));
+  let mut runtime = JsRuntime::new(RuntimeOptions {
+    module_loader: Some(loader.clone()),
+    validate_import_attributes_cb: Some(Box::new(validate_import_attrs)),
+    ..Default::default()
+  });
+
+  let module_map_rc = runtime.module_map().clone();
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {foo: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(err.to_string(), "Uncaught TypeError: boom!");
+  }
+}
+
+#[test]
+fn test_validate_import_attributes_default() {
+  let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));
+  let mut runtime = JsRuntime::new(RuntimeOptions {
+    module_loader: Some(loader.clone()),
+    ..Default::default()
+  });
+
+  let module_map_rc = runtime.module_map().clone();
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {foo: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(
+      err.to_string(),
+      "Uncaught TypeError: \"foo\" attribute is not supported."
+    );
+  }
+
+  {
+    let scope = &mut runtime.handle_scope();
+    let mut module_map = module_map_rc.borrow_mut();
+    let specifier_a = ascii_str!("file:///a.js");
+    let module_err = module_map
+      .new_es_module(
+        scope,
+        true,
+        specifier_a,
+        ascii_str!(
+          r#"
+          import jsonData from './c.json' with {type: "bar"};
+        "#
+        ),
+        false,
+      )
+      .unwrap_err();
+
+    let ModuleError::Exception(exc) = module_err else {
+      unreachable!();
+    };
+    let exception = v8::Local::new(scope, exc);
+    let err =
+      exception_to_err_result::<()>(scope, exception, false).unwrap_err();
+    assert_eq!(
+      err.to_string(),
+      "Uncaught TypeError: \"bar\" is not a valid module type."
+    );
+  }
+}
+
 #[tokio::test]
 async fn dyn_import_err() {
   let loader = Rc::new(CountingModuleLoader::new(StaticModuleLoader::new([])));