From 4ec9250c409fc0734e192d6571b0cad3cbc8a7ee Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 13 Jun 2024 18:29:27 -0400 Subject: [PATCH] fix(npm): use more relaxed package.json version constraint parsing (#24202) --- Cargo.lock | 4 +-- cli/Cargo.toml | 2 +- cli/args/package_json.rs | 34 +++++++++++++++---- cli/standalone/binary.rs | 17 +++++----- tests/specs/npm/npmrc_basic_auth/package.json | 2 +- .../package_json/invalid_value/error.ts.out | 2 +- .../package_json/invalid_value/task.out | 2 +- 7 files changed, 41 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12be389277..fcbac4293a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1853,9 +1853,9 @@ dependencies = [ [[package]] name = "deno_semver" -version = "0.5.4" +version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b49e14effd9df8ed261f7a1a34ac19bbaf0fa940c59bd19a6d8313cf41525e1c" +checksum = "389b5a8c2dd48cc1aad25396c92d7461ddb0fcfae1faf8e00205837c53e34d3e" dependencies = [ "monch", "once_cell", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 9c5cf3c434..6263dfb3e4 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -74,7 +74,7 @@ deno_lint = { version = "=0.60.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.21.4" deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_semver = "=0.5.4" +deno_semver = "=0.5.6" deno_task_shell = "=0.16.1" deno_terminal.workspace = true eszip = "=0.71.0" diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index 67481fd07a..b1ff39abc7 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -7,16 +7,16 @@ use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_npm::registry::parse_dep_entry_name_and_raw_version; use deno_runtime::deno_node::PackageJson; +use deno_semver::npm::NpmVersionReqParseError; use deno_semver::package::PackageReq; use deno_semver::VersionReq; -use deno_semver::VersionReqSpecifierParseError; use indexmap::IndexMap; use thiserror::Error; #[derive(Debug, Error, Clone)] pub enum PackageJsonDepValueParseError { #[error(transparent)] - Specifier(#[from] VersionReqSpecifierParseError), + VersionReq(#[from] NpmVersionReqParseError), #[error("Not implemented scheme '{scheme}'")] Unsupported { scheme: String }, } @@ -75,13 +75,13 @@ pub fn get_local_package_json_version_reqs( }); } let (name, version_req) = parse_dep_entry_name_and_raw_version(key, value); - let result = VersionReq::parse_from_specifier(version_req); + let result = VersionReq::parse_from_npm(version_req); match result { Ok(version_req) => Ok(PackageReq { name: name.to_string(), version_req, }), - Err(err) => Err(PackageJsonDepValueParseError::Specifier(err)), + Err(err) => Err(PackageJsonDepValueParseError::VersionReq(err)), } } @@ -208,7 +208,7 @@ mod test { let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); package_json.dependencies = Some(IndexMap::from([( "test".to_string(), - "1.x - 1.3".to_string(), + "%*(#$%()".to_string(), )])); let map = get_local_package_json_version_reqs_for_tests(&package_json); assert_eq!( @@ -217,8 +217,8 @@ mod test { "test".to_string(), Err( concat!( - "Invalid specifier version requirement. Unexpected character.\n", - " - 1.3\n", + "Invalid npm version requirement. Unexpected character.\n", + " %*(#$%()\n", " ~" ) .to_string() @@ -227,6 +227,26 @@ mod test { ); } + #[test] + fn test_get_local_package_json_version_reqs_range() { + let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); + package_json.dependencies = Some(IndexMap::from([( + "test".to_string(), + "1.x - 1.3".to_string(), + )])); + let map = get_local_package_json_version_reqs_for_tests(&package_json); + assert_eq!( + map, + IndexMap::from([( + "test".to_string(), + Ok(PackageReq { + name: "test".to_string(), + version_req: VersionReq::parse_from_npm("1.x - 1.3").unwrap() + }) + )]) + ); + } + #[test] fn test_get_local_package_json_version_reqs_skips_certain_specifiers() { let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 00b8d19f37..8df52e3eb4 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -24,6 +24,7 @@ use deno_core::futures::AsyncSeekExt; use deno_core::serde_json; use deno_core::url::Url; use deno_npm::NpmSystemInfo; +use deno_semver::npm::NpmVersionReqParseError; use deno_semver::package::PackageReq; use deno_semver::VersionReqSpecifierParseError; use log::Level; @@ -55,15 +56,15 @@ const MAGIC_TRAILER: &[u8; 8] = b"d3n0l4nd"; #[derive(Serialize, Deserialize)] enum SerializablePackageJsonDepValueParseError { - Specifier(String), + VersionReq(String), Unsupported { scheme: String }, } impl SerializablePackageJsonDepValueParseError { pub fn from_err(err: PackageJsonDepValueParseError) -> Self { match err { - PackageJsonDepValueParseError::Specifier(err) => { - Self::Specifier(err.source.to_string()) + PackageJsonDepValueParseError::VersionReq(err) => { + Self::VersionReq(err.source.to_string()) } PackageJsonDepValueParseError::Unsupported { scheme } => { Self::Unsupported { scheme } @@ -73,12 +74,10 @@ impl SerializablePackageJsonDepValueParseError { pub fn into_err(self) -> PackageJsonDepValueParseError { match self { - SerializablePackageJsonDepValueParseError::Specifier(source) => { - PackageJsonDepValueParseError::Specifier( - VersionReqSpecifierParseError { - source: monch::ParseErrorFailureError::new(source), - }, - ) + SerializablePackageJsonDepValueParseError::VersionReq(source) => { + PackageJsonDepValueParseError::VersionReq(NpmVersionReqParseError { + source: monch::ParseErrorFailureError::new(source), + }) } SerializablePackageJsonDepValueParseError::Unsupported { scheme } => { PackageJsonDepValueParseError::Unsupported { scheme } diff --git a/tests/specs/npm/npmrc_basic_auth/package.json b/tests/specs/npm/npmrc_basic_auth/package.json index 274d1ed7f4..13e305931e 100644 --- a/tests/specs/npm/npmrc_basic_auth/package.json +++ b/tests/specs/npm/npmrc_basic_auth/package.json @@ -2,7 +2,7 @@ "name": "npmrc_test", "version": "0.0.1", "dependencies": { - "@denotest/basic": "1.0.0", + "@denotest/basic": "^=1.0.0", "@denotest2/basic": "1.0.0" } } diff --git a/tests/testdata/package_json/invalid_value/error.ts.out b/tests/testdata/package_json/invalid_value/error.ts.out index faa811a306..2fd0940fe3 100644 --- a/tests/testdata/package_json/invalid_value/error.ts.out +++ b/tests/testdata/package_json/invalid_value/error.ts.out @@ -1,6 +1,6 @@ error: Parsing version constraints in the application-level package.json is more strict at the moment. -Invalid specifier version requirement. Unexpected character. +Invalid npm version requirement. Unexpected character. invalid stuff that won't parse ~ at file:///[WILDCARD]/error.ts:2:23 diff --git a/tests/testdata/package_json/invalid_value/task.out b/tests/testdata/package_json/invalid_value/task.out index 28e3c770d4..dd4a04b0db 100644 --- a/tests/testdata/package_json/invalid_value/task.out +++ b/tests/testdata/package_json/invalid_value/task.out @@ -1,4 +1,4 @@ -Warning Ignoring dependency '@denotest/cjs-default-export' in package.json because its version requirement failed to parse: Invalid specifier version requirement. Unexpected character. +Warning Ignoring dependency '@denotest/cjs-default-export' in package.json because its version requirement failed to parse: Invalid npm version requirement. Unexpected character. invalid stuff that won't parse ~ Task test echo 1