From fab57e2fc90a6812884ab2828ec9e0be9effcc8e Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Tue, 2 Apr 2024 11:19:04 -0700
Subject: [PATCH 1/5] Add an integration test for relative imports on
 SimpleModuleLoader

---
 core/engine/tests/assets/dir1/file1_1.js |  5 ++++
 core/engine/tests/assets/dir1/file1_2.js |  3 ++
 core/engine/tests/assets/file1.js        |  5 ++++
 core/engine/tests/imports.rs             | 36 ++++++++++++++++++++++++
 4 files changed, 49 insertions(+)
 create mode 100644 core/engine/tests/assets/dir1/file1_1.js
 create mode 100644 core/engine/tests/assets/dir1/file1_2.js
 create mode 100644 core/engine/tests/assets/file1.js
 create mode 100644 core/engine/tests/imports.rs

diff --git a/core/engine/tests/assets/dir1/file1_1.js b/core/engine/tests/assets/dir1/file1_1.js
new file mode 100644
index 00000000000..adc891a7a5c
--- /dev/null
+++ b/core/engine/tests/assets/dir1/file1_1.js
@@ -0,0 +1,5 @@
+import {file1_2} from './file1_2.js';
+
+export function file1_1() {
+    return 'file1_1' + '.' + file1_2();
+}
diff --git a/core/engine/tests/assets/dir1/file1_2.js b/core/engine/tests/assets/dir1/file1_2.js
new file mode 100644
index 00000000000..f06c7020373
--- /dev/null
+++ b/core/engine/tests/assets/dir1/file1_2.js
@@ -0,0 +1,3 @@
+export function file1_2() {
+    return 'file1_2';
+}
diff --git a/core/engine/tests/assets/file1.js b/core/engine/tests/assets/file1.js
new file mode 100644
index 00000000000..012c0579508
--- /dev/null
+++ b/core/engine/tests/assets/file1.js
@@ -0,0 +1,5 @@
+import {file1_1} from './dir1/file1_1.js';
+
+export function file1() {
+    return 'file1' + '..' + file1_1();
+}
diff --git a/core/engine/tests/imports.rs b/core/engine/tests/imports.rs
new file mode 100644
index 00000000000..1e66e82ad65
--- /dev/null
+++ b/core/engine/tests/imports.rs
@@ -0,0 +1,36 @@
+#![allow(unused_crate_dependencies, missing_docs)]
+
+use std::path::PathBuf;
+use std::rc::Rc;
+
+use boa_engine::builtins::promise::PromiseState;
+use boa_engine::module::SimpleModuleLoader;
+use boa_engine::{js_string, Context, JsValue, Source};
+
+/// Test that relative imports work with the simple module loader.
+#[test]
+fn subdirectories() {
+    let assets_dir =
+        PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()).join("tests/assets");
+
+    let loader = Rc::new(SimpleModuleLoader::new(assets_dir).unwrap());
+    let mut context = Context::builder()
+        .module_loader(loader.clone())
+        .build()
+        .unwrap();
+
+    let source = Source::from_bytes(b"import { file1 } from './file1.js'; file1()");
+    let module = boa_engine::Module::parse(source, None, &mut context).unwrap();
+    let result = module.load_link_evaluate(&mut context);
+
+    context.run_jobs();
+    match result.state() {
+        PromiseState::Pending => {}
+        PromiseState::Fulfilled(v) => {
+            assert_eq!(v, JsValue::String(js_string!("file1..file1_1.file1_2")));
+        }
+        PromiseState::Rejected(reason) => {
+            panic!("Module failed to load: {}", reason.display());
+        }
+    }
+}

From d216346cf4c7b7a8bedb826daf30cf490a06541a Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Tue, 2 Apr 2024 14:28:05 -0700
Subject: [PATCH 2/5] Add a path to Module (and expose it in Referrer)

This allows SimpleModuleLoader to resolve relative to the current
file (which this commit also does).

Fixes #3782
---
 core/engine/src/module/loader.rs | 44 ++++++++++++++++++++++++++++----
 core/engine/src/module/mod.rs    | 31 ++++++++++++++--------
 core/engine/tests/imports.rs     | 15 +++++++++--
 core/parser/src/source/mod.rs    | 16 +++++++++---
 examples/src/bin/synthetic.rs    |  1 +
 5 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/core/engine/src/module/loader.rs b/core/engine/src/module/loader.rs
index 5d9da5755b7..9cbde9ba62a 100644
--- a/core/engine/src/module/loader.rs
+++ b/core/engine/src/module/loader.rs
@@ -5,11 +5,11 @@ use rustc_hash::FxHashMap;
 use boa_gc::GcRefCell;
 use boa_parser::Source;
 
-use crate::script::Script;
 use crate::{
-    js_string, object::JsObject, realm::Realm, vm::ActiveRunnable, Context, JsError, JsNativeError,
-    JsResult, JsString,
+    Context, js_string, JsError, JsNativeError, JsResult, JsString, object::JsObject,
+    realm::Realm, vm::ActiveRunnable,
 };
+use crate::script::Script;
 
 use super::Module;
 
@@ -24,6 +24,17 @@ pub enum Referrer {
     Script(Script),
 }
 
+impl Referrer {
+    /// Gets the path of the referrer, if it has one.
+    pub fn path(&self) -> Option<&Path> {
+        match self {
+            Self::Module(module) => module.path(),
+            Self::Realm(_) => None,
+            Self::Script(_script) => None,
+        }
+    }
+}
+
 impl From<ActiveRunnable> for Referrer {
     fn from(value: ActiveRunnable) -> Self {
         match value {
@@ -176,17 +187,40 @@ impl SimpleModuleLoader {
 impl ModuleLoader for SimpleModuleLoader {
     fn load_imported_module(
         &self,
-        _referrer: Referrer,
+        referrer: Referrer,
         specifier: JsString,
         finish_load: Box<dyn FnOnce(JsResult<Module>, &mut Context)>,
         context: &mut Context,
     ) {
         let result = (|| {
+            // If the referrer has a path, we use it as the base for the specifier.
             let path = specifier
                 .to_std_string()
                 .map_err(|err| JsNativeError::typ().with_message(err.to_string()))?;
+
             let short_path = Path::new(&path);
-            let path = self.root.join(short_path);
+
+            let path = if let Some(p) = referrer.path().and_then(|p| p.parent()) {
+                let root = if p.is_absolute() {
+                    p.to_path_buf()
+                } else {
+                    self.root.join(p)
+                };
+                root.join(short_path)
+            } else {
+                self.root.join(short_path)
+            };
+
+            // Make sure we don't exit the root.
+            if !path.starts_with(&self.root) {
+                return Err(JsNativeError::typ()
+                    .with_message(format!(
+                        "path `{}` is outside the module root",
+                        path.display()
+                    ))
+                    .into());
+            }
+
             let path = path.canonicalize().map_err(|err| {
                 JsNativeError::typ()
                     .with_message(format!(
diff --git a/core/engine/src/module/mod.rs b/core/engine/src/module/mod.rs
index 83f3e3b4795..071f3b7cbee 100644
--- a/core/engine/src/module/mod.rs
+++ b/core/engine/src/module/mod.rs
@@ -21,27 +21,23 @@
 //! [spec]: https://tc39.es/ecma262/#sec-modules
 //! [module]: https://tc39.es/ecma262/#sec-abstract-module-records
 
-mod loader;
-mod namespace;
-mod source;
-mod synthetic;
-use boa_parser::source::ReadChar;
-pub use loader::*;
-pub use namespace::ModuleNamespace;
-use source::SourceTextModule;
-pub use synthetic::{SyntheticModule, SyntheticModuleInitializer};
-
 use std::cell::{Cell, RefCell};
 use std::collections::HashSet;
 use std::hash::Hash;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 
 use rustc_hash::FxHashSet;
 
 use boa_gc::{Finalize, Gc, GcRefCell, Trace};
 use boa_interner::Interner;
+use boa_parser::source::ReadChar;
 use boa_parser::{Parser, Source};
 use boa_profiler::Profiler;
+pub use loader::*;
+pub use namespace::ModuleNamespace;
+use source::SourceTextModule;
+pub use synthetic::{SyntheticModule, SyntheticModuleInitializer};
 
 use crate::{
     builtins::promise::{PromiseCapability, PromiseState},
@@ -51,6 +47,11 @@ use crate::{
     Context, HostDefined, JsError, JsResult, JsString, JsValue, NativeFunction,
 };
 
+mod loader;
+mod namespace;
+mod source;
+mod synthetic;
+
 /// ECMAScript's [**Abstract module record**][spec].
 ///
 /// [spec]: https://tc39.es/ecma262/#sec-abstract-module-records
@@ -75,6 +76,7 @@ struct ModuleRepr {
     namespace: GcRefCell<Option<JsObject>>,
     kind: ModuleKind,
     host_defined: HostDefined,
+    path: Option<PathBuf>,
 }
 
 /// The kind of a [`Module`].
@@ -155,6 +157,7 @@ impl Module {
         context: &mut Context,
     ) -> JsResult<Self> {
         let _timer = Profiler::global().start_event("Module parsing", "Main");
+        let path = src.path().map(|p| p.to_path_buf());
         let mut parser = Parser::new(src);
         parser.set_identifier(context.next_parser_identifier());
         let module = parser.parse_module(context.interner_mut())?;
@@ -167,6 +170,7 @@ impl Module {
                 namespace: GcRefCell::default(),
                 kind: ModuleKind::SourceText(src),
                 host_defined: HostDefined::default(),
+                path,
             }),
         })
     }
@@ -181,6 +185,7 @@ impl Module {
     pub fn synthetic(
         export_names: &[JsString],
         evaluation_steps: SyntheticModuleInitializer,
+        path: Option<PathBuf>,
         realm: Option<Realm>,
         context: &mut Context,
     ) -> Self {
@@ -194,6 +199,7 @@ impl Module {
                 namespace: GcRefCell::default(),
                 kind: ModuleKind::Synthetic(synth),
                 host_defined: HostDefined::default(),
+                path,
             }),
         }
     }
@@ -564,6 +570,11 @@ impl Module {
             })
             .clone()
     }
+
+    /// Returns the path of the module, if it was created from a file or assigned.
+    pub fn path(&self) -> Option<&Path> {
+        self.inner.path.as_deref()
+    }
 }
 
 impl PartialEq for Module {
diff --git a/core/engine/tests/imports.rs b/core/engine/tests/imports.rs
index 1e66e82ad65..6b7107e3cec 100644
--- a/core/engine/tests/imports.rs
+++ b/core/engine/tests/imports.rs
@@ -3,9 +3,9 @@
 use std::path::PathBuf;
 use std::rc::Rc;
 
+use boa_engine::{Context, js_string, JsValue, Source};
 use boa_engine::builtins::promise::PromiseState;
 use boa_engine::module::SimpleModuleLoader;
-use boa_engine::{js_string, Context, JsValue, Source};
 
 /// Test that relative imports work with the simple module loader.
 #[test]
@@ -19,7 +19,7 @@ fn subdirectories() {
         .build()
         .unwrap();
 
-    let source = Source::from_bytes(b"import { file1 } from './file1.js'; file1()");
+    let source = Source::from_bytes(b"export { file1 } from './file1.js';");
     let module = boa_engine::Module::parse(source, None, &mut context).unwrap();
     let result = module.load_link_evaluate(&mut context);
 
@@ -27,6 +27,17 @@ fn subdirectories() {
     match result.state() {
         PromiseState::Pending => {}
         PromiseState::Fulfilled(v) => {
+            assert!(v.is_undefined());
+
+            let foo = module
+                .namespace(&mut context)
+                .get(js_string!("file1"), &mut context)
+                .unwrap();
+            let v = foo
+                .as_callable()
+                .unwrap()
+                .call(&JsValue::undefined(), &[], &mut context)
+                .unwrap();
             assert_eq!(v, JsValue::String(js_string!("file1..file1_1.file1_2")));
         }
         PromiseState::Rejected(reason) => {
diff --git a/core/parser/src/source/mod.rs b/core/parser/src/source/mod.rs
index 19409f107d6..5fcf44194e3 100644
--- a/core/parser/src/source/mod.rs
+++ b/core/parser/src/source/mod.rs
@@ -1,8 +1,5 @@
 //! Boa parser input source types.
 
-mod utf16;
-mod utf8;
-
 use std::{
     fs::File,
     io::{self, BufReader, Read},
@@ -12,6 +9,9 @@ use std::{
 pub use utf16::UTF16Input;
 pub use utf8::UTF8Input;
 
+mod utf16;
+mod utf8;
+
 /// A source of ECMAScript code.
 ///
 /// [`Source`]s can be created from plain [`str`]s, file [`Path`]s or more generally, any [`Read`]
@@ -119,6 +119,13 @@ impl<'path, R: Read> Source<'path, UTF8Input<R>> {
     }
 }
 
+impl<'path, R> Source<'path, R> {
+    /// Returns the path (if any) of this source file.
+    pub fn path(&self) -> Option<&'path Path> {
+        self.path
+    }
+}
+
 /// This trait is used to abstract over the different types of input readers.
 pub trait ReadChar {
     /// Retrieves the next unicode code point. Returns `None` if the end of the input is reached.
@@ -131,9 +138,10 @@ pub trait ReadChar {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
     use std::io::Cursor;
 
+    use super::*;
+
     #[test]
     fn from_bytes() {
         let mut source = Source::from_bytes("'Hello' + 'World';");
diff --git a/examples/src/bin/synthetic.rs b/examples/src/bin/synthetic.rs
index 3d7c84f62da..ffab2c6ba47 100644
--- a/examples/src/bin/synthetic.rs
+++ b/examples/src/bin/synthetic.rs
@@ -175,6 +175,7 @@ fn create_operations_module(context: &mut Context) -> Module {
             (sum, sub, mult, div, sqrt),
         ),
         None,
+        None,
         context,
     )
 }

From 830964bbedb9e50dfb3a8546742a40942a294d8b Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Tue, 2 Apr 2024 15:04:25 -0700
Subject: [PATCH 3/5] cargo clippy and fmt

---
 core/engine/src/module/loader.rs |  7 ++++---
 core/engine/src/module/mod.rs    |  3 ++-
 core/engine/tests/imports.rs     | 13 ++++++++-----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/core/engine/src/module/loader.rs b/core/engine/src/module/loader.rs
index 9cbde9ba62a..9fcf9d73e57 100644
--- a/core/engine/src/module/loader.rs
+++ b/core/engine/src/module/loader.rs
@@ -5,11 +5,11 @@ use rustc_hash::FxHashMap;
 use boa_gc::GcRefCell;
 use boa_parser::Source;
 
+use crate::script::Script;
 use crate::{
-    Context, js_string, JsError, JsNativeError, JsResult, JsString, object::JsObject,
-    realm::Realm, vm::ActiveRunnable,
+    js_string, object::JsObject, realm::Realm, vm::ActiveRunnable, Context, JsError, JsNativeError,
+    JsResult, JsString,
 };
-use crate::script::Script;
 
 use super::Module;
 
@@ -26,6 +26,7 @@ pub enum Referrer {
 
 impl Referrer {
     /// Gets the path of the referrer, if it has one.
+    #[must_use]
     pub fn path(&self) -> Option<&Path> {
         match self {
             Self::Module(module) => module.path(),
diff --git a/core/engine/src/module/mod.rs b/core/engine/src/module/mod.rs
index 071f3b7cbee..9bf195f35ff 100644
--- a/core/engine/src/module/mod.rs
+++ b/core/engine/src/module/mod.rs
@@ -157,7 +157,7 @@ impl Module {
         context: &mut Context,
     ) -> JsResult<Self> {
         let _timer = Profiler::global().start_event("Module parsing", "Main");
-        let path = src.path().map(|p| p.to_path_buf());
+        let path = src.path().map(std::path::Path::to_path_buf);
         let mut parser = Parser::new(src);
         parser.set_identifier(context.next_parser_identifier());
         let module = parser.parse_module(context.interner_mut())?;
@@ -572,6 +572,7 @@ impl Module {
     }
 
     /// Returns the path of the module, if it was created from a file or assigned.
+    #[must_use]
     pub fn path(&self) -> Option<&Path> {
         self.inner.path.as_deref()
     }
diff --git a/core/engine/tests/imports.rs b/core/engine/tests/imports.rs
index 6b7107e3cec..2257d759e39 100644
--- a/core/engine/tests/imports.rs
+++ b/core/engine/tests/imports.rs
@@ -3,9 +3,9 @@
 use std::path::PathBuf;
 use std::rc::Rc;
 
-use boa_engine::{Context, js_string, JsValue, Source};
 use boa_engine::builtins::promise::PromiseState;
 use boa_engine::module::SimpleModuleLoader;
+use boa_engine::{js_string, Context, JsValue, Source};
 
 /// Test that relative imports work with the simple module loader.
 #[test]
@@ -29,16 +29,19 @@ fn subdirectories() {
         PromiseState::Fulfilled(v) => {
             assert!(v.is_undefined());
 
-            let foo = module
+            let foo_value = module
                 .namespace(&mut context)
                 .get(js_string!("file1"), &mut context)
-                .unwrap();
-            let v = foo
+                .unwrap()
                 .as_callable()
                 .unwrap()
                 .call(&JsValue::undefined(), &[], &mut context)
                 .unwrap();
-            assert_eq!(v, JsValue::String(js_string!("file1..file1_1.file1_2")));
+
+            assert_eq!(
+                foo_value,
+                JsValue::String(js_string!("file1..file1_1.file1_2"))
+            );
         }
         PromiseState::Rejected(reason) => {
             panic!("Module failed to load: {}", reason.display());

From ef842f821af8df03cfacb4604c9f456aac992cbd Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Tue, 2 Apr 2024 15:10:21 -0700
Subject: [PATCH 4/5] prettier

---
 core/engine/tests/assets/dir1/file1_1.js | 4 ++--
 core/engine/tests/assets/dir1/file1_2.js | 2 +-
 core/engine/tests/assets/file1.js        | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/core/engine/tests/assets/dir1/file1_1.js b/core/engine/tests/assets/dir1/file1_1.js
index adc891a7a5c..5d0002c21dc 100644
--- a/core/engine/tests/assets/dir1/file1_1.js
+++ b/core/engine/tests/assets/dir1/file1_1.js
@@ -1,5 +1,5 @@
-import {file1_2} from './file1_2.js';
+import { file1_2 } from "./file1_2.js";
 
 export function file1_1() {
-    return 'file1_1' + '.' + file1_2();
+  return "file1_1" + "." + file1_2();
 }
diff --git a/core/engine/tests/assets/dir1/file1_2.js b/core/engine/tests/assets/dir1/file1_2.js
index f06c7020373..bd9bcc1fdf9 100644
--- a/core/engine/tests/assets/dir1/file1_2.js
+++ b/core/engine/tests/assets/dir1/file1_2.js
@@ -1,3 +1,3 @@
 export function file1_2() {
-    return 'file1_2';
+  return "file1_2";
 }
diff --git a/core/engine/tests/assets/file1.js b/core/engine/tests/assets/file1.js
index 012c0579508..f047b485789 100644
--- a/core/engine/tests/assets/file1.js
+++ b/core/engine/tests/assets/file1.js
@@ -1,5 +1,5 @@
-import {file1_1} from './dir1/file1_1.js';
+import { file1_1 } from "./dir1/file1_1.js";
 
 export function file1() {
-    return 'file1' + '..' + file1_1();
+  return "file1" + ".." + file1_1();
 }

From 063caa6920809cc1a174929a04b2e76f456e7633 Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Wed, 3 Apr 2024 21:46:38 -0700
Subject: [PATCH 5/5] Fix merge error

---
 core/interop/src/lib.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs
index f048bf2f655..0aa8739d6c2 100644
--- a/core/interop/src/lib.rs
+++ b/core/interop/src/lib.rs
@@ -30,6 +30,7 @@ impl<T: IntoIterator<Item = (JsString, NativeFunction)> + Clone> IntoJsModule fo
                 })
             },
             None,
+            None,
             context,
         )
     }