From 1d696097664ce0e14b77f0c5c604900b2c1dc0b8 Mon Sep 17 00:00:00 2001 From: Boshen Date: Fri, 8 Dec 2023 16:04:20 +0800 Subject: [PATCH] throw recursion error when resolving cursed browser fields (#17) 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 | 25 +++++++++++-------- src/tests/browser_field.rs | 23 +++++++++++++++++ 8 files changed, 53 insertions(+), 14 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..18a2aaf9 100644 --- a/src/package_json.rs +++ b/src/package_json.rs @@ -151,15 +151,17 @@ impl PackageJson { 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); + let keys = map.keys().cloned().collect::>(); + for key in keys { + // Normalize the key if it looks like a file "foo.js" + if key.extension().is_some() { + map.insert(dir.normalize_with(&key), map[&key].clone()); + } + // Normalize the key if it is relative path "./relative" + if key.starts_with(".") { + if let Some(value) = map.remove(&key) { + map.insert(dir.normalize_with(&key), value); + } } } } @@ -224,7 +226,10 @@ impl PackageJson { path: &Path, request: Option<&str>, ) -> Result, ResolveError> { - let request = request.map_or(path, |r| Path::new(r)); + if self.browser_fields.is_empty() { + return Ok(None); + } + let request = request.map_or(path, Path::new); 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 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}"); + } +}