Skip to content

Fix Npm version parsing for version with build numbers. #528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
morended wants to merge 2 commits into from
Closed

Fix Npm version parsing for version with build numbers. #528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dac3a16
Fix Npm version parsing for version with build numbers.
Feb 24, 2026
6c9c05f
Add tests
Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Shared/Contracts/IBaseProjectManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ public interface IBaseProjectManager
/// <returns></returns>
public JsonElement? GetVersionElement(JsonDocument contentJSON, Version version);

/// <summary>
/// Gets the JSON element for a package version using string comparison (for non-standard formats like "1.0.2010022026").
/// </summary>
/// <param name="metadata"></param>
/// <param name="version"></param>
/// <returns></returns>
public JsonElement? GetVersionElement(JsonDocument contentJSON, String version);

/// <summary>
/// Gets all the versions of a package
/// </summary>
Expand Down
7 changes: 7 additions & 0 deletions src/Shared/PackageManagers/BaseProjectManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,13 @@ public virtual async Task<IPackageExistence> DetailedPackageVersionExistsAsync(P

/// <inheritdoc />
public virtual JsonElement? GetVersionElement(JsonDocument contentJSON, Version version)
{
string typeName = GetType().Name;
throw new NotImplementedException($"{typeName} does not implement GetVersions.");
}

/// <inheritdoc />
public virtual JsonElement? GetVersionElement(JsonDocument contentJSON, string version)
Copy link
Contributor

@michelleqyun michelleqyun Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method defined twice?

Copy link
Contributor Author

@morended morended Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the new method accepts string versions; I kept the original one for Version types for backwards compatibility

{
string typeName = GetType().Name;
throw new NotImplementedException($"{typeName} does not implement GetVersions.");
Copy link
Contributor

@michelleqyun michelleqyun Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be "does not implement GetVersionElement".

Expand Down
164 changes: 130 additions & 34 deletions src/Shared/PackageManagers/NPMProjectManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,16 +246,127 @@ public override async Task<IEnumerable<string>> EnumerateVersionsAsync(PackageUR
}

/// <summary>
/// Gets the latest version of the package
/// Helper method to find the latest version name, preferring dist-tags if available,
/// otherwise falling back to time-based comparison
/// </summary>
/// <param name="contentJSON"></param>
/// <returns>The latest version name</returns>
private string? GetLatestVersionName(JsonDocument contentJSON)
{
if (contentJSON is null) { return null; }

JsonElement root = contentJSON.RootElement;

try
{
// Try to get the "latest" version from dist-tags first
if (root.TryGetProperty("dist-tags", out JsonElement distTags) &&
distTags.TryGetProperty("latest", out JsonElement latestElement))
{
string? latestFromDistTag = latestElement.GetString();
if (!string.IsNullOrWhiteSpace(latestFromDistTag))
{
return latestFromDistTag;
}
}
}
catch (Exception ex)
{
Logger.Debug("Error getting latest version from dist-tags: {0}", ex.Message);
}

// Fallback to time-based comparison if dist-tags is empty or not available
return GetLatestVersionNameByTime(contentJSON);
}

/// <summary>
/// Helper method to find the latest version name based on published time
/// </summary>
/// <param name="contentJSON"></param>
/// <returns>The latest version name by published time</returns>
private string? GetLatestVersionNameByTime(JsonDocument contentJSON)
{
if (contentJSON is null) { return null; }

JsonElement root = contentJSON.RootElement;
string? latestVersionByTime = null;
DateTime? latestTime = null;

try
{
// Get the time metadata for all versions
if (!root.TryGetProperty("time", out JsonElement timeElement))
{
return null;
}

// Get all versions
if (!root.TryGetProperty("versions", out JsonElement versionsElement))
{
return null;
}

// Iterate through all versions and find the one with the latest publish time
foreach (JsonProperty versionProperty in versionsElement.EnumerateObject())
{
string versionName = versionProperty.Name;

// Get the publish time for this version
if (timeElement.TryGetProperty(versionName, out JsonElement versionTime))
{
try
{
string? timeString = versionTime.GetString();
if (!string.IsNullOrEmpty(timeString) && DateTime.TryParse(timeString, out DateTime publishTime))
{
if (latestTime is null || publishTime > latestTime)
{
latestTime = publishTime;
latestVersionByTime = versionName;
}
}
}
catch (Exception ex)
{
Logger.Debug("Error parsing publish time for version {0}: {1}", versionName, ex.Message);
}
}
}

return latestVersionByTime;
}
catch (Exception ex)
{
Logger.Debug("Error getting latest version name by time: {0}", ex.Message);
}

return null;
}

/// <summary>
/// Gets the latest version element based on dist-tags if available, otherwise by published time
/// </summary>
/// <param name="contentJSON"></param>
/// <returns></returns>
public JsonElement? GetLatestVersionElement(JsonDocument contentJSON)
{
List<Version> versions = GetVersions(contentJSON);
Version? maxVersion = GetLatestVersion(versions);
if (maxVersion is null) { return null; }
return GetVersionElement(contentJSON, maxVersion);
if (contentJSON is null) { return null; }

try
{
string? latestVersion = GetLatestVersionName(contentJSON);

if (!string.IsNullOrEmpty(latestVersion))
{
return GetVersionElement(contentJSON, latestVersion);
}
}
catch (Exception ex)
{
Logger.Debug("Error getting latest version element: {0}", ex.Message);
}

return null;
}

/// <inheritdoc />
Expand Down Expand Up @@ -303,8 +414,8 @@ public override Uri GetPackageAbsoluteUri(PackageURL purl)
metadata.ApiPackageUri = $"{ENV_NPM_API_ENDPOINT}/{metadata.Name}";
metadata.CreatedTime = ParseCreatedTime(contentJSON);

List<Version> versions = GetVersions(contentJSON);
Version? latestVersion = GetLatestVersion(versions);
// Get the latest version, preferring dist-tags if available, otherwise using time-based comparison
string? latestVersion = GetLatestVersionName(contentJSON);

if (purl.Version != null)
{
Expand All @@ -313,14 +424,13 @@ public override Uri GetPackageAbsoluteUri(PackageURL purl)
}
else
{
metadata.PackageVersion = latestVersion is null ? purl.Version : latestVersion?.ToString();
metadata.PackageVersion = latestVersion ?? purl.Version;
}

// if we found any version at all, get the information
if (metadata.PackageVersion != null)
{
Version versionToGet = new(metadata.PackageVersion);
JsonElement? versionElement = GetVersionElement(contentJSON, versionToGet);
JsonElement? versionElement = GetVersionElement(contentJSON, metadata.PackageVersion);
metadata.UploadTime = ParseUploadTime(contentJSON, metadata.PackageVersion);

if (versionElement != null)
Expand Down Expand Up @@ -478,7 +588,7 @@ is JsonElement.ArrayEnumerator enumeratorElement &&

if (latestVersion is not null)
{
metadata.LatestPackageVersion = latestVersion.ToString();
metadata.LatestPackageVersion = latestVersion;
}

return metadata;
Expand Down Expand Up @@ -521,6 +631,12 @@ is JsonElement.ArrayEnumerator enumeratorElement &&
}

public override JsonElement? GetVersionElement(JsonDocument? contentJSON, Version version)
{
return GetVersionElement(contentJSON, version.ToString());
}


public override JsonElement? GetVersionElement(JsonDocument? contentJSON, string version)
{
if (contentJSON is null) { return null; }
JsonElement root = contentJSON.RootElement;
Expand All @@ -530,9 +646,9 @@ is JsonElement.ArrayEnumerator enumeratorElement &&
JsonElement versionsJSON = root.GetProperty("versions");
foreach (JsonProperty versionProperty in versionsJSON.EnumerateObject())
{
if (string.Equals(versionProperty.Name, version.ToString(), StringComparison.InvariantCultureIgnoreCase))
if (string.Equals(versionProperty.Name, version, StringComparison.InvariantCultureIgnoreCase))
{
return versionsJSON.GetProperty(version.ToString());
return versionsJSON.GetProperty(version);
}
}
}
Expand All @@ -542,26 +658,6 @@ is JsonElement.ArrayEnumerator enumeratorElement &&
return null;
}

public override List<Version> GetVersions(JsonDocument? contentJSON)
{
List<Version> allVersions = new();
if (contentJSON is null) { return allVersions; }

JsonElement root = contentJSON.RootElement;
try
{
JsonElement versions = root.GetProperty("versions");
foreach (JsonProperty version in versions.EnumerateObject())
{
allVersions.Add(new Version(version.Name));
}
}
catch (KeyNotFoundException) { return allVersions; }
catch (InvalidOperationException) { return allVersions; }

return allVersions;
}

public override async Task<IPackageExistence> DetailedPackageExistsAsync(PackageURL purl, bool useCache = true)
{
Logger.Trace("DetailedPackageExists {0}", purl?.ToString());
Expand Down Expand Up @@ -788,7 +884,7 @@ protected async Task<Dictionary<PackageURL, double>> SearchRepoUrlsInPackageMeta
// TODO: If the latest version JSONElement doesnt have the repo infor, should we search all elements
// on that chance that one of them might have it?
JsonElement? versionJSON = string.IsNullOrEmpty(purl?.Version) ? GetLatestVersionElement(contentJSON) :
GetVersionElement(contentJSON, new Version(purl.Version));
GetVersionElement(contentJSON, purl.Version);

if (versionJSON is JsonElement notNullVersionJSON)
{
Expand Down
43 changes: 43 additions & 0 deletions src/oss-tests/ProjectManagerTests/NPMProjectManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public class NPMProjectManagerTests
{ "https://registry.npmjs.org/%40somosme/webflowutils", Resources.unpublishedpackage_json },
{ "https://registry.npmjs.org/%40angular/core", Resources.angular_core_json },
{ "https://registry.npmjs.org/%40achievementify/client", Resources.achievementify_client_json },
{ "https://registry.npmjs.org/%40adguard/dnr-rulesets", Resources.adguard_dnr_rulesets_json },
{ "https://registry.npmjs.org/ds-modal", Resources.ds_modal_json },
{ "https://registry.npmjs.org/monorepolint", Resources.monorepolint_json },
{ "https://registry.npmjs.org/rly-cli", Resources.rly_cli_json },
Expand Down Expand Up @@ -414,6 +415,48 @@ public async Task GetPackagesFromOwnerAsyncSucceeds_Async(string owner, string e
packages.Select(p => p.ToString()).Should().Contain(expectedPackage);
}

[Fact]
public async Task GetVersionElement_WithNonStandardVersionFormatAsync()
{
// Test non-standard version format like "4.0.20260218200111" (can't be parsed as SemVer but can be compared as strings)
PackageURL purl = new("pkg:npm/%40adguard/dnr-rulesets");
string? content = await _projectManager.Object.GetMetadataAsync(purl, useCache: false);
JsonDocument contentJSON = JsonDocument.Parse(content);

Copy link

@nanaymwu nanaymwu Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Additional line space is not needed here.

JsonElement? versionElement = _projectManager.Object.GetVersionElement(contentJSON, "4.0.20260218200111");

Assert.NotNull(versionElement);
Assert.Equal("4.0.20260218200111", versionElement?.GetProperty("version").GetString());
Copy link

@nanaymwu nanaymwu Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4.0.20260218200111 is the version number? Do we also want to test the package name?

Copy link
Contributor Author

@morended morended Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. [InlineData("pkg:npm/%40adguard/dnr-rulesets", "4.0.20260218200111")] This is the package in test

}

[Theory]
[InlineData("pkg:npm/lodash", "4.17.21")]
[InlineData("pkg:npm/%40angular/core", "13.2.6")]
[InlineData("pkg:npm/%40adguard/dnr-rulesets", "4.0.20260218200111")]
public async Task GetLatestVersionElement_WithValidJsonDocument_ReturnsLatestVersionElement(string purlString, string expectedLatestVersion)
{
// Test that GetLatestVersionElement returns the version element with the latest publish time
PackageURL purl = new(purlString);
string? content = await _projectManager.Object.GetMetadataAsync(purl, useCache: false);
JsonDocument contentJSON = JsonDocument.Parse(content);

JsonElement? latestVersionElement = _projectManager.Object.GetLatestVersionElement(contentJSON);

Assert.NotNull(latestVersionElement);

string? latestVersion = latestVersionElement?.GetProperty("version").GetString();
Assert.Equal(expectedLatestVersion, latestVersion);
}

[Fact]
public void GetLatestVersionElement_WithNullJsonDocument_ReturnsNull()
Copy link
Contributor

@michelleqyun michelleqyun Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add another test for testing getting the latest version from the dist tag?

Copy link
Contributor Author

@morended morended Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"GetLatestVersionElement_WithValidJsonDocument_ReturnsLatestVersionElement" is to test that scenario

{
// Test that GetLatestVersionElement returns null when given a null document
JsonElement? result = _projectManager.Object.GetLatestVersionElement(null);

Assert.Null(result);
}

private static void MockHttpFetchResponse(
HttpStatusCode statusCode,
string url,
Expand Down
Loading