From cb5cea6d268d589da667c666a9b7b238bea77d49 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 3 Apr 2024 22:01:23 -0700 Subject: [PATCH] Add a path to Module (and expose it in Referrer) (#3783) * Add an integration test for relative imports on SimpleModuleLoader * 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 * cargo clippy and fmt * prettier * Fix merge error --- core/engine/src/module/loader.rs | 39 +++++++++++++++++- core/engine/src/module/mod.rs | 32 ++++++++++----- 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 | 50 ++++++++++++++++++++++++ core/interop/src/lib.rs | 1 + core/parser/src/source/mod.rs | 16 ++++++-- examples/src/bin/synthetic.rs | 1 + 9 files changed, 136 insertions(+), 16 deletions(-) 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/src/module/loader.rs b/core/engine/src/module/loader.rs index 5d9da5755b..9fcf9d73e5 100644 --- a/core/engine/src/module/loader.rs +++ b/core/engine/src/module/loader.rs @@ -24,6 +24,18 @@ pub enum Referrer { Script(Script), } +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(), + Self::Realm(_) => None, + Self::Script(_script) => None, + } + } +} + impl From for Referrer { fn from(value: ActiveRunnable) -> Self { match value { @@ -176,17 +188,40 @@ impl SimpleModuleLoader { impl ModuleLoader for SimpleModuleLoader { fn load_imported_module( &self, - _referrer: Referrer, + referrer: Referrer, specifier: JsString, finish_load: Box, &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 83f3e3b479..9bf195f35f 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>, kind: ModuleKind, host_defined: HostDefined, + path: Option, } /// The kind of a [`Module`]. @@ -155,6 +157,7 @@ impl Module { context: &mut Context, ) -> JsResult { let _timer = Profiler::global().start_event("Module parsing", "Main"); + 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())?; @@ -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, realm: Option, 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,12 @@ impl Module { }) .clone() } + + /// 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() + } } impl PartialEq for Module { 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 0000000000..5d0002c21d --- /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 0000000000..bd9bcc1fdf --- /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 0000000000..f047b48578 --- /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 0000000000..2257d759e3 --- /dev/null +++ b/core/engine/tests/imports.rs @@ -0,0 +1,50 @@ +#![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"export { file1 } from './file1.js';"); + 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!(v.is_undefined()); + + let foo_value = module + .namespace(&mut context) + .get(js_string!("file1"), &mut context) + .unwrap() + .as_callable() + .unwrap() + .call(&JsValue::undefined(), &[], &mut context) + .unwrap(); + + assert_eq!( + foo_value, + JsValue::String(js_string!("file1..file1_1.file1_2")) + ); + } + PromiseState::Rejected(reason) => { + panic!("Module failed to load: {}", reason.display()); + } + } +} diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index f048bf2f65..0aa8739d6c 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -30,6 +30,7 @@ impl + Clone> IntoJsModule fo }) }, None, + None, context, ) } diff --git a/core/parser/src/source/mod.rs b/core/parser/src/source/mod.rs index 19409f107d..5fcf44194e 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> { } } +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 3d7c84f62d..ffab2c6ba4 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, ) }