From 5e4e324ceb657772f98bfae3c797e7417acc9837 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 1 Nov 2022 21:07:45 -0400 Subject: [PATCH] fix(lockfile): error if a referenced package id doesn't exist in list of packages (#16509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartek Iwańczuk --- cli/npm/resolution.rs | 84 +++++++---- cli/tests/integration/npm_tests.rs | 215 +++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 29 deletions(-) diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index 28a42bc333..3df2e4ce53 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -372,36 +372,45 @@ impl NpmResolutionSnapshot { lockfile: Arc>, api: &NpmRegistryApi, ) -> Result { - let mut package_reqs = HashMap::new(); - let mut packages_by_name: HashMap> = HashMap::new(); - let mut packages = HashMap::new(); + let mut package_reqs: HashMap; + let mut packages_by_name: HashMap>; + let mut packages: HashMap; { let lockfile = lockfile.lock(); + // pre-allocate collections + package_reqs = + HashMap::with_capacity(lockfile.content.npm.specifiers.len()); + packages = HashMap::with_capacity(lockfile.content.npm.packages.len()); + packages_by_name = + HashMap::with_capacity(lockfile.content.npm.packages.len()); // close enough + let mut verify_ids = + HashSet::with_capacity(lockfile.content.npm.packages.len()); + + // collect the specifiers to version mappings for (key, value) in &lockfile.content.npm.specifiers { let reference = NpmPackageReference::from_str(&format!("npm:{}", key)) .with_context(|| format!("Unable to parse npm specifier: {}", key))?; let package_id = NpmPackageId::deserialize_from_lock_file(value)?; package_reqs.insert(reference.req, package_id.version.clone()); + verify_ids.insert(package_id.clone()); } + // then the packages for (key, value) in &lockfile.content.npm.packages { let package_id = NpmPackageId::deserialize_from_lock_file(key)?; let mut dependencies = HashMap::default(); - for (name, specifier) in &value.dependencies { - dependencies.insert( - name.to_string(), - NpmPackageId::deserialize_from_lock_file(specifier)?, - ); - } + packages_by_name + .entry(package_id.name.to_string()) + .or_default() + .push(package_id.version.clone()); - for (name, id) in &dependencies { - packages_by_name - .entry(name.to_string()) - .or_default() - .push(id.version.clone()); + for (name, specifier) in &value.dependencies { + let dep_id = NpmPackageId::deserialize_from_lock_file(specifier)?; + dependencies.insert(name.to_string(), dep_id.clone()); + verify_ids.insert(dep_id); } let package = NpmResolutionPackage { @@ -415,32 +424,49 @@ impl NpmResolutionSnapshot { dependencies, }; - packages.insert(package_id.clone(), package); + packages.insert(package_id, package); + } + + // verify that all these ids exist in packages + for id in &verify_ids { + if !packages.contains_key(id) { + bail!( + "the lockfile ({}) is corrupt. You can recreate it with --lock-write", + lockfile.filename.display(), + ); + } } } let mut unresolved_tasks = Vec::with_capacity(packages_by_name.len()); - for package_id in packages.keys() { - let package_id = package_id.clone(); + // cache the package names in parallel in the registry api + for package_name in packages_by_name.keys() { + let package_name = package_name.clone(); let api = api.clone(); unresolved_tasks.push(tokio::task::spawn(async move { - let info = api.package_info(&package_id.name).await?; - Result::<_, AnyError>::Ok((package_id, info)) + api.package_info(&package_name).await?; + Result::<_, AnyError>::Ok(()) })); } for result in futures::future::join_all(unresolved_tasks).await { - let (package_id, info) = result??; + result??; + } - let version_and_info = get_resolved_package_version_and_info( - &package_id.name, - &NpmVersionReq::parse(&package_id.version.to_string()).unwrap(), - info, - None, - )?; - - let package = packages.get_mut(&package_id).unwrap(); - package.dist = version_and_info.info.dist; + // ensure the dist is set for each package + for package in packages.values_mut() { + // this will read from the memory cache now + let package_info = api.package_info(&package.id.name).await?; + let version_info = match package_info + .versions + .get(&package.id.version.to_string()) + { + Some(version_info) => version_info, + None => { + bail!("could not find '{}' specified in the lockfile. Maybe try again with --reload", package.id); + } + }; + package.dist = version_info.dist.clone(); } Ok(Self { diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 714fe25d45..a5b4431716 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -783,6 +783,221 @@ itest!(info_cli_chalk_json { http_server: true, }); +#[test] +fn lock_file_missing_top_level_package() { + let _server = http_server(); + + let deno_dir = util::new_deno_dir(); + let temp_dir = util::TempDir::new(); + + // write empty config file + temp_dir.write("deno.json", "{}"); + + // Lock file that is automatically picked up has been intentionally broken, + // by removing "cowsay" package from it. This test ensures that npm resolver + // snapshot can be successfully hydrated in such situation + let lock_file_content = r#"{ + "version": "2", + "remote": {}, + "npm": { + "specifiers": { "cowsay": "cowsay@1.5.0" }, + "packages": { + "ansi-regex@3.0.1": { + "integrity": "sha512-+O9Jct8wf++lXxxFc4hc8LsjaSq0HFzzL7cVsw8pRDIPdjKD2mT4ytDZlLuSBZ4cLKZFXIrMGO7DbQCtMJJMKw==", + "dependencies": {} + }, + "ansi-regex@5.0.1": { + "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", + "dependencies": {} + }, + "ansi-styles@4.3.0": { + "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", + "dependencies": { "color-convert": "color-convert@2.0.1" } + }, + "camelcase@5.3.1": { + "integrity": "sha512-L28STB170nwWS63UjtlEOE3dldQApaJXZkOI1uMFfzf3rRuPegHaHesyee+YxQ+W6SvRDQV6UrdOdRiR153wJg==", + "dependencies": {} + }, + "cliui@6.0.0": { + "integrity": "sha512-t6wbgtoCXvAzst7QgXxJYqPt0usEfbgQdftEPbLL/cvv6HPE5VgvqCuAIDR0NgU52ds6rFwqrgakNLrHEjCbrQ==", + "dependencies": { + "string-width": "string-width@4.2.3", + "strip-ansi": "strip-ansi@6.0.1", + "wrap-ansi": "wrap-ansi@6.2.0" + } + }, + "color-convert@2.0.1": { + "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", + "dependencies": { "color-name": "color-name@1.1.4" } + }, + "color-name@1.1.4": { + "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", + "dependencies": {} + }, + "decamelize@1.2.0": { + "integrity": "sha512-z2S+W9X73hAUUki+N+9Za2lBlun89zigOyGrsax+KUQ6wKW4ZoWpEYBkGhQjwAjjDCkWxhY0VKEhk8wzY7F5cA==", + "dependencies": {} + }, + "emoji-regex@8.0.0": { + "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==", + "dependencies": {} + }, + "find-up@4.1.0": { + "integrity": "sha512-PpOwAdQ/YlXQ2vj8a3h8IipDuYRi3wceVQQGYWxNINccq40Anw7BlsEXCMbt1Zt+OLA6Fq9suIpIWD0OsnISlw==", + "dependencies": { + "locate-path": "locate-path@5.0.0", + "path-exists": "path-exists@4.0.0" + } + }, + "get-caller-file@2.0.5": { + "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==", + "dependencies": {} + }, + "get-stdin@8.0.0": { + "integrity": "sha512-sY22aA6xchAzprjyqmSEQv4UbAAzRN0L2dQB0NlN5acTTK9Don6nhoc3eAbUnpZiCANAMfd/+40kVdKfFygohg==", + "dependencies": {} + }, + "is-fullwidth-code-point@2.0.0": { + "integrity": "sha512-VHskAKYM8RfSFXwee5t5cbN5PZeq1Wrh6qd5bkyiXIf6UQcN6w/A0eXM9r6t8d+GYOh+o6ZhiEnb88LN/Y8m2w==", + "dependencies": {} + }, + "is-fullwidth-code-point@3.0.0": { + "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==", + "dependencies": {} + }, + "locate-path@5.0.0": { + "integrity": "sha512-t7hw9pI+WvuwNJXwk5zVHpyhIqzg2qTlklJOf0mVxGSbe3Fp2VieZcduNYjaLDoy6p9uGpQEGWG87WpMKlNq8g==", + "dependencies": { "p-locate": "p-locate@4.1.0" } + }, + "p-limit@2.3.0": { + "integrity": "sha512-//88mFWSJx8lxCzwdAABTJL2MyWB12+eIY7MDL2SqLmAkeKU9qxRvWuSyTjm3FUmpBEMuFfckAIqEaVGUDxb6w==", + "dependencies": { "p-try": "p-try@2.2.0" } + }, + "p-locate@4.1.0": { + "integrity": "sha512-R79ZZ/0wAxKGu3oYMlz8jy/kbhsNrS7SKZ7PxEHBgJ5+F2mtFW2fK2cOtBh1cHYkQsbzFV7I+EoRKe6Yt0oK7A==", + "dependencies": { "p-limit": "p-limit@2.3.0" } + }, + "p-try@2.2.0": { + "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==", + "dependencies": {} + }, + "path-exists@4.0.0": { + "integrity": "sha512-ak9Qy5Q7jYb2Wwcey5Fpvg2KoAc/ZIhLSLOSBmRmygPsGwkVVt0fZa0qrtMz+m6tJTAHfZQ8FnmB4MG4LWy7/w==", + "dependencies": {} + }, + "require-directory@2.1.1": { + "integrity": "sha512-fGxEI7+wsG9xrvdjsrlmL22OMTTiHRwAMroiEeMgq8gzoLC/PQr7RsRDSTLUg/bZAZtF+TVIkHc6/4RIKrui+Q==", + "dependencies": {} + }, + "require-main-filename@2.0.0": { + "integrity": "sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg==", + "dependencies": {} + }, + "set-blocking@2.0.0": { + "integrity": "sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==", + "dependencies": {} + }, + "string-width@2.1.1": { + "integrity": "sha512-nOqH59deCq9SRHlxq1Aw85Jnt4w6KvLKqWVik6oA9ZklXLNIOlqg4F2yrT1MVaTjAqvVwdfeZ7w7aCvJD7ugkw==", + "dependencies": { + "is-fullwidth-code-point": "is-fullwidth-code-point@2.0.0", + "strip-ansi": "strip-ansi@4.0.0" + } + }, + "string-width@4.2.3": { + "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==", + "dependencies": { + "emoji-regex": "emoji-regex@8.0.0", + "is-fullwidth-code-point": "is-fullwidth-code-point@3.0.0", + "strip-ansi": "strip-ansi@6.0.1" + } + }, + "strip-ansi@4.0.0": { + "integrity": "sha512-4XaJ2zQdCzROZDivEVIDPkcQn8LMFSa8kj8Gxb/Lnwzv9A8VctNZ+lfivC/sV3ivW8ElJTERXZoPBRrZKkNKow==", + "dependencies": { "ansi-regex": "ansi-regex@3.0.1" } + }, + "strip-ansi@6.0.1": { + "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==", + "dependencies": { "ansi-regex": "ansi-regex@5.0.1" } + }, + "strip-final-newline@2.0.0": { + "integrity": "sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==", + "dependencies": {} + }, + "which-module@2.0.0": { + "integrity": "sha512-B+enWhmw6cjfVC7kS8Pj9pCrKSc5txArRyaYGe088shv/FGWH+0Rjx/xPgtsWfsUtS27FkP697E4DDhgrgoc0Q==", + "dependencies": {} + }, + "wrap-ansi@6.2.0": { + "integrity": "sha512-r6lPcBGxZXlIcymEu7InxDMhdW0KDxpLgoFLcguasxCaJ/SOIZwINatK9KY/tf+ZrlywOKU0UDj3ATXUBfxJXA==", + "dependencies": { + "ansi-styles": "ansi-styles@4.3.0", + "string-width": "string-width@4.2.3", + "strip-ansi": "strip-ansi@6.0.1" + } + }, + "y18n@4.0.3": { + "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==", + "dependencies": {} + }, + "yargs-parser@18.1.3": { + "integrity": "sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ==", + "dependencies": { + "camelcase": "camelcase@5.3.1", + "decamelize": "decamelize@1.2.0" + } + }, + "yargs@15.4.1": { + "integrity": "sha512-aePbxDmcYW++PaqBsJ+HYUFwCdv4LVvdnhBy78E57PIor8/OVvhMrADFFEDh8DHDFRv/O9i3lPhsENjO7QX0+A==", + "dependencies": { + "cliui": "cliui@6.0.0", + "decamelize": "decamelize@1.2.0", + "find-up": "find-up@4.1.0", + "get-caller-file": "get-caller-file@2.0.5", + "require-directory": "require-directory@2.1.1", + "require-main-filename": "require-main-filename@2.0.0", + "set-blocking": "set-blocking@2.0.0", + "string-width": "string-width@4.2.3", + "which-module": "which-module@2.0.0", + "y18n": "y18n@4.0.3", + "yargs-parser": "yargs-parser@18.1.3" + } + } + } + } + } + "#; + temp_dir.write("deno.lock", lock_file_content); + let main_contents = r#" + import cowsay from "npm:cowsay"; + console.log(cowsay); + "#; + temp_dir.write("main.ts", main_contents); + + let deno = util::deno_cmd_with_deno_dir(&deno_dir) + .current_dir(temp_dir.path()) + .arg("run") + .arg("--unstable") + .arg("--quiet") + .arg("--lock") + .arg("deno.lock") + .arg("-A") + .arg("main.ts") + .envs(env_vars()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert!(!output.status.success()); + + let stderr = String::from_utf8(output.stderr).unwrap(); + assert_eq!( + stderr, + "error: the lockfile (deno.lock) is corrupt. You can recreate it with --lock-write\n" + ); +} + fn env_vars_no_sync_download() -> Vec<(String, String)> { vec![ ("DENO_NODE_COMPAT_URL".to_string(), util::std_file_url()),