From f38e398b2d78fed0088ec410284666098eca58db Mon Sep 17 00:00:00 2001 From: Boshen Date: Fri, 8 Dec 2023 14:37:52 +0800 Subject: [PATCH] feat: throw recursion error when resolving recursed browser fields --- .../node_modules/recursive-file/a.js | 0 .../node_modules/recursive-file/b.js | 0 .../node_modules/recursive-file/c.js | 0 .../node_modules/recursive-file/d.js | 0 .../node_modules/recursive-file/package.json | 8 +++++ src/lib.rs | 11 ++++--- src/package_json.rs | 29 +++++++++---------- src/tests/browser_field.rs | 23 +++++++++++++++ 8 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/a.js create mode 100644 fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/b.js create mode 100644 fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/c.js create mode 100644 fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/d.js create mode 100644 fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/a.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/a.js new file mode 100644 index 00000000..e69de29b diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/b.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/b.js new file mode 100644 index 00000000..e69de29b diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/c.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/c.js new file mode 100644 index 00000000..e69de29b diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/d.js b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/d.js new file mode 100644 index 00000000..e69de29b diff --git a/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json new file mode 100644 index 00000000..c13fe23b --- /dev/null +++ b/fixtures/enhanced_resolve/test/fixtures/browser-module/node_modules/recursive-file/package.json @@ -0,0 +1,8 @@ +{ + "browser": { + "a.js": "./a", + "./b.js": "./b", + "c.js": "./d.js", + "./d.js": "./c.js" + } +} diff --git a/src/lib.rs b/src/lib.rs index c8a5784e..3eab20a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -790,13 +790,16 @@ impl ResolverGeneric { package_json: &PackageJson, ctx: &mut ResolveContext, ) -> ResolveState { - let Some(specifier) = package_json.resolve_browser_field(path, specifier)? else { + let Some(new_specifier) = package_json.resolve_browser_field(path, specifier)? else { return Ok(None); }; - if ctx.resolving_alias.as_ref().is_some_and(|s| s == specifier) { + if specifier.is_some_and(|s| s == new_specifier) { return Ok(None); } - let specifier = Specifier::parse(specifier).map_err(ResolveError::Specifier)?; + if ctx.resolving_alias.as_ref().is_some_and(|s| s == new_specifier) { + return Err(ResolveError::Recursion); + } + let specifier = Specifier::parse(new_specifier).map_err(ResolveError::Specifier)?; ctx.with_query_fragment(specifier.query, specifier.fragment); ctx.with_resolving_alias(specifier.path().to_string()); ctx.with_fully_specified(false); @@ -845,7 +848,7 @@ impl ResolverGeneric { if let Some(path) = self.load_alias_value( cached_path, alias_key, - new_specifier.path(), // pass in passed alias value + new_specifier.path(), // pass in parsed alias value specifier, ctx, )? { diff --git a/src/package_json.rs b/src/package_json.rs index 4bb26d9c..19feb07f 100644 --- a/src/package_json.rs +++ b/src/package_json.rs @@ -11,7 +11,7 @@ use indexmap::IndexMap; use rustc_hash::FxHasher; use serde::{Deserialize, Deserializer}; -use crate::{path::PathUtil, ResolveError, ResolveOptions}; +use crate::{ResolveError, ResolveOptions}; type FxIndexMap = IndexMap>; @@ -145,22 +145,15 @@ impl PackageJson { } // Dynamically create `browser_fields`. - let dir = path.parent().unwrap(); for object_path in &options.alias_fields { if let Some(browser_field) = Self::get_value_by_path(&package_json_value, object_path) { let mut browser_field = BrowserField::deserialize(browser_field)?; - // Normalize all relative paths to make browser_field a constant value lookup if let BrowserField::Map(map) = &mut browser_field { - let relative_paths = map - .keys() - .filter(|path| path.starts_with(".")) - .cloned() - .collect::>(); - for relative_path in relative_paths { - if let Some(value) = map.remove(&relative_path) { - let normalized_path = dir.normalize_with(relative_path); - map.insert(normalized_path, value); - } + // Make browser_field a constant value lookup by normalizing all relative paths. + for key in + map.keys().filter(|key| !key.starts_with(".")).cloned().collect::>() + { + map.insert(PathBuf::from("./").join(&key), map[&key].clone()); } } package_json.browser_fields.push(browser_field); @@ -224,12 +217,18 @@ impl PackageJson { path: &Path, request: Option<&str>, ) -> Result, ResolveError> { - let request = request.map_or(path, |r| Path::new(r)); + let request = match request { + Some(r) => PathBuf::from(r), + None => match path.strip_prefix(&self.path.parent().unwrap()) { + Ok(r) => Path::new("./").join(r), + Err(_) => return Ok(None), + }, + }; for browser in &self.browser_fields { // Only object is valid, all other types are invalid // https://github.com/webpack/enhanced-resolve/blob/3a28f47788de794d9da4d1702a3a583d8422cd48/lib/AliasFieldPlugin.js#L44-L52 if let BrowserField::Map(field_data) = browser { - if let Some(value) = field_data.get(request) { + if let Some(value) = field_data.get(request.as_path()) { return Self::alias_value(path, value); } } diff --git a/src/tests/browser_field.rs b/src/tests/browser_field.rs index d114d991..4fd91617 100644 --- a/src/tests/browser_field.rs +++ b/src/tests/browser_field.rs @@ -106,3 +106,26 @@ fn crypto_js() { let resolved_path = resolver.resolve(f.join("crypto-js"), "crypto").map(|r| r.full_path()); assert_eq!(resolved_path, Err(ResolveError::Ignored(f.join("crypto-js")))); } + +// https://github.com/webpack/webpack/blob/87660921808566ef3b8796f8df61bd79fc026108/test/cases/resolving/browser-field/index.js#L40-L43 +#[test] +fn recursive() { + let f = super::fixture().join("browser-module"); + + let resolver = Resolver::new(ResolveOptions { + alias_fields: vec![vec!["browser".into()]], + ..ResolveOptions::default() + }); + + let data = [ + ("should handle recursive file 1", f.clone(), "recursive-file/a"), + ("should handle recursive file 2", f.clone(), "recursive-file/b"), + ("should handle recursive file 3", f.clone(), "recursive-file/c"), + ("should handle recursive file 4", f.clone(), "recursive-file/d"), + ]; + + for (comment, path, request) in data { + let resolved_path = resolver.resolve(&path, request); + assert_eq!(resolved_path, Err(ResolveError::Recursion), "{comment} {path:?} {request}"); + } +}