diff --git a/src/cache.rs b/src/cache.rs index 8d54f36..49faa6d 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -109,6 +109,12 @@ impl Cache { #[derive(Clone)] pub struct CachedPath(Arc); +impl std::fmt::Debug for CachedPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.path.fmt(f) + } +} + impl Hash for CachedPath { fn hash(&self, state: &mut H) { self.0.hash.hash(state); diff --git a/src/context.rs b/src/context.rs index d03ce8c..57ee5f4 100644 --- a/src/context.rs +++ b/src/context.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use crate::error::ResolveError; +use crate::{cache::CachedPath, error::ResolveError}; #[derive(Debug, Default, Clone)] pub struct ResolveContext(ResolveContextImpl); @@ -25,7 +25,14 @@ pub struct ResolveContextImpl { /// The current resolving alias for bailing recursion alias. pub resolving_alias: Option, - /// For avoiding infinite recursion, which will cause stack overflow. + /// Stack-based recursion detection (ported from enhanced-resolve). + /// A duplicate `(path, specifier)` entry means we have entered a cycle. + /// Entries are unwound via `finish_resolve` so sibling fallback branches + /// don't see stale entries from earlier attempts. + resolve_stack: Vec<(CachedPath, String)>, + + /// Depth guard for non-repeating rewrite cycles where the specifier + /// changes on every hop (e.g. alias expansion `a → b/c → a/c/c → …`). depth: u8, } @@ -78,12 +85,38 @@ impl ResolveContext { self.resolving_alias = Some(alias); } - pub fn test_for_infinite_recursion(&mut self) -> Result<(), ResolveError> { + /// enhanced-resolve: Resolver.doResolve stack-based recursion detection. + /// + /// 1. Checks whether the same `(path, specifier)` pair already exists in + /// the resolve stack — a duplicate means a repeating cycle. + /// 2. Enforces a depth limit to catch non-repeating rewrite cycles where + /// the specifier changes on every hop. + pub fn test_for_infinite_recursion( + &mut self, + cached_path: &CachedPath, + specifier: &str, + ) -> Result<(), ResolveError> { + if self + .resolve_stack + .iter() + .any(|(p, s)| p == cached_path && s == specifier) + { + return Err(ResolveError::Recursion); + } + self.depth += 1; - // 64 should be more than enough for detecting infinite recursion. if self.depth > 32 { return Err(ResolveError::Recursion); } + + self + .resolve_stack + .push((cached_path.clone(), specifier.to_string())); Ok(()) } + + pub fn finish_resolve(&mut self) { + // just pop stack, DO NOT decrease depth to keep depth detection unchanged. + self.resolve_stack.pop(); + } } diff --git a/src/lib.rs b/src/lib.rs index 55662ac..47281f1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -299,17 +299,23 @@ impl ResolverGeneric { ctx: &'a mut Ctx, ) -> BoxFuture<'a, Result> { let fut = async move { - ctx.test_for_infinite_recursion()?; + ctx.test_for_infinite_recursion(cached_path, specifier)?; // enhanced-resolve: parse - let (parsed, try_fragment_as_path) = self.load_parse(cached_path, specifier, ctx).await?; - if let Some(path) = try_fragment_as_path { - return Ok(path); + let result = async { + let (parsed, try_fragment_as_path) = self.load_parse(cached_path, specifier, ctx).await?; + if let Some(path) = try_fragment_as_path { + return Ok(path); + } + + self + .require_without_parse(cached_path, parsed.path(), ctx) + .await } + .await; - self - .require_without_parse(cached_path, parsed.path(), ctx) - .await + ctx.finish_resolve(); + result }; Box::pin(fut) } @@ -1208,17 +1214,19 @@ impl ResolverGeneric { if module_specifier.is_some_and(|s| s == new_specifier) { return Ok(None); } - if ctx - .resolving_alias - .as_ref() - .is_some_and(|s| s == new_specifier) + // Detect self-reference (e.g. `{"./lib/main.js": "./lib/main.js"}`) + let is_self_reference = new_specifier.strip_prefix("./").is_some_and(|s| { + path + .strip_prefix(package_json.directory()) + .is_ok_and(|rel| rel == Path::new(s)) + }); + if is_self_reference + || ctx + .resolving_alias + .as_ref() + .is_some_and(|s| s == new_specifier) { - // Complete when resolving to self `{"./a.js": "./a.js"}` - if new_specifier - .strip_prefix("./") - .filter(|s| path.ends_with(Path::new(s))) - .is_some() - { + if is_self_reference { return if cached_path.is_file(&self.cache.fs, ctx).await { if self.check_restrictions(cached_path.path()) { Ok(Some(cached_path.clone())) diff --git a/src/tests/alias.rs b/src/tests/alias.rs index 7554217..b66440a 100644 --- a/src/tests/alias.rs +++ b/src/tests/alias.rs @@ -151,6 +151,72 @@ async fn infinite_recursion() { assert_eq!(resolution, Err(ResolveError::Recursion)); } +// Non-repeating rewrite cycle: specifier changes on every hop. +// `a → b/c`, `b → a/c` produces a/c/c → b/c/c/c → a/c/c/c/c → … +// The depth guard catches this even though no exact `(path, specifier)` pair repeats. +#[tokio::test] +#[cfg(not(target_os = "windows"))] +async fn non_repeating_rewrite_cycle() { + use std::path::Path; + + use super::memory_fs::MemoryFS; + use crate::ResolverGeneric; + + let f = Path::new("/"); + + let file_system = MemoryFS::new(&[("/placeholder", "")]); + + let resolver = ResolverGeneric::::new_with_file_system( + file_system, + ResolveOptions { + alias: vec![ + ("a".into(), vec![AliasValue::from("b/c")]), + ("b".into(), vec![AliasValue::from("a/c")]), + ], + modules: vec!["/".into()], + ..ResolveOptions::default() + }, + ); + let resolution = resolver.resolve(f, "a").await; + assert_eq!(resolution, Err(ResolveError::Recursion)); +} + +// Sibling fallback branches that share a `(path, specifier)` should not +// falsely report recursion — the stack must be unwound between attempts. +#[tokio::test] +#[cfg(not(target_os = "windows"))] +async fn sibling_fallback_no_false_recursion() { + use std::path::{Path, PathBuf}; + + use super::memory_fs::MemoryFS; + use crate::ResolverGeneric; + + let f = Path::new("/"); + + let file_system = MemoryFS::new(&[("/c/index", "")]); + + // Both branches eventually resolve `c`, only the second succeeds. + // Without stack unwinding this would falsely detect recursion on the + // second branch because `(/, c)` was already visited by the first. + let resolver = ResolverGeneric::::new_with_file_system( + file_system, + ResolveOptions { + alias: vec![ + ( + "a".into(), + vec![AliasValue::from("b"), AliasValue::from("c")], + ), + ("b".into(), vec![AliasValue::from("c")]), + ], + modules: vec!["/".into()], + ..ResolveOptions::default() + }, + ); + + let resolved = resolver.resolve(f, "a").await.map(|r| r.full_path()); + assert_eq!(resolved, Ok(PathBuf::from("/c/index"))); +} + fn check_slash(path: &Path) { let s = path.to_string_lossy().to_string(); #[cfg(target_os = "windows")]