Browse Source

Properly resolve paths in SimpleModuleLoader and add path to Referrer::Script (#3791)

* Properly resolve paths in SimpleModuleLoader and add path to Referrer::Script

* Add missing example

* Carry active runnable on frame push

* Duplicate the tests with proper paths for Windows/Unix

* Use the right root for absolute referrers

* ugh cargo fmt

* Limit doctest to unix

* Limit doctest to unix, for real

* cargo fmt

---------

Co-authored-by: jedel1043 <jedel0124@gmail.com>
pull/3792/head
Hans Larsen 8 months ago committed by GitHub
parent
commit
958045c9ac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 17
      .vscode/launch.json
  2. 10
      .vscode/tasks.json
  3. 34
      Cargo.lock
  4. 7
      core/engine/Cargo.toml
  5. 23
      core/engine/src/context/mod.rs
  6. 218
      core/engine/src/module/loader.rs
  7. 12
      core/engine/src/script.rs
  8. 6
      core/engine/src/vm/mod.rs
  9. 2
      core/engine/tests/imports.rs
  10. 39
      tests/tester/src/main.rs
  11. 16
      tests/tester/src/read.rs

17
.vscode/launch.json vendored

@ -33,6 +33,18 @@
],
"sourceLanguages": ["rust"],
"preLaunchTask": "Cargo Build boa_cli"
},
{
"type": "lldb",
"request": "launch",
"name": "Debug Boa (Tester)",
"windows": {
"program": "${workspaceFolder}/target/debug/boa_tester.exe"
},
"program": "${workspaceFolder}/target/debug/boa_tester",
"args": ["run", "-s", "${input:testPath}", "-vvv"],
"sourceLanguages": ["rust"],
"preLaunchTask": "Cargo Build boa_tester"
}
],
"inputs": [
@ -47,6 +59,11 @@
"description": "Relative path to the module root directory",
"default": "debug",
"type": "promptString"
},
{
"id": "testPath",
"description": "Relative path to the test from the test262 directory",
"type": "promptString"
}
]
}

10
.vscode/tasks.json vendored

@ -23,6 +23,16 @@
"clear": true
}
},
{
"type": "process",
"label": "Cargo Build boa_tester",
"command": "cargo",
"args": ["build", "-p", "boa_tester"],
"group": "build",
"presentation": {
"clear": true
}
},
{
"type": "process",
"label": "Run JS file",

34
Cargo.lock generated

@ -438,6 +438,7 @@ dependencies = [
"sys-locale",
"tap",
"temporal_rs",
"test-case",
"textwrap",
"thin-vec",
"thiserror",
@ -3735,6 +3736,39 @@ dependencies = [
"winapi-util",
]
[[package]]
name = "test-case"
version = "3.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eb2550dd13afcd286853192af8601920d959b14c401fcece38071d53bf0768a8"
dependencies = [
"test-case-macros",
]
[[package]]
name = "test-case-core"
version = "3.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "adcb7fd841cd518e279be3d5a3eb0636409487998a4aff22f3de87b81e88384f"
dependencies = [
"cfg-if",
"proc-macro2",
"quote",
"syn 2.0.57",
]
[[package]]
name = "test-case-macros"
version = "3.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5c89e72a01ed4c579669add59014b9a524d609c0c88c6a585ce37485879f6ffb"
dependencies = [
"proc-macro2",
"quote",
"syn 2.0.57",
"test-case-core",
]
[[package]]
name = "textwrap"
version = "0.16.1"

7
core/engine/Cargo.toml

@ -68,7 +68,7 @@ js = ["dep:web-time"]
[dependencies]
boa_interner.workspace = true
boa_gc = { workspace = true, features = [ "thin-vec" ] }
boa_gc = { workspace = true, features = ["thin-vec"] }
boa_profiler.workspace = true
boa_macros.workspace = true
boa_ast.workspace = true
@ -106,7 +106,7 @@ time.workspace = true
hashbrown.workspace = true
# intl deps
boa_icu_provider = {workspace = true, features = ["std"], optional = true }
boa_icu_provider = { workspace = true, features = ["std"], optional = true }
sys-locale = { version = "0.3.1", optional = true }
icu_provider = { workspace = true, optional = true }
icu_locid = { workspace = true, features = ["serde"], optional = true }
@ -116,7 +116,7 @@ icu_calendar = { workspace = true, default-features = false, optional = true }
icu_collator = { workspace = true, default-features = false, features = ["serde"], optional = true }
icu_plurals = { workspace = true, default-features = false, features = ["serde", "experimental"], optional = true }
icu_list = { workspace = true, default-features = false, features = ["serde"], optional = true }
icu_casemap = { workspace = true, default-features = false, features = ["serde"], optional = true}
icu_casemap = { workspace = true, default-features = false, features = ["serde"], optional = true }
icu_segmenter = { workspace = true, default-features = false, features = ["auto", "serde"], optional = true }
icu_decimal = { workspace = true, default-features = false, features = ["serde"], optional = true }
writeable = { workspace = true, optional = true }
@ -137,6 +137,7 @@ float-cmp = "0.9.0"
indoc.workspace = true
textwrap.workspace = true
futures-lite = "2.3.0"
test-case = "3.3.1"
[target.x86_64-unknown-linux-gnu.dev-dependencies]
jemallocator = "0.5.4"

23
core/engine/src/context/mod.rs

@ -1,20 +1,17 @@
//! The ECMAScript context.
mod hooks;
#[cfg(feature = "intl")]
pub(crate) mod icu;
pub mod intrinsics;
use std::{cell::Cell, path::Path, rc::Rc};
use boa_ast::StatementList;
use boa_interner::Interner;
use boa_parser::source::ReadChar;
use boa_profiler::Profiler;
pub use hooks::{DefaultHooks, HostHooks};
#[cfg(feature = "intl")]
pub use icu::IcuError;
use intrinsics::Intrinsics;
use std::{cell::Cell, path::Path, rc::Rc};
use crate::vm::RuntimeLimits;
use crate::{
builtins,
class::{Class, ClassBuilder},
@ -30,14 +27,14 @@ use crate::{
vm::{ActiveRunnable, CallFrame, Vm},
JsNativeError, JsResult, JsString, JsValue, Source,
};
use boa_ast::StatementList;
use boa_interner::Interner;
use boa_profiler::Profiler;
use crate::vm::RuntimeLimits;
use self::intrinsics::StandardConstructor;
mod hooks;
#[cfg(feature = "intl")]
pub(crate) mod icu;
pub mod intrinsics;
thread_local! {
static CANNOT_BLOCK_COUNTER: Cell<u64> = const { Cell::new(0) };
}

218
core/engine/src/module/loader.rs

@ -1,4 +1,4 @@
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};
use rustc_hash::FxHashMap;
@ -13,6 +13,97 @@ use crate::{
use super::Module;
/// Resolves paths from the referrer and the specifier, normalize the paths and ensure the path
/// is within a base. If the base is empty, that last verification will be skipped.
///
/// The returned specifier is a resolved absolute path that is guaranteed to be
/// a descendant of `base`. All path component that are either empty or `.` and
/// `..` have been resolved.
///
/// # Errors
/// This predicate will return an error if the specifier is relative but the referrer
/// does not have a path, or if the resolved path is outside `base`.
///
/// # Examples
/// ```
/// #[cfg(target_family = "unix")]
/// # {
/// # use std::path::Path;
/// # use boa_engine::{Context, js_string};
/// # use boa_engine::module::resolve_module_specifier;
/// assert_eq!(
/// resolve_module_specifier(
/// Some(Path::new("/base")),
/// &js_string!("../a.js"),
/// Some(Path::new("/base/hello/ref.js")),
/// &mut Context::default()
/// ),
/// Ok("/base/a.js".into())
/// );
/// # }
/// ```
pub fn resolve_module_specifier(
base: Option<&Path>,
specifier: &JsString,
referrer: Option<&Path>,
_context: &mut Context,
) -> JsResult<PathBuf> {
let base = base.map_or_else(|| PathBuf::from(""), PathBuf::from);
let referrer_dir = referrer.and_then(|p| p.parent());
let specifier = specifier.to_std_string_escaped();
let short_path = Path::new(&specifier);
// In ECMAScript, a path is considered relative if it starts with
// `./` or `../`. In Rust it's any path that start with `/`.
let is_relative = short_path.starts_with(".") || short_path.starts_with("..");
let long_path = if is_relative {
if let Some(r_path) = referrer_dir {
base.join(r_path).join(short_path)
} else {
return Err(JsError::from_opaque(
js_string!("relative path without referrer").into(),
));
}
} else {
base.join(&specifier)
};
if long_path.is_relative() {
return Err(JsError::from_opaque(
js_string!("resolved path is relative").into(),
));
}
// Normalize the path. We cannot use `canonicalize` here because it will fail
// if the path doesn't exist.
let path = long_path
.components()
.filter(|c| c != &Component::CurDir || c == &Component::Normal("".as_ref()))
.try_fold(PathBuf::new(), |mut acc, c| {
if c == Component::ParentDir {
if acc.as_os_str().is_empty() {
return Err(JsError::from_opaque(
js_string!("path is outside the module root").into(),
));
}
acc.pop();
} else {
acc.push(c);
}
Ok(acc)
})?;
if path.starts_with(&base) {
Ok(path)
} else {
Err(JsError::from_opaque(
js_string!("path is outside the module root").into(),
))
}
}
/// The referrer from which a load request of a module originates.
#[derive(Debug, Clone)]
pub enum Referrer {
@ -31,7 +122,7 @@ impl Referrer {
match self {
Self::Module(module) => module.path(),
Self::Realm(_) => None,
Self::Script(_script) => None,
Self::Script(script) => script.path(),
}
}
}
@ -194,53 +285,21 @@ impl ModuleLoader for SimpleModuleLoader {
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 = 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!(
"could not canonicalize path `{}`",
short_path.display()
))
.with_cause(JsError::from_opaque(js_string!(err.to_string()).into()))
})?;
let short_path = specifier.to_std_string_escaped();
let path =
resolve_module_specifier(Some(&self.root), &specifier, referrer.path(), context)?;
if let Some(module) = self.get(&path) {
return Ok(module);
}
let source = Source::from_filepath(&path).map_err(|err| {
JsNativeError::typ()
.with_message(format!("could not open file `{}`", short_path.display()))
.with_message(format!("could not open file `{short_path}`"))
.with_cause(JsError::from_opaque(js_string!(err.to_string()).into()))
})?;
let module = Module::parse(source, None, context).map_err(|err| {
JsNativeError::syntax()
.with_message(format!("could not parse module `{}`", short_path.display()))
.with_message(format!("could not parse module `{short_path}`"))
.with_cause(err)
})?;
self.insert(path, module.clone());
@ -250,3 +309,82 @@ impl ModuleLoader for SimpleModuleLoader {
finish_load(result, context);
}
}
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use test_case::test_case;
use super::*;
// Tests on Windows and Linux are different because of the path separator and the definition
// of absolute paths.
#[rustfmt::skip]
#[cfg(target_family = "unix")]
#[test_case(Some("/hello/ref.js"), "a.js", Ok("/base/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("/base/e.js"))]
#[test_case(Some("/base/ref.js"), "./f.js", Ok("/base/f.js"))]
#[test_case(Some("./ref.js"), "./g.js", Ok("/base/g.js"))]
#[test_case(Some("./other/ref.js"), "./other/h.js", Ok("/base/other/other/h.js"))]
#[test_case(Some("./other/ref.js"), "./other/../h1.js", Ok("/base/other/h1.js"))]
#[test_case(Some("./other/ref.js"), "./../h2.js", Ok("/base/h2.js"))]
#[test_case(None, "./i.js", Err(()))]
#[test_case(None, "j.js", Ok("/base/j.js"))]
#[test_case(None, "other/k.js", Ok("/base/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(ref_path: Option<&str>, spec: &str, expected: Result<&str, ()>) {
let base = PathBuf::from("/base");
let mut context = Context::default();
let spec = js_string!(spec);
let ref_path = ref_path.map(PathBuf::from);
let actual = resolve_module_specifier(
Some(&base),
&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"))]
#[test_case(Some("a:\\base\\ref.js"), ".\\b.js", Ok("a:\\base\\b.js"))]
#[test_case(Some("a:\\base\\other\\ref.js"), ".\\c.js", Ok("a:\\base\\other\\c.js"))]
#[test_case(Some("a:\\base\\other\\ref.js"), "..\\d.js", Ok("a:\\base\\d.js"))]
#[test_case(Some("a:\\base\\ref.js"), "e.js", Ok("a:\\base\\e.js"))]
#[test_case(Some("a:\\base\\ref.js"), ".\\f.js", Ok("a:\\base\\f.js"))]
#[test_case(Some(".\\ref.js"), ".\\g.js", Ok("a:\\base\\g.js"))]
#[test_case(Some(".\\other\\ref.js"), ".\\other\\h.js", Ok("a:\\base\\other\\other\\h.js"))]
#[test_case(Some(".\\other\\ref.js"), ".\\other\\..\\h1.js", Ok("a:\\base\\other\\h1.js"))]
#[test_case(Some(".\\other\\ref.js"), ".\\..\\h2.js", Ok("a:\\base\\h2.js"))]
#[test_case(None, ".\\i.js", Err(()))]
#[test_case(None, "j.js", Ok("a:\\base\\j.js"))]
#[test_case(None, "other\\k.js", Ok("a:\\base\\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(ref_path: Option<&str>, spec: &str, expected: Result<&str, ()>) {
let base = PathBuf::from("a:\\base");
let mut context = Context::default();
let spec = js_string!(spec);
let ref_path = ref_path.map(PathBuf::from);
let actual = resolve_module_specifier(
Some(&base),
&spec,
ref_path.as_deref(),
&mut context,
);
assert_eq!(actual.map_err(|_| ()), expected.map(PathBuf::from));
}
}

12
core/engine/src/script.rs

@ -8,10 +8,13 @@
//! [spec]: https://tc39.es/ecma262/#sec-scripts
//! [script]: https://tc39.es/ecma262/#sec-script-records
use std::path::{Path, PathBuf};
use rustc_hash::FxHashMap;
use boa_gc::{Finalize, Gc, GcRefCell, Trace};
use boa_parser::{source::ReadChar, Parser, Source};
use boa_profiler::Profiler;
use rustc_hash::FxHashMap;
use crate::{
bytecompiler::ByteCompiler,
@ -47,6 +50,7 @@ struct Inner {
codeblock: GcRefCell<Option<Gc<CodeBlock>>>,
loaded_modules: GcRefCell<FxHashMap<JsString, Module>>,
host_defined: HostDefined,
path: Option<PathBuf>,
}
impl Script {
@ -80,6 +84,7 @@ impl Script {
context: &mut Context,
) -> JsResult<Self> {
let _timer = Profiler::global().start_event("Script 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());
if context.is_strict() {
@ -97,6 +102,7 @@ impl Script {
codeblock: GcRefCell::default(),
loaded_modules: GcRefCell::default(),
host_defined: HostDefined::default(),
path,
}),
})
}
@ -212,4 +218,8 @@ impl Script {
Ok(())
}
pub(super) fn path(&self) -> Option<&Path> {
self.inner.path.as_deref()
}
}

6
core/engine/src/vm/mod.rs

@ -174,6 +174,12 @@ impl Vm {
);
}
// Keep carrying the last active runnable in case the current callframe
// yields.
if frame.active_runnable.is_none() {
frame.active_runnable = self.frames.last().and_then(|fr| fr.active_runnable.clone());
}
self.frames.push(frame);
}

2
core/engine/tests/imports.rs

@ -19,7 +19,7 @@ fn subdirectories() {
.build()
.unwrap();
let source = Source::from_bytes(b"export { file1 } from './file1.js';");
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);

39
tests/tester/src/main.rs

@ -9,38 +9,41 @@
clippy::cast_precision_loss
)]
mod edition;
mod exec;
mod read;
mod results;
use self::{
read::{read_harness, read_suite, read_test, MetaData, Negative, TestFlag},
results::{compare_results, write_json},
use std::{
ops::{Add, AddAssign},
path::{Path, PathBuf},
process::Command,
time::Instant,
};
use bitflags::bitflags;
use boa_engine::optimizer::OptimizerOptions;
use clap::{ArgAction, Parser, ValueHint};
use color_eyre::{
eyre::{bail, eyre, WrapErr},
Result,
};
use colored::Colorize;
use edition::SpecEdition;
use once_cell::sync::Lazy;
use read::ErrorType;
use rustc_hash::{FxHashMap, FxHashSet};
use serde::{
de::{Unexpected, Visitor},
Deserialize, Deserializer, Serialize,
};
use std::{
ops::{Add, AddAssign},
path::{Path, PathBuf},
process::Command,
time::Instant,
use boa_engine::optimizer::OptimizerOptions;
use edition::SpecEdition;
use read::ErrorType;
use self::{
read::{read_harness, read_suite, read_test, MetaData, Negative, TestFlag},
results::{compare_results, write_json},
};
mod edition;
mod exec;
mod read;
mod results;
static START: Lazy<Instant> = Lazy::new(Instant::now);
/// Structure that contains the configuration of the tester.
@ -225,7 +228,9 @@ fn main() -> Result<()> {
clone_test262(test262_commit, verbose)?;
Path::new(DEFAULT_TEST262_DIRECTORY)
};
}
.canonicalize();
let test262_path = &test262_path.wrap_err("could not get the Test262 path")?;
run_test_suite(
&config,

16
tests/tester/src/read.rs

@ -1,19 +1,21 @@
//! Module to read the list of test suites from disk.
use crate::{HarnessFile, Ignored};
use std::{
ffi::OsStr,
fs, io,
path::{Path, PathBuf},
};
use super::{Harness, Locale, Phase, Test, TestSuite};
use color_eyre::{
eyre::{eyre, WrapErr},
Result,
};
use rustc_hash::FxHashMap;
use serde::Deserialize;
use std::{
ffi::OsStr,
fs, io,
path::{Path, PathBuf},
};
use crate::{HarnessFile, Ignored};
use super::{Harness, Locale, Phase, Test, TestSuite};
/// Representation of the YAML metadata in Test262 tests.
#[derive(Debug, Clone, Deserialize)]

Loading…
Cancel
Save