From 57b8ce0aea1118dcfa653e370f73abac14b86d8d Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Tue, 25 Nov 2025 21:24:32 -0800 Subject: [PATCH 1/3] Migrate npm detectors from `Newtonsoft.Json` to `System.Text.Json` Replace `Newtonsoft.Json` usage in npm detector classes with `System.Text.Json`, continuing the project-wide migration effort. Changes: - Add `PackageJson` model and related types for deserializing `package.json` files - Add custom `JsonConverters` for polymorphic fields (`author`, `workspaces`, `engines`) - Refactor `NpmComponentUtilities` to use typed parameters instead of `JProperty` - Refactor `NpmLockfileDetectorBase` to use `JsonDocument` for lockfile parsing - Update `NpmComponentDetector` to use `PackageJson` model - Update `NpmComponentDetectorWithRoots` to use `PackageLockV1Dependency` model - Update `NpmLockfile3Detector` to use `PackageLockV3Package` model - Update `NpmUtilitiesTests` to use new API signatures The existing `PackageLock` contract models (V1, V2, V3) already used `System.Text.Json`, so this change leverages those models and adds the missing `PackageJson` model. `JsonSerializerOptions` configured with `AllowTrailingCommas=true` to handle npm's JSON format which sometimes includes trailing commas. --- .../npm/Contracts/PackageJson.cs | 81 ++++++ .../npm/Contracts/PackageJsonAuthor.cs | 27 ++ .../Contracts/PackageJsonAuthorConverter.cs | 81 ++++++ .../Contracts/PackageJsonEnginesConverter.cs | 99 ++++++++ .../PackageJsonWorkspacesConverter.cs | 102 ++++++++ .../npm/NpmComponentDetector.cs | 143 +++-------- .../npm/NpmComponentDetectorWithRoots.cs | 142 +++++++---- .../npm/NpmComponentUtilities.cs | 111 ++++----- .../npm/NpmLockfile3Detector.cs | 189 ++++++++------ .../npm/NpmLockfileDetectorBase.cs | 235 +++++++----------- .../NpmUtilitiesTests.cs | 204 ++++++--------- .../LinuxApplicationLayerExperimentTests.cs | 1 + 12 files changed, 859 insertions(+), 556 deletions(-) create mode 100644 src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJson.cs create mode 100644 src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthor.cs create mode 100644 src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthorConverter.cs create mode 100644 src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonEnginesConverter.cs create mode 100644 src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonWorkspacesConverter.cs diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJson.cs b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJson.cs new file mode 100644 index 000000000..bb6157c8b --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJson.cs @@ -0,0 +1,81 @@ +namespace Microsoft.ComponentDetection.Detectors.Npm.Contracts; + +using System.Collections.Generic; +using System.Text.Json.Serialization; + +/// +/// Represents a package.json file. +/// https://docs.npmjs.com/cli/v10/configuring-npm/package-json. +/// +public sealed record PackageJson +{ + /// + /// The name of the package. + /// + [JsonPropertyName("name")] + public string? Name { get; init; } + + /// + /// The version of the package. + /// + [JsonPropertyName("version")] + public string? Version { get; init; } + + /// + /// The author of the package. Can be a string or an object with name, email, and url fields. + /// + [JsonPropertyName("author")] + [JsonConverter(typeof(PackageJsonAuthorConverter))] + public PackageJsonAuthor? Author { get; init; } + + /// + /// If set to true, then npm will refuse to publish it. + /// + [JsonPropertyName("private")] + public bool? Private { get; init; } + + /// + /// The engines that the package is compatible with. + /// Can be an object mapping engine names to version ranges, or occasionally an array. + /// + [JsonPropertyName("engines")] + [JsonConverter(typeof(PackageJsonEnginesConverter))] + public IDictionary? Engines { get; init; } + + /// + /// Dependencies required to run the package. + /// + [JsonPropertyName("dependencies")] + public IDictionary? Dependencies { get; init; } + + /// + /// Dependencies only needed for development and testing. + /// + [JsonPropertyName("devDependencies")] + public IDictionary? DevDependencies { get; init; } + + /// + /// Dependencies that are optional. + /// + [JsonPropertyName("optionalDependencies")] + public IDictionary? OptionalDependencies { get; init; } + + /// + /// Dependencies that will be bundled when publishing the package. + /// + [JsonPropertyName("bundledDependencies")] + public IList? BundledDependencies { get; init; } + + /// + /// Peer dependencies - packages that the consumer must install. + /// + [JsonPropertyName("peerDependencies")] + public IDictionary? PeerDependencies { get; init; } + + /// + /// Workspaces configuration. Can be an array of glob patterns or an object with a packages field. + /// + [JsonPropertyName("workspaces")] + [JsonConverter(typeof(PackageJsonWorkspacesConverter))] + public IList? Workspaces { get; init; } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthor.cs b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthor.cs new file mode 100644 index 000000000..b9f775121 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthor.cs @@ -0,0 +1,27 @@ +namespace Microsoft.ComponentDetection.Detectors.Npm.Contracts; + +using System.Text.Json.Serialization; + +/// +/// Represents the author field in a package.json file. +/// +public sealed record PackageJsonAuthor +{ + /// + /// The name of the author. + /// + [JsonPropertyName("name")] + public string? Name { get; init; } + + /// + /// The email of the author. + /// + [JsonPropertyName("email")] + public string? Email { get; init; } + + /// + /// The URL of the author. + /// + [JsonPropertyName("url")] + public string? Url { get; init; } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthorConverter.cs b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthorConverter.cs new file mode 100644 index 000000000..b5b887660 --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonAuthorConverter.cs @@ -0,0 +1,81 @@ +namespace Microsoft.ComponentDetection.Detectors.Npm.Contracts; + +using System; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.RegularExpressions; + +/// +/// Converts the author field in a package.json file, which can be either a string or an object. +/// String format: "Name <email> (url)" where email and url are optional. +/// +public sealed partial class PackageJsonAuthorConverter : JsonConverter +{ + // Matches: Name (url) where email and url are optional + // Examples: + // "John Doe" + // "John Doe " + // "John Doe (https://example.com)" + // "John Doe (https://example.com)" + [GeneratedRegex(@"^(?([^<(]+?)?)[ \t]*(?:<(?([^>(]+?))>)?[ \t]*(?:\(([^)]+?)\)|$)", RegexOptions.Compiled)] + private static partial Regex AuthorStringPattern(); + + /// + public override PackageJsonAuthor? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return null; + } + + if (reader.TokenType == JsonTokenType.String) + { + var authorString = reader.GetString(); + if (string.IsNullOrWhiteSpace(authorString)) + { + return null; + } + + var match = AuthorStringPattern().Match(authorString); + if (!match.Success) + { + return null; + } + + var name = match.Groups["name"].Value.Trim(); + var email = match.Groups["email"].Value.Trim(); + + if (string.IsNullOrEmpty(name)) + { + return null; + } + + return new PackageJsonAuthor + { + Name = name, + Email = string.IsNullOrEmpty(email) ? null : email, + }; + } + + if (reader.TokenType == JsonTokenType.StartObject) + { + return JsonSerializer.Deserialize(ref reader, options); + } + + // Skip unexpected token types + reader.Skip(); + return null; + } + + /// + public override void Write(Utf8JsonWriter writer, PackageJsonAuthor? value, JsonSerializerOptions options) + { + if (value is null) + { + writer.WriteNullValue(); + return; + } + + JsonSerializer.Serialize(writer, value, options); + } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonEnginesConverter.cs b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonEnginesConverter.cs new file mode 100644 index 000000000..1fba1821d --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonEnginesConverter.cs @@ -0,0 +1,99 @@ +namespace Microsoft.ComponentDetection.Detectors.Npm.Contracts; + +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; + +/// +/// Converts the engines field in a package.json file. +/// Engines is typically an object mapping engine names to version ranges, +/// but can occasionally be an array of strings in malformed package.json files. +/// +public sealed class PackageJsonEnginesConverter : JsonConverter?> +{ + /// + public override IDictionary? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return null; + } + + if (reader.TokenType == JsonTokenType.StartObject) + { + var result = new Dictionary(); + + while (reader.Read() && reader.TokenType != JsonTokenType.EndObject) + { + if (reader.TokenType == JsonTokenType.PropertyName) + { + var propertyName = reader.GetString(); + reader.Read(); + + if (propertyName is not null && reader.TokenType == JsonTokenType.String) + { + var value = reader.GetString(); + if (value is not null) + { + result[propertyName] = value; + } + } + else + { + reader.Skip(); + } + } + } + + return result; + } + + if (reader.TokenType == JsonTokenType.StartArray) + { + // Some malformed package.json files have engines as an array + // We parse the array to check for known engine strings but return an empty dictionary + // since we can't map array values to key-value pairs + var result = new Dictionary(); + + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + if (reader.TokenType == JsonTokenType.String) + { + var value = reader.GetString(); + + // If the array contains strings like "vscode", we note it + // This matches the behavior of the original detector which checked for vscode engine + if (value is not null && value.Contains("vscode", StringComparison.OrdinalIgnoreCase)) + { + result["vscode"] = value; + } + } + } + + return result; + } + + // Skip unexpected token types + reader.Skip(); + return null; + } + + /// + public override void Write(Utf8JsonWriter writer, IDictionary? value, JsonSerializerOptions options) + { + if (value is null) + { + writer.WriteNullValue(); + return; + } + + writer.WriteStartObject(); + foreach (var kvp in value) + { + writer.WriteString(kvp.Key, kvp.Value); + } + + writer.WriteEndObject(); + } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonWorkspacesConverter.cs b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonWorkspacesConverter.cs new file mode 100644 index 000000000..ef30f7eef --- /dev/null +++ b/src/Microsoft.ComponentDetection.Detectors/npm/Contracts/PackageJsonWorkspacesConverter.cs @@ -0,0 +1,102 @@ +namespace Microsoft.ComponentDetection.Detectors.Npm.Contracts; + +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; + +/// +/// Converts the workspaces field in a package.json file. +/// Workspaces can be: +/// - An array of glob patterns: ["packages/*"] +/// - An object with a packages field: { "packages": ["packages/*"] }. +/// +public sealed class PackageJsonWorkspacesConverter : JsonConverter?> +{ + /// + public override IList? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return null; + } + + if (reader.TokenType == JsonTokenType.StartArray) + { + var result = new List(); + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + if (reader.TokenType == JsonTokenType.String) + { + var value = reader.GetString(); + if (value is not null) + { + result.Add(value); + } + } + } + + return result; + } + + if (reader.TokenType == JsonTokenType.StartObject) + { + // Parse object and look for "packages" field + IList? packages = null; + + while (reader.Read() && reader.TokenType != JsonTokenType.EndObject) + { + if (reader.TokenType == JsonTokenType.PropertyName) + { + var propertyName = reader.GetString(); + reader.Read(); + + if (string.Equals(propertyName, "packages", StringComparison.OrdinalIgnoreCase) && + reader.TokenType == JsonTokenType.StartArray) + { + packages = []; + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + if (reader.TokenType == JsonTokenType.String) + { + var value = reader.GetString(); + if (value is not null) + { + packages.Add(value); + } + } + } + } + else + { + reader.Skip(); + } + } + } + + return packages; + } + + // Skip unexpected token types + reader.Skip(); + return null; + } + + /// + public override void Write(Utf8JsonWriter writer, IList? value, JsonSerializerOptions options) + { + if (value is null) + { + writer.WriteNullValue(); + return; + } + + writer.WriteStartArray(); + foreach (var item in value) + { + writer.WriteStringValue(item); + } + + writer.WriteEndArray(); + } +} diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs index 7b5036fef..d44f5b541 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs @@ -1,23 +1,24 @@ -#nullable disable namespace Microsoft.ComponentDetection.Detectors.Npm; using System; using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Text.RegularExpressions; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using global::NuGet.Versioning; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.Internal; using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; using Microsoft.Extensions.Logging; -using Newtonsoft.Json.Linq; public class NpmComponentDetector : FileComponentDetector { - private static readonly Regex SingleAuthor = new Regex(@"^(?([^<(]+?)?)[ \t]*(?:<(?([^>(]+?))>)?[ \t]*(?:\(([^)]+?)\)|$)", RegexOptions.Compiled); + private static readonly JsonSerializerOptions JsonOptions = new() + { + AllowTrailingCommas = true, + PropertyNameCaseInsensitive = true, + }; public NpmComponentDetector( IComponentStreamEnumerableFactory componentStreamEnumerableFactory, @@ -29,14 +30,9 @@ public NpmComponentDetector( this.Logger = logger; } - /// Common delegate for Package.json JToken processing. - /// A JToken, usually corresponding to a package.json file. - /// Used in scenarios where one file path creates multiple JTokens, a false value indicates processing additional JTokens should be halted, proceed otherwise. - protected delegate bool JTokenProcessingDelegate(JToken token); - public override string Id { get; } = "Npm"; - public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Npm)]; + public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Npm)!]; public override IList SearchPatterns { get; } = ["package.json"]; @@ -51,37 +47,33 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID var filePath = file.Location; - string contents; - using (var reader = new StreamReader(file.Stream)) + try { - contents = await reader.ReadToEndAsync(cancellationToken); - } + var packageJson = await JsonSerializer.DeserializeAsync(file.Stream, JsonOptions, cancellationToken); + if (packageJson is null) + { + this.Logger.LogInformation("Could not deserialize {PackageJsonFile}.", filePath); + return; + } - await this.SafeProcessAllPackageJTokensAsync(filePath, contents, (token) => - { - if (token["name"] == null || token["version"] == null) + if (string.IsNullOrWhiteSpace(packageJson.Name) || string.IsNullOrWhiteSpace(packageJson.Version)) { this.Logger.LogInformation("{BadPackageJson} does not contain a name and/or version. These are required fields for a valid package.json file. It and its dependencies will not be registered.", filePath); - return false; + return; } - return this.ProcessIndividualPackageJTokens(filePath, singleFileComponentRecorder, token); - }); - } - - protected virtual Task ProcessAllPackageJTokensAsync(string contents, JTokenProcessingDelegate jtokenProcessor) - { - var o = JToken.Parse(contents); - jtokenProcessor(o); - return Task.CompletedTask; + this.ProcessPackageJson(filePath, singleFileComponentRecorder, packageJson); + } + catch (JsonException e) + { + this.Logger.LogInformation(e, "Could not parse JSON from file {PackageJsonFilePaths}.", filePath); + } } - protected virtual bool ProcessIndividualPackageJTokens(string filePath, ISingleFileComponentRecorder singleFileComponentRecorder, JToken packageJToken) + protected virtual bool ProcessPackageJson(string filePath, ISingleFileComponentRecorder singleFileComponentRecorder, PackageJson packageJson) { - var name = packageJToken["name"].ToString(); - var version = packageJToken["version"].ToString(); - var authorToken = packageJToken["author"]; - var enginesToken = packageJToken["engines"]; + var name = packageJson.Name!; + var version = packageJson.Version!; if (!SemanticVersion.TryParse(version, out _)) { @@ -91,26 +83,11 @@ protected virtual bool ProcessIndividualPackageJTokens(string filePath, ISingleF } var containsVsCodeEngine = false; - if (enginesToken != null) + if (packageJson.Engines is not null) { - if (enginesToken.Type == JTokenType.Array) + if (packageJson.Engines.ContainsKey("vscode")) { - var engineStrings = enginesToken - .Children() - .Where(t => t.Type == JTokenType.String) - .Select(t => t.ToString()) - .ToArray(); - if (engineStrings.Any(e => e.Contains("vscode"))) - { - containsVsCodeEngine = true; - } - } - else if (enginesToken.Type == JTokenType.Object) - { - if (enginesToken["vscode"] != null) - { - containsVsCodeEngine = true; - } + containsVsCodeEngine = true; } } @@ -120,72 +97,20 @@ protected virtual bool ProcessIndividualPackageJTokens(string filePath, ISingleF return false; } - var npmComponent = new NpmComponent(name, version, author: this.GetAuthor(authorToken, name, filePath)); + var author = this.GetAuthor(packageJson.Author, name, filePath); + var npmComponent = new NpmComponent(name, version, author: author); singleFileComponentRecorder.RegisterUsage(new DetectedComponent(npmComponent)); return true; } - private async Task SafeProcessAllPackageJTokensAsync(string sourceFilePath, string contents, JTokenProcessingDelegate jtokenProcessor) + private NpmAuthor? GetAuthor(PackageJsonAuthor? author, string packageName, string filePath) { - try - { - await this.ProcessAllPackageJTokensAsync(contents, jtokenProcessor); - } - catch (Exception e) - { - // If something went wrong, just ignore the component - this.Logger.LogInformation(e, "Could not parse Jtokens from file {PackageJsonFilePaths}.", sourceFilePath); - } - } - - private NpmAuthor GetAuthor(JToken authorToken, string packageName, string filePath) - { - var authorString = authorToken?.ToString(); - if (string.IsNullOrEmpty(authorString)) - { - return null; - } - - string authorName; - string authorEmail; - var authorMatch = SingleAuthor.Match(authorString); - /* - * for parsing author in Json Format - * for e.g. - * "author": { - * "name": "John Doe", - * "email": "johndoe@outlook.com", - * "name": "https://jd.com", - */ - if (authorToken.HasValues) - { - authorName = authorToken["name"]?.ToString(); - authorEmail = authorToken["email"]?.ToString(); - - /* - * for parsing author in single string format. - * for e.g. - * "author": "John Doe https://jd.com" - */ - } - else if (authorMatch.Success) - { - authorName = authorMatch.Groups["name"].ToString().Trim(); - authorEmail = authorMatch.Groups["email"].ToString().Trim(); - } - else - { - this.Logger.LogWarning("Unable to parse author:[{NpmAuthorString}] for package:[{NpmPackageName}] found at path:[{NpmPackageLocation}]. This may indicate an invalid npm package author, and author will not be registered.", authorString, packageName, filePath); - return null; - } - - if (string.IsNullOrEmpty(authorName)) + if (author is null || string.IsNullOrEmpty(author.Name)) { - this.Logger.LogWarning("Unable to parse author:[{NpmAuthorString}] for package:[{NpmPackageName}] found at path:[{NpmPackageLocation}]. This may indicate an invalid npm package author, and author will not be registered.", authorString, packageName, filePath); return null; } - return new NpmAuthor(authorName, authorEmail); + return new NpmAuthor(author.Name, author.Email); } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs index fbbc09525..b55c5e4a4 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs @@ -1,15 +1,20 @@ -#nullable disable namespace Microsoft.ComponentDetection.Detectors.Npm; using System.Collections.Generic; -using System.Linq; +using System.Text.Json; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; using Microsoft.Extensions.Logging; -using Newtonsoft.Json.Linq; public class NpmComponentDetectorWithRoots : NpmLockfileDetectorBase { + private static readonly JsonSerializerOptions JsonOptions = new() + { + AllowTrailingCommas = true, + PropertyNameCaseInsensitive = true, + }; + public NpmComponentDetectorWithRoots( IComponentStreamEnumerableFactory componentStreamEnumerableFactory, IObservableDirectoryWalkerFactory walkerFactory, @@ -34,69 +39,122 @@ public NpmComponentDetectorWithRoots(IPathUtilityService pathUtilityService) protected override bool IsSupportedLockfileVersion(int lockfileVersion) => lockfileVersion != 3; - protected override JToken ResolveDependencyObject(JToken packageLockJToken) => packageLockJToken["dependencies"]; - - protected override bool TryEnqueueFirstLevelDependencies( - Queue<(JProperty DependencyProperty, TypedComponent ParentComponent)> queue, - JToken dependencies, - IDictionary dependencyLookup, - TypedComponent parentComponent = null, - bool skipValidation = false) + protected override void ProcessLockfile( + ISingleFileComponentRecorder singleFileComponentRecorder, + PackageJson packageJson, + JsonDocument lockfile, + int lockfileVersion) { - if (dependencies == null) + var root = lockfile.RootElement; + + // Get dependencies from lockfile + if (!root.TryGetProperty("dependencies", out var dependenciesElement)) { - return true; + return; } - var isValid = true; - - foreach (var dependency in dependencies.Cast()) + // Build dependency lookup + var dependencyLookup = new Dictionary(); + foreach (var dep in dependenciesElement.EnumerateObject()) { - if (dependency?.Name == null) + var dependency = JsonSerializer.Deserialize(dep.Value.GetRawText(), JsonOptions); + if (dependency is not null) { - continue; + dependencyLookup[dep.Name] = (dep.Name, dependency); } + } - var inLock = dependencyLookup.TryGetValue(dependency.Name, out var dependencyProperty); - if (inLock) - { - queue.Enqueue((dependencyProperty, parentComponent)); - } - else if (skipValidation) + // Collect all top-level dependencies from package.json + var topLevelDependencies = new Queue<(string Name, PackageLockV1Dependency Dependency, TypedComponent? Parent)>(); + + this.EnqueueDependencies(topLevelDependencies, packageJson.Dependencies, dependencyLookup, null); + this.EnqueueDependencies(topLevelDependencies, packageJson.DevDependencies, dependencyLookup, null); + this.EnqueueDependencies(topLevelDependencies, packageJson.OptionalDependencies, dependencyLookup, null); + + // Process each top-level dependency + while (topLevelDependencies.Count > 0) + { + var (name, lockDependency, _) = topLevelDependencies.Dequeue(); + + var component = this.CreateComponent(name, lockDependency.Version, lockDependency.Integrity); + if (component is null) { + continue; } - else + + var previouslyAddedComponents = new HashSet { component.Id }; + var subQueue = new Queue<(string Name, PackageLockV1Dependency Dependency, TypedComponent Parent)>(); + + // Record the top-level component + this.RecordComponent(singleFileComponentRecorder, component, lockDependency.Dev ?? false, component); + + // Enqueue nested dependencies and requires + this.EnqueueNestedDependencies(subQueue, lockDependency, dependencyLookup, component); + + // Process sub-dependencies + while (subQueue.Count > 0) { - isValid = false; + var (subName, subDependency, parentComponent) = subQueue.Dequeue(); + + var subComponent = this.CreateComponent(subName, subDependency.Version, subDependency.Integrity); + if (subComponent is null || previouslyAddedComponents.Contains(subComponent.Id)) + { + continue; + } + + previouslyAddedComponents.Add(subComponent.Id); + + this.RecordComponent(singleFileComponentRecorder, subComponent, subDependency.Dev ?? false, component, parentComponent.Id); + + this.EnqueueNestedDependencies(subQueue, subDependency, dependencyLookup, subComponent); } } - - return isValid; } - protected override void EnqueueAllDependencies( - IDictionary dependencyLookup, - ISingleFileComponentRecorder singleFileComponentRecorder, - Queue<(JProperty CurrentSubDependency, TypedComponent ParentComponent)> subQueue, - JProperty currentDependency, - TypedComponent typedComponent) + private void EnqueueDependencies( + Queue<(string Name, PackageLockV1Dependency Dependency, TypedComponent? Parent)> queue, + IDictionary? dependencies, + Dictionary dependencyLookup, + TypedComponent? parent) { - this.EnqueueDependencies(subQueue, currentDependency.Value["dependencies"], parentComponent: typedComponent); - this.TryEnqueueFirstLevelDependencies(subQueue, currentDependency.Value["requires"], dependencyLookup, parentComponent: typedComponent); + if (dependencies is null) + { + return; + } + + foreach (var dep in dependencies) + { + if (dependencyLookup.TryGetValue(dep.Key, out var lockDep)) + { + queue.Enqueue((lockDep.Name, lockDep.Dependency, parent)); + } + } } - private void EnqueueDependencies(Queue<(JProperty Dependency, TypedComponent ParentComponent)> queue, JToken dependencies, TypedComponent parentComponent) + private void EnqueueNestedDependencies( + Queue<(string Name, PackageLockV1Dependency Dependency, TypedComponent Parent)> queue, + PackageLockV1Dependency dependency, + Dictionary dependencyLookup, + TypedComponent parent) { - if (dependencies == null) + // Enqueue nested dependencies (these are local to this package) + if (dependency.Dependencies is not null) { - return; + foreach (var (name, nestedDep) in dependency.Dependencies) + { + queue.Enqueue((name, nestedDep, parent)); + } } - foreach (var dependency in dependencies.Cast()) + // Enqueue requires (these reference the top-level lookup) + if (dependency.Requires is not null) { - if (dependency != null) + foreach (var req in dependency.Requires) { - queue.Enqueue((dependency, parentComponent)); + if (dependencyLookup.TryGetValue(req.Key, out var lockDep)) + { + queue.Enqueue((lockDep.Name, lockDep.Dependency, parent)); + } } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentUtilities.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentUtilities.cs index 12effe1f3..35dcb053e 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentUtilities.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentUtilities.cs @@ -1,20 +1,25 @@ -#nullable disable namespace Microsoft.ComponentDetection.Detectors.Npm; using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.Json; using System.Text.RegularExpressions; using global::NuGet.Versioning; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; public static class NpmComponentUtilities { + private static readonly JsonSerializerOptions JsonOptions = new() + { + AllowTrailingCommas = true, + PropertyNameCaseInsensitive = true, + }; + private static readonly Regex UnsafeCharactersRegex = new Regex( @"[?<>#%{}|`'^\\~\[\]""\s\x7f]|[\x00-\x1f]|[\x80-\xff]", RegexOptions.Compiled); @@ -22,9 +27,8 @@ public static class NpmComponentUtilities public static readonly string NodeModules = "node_modules"; public static readonly string LockFile3EnvFlag = "CD_LOCKFILE_V3_ENABLED"; - public static void TraverseAndRecordComponents(JProperty currentDependency, ISingleFileComponentRecorder singleFileComponentRecorder, TypedComponent component, TypedComponent explicitReferencedDependency, string parentComponentId = null) + public static void TraverseAndRecordComponents(bool isDevDependency, ISingleFileComponentRecorder singleFileComponentRecorder, TypedComponent component, TypedComponent explicitReferencedDependency, string? parentComponentId = null) { - var isDevDependency = currentDependency.Value["dev"] is JValue devJValue && (bool)devJValue; AddOrUpdateDetectedComponent(singleFileComponentRecorder, component, isDevDependency, parentComponentId, isExplicitReferencedDependency: string.Equals(component.Id, explicitReferencedDependency.Id)); } @@ -32,7 +36,7 @@ public static DetectedComponent AddOrUpdateDetectedComponent( ISingleFileComponentRecorder singleFileComponentRecorder, TypedComponent component, bool isDevDependency, - string parentComponentId = null, + string? parentComponentId = null, bool isExplicitReferencedDependency = false) { var newComponent = new DetectedComponent(component); @@ -40,33 +44,30 @@ public static DetectedComponent AddOrUpdateDetectedComponent( return singleFileComponentRecorder.GetComponent(component.Id); } - public static TypedComponent GetTypedComponent(JProperty currentDependency, string npmRegistryHost, ILogger logger) + public static TypedComponent? GetTypedComponent(string name, string? version, string? hash, string npmRegistryHost, ILogger logger) { - var name = GetModuleName(currentDependency.Name); - - var version = currentDependency.Value["version"]?.ToString(); - var hash = currentDependency.Value["integrity"]?.ToString(); // https://docs.npmjs.com/configuring-npm/package-lock-json.html#integrity + var moduleName = GetModuleName(name); - if (!IsPackageNameValid(name)) + if (!IsPackageNameValid(moduleName)) { - logger.LogInformation("The package name {PackageName} is invalid or unsupported and a component will not be recorded.", name); + logger.LogInformation("The package name {PackageName} is invalid or unsupported and a component will not be recorded.", moduleName); return null; } - if (!SemanticVersion.TryParse(version, out var result) && !TryParseNpmVersion(npmRegistryHost, name, version, out result)) + if (version is null || (!SemanticVersion.TryParse(version, out var result) && !TryParseNpmVersion(npmRegistryHost, moduleName, version, out result))) { - logger.LogInformation("Version string {ComponentVersion} for component {ComponentName} is invalid or unsupported and a component will not be recorded.", version, name); + logger.LogInformation("Version string {ComponentVersion} for component {ComponentName} is invalid or unsupported and a component will not be recorded.", version, moduleName); return null; } - version = result.ToString(); - TypedComponent component = new NpmComponent(name, version, hash); + var versionString = result!.ToString(); + TypedComponent component = new NpmComponent(moduleName, versionString, hash); return component; } - public static bool TryParseNpmVersion(string npmRegistryHost, string packageName, string versionString, out SemanticVersion version) + public static bool TryParseNpmVersion(string npmRegistryHost, string packageName, string? versionString, out SemanticVersion? version) { - if (Uri.TryCreate(versionString, UriKind.Absolute, out var parsedUri)) + if (versionString is not null && Uri.TryCreate(versionString, UriKind.Absolute, out var parsedUri)) { if (string.Equals(npmRegistryHost, parsedUri.Host, StringComparison.OrdinalIgnoreCase)) { @@ -78,7 +79,7 @@ public static bool TryParseNpmVersion(string npmRegistryHost, string packageName return false; } - public static bool TryParseNpmRegistryVersion(string packageName, Uri versionString, out SemanticVersion version) + public static bool TryParseNpmRegistryVersion(string packageName, Uri versionString, out SemanticVersion? version) { try { @@ -98,31 +99,24 @@ public static IDictionary> TryGetAllPackageJso { yarnWorkspaces = []; - using var file = new StreamReader(stream); - using var reader = new JsonTextReader(file); - - IDictionary dependencies = new Dictionary(); - IDictionary devDependencies = new Dictionary(); - - var o = JToken.ReadFrom(reader); + var packageJson = JsonSerializer.Deserialize(stream, JsonOptions); + if (packageJson is null) + { + return new Dictionary>(); + } - if (o["private"] != null && o["private"].Value() && o["workspaces"] != null) + if (packageJson.Private == true && packageJson.Workspaces is not null) { - if (o["workspaces"] is JArray) - { - yarnWorkspaces = o["workspaces"].Values().ToList(); - } - else if (o["workspaces"] is JObject && o["workspaces"]["packages"] != null && o["workspaces"]["packages"] is JArray) - { - yarnWorkspaces = o["workspaces"]["packages"].Values().ToList(); - } + yarnWorkspaces = packageJson.Workspaces.ToList(); } - dependencies = PullDependenciesFromJToken(o, "dependencies"); - dependencies = dependencies.Concat(PullDependenciesFromJToken(o, "optionalDependencies")).ToDictionary(x => x.Key, x => x.Value); - devDependencies = PullDependenciesFromJToken(o, "devDependencies"); + var dependencies = packageJson.Dependencies ?? new Dictionary(); + var optionalDependencies = packageJson.OptionalDependencies ?? new Dictionary(); + var devDependencies = packageJson.DevDependencies ?? new Dictionary(); - var returnedDependencies = AttachDevInformationToDependencies(dependencies, false); + var allDependencies = dependencies.Concat(optionalDependencies).ToDictionary(x => x.Key, x => x.Value); + + var returnedDependencies = AttachDevInformationToDependencies(allDependencies, false); return returnedDependencies.Concat(AttachDevInformationToDependencies(devDependencies, true)).GroupBy(x => x.Key).ToDictionary(x => x.Key, x => x.First().Value); } @@ -144,6 +138,19 @@ public static string GetModuleName(string name) return name; } + internal static bool IsPackageNameValid(string name) + { + if (Uri.TryCreate(name, UriKind.Absolute, out _)) + { + return false; + } + + return !(name.Length >= 214 + || name.StartsWith('.') + || name.StartsWith('_') + || UnsafeCharactersRegex.IsMatch(name)); + } + private static IDictionary> AttachDevInformationToDependencies(IDictionary dependencies, bool isDev) { IDictionary> returnedDependencies = new Dictionary>(); @@ -167,28 +174,4 @@ private static IDictionary> AttachDevInformati return returnedDependencies; } - - private static IDictionary PullDependenciesFromJToken(JToken jObject, string dependencyType) - { - IDictionary dependencyJObject = new Dictionary(); - if (jObject[dependencyType] != null) - { - dependencyJObject = (JObject)jObject[dependencyType]; - } - - return dependencyJObject.ToDictionary(x => x.Key, x => (string)x.Value); - } - - private static bool IsPackageNameValid(string name) - { - if (Uri.TryCreate(name, UriKind.Absolute, out _)) - { - return false; - } - - return !(name.Length >= 214 - || name.StartsWith('.') - || name.StartsWith('_') - || UnsafeCharactersRegex.IsMatch(name)); - } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs index d0b842e12..69015f3fa 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs @@ -1,17 +1,23 @@ -#nullable disable namespace Microsoft.ComponentDetection.Detectors.Npm; using System.Collections.Generic; using System.Linq; +using System.Text.Json; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; using Microsoft.Extensions.Logging; -using Newtonsoft.Json.Linq; public class NpmLockfile3Detector : NpmLockfileDetectorBase { private static readonly string NodeModules = NpmComponentUtilities.NodeModules; + private static readonly JsonSerializerOptions JsonOptions = new() + { + AllowTrailingCommas = true, + PropertyNameCaseInsensitive = true, + }; + public NpmLockfile3Detector( IComponentStreamEnumerableFactory componentStreamEnumerableFactory, IObservableDirectoryWalkerFactory walkerFactory, @@ -36,125 +42,162 @@ public NpmLockfile3Detector(IPathUtilityService pathUtilityService) protected override bool IsSupportedLockfileVersion(int lockfileVersion) => lockfileVersion == 3; - protected override JToken ResolveDependencyObject(JToken packageLockJToken) => packageLockJToken["packages"]; - - protected override bool TryEnqueueFirstLevelDependencies( - Queue<(JProperty DependencyProperty, TypedComponent ParentComponent)> queue, - JToken dependencies, - IDictionary dependencyLookup, - TypedComponent parentComponent = null, - bool skipValidation = false) + protected override void ProcessLockfile( + ISingleFileComponentRecorder singleFileComponentRecorder, + PackageJson packageJson, + JsonDocument lockfile, + int lockfileVersion) { - if (dependencies == null) + var root = lockfile.RootElement; + + // Get packages from lockfile (v3 uses "packages" instead of "dependencies") + if (!root.TryGetProperty("packages", out var packagesElement)) { - return true; + return; } - var isValid = true; - - foreach (var dependency in dependencies.Cast()) + // Build package lookup - keys are paths like "node_modules/lodash" or "node_modules/a/node_modules/b" + var packageLookup = new Dictionary(); + foreach (var pkg in packagesElement.EnumerateObject()) { - if (dependency?.Name == null) + if (string.IsNullOrEmpty(pkg.Name)) { - continue; + continue; // Skip the root package (empty key) } - var inLock = dependencyLookup.TryGetValue($"{NodeModules}/{dependency.Name}", out var dependencyProperty); - if (inLock) + var package = JsonSerializer.Deserialize(pkg.Value.GetRawText(), JsonOptions); + if (package is not null) { - queue.Enqueue((dependencyProperty, parentComponent)); + packageLookup[pkg.Name] = (pkg.Name, package); } - else if (skipValidation) + } + + // Collect all top-level dependencies from package.json + var topLevelDependencies = new Queue<(string Path, PackageLockV3Package Package, TypedComponent? Parent)>(); + + this.EnqueueDependencies(topLevelDependencies, packageJson.Dependencies, packageLookup, null); + this.EnqueueDependencies(topLevelDependencies, packageJson.DevDependencies, packageLookup, null); + this.EnqueueDependencies(topLevelDependencies, packageJson.OptionalDependencies, packageLookup, null); + + // Process each top-level dependency + while (topLevelDependencies.Count > 0) + { + var (path, lockPackage, _) = topLevelDependencies.Dequeue(); + var name = NpmComponentUtilities.GetModuleName(path); + + var component = this.CreateComponent(name, lockPackage.Version, lockPackage.Integrity); + if (component is null) { + continue; } - else + + var previouslyAddedComponents = new HashSet { component.Id }; + var subQueue = new Queue<(string Path, PackageLockV3Package Package, TypedComponent Parent)>(); + + // Record the top-level component + this.RecordComponent(singleFileComponentRecorder, component, lockPackage.Dev ?? false, component); + + // Enqueue nested dependencies + this.EnqueueNestedDependencies(subQueue, path, lockPackage, packageLookup, singleFileComponentRecorder, component); + + // Process sub-dependencies + while (subQueue.Count > 0) { - isValid = false; + var (subPath, subPackage, parentComponent) = subQueue.Dequeue(); + var subName = NpmComponentUtilities.GetModuleName(subPath); + + var subComponent = this.CreateComponent(subName, subPackage.Version, subPackage.Integrity); + if (subComponent is null || previouslyAddedComponents.Contains(subComponent.Id)) + { + continue; + } + + previouslyAddedComponents.Add(subComponent.Id); + + this.RecordComponent(singleFileComponentRecorder, subComponent, subPackage.Dev ?? false, component, parentComponent.Id); + + this.EnqueueNestedDependencies(subQueue, subPath, subPackage, packageLookup, singleFileComponentRecorder, subComponent); } } - - return isValid; } - protected override void EnqueueAllDependencies( - IDictionary dependencyLookup, - ISingleFileComponentRecorder singleFileComponentRecorder, - Queue<(JProperty CurrentSubDependency, TypedComponent ParentComponent)> subQueue, - JProperty currentDependency, - TypedComponent typedComponent) => - this.TryEnqueueFirstLevelDependenciesLockfile3( - subQueue, - currentDependency.Value["dependencies"], - dependencyLookup, - singleFileComponentRecorder, - parentComponent: typedComponent); - - private void TryEnqueueFirstLevelDependenciesLockfile3( - Queue<(JProperty DependencyProperty, TypedComponent ParentComponent)> queue, - JToken dependencies, - IDictionary dependencyLookup, - ISingleFileComponentRecorder componentRecorder, - TypedComponent parentComponent) + private void EnqueueDependencies( + Queue<(string Path, PackageLockV3Package Package, TypedComponent? Parent)> queue, + IDictionary? dependencies, + Dictionary packageLookup, + TypedComponent? parent) { - if (dependencies == null) + if (dependencies is null) { return; } - foreach (var dependency in dependencies.Cast()) + foreach (var dep in dependencies) { - if (dependency?.Name == null) + var path = $"{NodeModules}/{dep.Key}"; + if (packageLookup.TryGetValue(path, out var lockPkg)) { - continue; + queue.Enqueue((lockPkg.Path, lockPkg.Package, parent)); } + } + } + private void EnqueueNestedDependencies( + Queue<(string Path, PackageLockV3Package Package, TypedComponent Parent)> queue, + string currentPath, + PackageLockV3Package package, + Dictionary packageLookup, + ISingleFileComponentRecorder componentRecorder, + TypedComponent parent) + { + if (package.Dependencies is null) + { + return; + } + + foreach (var dep in package.Dependencies) + { // First, check if there is an entry in the lockfile for this dependency nested in its ancestors - var ancestors = componentRecorder.DependencyGraph.GetAncestors(parentComponent.Id); - ancestors.Add(parentComponent.Id); + var ancestors = componentRecorder.DependencyGraph.GetAncestors(parent.Id); + ancestors.Add(parent.Id); - // remove version information + // Remove version information from ancestor IDs ancestors = ancestors.Select(x => x.Split(' ')[0]).ToList(); - var possibleDepPaths = ancestors - .Select((t, i) => ancestors.TakeLast(ancestors.Count - i)); // depth-first search + var found = false; - var inLock = false; - JProperty dependencyProperty; - foreach (var possibleDepPath in possibleDepPaths) + // Depth-first search through ancestors + for (var i = 0; i < ancestors.Count && !found; i++) { + var possiblePath = ancestors.Skip(i).ToList(); var ancestorNodeModulesPath = string.Format( "{0}/{1}/{0}/{2}", NodeModules, - string.Join($"/{NodeModules}/", possibleDepPath), - dependency.Name); - - // Does this exist? - inLock = dependencyLookup.TryGetValue(ancestorNodeModulesPath, out dependencyProperty); + string.Join($"/{NodeModules}/", possiblePath), + dep.Key); - if (!inLock) + if (packageLookup.TryGetValue(ancestorNodeModulesPath, out var nestedPkg)) { - continue; + this.Logger.LogDebug("Found nested dependency {Dependency} in {AncestorNodeModulesPath}", dep.Key, ancestorNodeModulesPath); + queue.Enqueue((nestedPkg.Path, nestedPkg.Package, parent)); + found = true; } - - this.Logger.LogDebug("Found nested dependency {Dependency} in {AncestorNodeModulesPath}", dependency.Name, ancestorNodeModulesPath); - queue.Enqueue((dependencyProperty, parentComponent)); - break; } - if (inLock) + if (found) { continue; } - // If not, check if there is an entry in the lockfile for this dependency at the top level - inLock = dependencyLookup.TryGetValue($"{NodeModules}/{dependency.Name}", out dependencyProperty); - if (inLock) + // If not found in ancestors, check at the top level + var topLevelPath = $"{NodeModules}/{dep.Key}"; + if (packageLookup.TryGetValue(topLevelPath, out var topLevelPkg)) { - queue.Enqueue((dependencyProperty, parentComponent)); + queue.Enqueue((topLevelPkg.Path, topLevelPkg.Package, parent)); } else { - this.Logger.LogWarning("Could not find dependency {Dependency} in lockfile", dependency.Name); + this.Logger.LogWarning("Could not find dependency {Dependency} in lockfile", dep.Key); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs index 690b13172..9c6e7c7e8 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs @@ -1,4 +1,3 @@ -#nullable disable namespace Microsoft.ComponentDetection.Detectors.Npm; using System; @@ -6,15 +5,15 @@ namespace Microsoft.ComponentDetection.Detectors.Npm; using System.IO; using System.Linq; using System.Reactive.Linq; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.ComponentDetection.Common; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.Internal; using Microsoft.ComponentDetection.Contracts.TypedComponent; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; public abstract class NpmLockfileDetectorBase : FileComponentDetector { @@ -22,6 +21,17 @@ public abstract class NpmLockfileDetectorBase : FileComponentDetector private const string LernaSearchPattern = "lerna.json"; + private static readonly JsonSerializerOptions JsonOptions = new() + { + AllowTrailingCommas = true, + PropertyNameCaseInsensitive = true, + }; + + private static readonly JsonDocumentOptions JsonDocumentOptions = new() + { + AllowTrailingCommas = true, + }; + private readonly object lernaFilesLock = new object(); /// @@ -43,12 +53,7 @@ protected NpmLockfileDetectorBase( protected NpmLockfileDetectorBase(IPathUtilityService pathUtilityService) => this.pathUtilityService = pathUtilityService; - /// Common delegate for Package.json JToken processing. - /// A JToken, usually corresponding to a package.json file. - /// Used in scenarios where one file path creates multiple JTokens, a false value indicates processing additional JTokens should be halted, proceed otherwise. - protected delegate bool JTokenProcessingDelegate(JToken token); - - public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Npm)]; + public override IEnumerable Categories => [Enum.GetName(typeof(DetectorClass), DetectorClass.Npm)!]; public override IList SearchPatterns { get; } = ["package-lock.json", "npm-shrinkwrap.json", LernaSearchPattern]; @@ -61,21 +66,11 @@ protected NpmLockfileDetectorBase( protected abstract bool IsSupportedLockfileVersion(int lockfileVersion); - protected abstract JToken ResolveDependencyObject(JToken packageLockJToken); - - protected abstract void EnqueueAllDependencies( - IDictionary dependencyLookup, + protected abstract void ProcessLockfile( ISingleFileComponentRecorder singleFileComponentRecorder, - Queue<(JProperty CurrentSubDependency, TypedComponent ParentComponent)> subQueue, - JProperty currentDependency, - TypedComponent typedComponent); - - protected abstract bool TryEnqueueFirstLevelDependencies( - Queue<(JProperty DependencyProperty, TypedComponent ParentComponent)> queue, - JToken dependencies, - IDictionary dependencyLookup, - TypedComponent parentComponent = null, - bool skipValidation = false); + PackageJson packageJson, + JsonDocument lockfile, + int lockfileVersion); protected override Task> OnPrepareDetectionAsync( IObservable processRequests, @@ -118,83 +113,94 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID lernaFile.Location, file.Location)); - await this.SafeProcessAllPackageJTokensAsync(file, (token) => + try { - if (!foundUnderLerna && - (token["name"] == null || - token["version"] == null || - string.IsNullOrWhiteSpace(token["name"].Value()) || - string.IsNullOrWhiteSpace(token["version"].Value()))) + using var lockfileDocument = await JsonDocument.ParseAsync(file.Stream, JsonDocumentOptions, cancellationToken); + var root = lockfileDocument.RootElement; + + // Validate name and version unless under lerna + if (!foundUnderLerna) { - this.Logger.LogInformation("{PackageLogJsonFile} does not contain a valid name and/or version. These are required fields for a valid package-lock.json file. It and its dependencies will not be registered.", file.Location); - return false; + var hasName = root.TryGetProperty("name", out var nameElement) && nameElement.ValueKind == JsonValueKind.String && !string.IsNullOrWhiteSpace(nameElement.GetString()); + var hasVersion = root.TryGetProperty("version", out var versionElement) && versionElement.ValueKind == JsonValueKind.String && !string.IsNullOrWhiteSpace(versionElement.GetString()); + + if (!hasName || !hasVersion) + { + this.Logger.LogInformation("{PackageLogJsonFile} does not contain a valid name and/or version. These are required fields for a valid package-lock.json file. It and its dependencies will not be registered.", file.Location); + return; + } } - this.ProcessIndividualPackageJTokens(singleFileComponentRecorder, token, packageJsonComponentStream, skipValidation: foundUnderLerna); - return true; - }); - } + var lockfileVersion = root.TryGetProperty("lockfileVersion", out var lockfileVersionElement) ? lockfileVersionElement.GetInt32() : 1; + this.RecordLockfileVersion(lockfileVersion); - protected async Task ProcessAllPackageJTokensAsync(IComponentStream componentStream, JTokenProcessingDelegate jtokenProcessor) - { - try - { - if (!componentStream.Stream.CanRead) + if (!this.IsSupportedLockfileVersion(lockfileVersion)) { - componentStream.Stream.ReadByte(); + return; } - } - catch (Exception ex) - { - this.Logger.LogInformation(ex, "Could not read {ComponentStreamFile} file.", componentStream.Location); - return; - } - using var file = new StreamReader(componentStream.Stream); - using var reader = new JsonTextReader(file); - - var o = await JToken.ReadFromAsync(reader); - jtokenProcessor(o); - return; - } + // Read package.json files + var packageJsons = new List(); + foreach (var stream in packageJsonComponentStream) + { + try + { + var packageJson = await JsonSerializer.DeserializeAsync(stream.Stream, JsonOptions, cancellationToken); + if (packageJson is not null) + { + packageJsons.Add(packageJson); + } + } + catch (JsonException ex) + { + this.Logger.LogWarning(ex, "Could not parse package.json at {Location}", stream.Location); + } + } - private void ProcessIndividualPackageJTokens(ISingleFileComponentRecorder singleFileComponentRecorder, JToken packageLockJToken, IEnumerable packageJsonComponentStream, bool skipValidation = false) - { - var lockfileVersion = packageLockJToken.Value("lockfileVersion"); - this.RecordLockfileVersion(lockfileVersion); + if (packageJsons.Count == 0) + { + throw new InvalidOperationException(string.Format("InvalidPackageJson -- There must be a package.json file at '{0}' for components to be registered", singleFileComponentRecorder.ManifestFileLocation)); + } - if (!this.IsSupportedLockfileVersion(lockfileVersion)) + // Process each package.json against the lockfile + foreach (var packageJson in packageJsons) + { + this.ProcessLockfile(singleFileComponentRecorder, packageJson, lockfileDocument, lockfileVersion); + } + } + catch (JsonException ex) { - return; + this.Logger.LogInformation(ex, "Could not parse JSON from {ComponentLocation} file.", file.Location); } - - var dependencies = this.ResolveDependencyObject(packageLockJToken); - var topLevelDependencies = new Queue<(JProperty, TypedComponent)>(); - - var dependencyLookup = dependencies?.Children().ToDictionary(dependency => dependency.Name) ?? []; - - foreach (var stream in packageJsonComponentStream) + catch (InvalidOperationException ex) { - using var file = new StreamReader(stream.Stream); - using var reader = new JsonTextReader(file); - - var packageJsonToken = JToken.ReadFrom(reader); - var enqueued = this.TryEnqueueFirstLevelDependencies(topLevelDependencies, packageJsonToken["dependencies"], dependencyLookup, skipValidation: skipValidation); - enqueued = enqueued && this.TryEnqueueFirstLevelDependencies(topLevelDependencies, packageJsonToken["devDependencies"], dependencyLookup, skipValidation: skipValidation); - enqueued = enqueued && this.TryEnqueueFirstLevelDependencies(topLevelDependencies, packageJsonToken["optionalDependencies"], dependencyLookup, skipValidation: skipValidation); - if (!enqueued) - { - // This represents a mismatch between lock file and package.json, break out and do not register anything for these files - throw new InvalidOperationException(string.Format("InvalidPackageJson -- There was a mismatch between the components in the package.json and the lock file at '{0}'", singleFileComponentRecorder.ManifestFileLocation)); - } + // Log and continue - this can happen when package.json is missing + this.Logger.LogInformation(ex, "Could not process {ComponentLocation} file.", file.Location); } - - if (!packageJsonComponentStream.Any()) + catch (Exception e) { - throw new InvalidOperationException(string.Format("InvalidPackageJson -- There must be a package.json file at '{0}' for components to be registered", singleFileComponentRecorder.ManifestFileLocation)); + this.Logger.LogInformation(e, "Could not process {ComponentLocation} file.", file.Location); } + } - this.TraverseRequirementAndDependencyTree(topLevelDependencies, dependencyLookup, singleFileComponentRecorder); + protected TypedComponent? CreateComponent(string name, string? version, string? integrity) + { + return NpmComponentUtilities.GetTypedComponent(name, version, integrity, NpmRegistryHost, this.Logger); + } + + protected void RecordComponent( + ISingleFileComponentRecorder recorder, + TypedComponent component, + bool isDevDependency, + TypedComponent? explicitReferencedDependency = null, + string? parentComponentId = null) + { + NpmComponentUtilities.TraverseAndRecordComponents( + isDevDependency, + recorder, + component, + explicitReferencedDependency ?? component, + parentComponentId); } private IObservable RemoveNodeModuleNestedFiles(IObservable componentStreams) @@ -209,7 +215,7 @@ private IObservable RemoveNodeModuleNestedFiles(IObservable RemoveNodeModuleNestedFiles(IObservable RemoveNodeModuleNestedFiles(IObservable RemoveNodeModuleNestedFiles(IObservable topLevelDependencies, - IDictionary dependencyLookup, - ISingleFileComponentRecorder singleFileComponentRecorder) - { - // iterate through everything for a top level dependency with a single explicitReferencedDependency value - foreach (var (currentDependency, _) in topLevelDependencies) - { - var typedComponent = NpmComponentUtilities.GetTypedComponent(currentDependency, NpmRegistryHost, this.Logger); - if (typedComponent == null) - { - continue; - } - - var previouslyAddedComponents = new HashSet { typedComponent.Id }; - var subQueue = new Queue<(JProperty, TypedComponent)>(); - - NpmComponentUtilities.TraverseAndRecordComponents(currentDependency, singleFileComponentRecorder, typedComponent, explicitReferencedDependency: typedComponent); - - this.EnqueueAllDependencies(dependencyLookup, singleFileComponentRecorder, subQueue, currentDependency, typedComponent); - - while (subQueue.Count != 0) - { - var (currentSubDependency, parentComponent) = subQueue.Dequeue(); - - var typedSubComponent = NpmComponentUtilities.GetTypedComponent(currentSubDependency, NpmRegistryHost, this.Logger); - - // only process components that we haven't seen before that have been brought in by the same explicitReferencedDependency, resolves circular npm 'requires' loop - if (typedSubComponent == null || previouslyAddedComponents.Contains(typedSubComponent.Id)) - { - continue; - } - - previouslyAddedComponents.Add(typedSubComponent.Id); - - NpmComponentUtilities.TraverseAndRecordComponents(currentSubDependency, singleFileComponentRecorder, typedSubComponent, explicitReferencedDependency: typedComponent, parentComponent.Id); - - this.EnqueueAllDependencies(dependencyLookup, singleFileComponentRecorder, subQueue, currentSubDependency, typedSubComponent); - } - } - } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/NpmUtilitiesTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/NpmUtilitiesTests.cs index 4e26dcf73..f3e89bccd 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/NpmUtilitiesTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/NpmUtilitiesTests.cs @@ -1,7 +1,6 @@ #nullable disable namespace Microsoft.ComponentDetection.Detectors.Tests; -using System.Linq; using AwesomeAssertions; using Microsoft.ComponentDetection.Common.DependencyGraph; using Microsoft.ComponentDetection.Contracts; @@ -11,7 +10,6 @@ namespace Microsoft.ComponentDetection.Detectors.Tests; using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Newtonsoft.Json.Linq; [TestClass] [TestCategory("Governance/All")] @@ -29,22 +27,17 @@ public void TestInitialize() [TestMethod] public void TestGetTypedComponent() { - var json = @"{ - ""async"": { - ""version"": ""2.3.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; + var componentFromMethod = NpmComponentUtilities.GetTypedComponent( + "async", + "2.3.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); - var j = JObject.Parse(json); + componentFromMethod.Should().NotBeNull(); + componentFromMethod.Type.Should().Be(ComponentType.Npm); - var componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - - componentFromJProperty.Should().NotBeNull(); - componentFromJProperty.Type.Should().Be(ComponentType.Npm); - - var npmComponent = (NpmComponent)componentFromJProperty; + var npmComponent = (NpmComponent)componentFromMethod; npmComponent.Name.Should().Be("async"); npmComponent.Version.Should().Be("2.3.0"); } @@ -52,84 +45,64 @@ public void TestGetTypedComponent() [TestMethod] public void TestGetTypedComponent_FailsOnMalformed() { - var json = @"{ - ""async"": { - ""version"": ""NOTAVERSION"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; - - var j = JObject.Parse(json); - - var componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - - componentFromJProperty.Should().BeNull(); + var componentFromMethod = NpmComponentUtilities.GetTypedComponent( + "async", + "NOTAVERSION", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + + componentFromMethod.Should().BeNull(); } [TestMethod] public void TestGetTypedComponent_FailsOnInvalidPackageName() { - var jsonInvalidCharacter = @"{ - ""async<"": { - ""version"": ""1.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; - - var j = JObject.Parse(jsonInvalidCharacter); - var componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - componentFromJProperty.Should().BeNull(); - - var jsonUrlName = @"{ - ""http://thisis/my/packagename"": { - ""version"": ""1.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; - - j = JObject.Parse(jsonUrlName); - componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - componentFromJProperty.Should().BeNull(); - - var jsonInvalidInitialCharacter1 = @"{ - ""_async"": { - ""version"": ""1.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; - - j = JObject.Parse(jsonInvalidInitialCharacter1); - componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - componentFromJProperty.Should().BeNull(); - - var jsonInvalidInitialCharacter2 = @"{ - "".async"": { - ""version"": ""1.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }, - }"; - - j = JObject.Parse(jsonInvalidInitialCharacter2); - componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - componentFromJProperty.Should().BeNull(); - + // Invalid character + var componentFromMethod = NpmComponentUtilities.GetTypedComponent( + "async<", + "1.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + componentFromMethod.Should().BeNull(); + + // URL name + componentFromMethod = NpmComponentUtilities.GetTypedComponent( + "http://thisis/my/packagename", + "1.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + componentFromMethod.Should().BeNull(); + + // Invalid initial character _ + componentFromMethod = NpmComponentUtilities.GetTypedComponent( + "_async", + "1.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + componentFromMethod.Should().BeNull(); + + // Invalid initial character . + componentFromMethod = NpmComponentUtilities.GetTypedComponent( + ".async", + "1.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + componentFromMethod.Should().BeNull(); + + // Long name var longPackageName = new string('a', 214); - var jsonLongName = $@"{{ - ""{longPackageName}"": {{ - ""version"": ""1.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="" - }}, - }}"; - - j = JObject.Parse(jsonLongName); - componentFromJProperty = NpmComponentUtilities.GetTypedComponent(j.Children().Single(), "registry.npmjs.org", this.loggerMock.Object); - componentFromJProperty.Should().BeNull(); + componentFromMethod = NpmComponentUtilities.GetTypedComponent( + longPackageName, + "1.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + componentFromMethod.Should().BeNull(); } [TestMethod] @@ -146,33 +119,20 @@ public void TestTryParseNpmVersion() [TestMethod] public void TestTraverseAndGetRequirementsAndDependencies() { - var json = @"{ - ""archiver"": { - ""version"": ""2.3.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="", - ""dependencies"": { - ""archiver-utils"": { - ""version"": ""1.3.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/archiver-utils/-/archiver-utils-1.3.0.tgz"", - ""integrity"": ""sha1-PRT306DRK/NZUaVL07iuqH7nWPg="" - } - } - }, - }"; - - var jsonChildren = JObject.Parse(json).Children(); - var currentDependency = jsonChildren.Single(); - var dependencyLookup = jsonChildren.ToDictionary(dependency => dependency.Name); - - var typedComponent = NpmComponentUtilities.GetTypedComponent(currentDependency, "registry.npmjs.org", this.loggerMock.Object); + var typedComponent = NpmComponentUtilities.GetTypedComponent( + "archiver", + "2.3.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); + var componentRecorder = new ComponentRecorder(); var singleFileComponentRecorder1 = componentRecorder.CreateSingleFileComponentRecorder("/this/is/a/test/path/"); var singleFileComponentRecorder2 = componentRecorder.CreateSingleFileComponentRecorder("/this/is/a/different/path/"); - NpmComponentUtilities.TraverseAndRecordComponents(currentDependency, singleFileComponentRecorder1, typedComponent, typedComponent); - NpmComponentUtilities.TraverseAndRecordComponents(currentDependency, singleFileComponentRecorder2, typedComponent, typedComponent); + NpmComponentUtilities.TraverseAndRecordComponents(false, singleFileComponentRecorder1, typedComponent, typedComponent); + NpmComponentUtilities.TraverseAndRecordComponents(false, singleFileComponentRecorder2, typedComponent, typedComponent); componentRecorder.GetDetectedComponents().Should().ContainSingle(); componentRecorder.GetComponent(typedComponent.Id).Should().NotBeNull(); @@ -184,29 +144,21 @@ public void TestTraverseAndGetRequirementsAndDependencies() graph2.GetExplicitReferencedDependencyIds(typedComponent.Id).Should().Contain(typedComponent.Id); componentRecorder.GetEffectiveDevDependencyValue(typedComponent.Id).GetValueOrDefault(true).Should().BeFalse(); - var json1 = @"{ - ""test"": { - ""version"": ""2.0.0"", - ""resolved"": ""https://mseng.pkgs.visualstudio.com/_packaging/VsoMicrosoftExternals/npm/registry/async/-/async-2.3.0.tgz"", - ""integrity"": ""sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k="", - ""dev"": ""true"" - }, - }"; - - var jsonChildren1 = JObject.Parse(json1).Children(); - var currentDependency1 = jsonChildren1.Single(); - var dependencyLookup1 = jsonChildren1.ToDictionary(dependency => dependency.Name); - - var typedComponent1 = NpmComponentUtilities.GetTypedComponent(currentDependency1, "registry.npmjs.org", this.loggerMock.Object); + var typedComponent1 = NpmComponentUtilities.GetTypedComponent( + "test", + "2.0.0", + "sha1-EBPRBRBH3TIP4k5JTVxm7K9hR9k=", + "registry.npmjs.org", + this.loggerMock.Object); - NpmComponentUtilities.TraverseAndRecordComponents(currentDependency1, singleFileComponentRecorder2, typedComponent1, typedComponent1); + NpmComponentUtilities.TraverseAndRecordComponents(true, singleFileComponentRecorder2, typedComponent1, typedComponent1); componentRecorder.GetDetectedComponents().Should().HaveCount(2); graph2.GetExplicitReferencedDependencyIds(typedComponent1.Id).Should().Contain(typedComponent1.Id); componentRecorder.GetEffectiveDevDependencyValue(typedComponent1.Id).GetValueOrDefault(false).Should().BeTrue(); - NpmComponentUtilities.TraverseAndRecordComponents(currentDependency1, singleFileComponentRecorder2, typedComponent, typedComponent1, parentComponentId: typedComponent1.Id); + NpmComponentUtilities.TraverseAndRecordComponents(true, singleFileComponentRecorder2, typedComponent, typedComponent1, parentComponentId: typedComponent1.Id); componentRecorder.GetDetectedComponents().Should().HaveCount(2); var explicitlyReferencedDependencyIds = graph2.GetExplicitReferencedDependencyIds(typedComponent.Id); diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs index d5fd76c56..396d6d458 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs @@ -1,3 +1,4 @@ +#nullable disable namespace Microsoft.ComponentDetection.Orchestrator.Tests.Experiments; using AwesomeAssertions; From 291c8034ee56d47e8d1e9079b414cdcdcf1eea7f Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Mon, 1 Dec 2025 21:47:15 +0000 Subject: [PATCH 2/3] Add unit tests for custom json converters --- .../PackageJsonAuthorConverterTests.cs | 205 ++++++++++++++++++ .../Contracts/PackageJsonConvertersTests.cs | 82 +++++++ .../PackageJsonEnginesConverterTests.cs | 185 ++++++++++++++++ .../PackageJsonWorkspacesConverterTests.cs | 203 +++++++++++++++++ 4 files changed, 675 insertions(+) create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonAuthorConverterTests.cs create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonConvertersTests.cs create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonEnginesConverterTests.cs create mode 100644 test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonWorkspacesConverterTests.cs diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonAuthorConverterTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonAuthorConverterTests.cs new file mode 100644 index 000000000..2186660f0 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonAuthorConverterTests.cs @@ -0,0 +1,205 @@ +#nullable disable +namespace Microsoft.ComponentDetection.Detectors.Tests.Npm.Contracts; + +using System.Text.Json; +using AwesomeAssertions; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class PackageJsonAuthorConverterTests +{ + private static readonly JsonSerializerOptions Options = new() + { + PropertyNameCaseInsensitive = true, + }; + + [TestMethod] + public void ParsesStringWithNameOnly() + { + var json = """{ "author": "John Doe" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().BeNull(); + result.Author.Url.Should().BeNull(); + } + + [TestMethod] + public void ParsesStringWithNameAndEmail() + { + var json = """{ "author": "John Doe " }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().Be("john@example.com"); + } + + [TestMethod] + public void ParsesStringWithNameEmailAndUrl() + { + var json = """{ "author": "John Doe (https://example.com)" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().Be("john@example.com"); + } + + [TestMethod] + public void ParsesStringWithNameAndUrl_NoEmail() + { + var json = @"{ ""author"": ""John Doe (https://example.com)"" }"; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().BeNull(); + } + + [TestMethod] + public void ParsesObjectFormat() + { + var json = """{ "author": { "name": "John Doe", "email": "john@example.com", "url": "https://example.com" } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().Be("john@example.com"); + result.Author.Url.Should().Be("https://example.com"); + } + + [TestMethod] + public void ParsesObjectWithNameOnly() + { + var json = """{ "author": { "name": "John Doe" } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().BeNull(); + result.Author.Url.Should().BeNull(); + } + + [TestMethod] + public void ReturnsNullForNullValue() + { + var json = """{ "author": null }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().BeNull(); + } + + [TestMethod] + public void ReturnsNullForEmptyString() + { + var json = """{ "author": "" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().BeNull(); + } + + [TestMethod] + public void ReturnsNullForWhitespaceOnlyString() + { + var json = """{ "author": " " }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().BeNull(); + } + + [TestMethod] + public void HandlesStringWithExtraWhitespace() + { + var json = """{ "author": " John Doe " }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Author.Email.Should().Be("john@example.com"); + } + + [TestMethod] + public void HandlesMissingAuthorField() + { + var json = """{ "name": "test-package" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().BeNull(); + } + + [TestMethod] + public void SkipsUnexpectedTokenTypes() + { + // Author as a number (malformed) + var json = """{ "author": 12345 }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Author.Should().BeNull(); + } + + [TestMethod] + public void CanSerializeAuthorObject() + { + var packageJson = new PackageJson + { + Author = new PackageJsonAuthor + { + Name = "John Doe", + Email = "john@example.com", + }, + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Author.Should().NotBeNull(); + deserialized.Author.Name.Should().Be("John Doe"); + deserialized.Author.Email.Should().Be("john@example.com"); + } + + [TestMethod] + public void CanSerializeNullAuthor() + { + var packageJson = new PackageJson + { + Name = "test-package", + Author = null, + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Author.Should().BeNull(); + } +} diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonConvertersTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonConvertersTests.cs new file mode 100644 index 000000000..f8a502afe --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonConvertersTests.cs @@ -0,0 +1,82 @@ +#nullable disable +namespace Microsoft.ComponentDetection.Detectors.Tests.Npm.Contracts; + +using System.Text.Json; +using AwesomeAssertions; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Integration tests for PackageJson deserialization using all converters together. +/// +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class PackageJsonConvertersTests +{ + private static readonly JsonSerializerOptions Options = new() + { + PropertyNameCaseInsensitive = true, + }; + + [TestMethod] + public void ParseCompletePackageJson() + { + var json = """ + { + "name": "test-package", + "version": "1.0.0", + "author": "John Doe ", + "engines": { "node": ">=14.0.0" }, + "workspaces": ["packages/*"] + } + """; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Name.Should().Be("test-package"); + result.Version.Should().Be("1.0.0"); + result.Author.Should().NotBeNull(); + result.Author.Name.Should().Be("John Doe"); + result.Engines.Should().NotBeNull(); + result.Engines["node"].Should().Be(">=14.0.0"); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().Contain("packages/*"); + } + + [TestMethod] + public void HandleAllNullableFields() + { + var json = """ + { + "name": "test-package", + "author": null, + "engines": null, + "workspaces": null + } + """; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Name.Should().Be("test-package"); + result.Author.Should().BeNull(); + result.Engines.Should().BeNull(); + result.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void HandleMinimalPackageJson() + { + var json = """{ "name": "minimal" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Name.Should().Be("minimal"); + result.Author.Should().BeNull(); + result.Engines.Should().BeNull(); + result.Workspaces.Should().BeNull(); + } +} diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonEnginesConverterTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonEnginesConverterTests.cs new file mode 100644 index 000000000..864a4a578 --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonEnginesConverterTests.cs @@ -0,0 +1,185 @@ +#nullable disable +namespace Microsoft.ComponentDetection.Detectors.Tests.Npm.Contracts; + +using System.Collections.Generic; +using System.Text.Json; +using AwesomeAssertions; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class PackageJsonEnginesConverterTests +{ + private static readonly JsonSerializerOptions Options = new() + { + PropertyNameCaseInsensitive = true, + }; + + [TestMethod] + public void ParsesObjectFormat() + { + var json = """{ "engines": { "node": ">=14.0.0", "npm": ">=6.0.0" } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().HaveCount(2); + result.Engines["node"].Should().Be(">=14.0.0"); + result.Engines["npm"].Should().Be(">=6.0.0"); + } + + [TestMethod] + public void ParsesSingleEngine() + { + var json = """{ "engines": { "node": "^16.0.0" } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().HaveCount(1); + result.Engines["node"].Should().Be("^16.0.0"); + } + + [TestMethod] + public void ParsesEmptyObject() + { + var json = """{ "engines": {} }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().BeEmpty(); + } + + [TestMethod] + public void ReturnsNullForNullValue() + { + var json = """{ "engines": null }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().BeNull(); + } + + [TestMethod] + public void HandlesMalformedArrayFormat() + { + // Some malformed package.json files have engines as an array + var json = """{ "engines": ["node >= 14"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + + // Array format returns empty dictionary since we can't map to key-value pairs + result.Engines.Should().BeEmpty(); + } + + [TestMethod] + public void HandlesArrayWithVscodeEngine() + { + // When array contains vscode, it should be captured + var json = """{ "engines": ["vscode ^1.60.0", "node >= 14"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().ContainKey("vscode"); + result.Engines["vscode"].Should().Be("vscode ^1.60.0"); + } + + [TestMethod] + public void HandlesArrayWithVscodeUpperCase() + { + var json = """{ "engines": ["VSCODE >= 1.0.0"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().ContainKey("vscode"); + } + + [TestMethod] + public void SkipsNonStringValuesInObject() + { + // If a value is not a string, it should be skipped + var json = """{ "engines": { "node": ">=14.0.0", "invalid": 123 } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().NotBeNull(); + result.Engines.Should().HaveCount(1); + result.Engines["node"].Should().Be(">=14.0.0"); + } + + [TestMethod] + public void HandlesMissingEnginesField() + { + var json = """{ "name": "test-package" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().BeNull(); + } + + [TestMethod] + public void SkipsUnexpectedTokenTypes() + { + // Engines as a string (malformed) + var json = """{ "engines": "node >= 14" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Engines.Should().BeNull(); + } + + [TestMethod] + public void CanSerializeEngines() + { + var packageJson = new PackageJson + { + Engines = new Dictionary + { + ["node"] = ">=14.0.0", + ["npm"] = ">=6.0.0", + }, + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Engines.Should().NotBeNull(); + deserialized.Engines.Should().HaveCount(2); + deserialized.Engines["node"].Should().Be(">=14.0.0"); + deserialized.Engines["npm"].Should().Be(">=6.0.0"); + } + + [TestMethod] + public void CanSerializeNullEngines() + { + var packageJson = new PackageJson + { + Name = "test-package", + Engines = null, + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Engines.Should().BeNull(); + } +} diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonWorkspacesConverterTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonWorkspacesConverterTests.cs new file mode 100644 index 000000000..a8ad82e2a --- /dev/null +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/npm/Contracts/PackageJsonWorkspacesConverterTests.cs @@ -0,0 +1,203 @@ +#nullable disable +namespace Microsoft.ComponentDetection.Detectors.Tests.Npm.Contracts; + +using System.Text.Json; +using AwesomeAssertions; +using Microsoft.ComponentDetection.Detectors.Npm.Contracts; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +[TestCategory("Governance/All")] +[TestCategory("Governance/ComponentDetection")] +public class PackageJsonWorkspacesConverterTests +{ + private static readonly JsonSerializerOptions Options = new() + { + PropertyNameCaseInsensitive = true, + }; + + [TestMethod] + public void ParsesArrayFormat() + { + var json = """{ "workspaces": ["packages/*", "apps/*"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(2); + result.Workspaces.Should().Contain("packages/*"); + result.Workspaces.Should().Contain("apps/*"); + } + + [TestMethod] + public void ParsesSingleWorkspace() + { + var json = """{ "workspaces": ["packages/*"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(1); + result.Workspaces.Should().Contain("packages/*"); + } + + [TestMethod] + public void ParsesEmptyArray() + { + var json = """{ "workspaces": [] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().BeEmpty(); + } + + [TestMethod] + public void ParsesObjectWithPackagesField() + { + var json = """{ "workspaces": { "packages": ["packages/*", "apps/*"] } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(2); + result.Workspaces.Should().Contain("packages/*"); + result.Workspaces.Should().Contain("apps/*"); + } + + [TestMethod] + public void ParsesObjectWithEmptyPackagesArray() + { + var json = """{ "workspaces": { "packages": [] } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().BeEmpty(); + } + + [TestMethod] + public void ReturnsNullForObjectWithoutPackages() + { + // Object format without packages field + var json = """{ "workspaces": { "nohoist": ["**/react-native"] } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void ReturnsNullForNullValue() + { + var json = """{ "workspaces": null }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void HandlesMissingWorkspacesField() + { + var json = """{ "name": "test-package" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void SkipsUnexpectedTokenTypes() + { + // Workspaces as a string (malformed) + var json = """{ "workspaces": "packages/*" }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void IgnoresNonStringValuesInArray() + { + var json = """{ "workspaces": ["packages/*", 123, "apps/*"] }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(2); + result.Workspaces.Should().Contain("packages/*"); + result.Workspaces.Should().Contain("apps/*"); + } + + [TestMethod] + public void HandlesObjectWithOtherFieldsAndPackages() + { + // Yarn workspaces can have both packages and nohoist fields + var json = """{ "workspaces": { "packages": ["packages/*"], "nohoist": ["**/react-native"] } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(1); + result.Workspaces.Should().Contain("packages/*"); + } + + [TestMethod] + public void CanSerializeWorkspaces() + { + var packageJson = new PackageJson + { + Workspaces = ["packages/*", "apps/*"], + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Workspaces.Should().NotBeNull(); + deserialized.Workspaces.Should().HaveCount(2); + deserialized.Workspaces.Should().Contain("packages/*"); + deserialized.Workspaces.Should().Contain("apps/*"); + } + + [TestMethod] + public void CanSerializeNullWorkspaces() + { + var packageJson = new PackageJson + { + Name = "test-package", + Workspaces = null, + }; + + var json = JsonSerializer.Serialize(packageJson, Options); + var deserialized = JsonSerializer.Deserialize(json, Options); + + deserialized.Should().NotBeNull(); + deserialized.Workspaces.Should().BeNull(); + } + + [TestMethod] + public void IsCaseInsensitiveForPackagesField() + { + var json = """{ "workspaces": { "PACKAGES": ["packages/*"] } }"""; + + var result = JsonSerializer.Deserialize(json, Options); + + result.Should().NotBeNull(); + result.Workspaces.Should().NotBeNull(); + result.Workspaces.Should().HaveCount(1); + } +} From a2a68cc4b8d1e4297532f3e6f023d1e8a27f3269 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Mon, 1 Dec 2025 21:51:41 +0000 Subject: [PATCH 3/3] PR comments --- .../npm/NpmComponentDetector.cs | 7 ++----- .../npm/NpmComponentDetectorWithRoots.cs | 19 +++++++++---------- .../npm/NpmLockfile3Detector.cs | 11 +++++------ 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs index d44f5b541..1ee6a4e3e 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetector.cs @@ -83,12 +83,9 @@ protected virtual bool ProcessPackageJson(string filePath, ISingleFileComponentR } var containsVsCodeEngine = false; - if (packageJson.Engines is not null) + if (packageJson.Engines is not null && packageJson.Engines.ContainsKey("vscode")) { - if (packageJson.Engines.ContainsKey("vscode")) - { - containsVsCodeEngine = true; - } + containsVsCodeEngine = true; } if (containsVsCodeEngine) diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs index b55c5e4a4..2ee6316c7 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmComponentDetectorWithRoots.cs @@ -1,6 +1,7 @@ namespace Microsoft.ComponentDetection.Detectors.Npm; using System.Collections.Generic; +using System.Linq; using System.Text.Json; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.TypedComponent; @@ -122,12 +123,11 @@ private void EnqueueDependencies( return; } - foreach (var dep in dependencies) + foreach (var (name, dependency) in dependencies.Keys + .Where(dependencyLookup.ContainsKey) + .Select(key => dependencyLookup[key])) { - if (dependencyLookup.TryGetValue(dep.Key, out var lockDep)) - { - queue.Enqueue((lockDep.Name, lockDep.Dependency, parent)); - } + queue.Enqueue((name, dependency, parent)); } } @@ -149,12 +149,11 @@ private void EnqueueNestedDependencies( // Enqueue requires (these reference the top-level lookup) if (dependency.Requires is not null) { - foreach (var req in dependency.Requires) + foreach (var (name, dep) in dependency.Requires.Keys + .Where(dependencyLookup.ContainsKey) + .Select(key => dependencyLookup[key])) { - if (dependencyLookup.TryGetValue(req.Key, out var lockDep)) - { - queue.Enqueue((lockDep.Name, lockDep.Dependency, parent)); - } + queue.Enqueue((name, dep, parent)); } } } diff --git a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs index 69015f3fa..f2916f10b 100644 --- a/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfile3Detector.cs @@ -132,13 +132,12 @@ private void EnqueueDependencies( return; } - foreach (var dep in dependencies) + foreach (var (path, package) in dependencies.Keys + .Select(key => $"{NodeModules}/{key}") + .Where(packageLookup.ContainsKey) + .Select(path => packageLookup[path])) { - var path = $"{NodeModules}/{dep.Key}"; - if (packageLookup.TryGetValue(path, out var lockPkg)) - { - queue.Enqueue((lockPkg.Path, lockPkg.Package, parent)); - } + queue.Enqueue((path, package, parent)); } }