From e522131ef3ede0e142673b2af2a9b84d3aa608b0 Mon Sep 17 00:00:00 2001 From: Charles Coggins Date: Fri, 20 Dec 2024 13:46:03 -0600 Subject: [PATCH] Fix `msbuild` parser allowing missing versions This change updates the `msbuild` lockfile parser so that it will not allow for missing names and or versions. Before this change, a `*.csproj` file parsed as a lockfile (e.g., with type of `msbuild`) would show entries with missing name or version as an empty string. This causes analysis failures when submitting to the API since a valid package descriptor requires a non-empty `version` field. Dependency management in NuGet's "Central Package Management" (CPM) allows for `*.csproj` files containing `PackageReference` elements without a `Version` attribute. Those versions will be included, along with the fully transitive set, in the `packages.lock.json` lockfile generated by NuGet. When both files are present in the same directory, the changes in this patch will correctly parse both. For more about CPM, see this reference: https://devblogs.microsoft.com/nuget/introducing-central-package-management/ The `sample.csproj` test fixture was updated to include an entry with neither a name or version and another entry with a name but no version. --- CHANGELOG.md | 4 ++++ lockfile/src/csharp.rs | 24 +++++++++++++++--------- tests/fixtures/sample.csproj | 2 ++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a8d45586..3f3ebe220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `phylum exception` subcommand for managing suppressions +### Fixed + +- `msbuild` lockfile parser allowing missing names and versions + ## 7.2.0 - 2024-12-10 ### Added diff --git a/lockfile/src/csharp.rs b/lockfile/src/csharp.rs index 700275236..6fd81cb56 100644 --- a/lockfile/src/csharp.rs +++ b/lockfile/src/csharp.rs @@ -95,10 +95,10 @@ pub struct CSProj; #[derive(Debug, Deserialize, PartialEq, Eq)] pub struct PackageReference { #[serde(alias = "@Include", default)] - pub name: String, + pub name: Option, #[serde(alias = "@Version", alias = "@version", alias = "Version", default)] - pub version: String, + pub version: Option, } #[derive(Debug, Deserialize, PartialEq, Eq)] @@ -113,13 +113,15 @@ struct Project { pub item_groups: Vec, } -impl From for Package { - fn from(pkg_ref: PackageReference) -> Self { - Self { - name: pkg_ref.name, - version: PackageVersion::FirstParty(pkg_ref.version), +impl TryFrom for Package { + type Error = (); + + fn try_from(pkg_ref: PackageReference) -> Result { + Ok(Self { + name: pkg_ref.name.ok_or(())?, + version: PackageVersion::FirstParty(pkg_ref.version.ok_or(())?), package_type: PackageType::Nuget, - } + }) } } @@ -130,7 +132,11 @@ impl From for Vec { for item_group in proj.item_groups { if !item_group.dependencies.is_empty() { deps.extend( - item_group.dependencies.into_iter().map(Package::from).collect::>(), + item_group + .dependencies + .into_iter() + .filter_map(|pkg| Package::try_from(pkg).ok()) + .collect::>(), ); } } diff --git a/tests/fixtures/sample.csproj b/tests/fixtures/sample.csproj index d599e439c..416d3cef4 100644 --- a/tests/fixtures/sample.csproj +++ b/tests/fixtures/sample.csproj @@ -10,6 +10,7 @@ + @@ -20,6 +21,7 @@ 1.5.0 +