From 75ccc2ea6f950efbbaada24d9ea16fc958ffc59b Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 1 Apr 2024 17:34:42 -0700 Subject: [PATCH 01/18] Add an embed_module!() macro to boa_interop The macro creates a ModuleLoader that includes all JS files from a directory. --- Cargo.lock | 1 + core/interop/Cargo.toml | 1 + core/interop/src/lib.rs | 1 + core/interop/src/loaders.rs | 1 + core/interop/src/loaders/embedded.rs | 162 ++++++++++++++++++++++ core/interop/tests/embedded.rs | 63 +++++++++ core/interop/tests/embedded/dir1/file3.js | 6 + core/interop/tests/embedded/dir1/file4.js | 3 + core/interop/tests/embedded/file1.js | 6 + core/interop/tests/embedded/file2.js | 3 + core/macros/src/embedded_module_loader.rs | 122 ++++++++++++++++ core/macros/src/lib.rs | 13 ++ 12 files changed, 382 insertions(+) create mode 100644 core/interop/src/loaders/embedded.rs create mode 100644 core/interop/tests/embedded.rs create mode 100644 core/interop/tests/embedded/dir1/file3.js create mode 100644 core/interop/tests/embedded/dir1/file4.js create mode 100644 core/interop/tests/embedded/file1.js create mode 100644 core/interop/tests/embedded/file2.js create mode 100644 core/macros/src/embedded_module_loader.rs diff --git a/Cargo.lock b/Cargo.lock index 0d51cc0bbc4..98192476e4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -508,6 +508,7 @@ version = "0.18.0" dependencies = [ "boa_engine", "boa_gc", + "boa_macros", "rustc-hash", ] diff --git a/core/interop/Cargo.toml b/core/interop/Cargo.toml index 454b2b1c5c1..ad3cae0c54e 100644 --- a/core/interop/Cargo.toml +++ b/core/interop/Cargo.toml @@ -13,6 +13,7 @@ rust-version.workspace = true [dependencies] boa_engine.workspace = true boa_gc.workspace = true +boa_macros.workspace = true rustc-hash = { workspace = true, features = ["std"] } [lints] diff --git a/core/interop/src/lib.rs b/core/interop/src/lib.rs index f048bf2f655..d30e0c24632 100644 --- a/core/interop/src/lib.rs +++ b/core/interop/src/lib.rs @@ -4,6 +4,7 @@ use std::cell::RefCell; use boa_engine::module::SyntheticModuleInitializer; use boa_engine::{Context, JsString, JsValue, Module, NativeFunction}; +pub use boa_macros; pub mod loaders; diff --git a/core/interop/src/loaders.rs b/core/interop/src/loaders.rs index 663210dde87..fb59f4fc603 100644 --- a/core/interop/src/loaders.rs +++ b/core/interop/src/loaders.rs @@ -3,4 +3,5 @@ pub use hashmap::HashMapModuleLoader; +pub mod embedded; pub mod hashmap; diff --git a/core/interop/src/loaders/embedded.rs b/core/interop/src/loaders/embedded.rs new file mode 100644 index 00000000000..11a3d709339 --- /dev/null +++ b/core/interop/src/loaders/embedded.rs @@ -0,0 +1,162 @@ +//! Embedded module loader. Creates a `ModuleLoader` instance that contains +//! files embedded in the binary at build time. + +use std::cell::RefCell; +use std::collections::HashMap; +use std::path::Path; + +use boa_engine::module::{ModuleLoader, Referrer}; +use boa_engine::{Context, JsError, JsNativeError, JsResult, JsString, Module, Source}; + +fn normalize_specifier(specifier: &JsString) -> JsResult { + let specifier = specifier.to_std_string_escaped(); + let components = specifier.split('/').collect::>(); + let specifier = components + .into_iter() + .try_fold(String::new(), |mut acc, component| { + if component == "." { + return Ok(acc); + } + + if component == ".." { + if acc.is_empty() { + return Err(JsError::from_native( + JsNativeError::typ().with_message("invalid specifier".to_string()), + )); + } + + acc.pop(); + return Ok(acc); + } + + if !acc.is_empty() { + acc.push('/'); + } + acc.push_str(component); + Ok(acc) + })?; + + Ok(specifier.into()) +} + +/// Create a module loader that embeds files from the filesystem at build +/// time. This is useful for bundling assets with the binary. +/// +/// By default, will error if the total file size exceeds 1MB. This can be +/// changed by specifying the `max_size` parameter. +/// +/// The embedded module will only contain files that have the `.js`, `.mjs`, +/// or `.cjs` extension. +#[macro_export] +macro_rules! embed_module { + ($path: literal, max_size = $max_size: literal) => { + $crate::loaders::embedded::EmbeddedModuleLoader::from_iter( + $crate::boa_macros::embed_module_inner!($path, $max_size), + ) + }; + ($path: literal) => { + embed_module!($path, max_size = 1_048_576) + }; +} + +#[derive(Debug, Clone)] +enum EmbeddedModuleEntry { + Source(JsString, &'static [u8]), + Module(Module), + Error(JsError), +} + +impl EmbeddedModuleEntry { + fn from_source(path: JsString, source: &'static [u8]) -> Self { + Self::Source(path, source) + } + + fn cache(&mut self, context: &mut Context) -> JsResult<&Module> { + if let Self::Source(path, source) = self { + let mut bytes: &[u8] = source; + let path = path.to_std_string_escaped(); + let source = Source::from_reader(&mut bytes, Some(Path::new(&path))); + match Module::parse(source, None, context) { + Ok(module) => { + *self = Self::Module(module); + } + Err(err) => { + *self = Self::Error(err); + } + } + }; + + match self { + Self::Module(module) => Ok(module), + Self::Error(err) => Err(err.clone()), + EmbeddedModuleEntry::Source(_, _) => unreachable!(), + } + } + + fn as_module(&self) -> Option<&Module> { + match self { + Self::Module(module) => Some(module), + _ => None, + } + } +} + +/// The resulting type of creating an embedded module loader. +#[derive(Debug, Clone)] +#[allow(clippy::module_name_repetitions)] +pub struct EmbeddedModuleLoader { + map: HashMap>, +} + +impl FromIterator<(JsString, &'static [u8])> for EmbeddedModuleLoader { + fn from_iter>(iter: T) -> Self { + Self { + map: iter + .into_iter() + .map(|(path, source)| { + ( + path.clone(), + RefCell::new(EmbeddedModuleEntry::from_source(path, source)), + ) + }) + .collect(), + } + } +} + +impl ModuleLoader for EmbeddedModuleLoader { + fn load_imported_module( + &self, + _referrer: Referrer, + specifier: JsString, + finish_load: Box, &mut Context)>, + context: &mut Context, + ) { + let specifier = match normalize_specifier(&specifier) { + Ok(specifier) => specifier, + Err(err) => { + finish_load(Err(err), context); + return; + } + }; + + if let Some(module) = self.map.get(&specifier) { + let mut embedded = module.borrow_mut(); + let module = embedded.cache(context); + + finish_load(module.cloned(), context); + } else { + let err = JsNativeError::typ().with_message(format!( + "could not find module `{}`", + specifier.to_std_string_escaped() + )); + finish_load(Err(err.into()), context); + } + } + + fn get_module(&self, specifier: JsString) -> Option { + self.map + .get(&specifier) + .and_then(|module| module.borrow().as_module().cloned()) + } +} diff --git a/core/interop/tests/embedded.rs b/core/interop/tests/embedded.rs new file mode 100644 index 00000000000..9fbdea38d05 --- /dev/null +++ b/core/interop/tests/embedded.rs @@ -0,0 +1,63 @@ +#![allow(unused_crate_dependencies)] + +use std::rc::Rc; + +use boa_engine::builtins::promise::PromiseState; +use boa_engine::module::ModuleLoader; +use boa_engine::{js_string, Context, JsString, JsValue, Module, Source}; +use boa_interop::embed_module; + +#[test] +fn simple() { + let module_loader = Rc::new(embed_module!("tests/embedded/")); + let mut context = Context::builder() + .module_loader(module_loader.clone()) + .build() + .unwrap(); + + // Resolving modules that exist but haven't been cached yet should return None. + assert_eq!(module_loader.get_module(JsString::from("file1.js")), None); + assert_eq!( + module_loader.get_module(JsString::from("non-existent.js")), + None + ); + + let module = Module::parse( + Source::from_bytes(b"export { bar } from 'file1.js';"), + None, + &mut context, + ) + .expect("failed to parse module"); + let promise = module.load_link_evaluate(&mut context); + context.run_jobs(); + + match promise.state() { + PromiseState::Fulfilled(value) => { + assert!( + value.is_undefined(), + "Expected undefined, got {}", + value.display() + ); + + let bar = module + .namespace(&mut context) + .get(js_string!("bar"), &mut context) + .unwrap() + .as_callable() + .cloned() + .unwrap(); + let value = bar.call(&JsValue::undefined(), &[], &mut context).unwrap(); + assert_eq!( + value.as_number(), + Some(6.), + "Expected 6, got {}", + value.display() + ); + } + PromiseState::Rejected(err) => panic!( + "promise was not fulfilled: {:?}", + err.to_string(&mut context) + ), + PromiseState::Pending => panic!("Promise was not settled"), + } +} diff --git a/core/interop/tests/embedded/dir1/file3.js b/core/interop/tests/embedded/dir1/file3.js new file mode 100644 index 00000000000..d2f1b8f8760 --- /dev/null +++ b/core/interop/tests/embedded/dir1/file3.js @@ -0,0 +1,6 @@ +// Enable this when https://github.com/boa-dev/boa/pull/3781 is fixed and merged. +// export { foo } from "./file4.js"; + +export function foo() { + return 3; +} diff --git a/core/interop/tests/embedded/dir1/file4.js b/core/interop/tests/embedded/dir1/file4.js new file mode 100644 index 00000000000..58cc5fc0181 --- /dev/null +++ b/core/interop/tests/embedded/dir1/file4.js @@ -0,0 +1,3 @@ +export function foo() { + return 3; +} diff --git a/core/interop/tests/embedded/file1.js b/core/interop/tests/embedded/file1.js new file mode 100644 index 00000000000..4f918bf9a79 --- /dev/null +++ b/core/interop/tests/embedded/file1.js @@ -0,0 +1,6 @@ +import {foo} from './file2.js'; +import {foo as foo2} from './dir1/file3.js'; + +export function bar() { + return foo() + foo2() + 1; +} diff --git a/core/interop/tests/embedded/file2.js b/core/interop/tests/embedded/file2.js new file mode 100644 index 00000000000..d09103bc5d5 --- /dev/null +++ b/core/interop/tests/embedded/file2.js @@ -0,0 +1,3 @@ +export function foo() { + return 2; +} diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs new file mode 100644 index 00000000000..2019d16bd13 --- /dev/null +++ b/core/macros/src/embedded_module_loader.rs @@ -0,0 +1,122 @@ +//! Embedded module loader. Creates a `ModuleLoader` instance that contains +//! files embedded in the binary at build time. + +use proc_macro::TokenStream; +use std::path::PathBuf; + +use quote::quote; +use syn::{parse::Parse, LitInt, LitStr, Token}; + +struct EmbedModuleMacroInput { + path: LitStr, + max_size: u64, +} + +impl Parse for EmbedModuleMacroInput { + fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + let path = input.parse()?; + let _comma: Token![,] = input.parse()?; + let max_size = input.parse::()?.base10_parse()?; + + Ok(Self { path, max_size }) + } +} + +/// List all the files readable from the given directory, recursively. +fn find_all_files(dir: &mut std::fs::ReadDir, root: &PathBuf) -> Vec { + let mut files = Vec::new(); + for entry in dir { + let Ok(entry) = entry else { + continue; + }; + + let path = entry.path(); + if path.is_dir() { + let Ok(mut sub_dir) = std::fs::read_dir(&path) else { + continue; + }; + files.append(&mut find_all_files(&mut sub_dir, root)); + } else if let Ok(path) = path.strip_prefix(root) { + files.push(path.to_path_buf()); + } + } + files +} + +/// Implementation of the `embed_module_inner!` macro. +/// This should not be used directly. Use the `embed_module!` macro from [`boa_interop`] +/// instead. +pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { + let manifest_dir = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap_or_default()); + + let input = syn::parse_macro_input!(input as EmbedModuleMacroInput); + + let root = manifest_dir.join(input.path.value()); + let max_size = input.max_size; + + let mut dir = match std::fs::read_dir(root.clone()) { + Ok(dir) => dir, + Err(e) => { + return syn::Error::new_spanned( + input.path.clone(), + format!("Error reading directory: {e}"), + ) + .to_compile_error() + .into(); + } + }; + + let mut total = 0; + let files = find_all_files(&mut dir, &root); + + let inner = match files.into_iter().try_fold(quote! {}, |acc, relative_path| { + let path = root.join(&relative_path); + let ext = path.extension().unwrap_or_default(); + let absolute_path = manifest_dir.join(&path).to_string_lossy().to_string(); + let Some(relative_path) = relative_path.to_str() else { + return Err(syn::Error::new_spanned( + input.path.clone(), + "Path has non-Unicode characters", + )); + }; + + // Check the size. + let size = std::fs::metadata(&path) + .map_err(|e| { + syn::Error::new_spanned(input.path.clone(), format!("Error reading file size: {e}")) + })? + .len(); + + total += size; + if total > max_size { + return Err(syn::Error::new_spanned( + input.path.clone(), + "Total embedded file size exceeds the maximum size", + )); + } + + if ext == "js" || ext == "mjs" || ext == "cjs" { + Ok(quote! { + #acc + + ( + ::boa_engine::js_string!(#relative_path), + include_bytes!(#absolute_path).as_ref(), + ), + }) + } else { + Ok(acc) + } + }) { + Ok(inner) => inner, + Err(e) => return e.to_compile_error().into(), + }; + + let stream = quote! { + [ + #inner + ] + }; + + stream.into() +} diff --git a/core/macros/src/lib.rs b/core/macros/src/lib.rs index 56b19a71e08..999b1737616 100644 --- a/core/macros/src/lib.rs +++ b/core/macros/src/lib.rs @@ -16,6 +16,19 @@ use syn::{ }; use synstructure::{decl_derive, AddBounds, Structure}; +mod embedded_module_loader; + +/// Implementation of the inner iterator of the `embed_module!` macro. All +/// arguments are required. +/// +/// # Warning +/// This should not be used directly as is, and instead should be used through +/// the `embed_module!` macro in [`boa_interop`] for convenience. +#[proc_macro] +pub fn embed_module_inner(input: TokenStream) -> TokenStream { + embedded_module_loader::embed_module_impl(input) +} + struct Static { literal: LitStr, ident: Ident, From 06551cb8ec63dedffb2598372b8b310a9c6f03da Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:08:06 -0700 Subject: [PATCH 02/18] Add description of function --- core/interop/src/loaders/embedded.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/interop/src/loaders/embedded.rs b/core/interop/src/loaders/embedded.rs index 11a3d709339..c58c5674e4b 100644 --- a/core/interop/src/loaders/embedded.rs +++ b/core/interop/src/loaders/embedded.rs @@ -8,6 +8,7 @@ use std::path::Path; use boa_engine::module::{ModuleLoader, Referrer}; use boa_engine::{Context, JsError, JsNativeError, JsResult, JsString, Module, Source}; +/// Normalize a specifier to remove `.` and `..` components. fn normalize_specifier(specifier: &JsString) -> JsResult { let specifier = specifier.to_std_string_escaped(); let components = specifier.split('/').collect::>(); From d7a3c7df4beb37756f346c617c3369170aeef1ed Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:09:26 -0700 Subject: [PATCH 03/18] Run prettier --- core/interop/tests/embedded/dir1/file3.js | 2 +- core/interop/tests/embedded/dir1/file4.js | 2 +- core/interop/tests/embedded/file1.js | 6 +++--- core/interop/tests/embedded/file2.js | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/interop/tests/embedded/dir1/file3.js b/core/interop/tests/embedded/dir1/file3.js index d2f1b8f8760..d5e6c81469f 100644 --- a/core/interop/tests/embedded/dir1/file3.js +++ b/core/interop/tests/embedded/dir1/file3.js @@ -2,5 +2,5 @@ // export { foo } from "./file4.js"; export function foo() { - return 3; + return 3; } diff --git a/core/interop/tests/embedded/dir1/file4.js b/core/interop/tests/embedded/dir1/file4.js index 58cc5fc0181..c7d1d5f7172 100644 --- a/core/interop/tests/embedded/dir1/file4.js +++ b/core/interop/tests/embedded/dir1/file4.js @@ -1,3 +1,3 @@ export function foo() { - return 3; + return 3; } diff --git a/core/interop/tests/embedded/file1.js b/core/interop/tests/embedded/file1.js index 4f918bf9a79..10b049b34e9 100644 --- a/core/interop/tests/embedded/file1.js +++ b/core/interop/tests/embedded/file1.js @@ -1,6 +1,6 @@ -import {foo} from './file2.js'; -import {foo as foo2} from './dir1/file3.js'; +import { foo } from "./file2.js"; +import { foo as foo2 } from "./dir1/file3.js"; export function bar() { - return foo() + foo2() + 1; + return foo() + foo2() + 1; } diff --git a/core/interop/tests/embedded/file2.js b/core/interop/tests/embedded/file2.js index d09103bc5d5..2c990085cef 100644 --- a/core/interop/tests/embedded/file2.js +++ b/core/interop/tests/embedded/file2.js @@ -1,3 +1,3 @@ export function foo() { - return 2; + return 2; } From 6ed8d219ae75076bff5b50b82af03f03cd24f809 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:14:19 -0700 Subject: [PATCH 04/18] Remove reference to a unaccessible crate --- core/macros/src/embedded_module_loader.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index 2019d16bd13..e0aa640db51 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -44,8 +44,8 @@ fn find_all_files(dir: &mut std::fs::ReadDir, root: &PathBuf) -> Vec { } /// Implementation of the `embed_module_inner!` macro. -/// This should not be used directly. Use the `embed_module!` macro from [`boa_interop`] -/// instead. +/// This should not be used directly. Use the `embed_module!` macro from the `boa_interop` +/// crate instead. pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { let manifest_dir = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap_or_default()); From 9118d59cc1b0ca4f7531331d36d7dfd0f806222a Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:17:28 -0700 Subject: [PATCH 05/18] Remove one more reference to a unaccessible crate --- core/macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/macros/src/lib.rs b/core/macros/src/lib.rs index 999b1737616..600b566bdb3 100644 --- a/core/macros/src/lib.rs +++ b/core/macros/src/lib.rs @@ -23,7 +23,7 @@ mod embedded_module_loader; /// /// # Warning /// This should not be used directly as is, and instead should be used through -/// the `embed_module!` macro in [`boa_interop`] for convenience. +/// the `embed_module!` macro in `boa_interop` for convenience. #[proc_macro] pub fn embed_module_inner(input: TokenStream) -> TokenStream { embedded_module_loader::embed_module_impl(input) From 21980f8e080008dbf2136f8e5fd524dd6b8d558a Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:25:32 -0700 Subject: [PATCH 06/18] Disable test that plays with paths on Windows --- core/interop/tests/embedded.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/interop/tests/embedded.rs b/core/interop/tests/embedded.rs index 9fbdea38d05..3de8668b1a8 100644 --- a/core/interop/tests/embedded.rs +++ b/core/interop/tests/embedded.rs @@ -7,6 +7,7 @@ use boa_engine::module::ModuleLoader; use boa_engine::{js_string, Context, JsString, JsValue, Module, Source}; use boa_interop::embed_module; +#[cfg(target_family = "unix")] #[test] fn simple() { let module_loader = Rc::new(embed_module!("tests/embedded/")); From 8b1adc83d72d47fdac0233864db71260f8033e02 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 14:35:00 -0700 Subject: [PATCH 07/18] Block the whole test module instead of just the fn --- core/interop/tests/embedded.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/interop/tests/embedded.rs b/core/interop/tests/embedded.rs index 3de8668b1a8..fe689a87d9b 100644 --- a/core/interop/tests/embedded.rs +++ b/core/interop/tests/embedded.rs @@ -1,3 +1,4 @@ +#![cfg(target_family = "unix")] #![allow(unused_crate_dependencies)] use std::rc::Rc; @@ -7,7 +8,6 @@ use boa_engine::module::ModuleLoader; use boa_engine::{js_string, Context, JsString, JsValue, Module, Source}; use boa_interop::embed_module; -#[cfg(target_family = "unix")] #[test] fn simple() { let module_loader = Rc::new(embed_module!("tests/embedded/")); From 0466e19ddd841c35cb5e268a9c28091bd4edd4f0 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Sat, 6 Apr 2024 16:11:08 -0700 Subject: [PATCH 08/18] This is a bit insane --- core/interop/tests/embedded.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/interop/tests/embedded.rs b/core/interop/tests/embedded.rs index fe689a87d9b..b1b0ba8dfac 100644 --- a/core/interop/tests/embedded.rs +++ b/core/interop/tests/embedded.rs @@ -1,4 +1,3 @@ -#![cfg(target_family = "unix")] #![allow(unused_crate_dependencies)] use std::rc::Rc; @@ -10,7 +9,11 @@ use boa_interop::embed_module; #[test] fn simple() { + #[cfg(target_family = "unix")] let module_loader = Rc::new(embed_module!("tests/embedded/")); + #[cfg(target_family = "windows")] + let module_loader = Rc::new(embed_module!("tests\\embedded\\")); + let mut context = Context::builder() .module_loader(module_loader.clone()) .build() From d6030d77728f1da26446549cf4aea28840c50540 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 11:10:36 -0700 Subject: [PATCH 09/18] Replace path separators into JavaScript specifier separators --- core/macros/src/embedded_module_loader.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index e0aa640db51..752c7394353 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -4,6 +4,10 @@ use proc_macro::TokenStream; use std::path::PathBuf; +use quote::quote; +use proc_macro::TokenStream; +use std::path::PathBuf; + use quote::quote; use syn::{parse::Parse, LitInt, LitStr, Token}; @@ -80,6 +84,10 @@ pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { )); }; + // Replace component separators by `/`. JavaScript always use `/` as path separator, + // regardless of the platform. + let relative_path = relative_path.replace(std::path::MAIN_SEPARATOR, "/"); + // Check the size. let size = std::fs::metadata(&path) .map_err(|e| { From 40c7b272c7dd9d0a7d835f430843555da4666d26 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 11:11:02 -0700 Subject: [PATCH 10/18] cargo fmt --- core/macros/src/embedded_module_loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index 752c7394353..e6825bc64d8 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -4,8 +4,8 @@ use proc_macro::TokenStream; use std::path::PathBuf; -use quote::quote; use proc_macro::TokenStream; +use quote::quote; use std::path::PathBuf; use quote::quote; From 1e501fc1b0b59ab2bf087ee773b8ec1244329a99 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Tue, 9 Apr 2024 11:11:41 -0700 Subject: [PATCH 11/18] cargo fmt part deux --- core/macros/src/embedded_module_loader.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index e6825bc64d8..077596b223c 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -4,10 +4,6 @@ use proc_macro::TokenStream; use std::path::PathBuf; -use proc_macro::TokenStream; -use quote::quote; -use std::path::PathBuf; - use quote::quote; use syn::{parse::Parse, LitInt, LitStr, Token}; From b174098ca589f5e5f331889837409ee2aa141ab5 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 15 Apr 2024 13:26:02 -0700 Subject: [PATCH 12/18] fix some issues with relative path and pathing on windows --- core/interop/src/loaders/embedded.rs | 66 ++++++++--------------- core/interop/tests/embedded.rs | 6 +-- core/interop/tests/embedded/dir1/file3.js | 6 +-- core/macros/src/embedded_module_loader.rs | 28 ++++------ 4 files changed, 36 insertions(+), 70 deletions(-) diff --git a/core/interop/src/loaders/embedded.rs b/core/interop/src/loaders/embedded.rs index c58c5674e4b..e6dfd6abc26 100644 --- a/core/interop/src/loaders/embedded.rs +++ b/core/interop/src/loaders/embedded.rs @@ -8,38 +8,6 @@ use std::path::Path; use boa_engine::module::{ModuleLoader, Referrer}; use boa_engine::{Context, JsError, JsNativeError, JsResult, JsString, Module, Source}; -/// Normalize a specifier to remove `.` and `..` components. -fn normalize_specifier(specifier: &JsString) -> JsResult { - let specifier = specifier.to_std_string_escaped(); - let components = specifier.split('/').collect::>(); - let specifier = components - .into_iter() - .try_fold(String::new(), |mut acc, component| { - if component == "." { - return Ok(acc); - } - - if component == ".." { - if acc.is_empty() { - return Err(JsError::from_native( - JsNativeError::typ().with_message("invalid specifier".to_string()), - )); - } - - acc.pop(); - return Ok(acc); - } - - if !acc.is_empty() { - acc.push('/'); - } - acc.push_str(component); - Ok(acc) - })?; - - Ok(specifier.into()) -} - /// Create a module loader that embeds files from the filesystem at build /// time. This is useful for bundling assets with the binary. /// @@ -109,15 +77,16 @@ pub struct EmbeddedModuleLoader { map: HashMap>, } -impl FromIterator<(JsString, &'static [u8])> for EmbeddedModuleLoader { - fn from_iter>(iter: T) -> Self { +impl FromIterator<(&'static str, &'static [u8])> for EmbeddedModuleLoader { + fn from_iter>(iter: T) -> Self { Self { map: iter .into_iter() .map(|(path, source)| { + let p = JsString::from(path); ( - path.clone(), - RefCell::new(EmbeddedModuleEntry::from_source(path, source)), + p.clone(), + RefCell::new(EmbeddedModuleEntry::from_source(p, source)), ) }) .collect(), @@ -128,20 +97,29 @@ impl FromIterator<(JsString, &'static [u8])> for EmbeddedModuleLoader { impl ModuleLoader for EmbeddedModuleLoader { fn load_imported_module( &self, - _referrer: Referrer, + referrer: Referrer, specifier: JsString, finish_load: Box, &mut Context)>, context: &mut Context, ) { - let specifier = match normalize_specifier(&specifier) { - Ok(specifier) => specifier, - Err(err) => { - finish_load(Err(err), context); - return; - } + let Ok(specifier_path) = boa_engine::module::resolve_module_specifier( + None, + &specifier, + referrer.path(), + context, + ) else { + let err = JsNativeError::typ().with_message(format!( + "could not resolve module specifier `{}`", + specifier.to_std_string_escaped() + )); + finish_load(Err(err.into()), context); + return; }; - if let Some(module) = self.map.get(&specifier) { + if let Some(module) = self + .map + .get(&JsString::from(specifier_path.to_string_lossy().as_ref())) + { let mut embedded = module.borrow_mut(); let module = embedded.cache(context); diff --git a/core/interop/tests/embedded.rs b/core/interop/tests/embedded.rs index b1b0ba8dfac..1ef0c4c4516 100644 --- a/core/interop/tests/embedded.rs +++ b/core/interop/tests/embedded.rs @@ -20,14 +20,14 @@ fn simple() { .unwrap(); // Resolving modules that exist but haven't been cached yet should return None. - assert_eq!(module_loader.get_module(JsString::from("file1.js")), None); + assert_eq!(module_loader.get_module(JsString::from("/file1.js")), None); assert_eq!( - module_loader.get_module(JsString::from("non-existent.js")), + module_loader.get_module(JsString::from("/non-existent.js")), None ); let module = Module::parse( - Source::from_bytes(b"export { bar } from 'file1.js';"), + Source::from_bytes(b"export { bar } from '/file1.js';"), None, &mut context, ) diff --git a/core/interop/tests/embedded/dir1/file3.js b/core/interop/tests/embedded/dir1/file3.js index d5e6c81469f..756e416851e 100644 --- a/core/interop/tests/embedded/dir1/file3.js +++ b/core/interop/tests/embedded/dir1/file3.js @@ -1,6 +1,2 @@ // Enable this when https://github.com/boa-dev/boa/pull/3781 is fixed and merged. -// export { foo } from "./file4.js"; - -export function foo() { - return 3; -} +export {foo} from "./file4.js"; diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index 077596b223c..88d3918fde4 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -5,7 +5,7 @@ use proc_macro::TokenStream; use std::path::PathBuf; use quote::quote; -use syn::{parse::Parse, LitInt, LitStr, Token}; +use syn::{LitInt, LitStr, parse::Parse, Token}; struct EmbedModuleMacroInput { path: LitStr, @@ -71,7 +71,6 @@ pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { let inner = match files.into_iter().try_fold(quote! {}, |acc, relative_path| { let path = root.join(&relative_path); - let ext = path.extension().unwrap_or_default(); let absolute_path = manifest_dir.join(&path).to_string_lossy().to_string(); let Some(relative_path) = relative_path.to_str() else { return Err(syn::Error::new_spanned( @@ -79,10 +78,7 @@ pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { "Path has non-Unicode characters", )); }; - - // Replace component separators by `/`. JavaScript always use `/` as path separator, - // regardless of the platform. - let relative_path = relative_path.replace(std::path::MAIN_SEPARATOR, "/"); + let relative_path = format!("/{}", relative_path.replace(std::path::MAIN_SEPARATOR, "/")); // Check the size. let size = std::fs::metadata(&path) @@ -99,18 +95,14 @@ pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { )); } - if ext == "js" || ext == "mjs" || ext == "cjs" { - Ok(quote! { - #acc - - ( - ::boa_engine::js_string!(#relative_path), - include_bytes!(#absolute_path).as_ref(), - ), - }) - } else { - Ok(acc) - } + Ok(quote! { + #acc + + ( + #relative_path, + include_bytes!(#absolute_path).as_ref(), + ), + }) }) { Ok(inner) => inner, Err(e) => return e.to_compile_error().into(), From b3d4a0678483c335a95ac6be3b1080bd15a1b4dc Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 15 Apr 2024 14:44:43 -0700 Subject: [PATCH 13/18] fix module resolver when there are no base path --- core/engine/src/module/loader.rs | 49 ++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/core/engine/src/module/loader.rs b/core/engine/src/module/loader.rs index 4a5ee35a99c..d2df3923b89 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; @@ -48,7 +48,7 @@ pub fn resolve_module_specifier( referrer: Option<&Path>, _context: &mut Context, ) -> JsResult { - let base = base.map_or_else(|| PathBuf::from(""), PathBuf::from); + let base_path = base.map_or_else(|| PathBuf::from(""), PathBuf::from); let referrer_dir = referrer.and_then(|p| p.parent()); let specifier = specifier.to_std_string_escaped(); @@ -65,17 +65,17 @@ pub fn resolve_module_specifier( let long_path = if is_relative { if let Some(r_path) = referrer_dir { - base.join(r_path).join(short_path) + base_path.join(r_path).join(short_path) } else { return Err(JsError::from_opaque( js_string!("relative path without referrer").into(), )); } } else { - base.join(&specifier) + base_path.join(&specifier) }; - if long_path.is_relative() { + if long_path.is_relative() && base.is_some() { return Err(JsError::from_opaque( js_string!("resolved path is relative").into(), )); @@ -100,7 +100,7 @@ pub fn resolve_module_specifier( Ok(acc) })?; - if path.starts_with(&base) { + if path.starts_with(&base_path) { Ok(path) } else { Err(JsError::from_opaque( @@ -371,6 +371,39 @@ mod tests { assert_eq!(actual.map_err(|_| ()), expected.map(PathBuf::from)); } + // This tests the same cases as the previous test, but without a base path. + #[rustfmt::skip] + #[cfg(target_family = "unix")] + #[test_case(Some("hello/ref.js"), "a.js", Ok("a.js"))] + #[test_case(Some("base/ref.js"), "./b.js", Ok("base/b.js"))] + #[test_case(Some("base/other/ref.js"), "./c.js", Ok("base/other/c.js"))] + #[test_case(Some("base/other/ref.js"), "../d.js", Ok("base/d.js"))] + #[test_case(Some("base/ref.js"), "e.js", Ok("e.js"))] + #[test_case(Some("base/ref.js"), "./f.js", Ok("base/f.js"))] + #[test_case(Some("./ref.js"), "./g.js", Ok("g.js"))] + #[test_case(Some("./other/ref.js"), "./other/h.js", Ok("other/other/h.js"))] + #[test_case(Some("./other/ref.js"), "./other/../h1.js", Ok("other/h1.js"))] + #[test_case(Some("./other/ref.js"), "./../h2.js", Ok("h2.js"))] + #[test_case(None, "./i.js", Err(()))] + #[test_case(None, "j.js", Ok("j.js"))] + #[test_case(None, "other/k.js", Ok("other/k.js"))] + #[test_case(None, "other/../../l.js", Err(()))] + #[test_case(Some("/base/ref.js"), "other/../../m.js", Err(()))] + #[test_case(None, "../n.js", Err(()))] + fn resolve_test_no_base(ref_path: Option<&str>, spec: &str, expected: Result<&str, ()>) { + let mut context = Context::default(); + let spec = js_string!(spec); + let ref_path = ref_path.map(PathBuf::from); + + let actual = resolve_module_specifier( + None, + &spec, + ref_path.as_deref(), + &mut context, + ); + assert_eq!(actual.map_err(|_| ()), expected.map(PathBuf::from)); + } + #[rustfmt::skip] #[cfg(target_family = "windows")] #[test_case(Some("a:\\hello\\ref.js"), "a.js", Ok("a:\\base\\a.js"))] From d1b6dced0d0a9a370dbcc9f4c821517688da0b11 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 15 Apr 2024 14:47:52 -0700 Subject: [PATCH 14/18] use the platform's path separator --- core/macros/src/embedded_module_loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index 88d3918fde4..069a1948463 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -78,7 +78,7 @@ pub(crate) fn embed_module_impl(input: TokenStream) -> TokenStream { "Path has non-Unicode characters", )); }; - let relative_path = format!("/{}", relative_path.replace(std::path::MAIN_SEPARATOR, "/")); + let relative_path = format!("{}{}", std::path::MAIN_SEPARATOR, relative_path); // Check the size. let size = std::fs::metadata(&path) From 22f4ce95741e5f72922f0b94eda2c9995e944f4c Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 15 Apr 2024 14:52:45 -0700 Subject: [PATCH 15/18] cargo fmt --- core/engine/src/module/loader.rs | 6 +++--- core/macros/src/embedded_module_loader.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/engine/src/module/loader.rs b/core/engine/src/module/loader.rs index d2df3923b89..0f280444dd1 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; diff --git a/core/macros/src/embedded_module_loader.rs b/core/macros/src/embedded_module_loader.rs index 069a1948463..8a4963317c9 100644 --- a/core/macros/src/embedded_module_loader.rs +++ b/core/macros/src/embedded_module_loader.rs @@ -5,7 +5,7 @@ use proc_macro::TokenStream; use std::path::PathBuf; use quote::quote; -use syn::{LitInt, LitStr, parse::Parse, Token}; +use syn::{parse::Parse, LitInt, LitStr, Token}; struct EmbedModuleMacroInput { path: LitStr, From e8d90479036561793d3879e0e088e9fa6c1447ee Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Mon, 15 Apr 2024 15:18:09 -0700 Subject: [PATCH 16/18] prettier --- core/interop/tests/embedded/dir1/file3.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/interop/tests/embedded/dir1/file3.js b/core/interop/tests/embedded/dir1/file3.js index 756e416851e..57695a2642c 100644 --- a/core/interop/tests/embedded/dir1/file3.js +++ b/core/interop/tests/embedded/dir1/file3.js @@ -1,2 +1,2 @@ // Enable this when https://github.com/boa-dev/boa/pull/3781 is fixed and merged. -export {foo} from "./file4.js"; +export { foo } from "./file4.js"; From 3eb5eeb7b1e31c8011fc322f706673b3da25fb5e Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 17 Apr 2024 09:50:21 -0700 Subject: [PATCH 17/18] Remove caching of the error --- core/interop/src/loaders/embedded.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/interop/src/loaders/embedded.rs b/core/interop/src/loaders/embedded.rs index e6dfd6abc26..1d6862cead4 100644 --- a/core/interop/src/loaders/embedded.rs +++ b/core/interop/src/loaders/embedded.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; use std::path::Path; use boa_engine::module::{ModuleLoader, Referrer}; -use boa_engine::{Context, JsError, JsNativeError, JsResult, JsString, Module, Source}; +use boa_engine::{Context, JsNativeError, JsResult, JsString, Module, Source}; /// Create a module loader that embeds files from the filesystem at build /// time. This is useful for bundling assets with the binary. @@ -32,7 +32,6 @@ macro_rules! embed_module { enum EmbeddedModuleEntry { Source(JsString, &'static [u8]), Module(Module), - Error(JsError), } impl EmbeddedModuleEntry { @@ -50,14 +49,13 @@ impl EmbeddedModuleEntry { *self = Self::Module(module); } Err(err) => { - *self = Self::Error(err); + return Err(err); } } }; match self { Self::Module(module) => Ok(module), - Self::Error(err) => Err(err.clone()), EmbeddedModuleEntry::Source(_, _) => unreachable!(), } } From f2ba245f3b6b212e50936fc91df06163e2b5e2b0 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 17 Apr 2024 10:16:28 -0700 Subject: [PATCH 18/18] Pedantic clippy gonna pedant --- core/interop/src/loaders/embedded.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/interop/src/loaders/embedded.rs b/core/interop/src/loaders/embedded.rs index 1d6862cead4..0f89c0a2cdc 100644 --- a/core/interop/src/loaders/embedded.rs +++ b/core/interop/src/loaders/embedded.rs @@ -63,7 +63,7 @@ impl EmbeddedModuleEntry { fn as_module(&self) -> Option<&Module> { match self { Self::Module(module) => Some(module), - _ => None, + Self::Source(_, _) => None, } } }