Skip to content

Commit d534ee9

Browse files
Copilotjcfiorenzanogrvillic
authored
Fix NullReferenceException in NpmLockfileDetectorBase when dependencies object is missing (#1437)
* Initial plan for issue * Start investigation of NullReferenceException in NpmLockfileDetectorBase Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> * Fix NullReferenceException in NpmLockfileDetectorBase when dependencies object is missing - Add null check for dependencies object in ProcessIndividualPackageJTokens method - Use null conditional operator to safely handle missing dependencies/packages properties - Add comprehensive test cases for both lockfile v2 (dependencies) and v3 (packages) scenarios - Ensure backward compatibility and no regression in existing functionality Resolves issue where package-lock.json files without dependencies/packages properties would cause NullReferenceException when calling .Children<JProperty>() on null JToken. Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> * Revert global.json SDK version change back to 8.0.408 Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> * Remove TestNpmDetector_PackageLockWithNullDependenciesValue_ShouldHandleGracefully test as requested Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> * Remove TestNpmDetector_PackageLockWithNullPackagesValue_ShouldHandleGracefully test as requested Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> * Remove comment as requested Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jcfiorenzano <5582507+jcfiorenzano@users.noreply.github.com> Co-authored-by: Greg Villicana <58237075+grvillic@users.noreply.github.com>
1 parent adabadd commit d534ee9

3 files changed

Lines changed: 169 additions & 1 deletion

File tree

‎src/Microsoft.ComponentDetection.Detectors/npm/NpmLockfileDetectorBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ private void ProcessIndividualPackageJTokens(ISingleFileComponentRecorder single
170170
var dependencies = this.ResolveDependencyObject(packageLockJToken);
171171
var topLevelDependencies = new Queue<(JProperty, TypedComponent)>();
172172

173-
var dependencyLookup = dependencies.Children<JProperty>().ToDictionary(dependency => dependency.Name);
173+
var dependencyLookup = dependencies?.Children<JProperty>().ToDictionary(dependency => dependency.Name) ?? [];
174174

175175
foreach (var stream in packageJsonComponentStream)
176176
{

‎test/Microsoft.ComponentDetection.Detectors.Tests/NpmDetectorWithRootsTests.cs‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,4 +609,88 @@ public async Task TestNpmDetector_DependencyGraphIsCreatedAsync()
609609
dependencyGraph.GetDependenciesForComponent(componentBId).Should().Contain(componentCId);
610610
dependencyGraph.GetDependenciesForComponent(componentCId).Should().BeEmpty();
611611
}
612+
613+
[TestMethod]
614+
public async Task TestNpmDetector_PackageLockWithoutDependenciesObject_ShouldHandleGracefully()
615+
{
616+
// This test reproduces the NullReferenceException issue when package-lock.json doesn't contain a "dependencies" object
617+
var packageLockJson = @"{
618+
""name"": ""test"",
619+
""version"": ""1.0.0"",
620+
""lockfileVersion"": 2
621+
}";
622+
623+
var packageJsonContents = @"{
624+
""name"": ""test"",
625+
""version"": ""1.0.0""
626+
}";
627+
628+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
629+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
630+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
631+
.ExecuteDetectorAsync();
632+
633+
// The detector should handle the missing "dependencies" object gracefully without throwing
634+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
635+
var detectedComponents = componentRecorder.GetDetectedComponents();
636+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected
637+
}
638+
639+
[TestMethod]
640+
public async Task TestNpmDetector_PackageLockMissingDependenciesButPackageJsonHasDependencies_ShouldHandleGracefully()
641+
{
642+
// This test reproduces a more specific scenario where package.json has dependencies but package-lock.json is missing dependencies
643+
var packageLockJson = @"{
644+
""name"": ""test"",
645+
""version"": ""1.0.0"",
646+
""lockfileVersion"": 2
647+
}";
648+
649+
var packageJsonContents = @"{
650+
""name"": ""test"",
651+
""version"": ""1.0.0"",
652+
""dependencies"": {
653+
""lodash"": ""^4.17.21""
654+
}
655+
}";
656+
657+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
658+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
659+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
660+
.ExecuteDetectorAsync();
661+
662+
// The detector should handle the missing "dependencies" object gracefully without throwing
663+
// This may result in processing failure but should not throw NullReferenceException
664+
var detectedComponents = componentRecorder.GetDetectedComponents();
665+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected since dependencies is missing
666+
}
667+
668+
[TestMethod]
669+
public async Task TestNpmDetector_PackageLockMissingDependenciesProperty_ShouldNotThrowNullReferenceException()
670+
{
671+
// This test reproduces the exact NullReferenceException scenario from the issue:
672+
// package-lock.json doesn't contain a "dependencies" property at all
673+
var packageLockJson = @"{
674+
""name"": ""test"",
675+
""version"": ""1.0.0"",
676+
""lockfileVersion"": 2
677+
}";
678+
679+
var packageJsonContents = @"{
680+
""name"": ""test"",
681+
""version"": ""1.0.0""
682+
}";
683+
684+
// Before the fix, this would throw a NullReferenceException because
685+
// packageLockJToken["dependencies"] returns null, and calling .Children<JProperty>() on null throws
686+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
687+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
688+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
689+
.ExecuteDetectorAsync();
690+
691+
// The detector should handle the missing "dependencies" property gracefully without throwing
692+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
693+
var detectedComponents = componentRecorder.GetDetectedComponents();
694+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected since dependencies is missing
695+
}
612696
}

‎test/Microsoft.ComponentDetection.Detectors.Tests/NpmLockfile3DetectorTests.cs‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,4 +284,88 @@ public async Task TestNpmDetector_NestedNodeModulesV3Async()
284284
dependencyGraph.GetDependenciesForComponent(componentAId).Should().Contain(componentBId);
285285
dependencyGraph.GetDependenciesForComponent(componentBId).Should().BeEmpty();
286286
}
287+
288+
[TestMethod]
289+
public async Task TestNpmDetector_PackageLockWithoutPackagesObject_ShouldHandleGracefully()
290+
{
291+
// This test reproduces the NullReferenceException issue when package-lock.json doesn't contain a "packages" object
292+
var packageLockJson = @"{
293+
""name"": ""test"",
294+
""version"": ""1.0.0"",
295+
""lockfileVersion"": 3
296+
}";
297+
298+
var packageJsonContents = @"{
299+
""name"": ""test"",
300+
""version"": ""1.0.0""
301+
}";
302+
303+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
304+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
305+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
306+
.ExecuteDetectorAsync();
307+
308+
// The detector should handle the missing "packages" object gracefully without throwing
309+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
310+
var detectedComponents = componentRecorder.GetDetectedComponents();
311+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected
312+
}
313+
314+
[TestMethod]
315+
public async Task TestNpmDetector_PackageLockMissingPackagesButPackageJsonHasDependencies_ShouldHandleGracefully()
316+
{
317+
// This test reproduces a more specific scenario where package.json has dependencies but package-lock.json is missing packages
318+
var packageLockJson = @"{
319+
""name"": ""test"",
320+
""version"": ""1.0.0"",
321+
""lockfileVersion"": 3
322+
}";
323+
324+
var packageJsonContents = @"{
325+
""name"": ""test"",
326+
""version"": ""1.0.0"",
327+
""dependencies"": {
328+
""lodash"": ""^4.17.21""
329+
}
330+
}";
331+
332+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
333+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
334+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
335+
.ExecuteDetectorAsync();
336+
337+
// The detector should handle the missing "packages" object gracefully without throwing
338+
// This may result in processing failure but should not throw NullReferenceException
339+
var detectedComponents = componentRecorder.GetDetectedComponents();
340+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected since packages is missing
341+
}
342+
343+
[TestMethod]
344+
public async Task TestNpmDetector_PackageLockMissingPackagesProperty_ShouldNotThrowNullReferenceException()
345+
{
346+
// This test reproduces the exact NullReferenceException scenario from the issue:
347+
// package-lock.json doesn't contain a "packages" property at all
348+
var packageLockJson = @"{
349+
""name"": ""test"",
350+
""version"": ""1.0.0"",
351+
""lockfileVersion"": 3
352+
}";
353+
354+
var packageJsonContents = @"{
355+
""name"": ""test"",
356+
""version"": ""1.0.0""
357+
}";
358+
359+
// Before the fix, this would throw a NullReferenceException because
360+
// packageLockJToken["packages"] returns null, and calling .Children<JProperty>() on null throws
361+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
362+
.WithFile(this.packageLockJsonFileName, packageLockJson, this.packageLockJsonSearchPatterns)
363+
.WithFile(this.packageJsonFileName, packageJsonContents, this.packageJsonSearchPattern)
364+
.ExecuteDetectorAsync();
365+
366+
// The detector should handle the missing "packages" property gracefully without throwing
367+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
368+
var detectedComponents = componentRecorder.GetDetectedComponents();
369+
detectedComponents.Should().BeEmpty(); // No dependencies should be detected since packages is missing
370+
}
287371
}

0 commit comments

Comments
 (0)