Skip to content

Commit f25b012

Browse files
ericstjericstj
authored
Fix reliability issues seen in DotNet experiment (#1382)
1 parent cd49c50 commit f25b012

3 files changed

Lines changed: 144 additions & 22 deletions

File tree

‎src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class DotNetComponentDetector : FileComponentDetector, IExperimentalDetec
2626
private readonly IPathUtilityService pathUtilityService;
2727
private readonly LockFileFormat lockFileFormat = new();
2828
private readonly ConcurrentDictionary<string, string?> sdkVersionCache = [];
29+
private readonly JsonDocumentOptions jsonDocumentOptions = new() { CommentHandling = JsonCommentHandling.Skip };
2930
private string? sourceDirectory;
3031
private string? sourceFileRootDirectory;
3132

@@ -137,6 +138,12 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID
137138
{
138139
var lockFile = this.lockFileFormat.Read(processRequest.ComponentStream.Stream, processRequest.ComponentStream.Location);
139140

141+
if (lockFile.PackageSpec is null)
142+
{
143+
this.Logger.LogWarning("Lock file {LockFilePath} does not contain a PackageSpec.", processRequest.ComponentStream.Location);
144+
return;
145+
}
146+
140147
var projectAssetsDirectory = this.pathUtilityService.GetParentDirectory(processRequest.ComponentStream.Location);
141148
var projectPath = lockFile.PackageSpec.RestoreMetadata.ProjectPath;
142149
var projectOutputPath = lockFile.PackageSpec.RestoreMetadata.OutputPath;
@@ -184,24 +191,31 @@ protected override async Task OnFileFoundAsync(ProcessRequest processRequest, ID
184191

185192
private string? GetProjectType(string projectOutputPath, string projectName, CancellationToken cancellationToken)
186193
{
187-
if (this.directoryUtilityService.Exists(projectOutputPath))
194+
if (this.directoryUtilityService.Exists(projectOutputPath) &&
195+
projectName is not null &&
196+
projectName.IndexOfAny(Path.GetInvalidFileNameChars()) == -1)
188197
{
189-
var namePattern = projectName ?? "*";
190-
191-
// look for the compiled output, first as dll then as exe.
192-
var candidates = this.directoryUtilityService.EnumerateFiles(projectOutputPath, namePattern + ".dll", SearchOption.AllDirectories)
193-
.Concat(this.directoryUtilityService.EnumerateFiles(projectOutputPath, namePattern + ".exe", SearchOption.AllDirectories));
194-
foreach (var candidate in candidates)
198+
try
195199
{
196-
try
200+
// look for the compiled output, first as dll then as exe.
201+
var candidates = this.directoryUtilityService.EnumerateFiles(projectOutputPath, projectName + ".dll", SearchOption.AllDirectories)
202+
.Concat(this.directoryUtilityService.EnumerateFiles(projectOutputPath, projectName + ".exe", SearchOption.AllDirectories));
203+
foreach (var candidate in candidates)
197204
{
198-
return this.IsApplication(candidate) ? "application" : "library";
199-
}
200-
catch (Exception e)
201-
{
202-
this.Logger.LogWarning("Failed to open output assembly {AssemblyPath} error {Message}.", candidate, e.Message);
205+
try
206+
{
207+
return this.IsApplication(candidate) ? "application" : "library";
208+
}
209+
catch (Exception e)
210+
{
211+
this.Logger.LogWarning("Failed to open output assembly {AssemblyPath} error {Message}.", candidate, e.Message);
212+
}
203213
}
204214
}
215+
catch (IOException e)
216+
{
217+
this.Logger.LogWarning("Failed to enumerate output directory {OutputPath} error {Message}.", projectOutputPath, e.Message);
218+
}
205219
}
206220

207221
return null;
@@ -246,7 +260,7 @@ private bool IsApplication(string assemblyPath)
246260

247261
if (string.IsNullOrWhiteSpace(sdkVersion))
248262
{
249-
var globalJson = await JsonDocument.ParseAsync(this.fileUtilityService.MakeFileStream(globalJsonPath), cancellationToken: cancellationToken);
263+
var globalJson = await JsonDocument.ParseAsync(this.fileUtilityService.MakeFileStream(globalJsonPath), cancellationToken: cancellationToken, options: this.jsonDocumentOptions).ConfigureAwait(false);
250264
if (globalJson.RootElement.TryGetProperty("sdk", out var sdk))
251265
{
252266
if (sdk.TryGetProperty("version", out var version))
@@ -256,14 +270,21 @@ private bool IsApplication(string assemblyPath)
256270
}
257271
}
258272

259-
var globalJsonComponent = new DetectedComponent(new DotNetComponent(sdkVersion));
260-
var recorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(globalJsonPath);
261-
recorder.RegisterUsage(globalJsonComponent, isExplicitReferencedDependency: true);
273+
if (!string.IsNullOrWhiteSpace(sdkVersion))
274+
{
275+
var globalJsonComponent = new DetectedComponent(new DotNetComponent(sdkVersion));
276+
var recorder = this.ComponentRecorder.CreateSingleFileComponentRecorder(globalJsonPath);
277+
recorder.RegisterUsage(globalJsonComponent, isExplicitReferencedDependency: true);
278+
return sdkVersion;
279+
}
280+
281+
// global.json may be malformed, or the sdk version may not be specified.
262282
}
263-
else if (projectDirectory.Equals(this.sourceDirectory, StringComparison.OrdinalIgnoreCase) ||
264-
projectDirectory.Equals(this.sourceFileRootDirectory, StringComparison.OrdinalIgnoreCase) ||
265-
string.IsNullOrEmpty(parentDirectory) ||
266-
projectDirectory.Equals(parentDirectory, StringComparison.OrdinalIgnoreCase))
283+
284+
if (projectDirectory.Equals(this.sourceDirectory, StringComparison.OrdinalIgnoreCase) ||
285+
projectDirectory.Equals(this.sourceFileRootDirectory, StringComparison.OrdinalIgnoreCase) ||
286+
string.IsNullOrEmpty(parentDirectory) ||
287+
projectDirectory.Equals(parentDirectory, StringComparison.OrdinalIgnoreCase))
267288
{
268289
// if we are at the source directory, source file root, or have reached a root directory, run `dotnet --version`
269290
// this could fail if dotnet is not on the path, or if the global.json is malformed

‎src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDiction
9292

9393
if (lockFile.PackageSpec == null)
9494
{
95-
throw new FormatException("Lockfile did not contain a PackageSpec");
95+
this.Logger.LogWarning("Lock file {LockFilePath} does not contain a PackageSpec.", processRequest.ComponentStream.Location);
96+
return Task.CompletedTask;
9697
}
9798

9899
var explicitReferencedDependencies = this.GetTopLevelLibraries(lockFile)

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,19 @@ private static Stream GlobalJson(string sdkVersion)
223223
return stream;
224224
}
225225

226+
private static Stream StreamFromString(string content)
227+
{
228+
var stream = new MemoryStream();
229+
using (var writer = new StreamWriter(stream, leaveOpen: true))
230+
{
231+
writer.Write(content);
232+
writer.Flush();
233+
stream.Position = 0;
234+
}
235+
236+
return stream;
237+
}
238+
226239
[TestMethod]
227240
public async Task TestDotNetDetectorWithNoFiles_ReturnsSuccessfullyAsync()
228241
{
@@ -256,6 +269,70 @@ public async Task TestDotNetDetectorGlobalJson_ReturnsSDKVersion()
256269
discoveredComponents.Where(component => component.Component.Id == "42.0.800 net8.0 unknown - DotNet").Should().ContainSingle();
257270
}
258271

272+
[TestMethod]
273+
public async Task TestDotNetDetectorGlobalJsonWithComments_ReturnsSDKVersion()
274+
{
275+
var projectPath = Path.Combine(RootDir, "path", "to", "project");
276+
var projectAssets = ProjectAssets("projectName", "does-not-exist", projectPath, "net8.0");
277+
278+
var globalJson = StreamFromString("""
279+
// comment
280+
{
281+
// comment
282+
"sdk": {
283+
// comment
284+
"version": "8.2.100"
285+
}
286+
}
287+
""");
288+
this.AddFile(projectPath, null);
289+
this.AddFile(Path.Combine(RootDir, "path", "global.json"), globalJson);
290+
this.SetCommandResult(-1);
291+
292+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
293+
.WithFile("project.assets.json", projectAssets)
294+
.ExecuteDetectorAsync();
295+
296+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
297+
298+
var detectedComponents = componentRecorder.GetDetectedComponents();
299+
detectedComponents.Should().HaveCount(2);
300+
301+
var discoveredComponents = detectedComponents.ToArray();
302+
discoveredComponents.Where(component => component.Component.Id == "8.2.100 unknown unknown - DotNet").Should().ContainSingle();
303+
discoveredComponents.Where(component => component.Component.Id == "8.2.100 net8.0 unknown - DotNet").Should().ContainSingle();
304+
}
305+
306+
[TestMethod]
307+
public async Task TestDotNetDetectorGlobalJsonWithoutVersion()
308+
{
309+
var projectPath = Path.Combine(RootDir, "path", "to", "project");
310+
var projectAssets = ProjectAssets("projectName", "does-not-exist", projectPath, "net8.0");
311+
var globalJson = StreamFromString("""
312+
{
313+
"msbuild-sdks": {
314+
"Microsoft.Build.NoTargets": "3.5.0",
315+
"Microsoft.DotNet.Arcade.Sdk": "10.0.0-beta.25206.1"
316+
}
317+
}
318+
""");
319+
this.AddFile(projectPath, null);
320+
this.AddFile(Path.Combine(RootDir, "path", "global.json"), globalJson);
321+
this.SetCommandResult(-1);
322+
323+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
324+
.WithFile("project.assets.json", projectAssets)
325+
.ExecuteDetectorAsync();
326+
327+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
328+
329+
var detectedComponents = componentRecorder.GetDetectedComponents();
330+
detectedComponents.Should().ContainSingle();
331+
332+
var discoveredComponents = detectedComponents.ToArray();
333+
discoveredComponents.Where(component => component.Component.Id == "unknown net8.0 unknown - DotNet").Should().ContainSingle();
334+
}
335+
259336
[TestMethod]
260337
public async Task TestDotNetDetectorGlobalJsonRollForward_ReturnsSDKVersion()
261338
{
@@ -370,6 +447,29 @@ public async Task TestDotNetDetectorNoGlobalJsonNoDotnet_ReturnsUnknownVersion()
370447
discoveredComponents.Where(component => component.Component.Id == "unknown net8.0 unknown - DotNet").Should().ContainSingle();
371448
}
372449

450+
[TestMethod]
451+
public async Task TestDotNetDetectorInvalidProjectName_ReturnsUnknownVersion()
452+
{
453+
var projectPath = Path.Combine(RootDir, "path", "to", "project");
454+
var outputPath = Path.Combine(projectPath, "obj");
455+
var projectAssets = ProjectAssets("/foo/bar/projectName", outputPath, projectPath, "net8.0");
456+
this.AddFile(projectPath, null);
457+
this.AddDirectory(outputPath);
458+
this.SetCommandResult(-1);
459+
460+
var (scanResult, componentRecorder) = await this.DetectorTestUtility
461+
.WithFile("project.assets.json", projectAssets)
462+
.ExecuteDetectorAsync();
463+
464+
scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
465+
466+
var detectedComponents = componentRecorder.GetDetectedComponents();
467+
detectedComponents.Should().ContainSingle();
468+
469+
var discoveredComponents = detectedComponents.ToArray();
470+
discoveredComponents.Where(component => component.Component.Id == "unknown net8.0 unknown - DotNet").Should().ContainSingle();
471+
}
472+
373473
[TestMethod]
374474
public async Task TestDotNetDetectorMultipleTargetFrameworks()
375475
{

0 commit comments

Comments
 (0)