Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 201 additions & 0 deletions src/Build.UnitTests/Instance/ProjectInstance_ImportEdges_Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Linq;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Execution;
using Microsoft.Build.Framework;
using Microsoft.Build.UnitTests;
using Microsoft.Build.UnitTests.BackEnd;
using Shouldly;
using Xunit;

namespace Microsoft.Build.UnitTests.OM.Instance
{
/// <summary>
/// Tests for the import edge feature: structured import graph data on ProjectInstance
/// exposed to tasks via EngineServices.GetImportEdges().
/// </summary>
public class ProjectInstance_ImportEdges_Tests
{
private readonly ITestOutputHelper _output;

public ProjectInstance_ImportEdges_Tests(ITestOutputHelper output)
{
_output = output;
}

[Fact]
public void ImportEdgesArePopulatedFromEvaluation()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string import2Content = "<Project />";
var import2File = env.CreateFile("import2.targets", import2Content);

string import1Content = $"""
<Project>
<Import Project="{import2File.Path}" />
</Project>
""";
var import1File = env.CreateFile("import1.targets", import1Content);

string projectContent = $"""
<Project>
<Import Project="{import1File.Path}" />
</Project>
""";
var projectFile = env.CreateFile("test.proj", projectContent);

using var collection = new ProjectCollection();
var project = new Project(projectFile.Path, globalProperties: null, toolsVersion: null, collection);
ProjectInstance instance = project.CreateProjectInstance();

// Should have the flat import paths
instance.ImportPaths.ShouldContain(import1File.Path);
instance.ImportPaths.ShouldContain(import2File.Path);

// Should have structured import edges
var edges = instance.ImportEdges;
edges.ShouldNotBeNull();
edges.Count.ShouldBe(2);

// Edge: project -> import1
var edge1 = edges.First(e => e.ImportedProjectPath == import1File.Path);
edge1.ImportingProjectPath.ShouldBe(projectFile.Path);
edge1.SdkName.ShouldBeNull();

// Edge: import1 -> import2
var edge2 = edges.First(e => e.ImportedProjectPath == import2File.Path);
edge2.ImportingProjectPath.ShouldBe(import1File.Path);
edge2.SdkName.ShouldBeNull();
}

[Fact]
public void ImportEdgesExcludeRootProject()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string projectContent = "<Project />";
var projectFile = env.CreateFile("test.proj", projectContent);

using var collection = new ProjectCollection();
var project = new Project(projectFile.Path, globalProperties: null, toolsVersion: null, collection);
ProjectInstance instance = project.CreateProjectInstance();

// A project with no imports should have zero edges
instance.ImportEdges.ShouldNotBeNull();
instance.ImportEdges.Count.ShouldBe(0);
}

[Fact]
public void ImportEdgesSurviveDeepCopy()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string importContent = "<Project />";
var importFile = env.CreateFile("import.targets", importContent);

string projectContent = $"""
<Project>
<Import Project="{importFile.Path}" />
</Project>
""";
var projectFile = env.CreateFile("test.proj", projectContent);

using var collection = new ProjectCollection();
var project = new Project(projectFile.Path, globalProperties: null, toolsVersion: null, collection);
ProjectInstance original = project.CreateProjectInstance();
ProjectInstance copy = original.DeepCopy();

copy.ImportEdges.ShouldNotBeNull();
copy.ImportEdges.Count.ShouldBe(1);
copy.ImportEdges[0].ImportedProjectPath.ShouldBe(importFile.Path);
copy.ImportEdges[0].ImportingProjectPath.ShouldBe(projectFile.Path);
}

[Fact]
public void ImportEdgesSerializedWhenPropertyIsSet()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string importContent = "<Project />";
var importFile = env.CreateFile("import.targets", importContent);

string projectContent = $"""
<Project>
<PropertyGroup>
<MSBuildProvideImportGraph>true</MSBuildProvideImportGraph>
</PropertyGroup>
<Import Project="{importFile.Path}" />
</Project>
""";
var projectFile = env.CreateFile("test.proj", projectContent);

using var collection = new ProjectCollection();
var project = new Project(projectFile.Path, globalProperties: null, toolsVersion: null, collection);
ProjectInstance original = project.CreateProjectInstance();
original.TranslateEntireState = true;

// Verify edges exist before serialization
original.ImportEdges.ShouldNotBeNull();
original.ImportEdges.Count.ShouldBe(1);

// Round-trip through serialization
((ITranslatable)original).Translate(TranslationHelpers.GetWriteTranslator());
ProjectInstance deserialized = ProjectInstance.FactoryForDeserialization(TranslationHelpers.GetReadTranslator());

// Edges should survive serialization when property is set
deserialized.ImportEdges.ShouldNotBeNull();
deserialized.ImportEdges.Count.ShouldBe(1);
deserialized.ImportEdges[0].ImportedProjectPath.ShouldBe(importFile.Path);
deserialized.ImportEdges[0].ImportingProjectPath.ShouldBe(projectFile.Path);
deserialized.ImportEdges[0].SdkName.ShouldBeNull();
}

[Fact]
public void ImportEdgesNotSerializedWithoutProperty()
{
using TestEnvironment env = TestEnvironment.Create(_output);

string importContent = "<Project />";
var importFile = env.CreateFile("import.targets", importContent);

string projectContent = $"""
<Project>
<Import Project="{importFile.Path}" />
</Project>
""";
var projectFile = env.CreateFile("test.proj", projectContent);

using var collection = new ProjectCollection();
var project = new Project(projectFile.Path, globalProperties: null, toolsVersion: null, collection);
ProjectInstance original = project.CreateProjectInstance();
original.TranslateEntireState = true;

// Edges exist on the original
original.ImportEdges.ShouldNotBeNull();
original.ImportEdges.Count.ShouldBe(1);

// Round-trip through serialization
((ITranslatable)original).Translate(TranslationHelpers.GetWriteTranslator());
ProjectInstance deserialized = ProjectInstance.FactoryForDeserialization(TranslationHelpers.GetReadTranslator());

// Without the opt-in property, edges should NOT be serialized
deserialized.ImportEdges.ShouldBeNull();
}

[Fact]
public void ProjectImportEdgeToString()
{
var edge = new ProjectImportEdge(@"C:\imported.targets", @"C:\project.csproj");
edge.ToString().ShouldContain(@"C:\project.csproj");
edge.ToString().ShouldContain(@"C:\imported.targets");
edge.ToString().ShouldContain("->");

var sdkEdge = new ProjectImportEdge(@"C:\sdk.targets", @"C:\project.csproj", "Microsoft.NET.Sdk");
sdkEdge.ToString().ShouldContain("SDK: Microsoft.NET.Sdk");
}
}
}
4 changes: 4 additions & 0 deletions src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,10 @@ public override bool LogsMessagesOfImportance(MessageImportance importance)

public override bool IsOutOfProcRarNodeEnabled => _taskHost._host.BuildParameters.EnableRarNode;

/// <inheritdoc/>
public override IReadOnlyList<ProjectImportEdge> ImportEdges =>
_taskHost._requestEntry.RequestConfiguration.Project?.ImportEdges;

#if FEATURE_REPORTFILEACCESSES
/// <summary>
/// Reports a file access from a task.
Expand Down
93 changes: 90 additions & 3 deletions src/Build/Instance/ProjectInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ private ProjectInstance(ProjectInstance that, bool isImmutable, RequestedProject
ImportPaths = new ObjectModel.ReadOnlyCollection<string>(_importPaths);
_importPathsIncludingDuplicates = that._importPathsIncludingDuplicates;
ImportPathsIncludingDuplicates = new ObjectModel.ReadOnlyCollection<string>(_importPathsIncludingDuplicates);
ImportEdges = that.ImportEdges;

this.EvaluatedItemElements = that.EvaluatedItemElements;

Expand Down Expand Up @@ -1260,6 +1261,18 @@ public IDictionary<string, ProjectItemDefinitionInstance> ItemDefinitions
/// </summary>
public IReadOnlyList<string> ImportPathsIncludingDuplicates { get; private set; }

/// <summary>
/// Gets the structured import edges discovered during evaluation, representing the graph
/// of <c>&lt;Import&gt;</c> relationships between project files.
/// </summary>
/// <remarks>
/// Each edge connects an importing file to the file it imported. The root project's own
/// entry is excluded; only real import relationships are represented.
/// May be <see langword="null"/> on out-of-process nodes if the project did not opt in
/// to serializing import edges via the <c>MSBuildProvideImportGraph</c> property.
/// </remarks>
internal IReadOnlyList<ProjectImportEdge> ImportEdges { get; private set; }

/// <summary>
/// DefaultTargets specified in the project, or
/// the logically first target if no DefaultTargets is
Expand Down Expand Up @@ -2536,8 +2549,10 @@ private void TranslateAllState(ITranslator translator)
ProjectItemDefinitionInstance.FactoryForDeserialization,
capacity => new RetrievableEntryHashSet<ProjectItemDefinitionInstance>(capacity, MSBuildNameIgnoreCaseComparer.Default));

// ignore _importPaths/ImportPaths. Only used by public API users, not nodes
// ignore _importPathsIncludingDuplicates/ImportPathsIncludingDuplicates. Only used by public API users, not nodes
// ImportPaths/ImportPathsIncludingDuplicates are not serialized — only used by public API consumers.
// ImportEdges are conditionally serialized when MSBuildProvideImportGraph is set (see TranslateImportEdges).

TranslateImportEdges(translator);
}

private void TranslateToolsetSpecificState(ITranslator translator)
Expand All @@ -2549,6 +2564,69 @@ private void TranslateToolsetSpecificState(ITranslator translator)
translator.Translate(ref _subToolsetVersion);
}

/// <summary>
/// Conditionally serializes import edge data across nodes.
/// The data is only written when the project has opted in via the
/// <c>MSBuildProvideImportGraph</c> property.
/// </summary>
private void TranslateImportEdges(ITranslator translator)
{
bool hasImportEdges = false;

if (translator.Mode == TranslationDirection.WriteToStream)
{
hasImportEdges = ImportEdges is { Count: > 0 }
&& string.Equals(
_properties?.GetProperty(Constants.MSBuildProvideImportGraphPropertyName)?.EvaluatedValue,
"true",
StringComparison.OrdinalIgnoreCase);
}
Comment on lines +2574 to +2583

// Bidirectional: on write this persists the flag, on read it recovers it.
translator.Translate(ref hasImportEdges);

if (!hasImportEdges)
{
return;
}

if (translator.Mode == TranslationDirection.WriteToStream)
{
int count = ImportEdges.Count;
translator.Translate(ref count);

for (int i = 0; i < count; i++)
{
ProjectImportEdge edge = ImportEdges[i];
string importedPath = edge.ImportedProjectPath;
string importingPath = edge.ImportingProjectPath;
string sdkName = edge.SdkName;
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
Comment on lines +2601 to +2606
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Nullable safety gap in deserialization

string importedPath = null;
string importingPath = null;
string sdkName = null;
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));

importedPath is initialized to null and passed to Translate(ref string). Per BinaryTranslator.TranslateNullable: if the stream marker is false, the method returns early and importedPath remains null. This null is then passed to new ProjectImportEdge(importedPath, ...) where ImportedProjectPath is declared as non-nullable string.

While the writing path guarantees non-null values (they come from FullPath which is validated), corrupted or forward-incompatible streams could trigger this path.

Consider adding a defensive null check:

translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
if (importedPath is not null)
{
    edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));
}

Or at minimum suppress the nullable warning with importedPath! to document the invariant explicitly.

}
}
else
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[MODERATE] Correctness — Null importedPath silently violates non-nullable contract

During deserialization the code initialises:

string importedPath = null;
translator.Translate(ref importedPath);
// ...
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));

ITranslator.Translate(ref string) wraps the value in a nullable-marker protocol: on the read side, TranslateNullable reads one boolean; if false, it returns early and leaves the local unchanged (still null). So if the stream ever marks the path as null (corrupted packet, version skew, or crafted stream), importedPath stays null and gets silently passed to ProjectImportEdge whose first positional parameter is string ImportedProjectPath — non-nullable by declaration. This will produce an ImportEdges list whose entries have ImportedProjectPath == null, causing NREs in any caller that unconditionally dereferences it.

In practice the writer never emits a null ImportedProjectPath because resolvedImport.ImportedProject.FullPath is always populated. But the deserializer has no local guard.

Recommendation:

translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);

// Guard against a corrupted or version-mismatched stream.
if (importedPath is null)
{
    continue; // or throw InvalidProjectFileException
}

edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));

int count = 0;
translator.Translate(ref count);

var edges = new List<ProjectImportEdge>(count);
for (int i = 0; i < count; i++)
{
string importedPath = null;
string importingPath = null;
string sdkName = null;
translator.Translate(ref importedPath);
translator.Translate(ref importingPath);
translator.Translate(ref sdkName);
edges.Add(new ProjectImportEdge(importedPath, importingPath, sdkName));
}

ImportEdges = edges.AsReadOnly();
}
}

private void TranslateProperties(ITranslator translator)
{
translator.TranslateDictionary(ref _environmentVariableProperties, ProjectPropertyInstance.FactoryForDeserialization);
Expand Down Expand Up @@ -3352,17 +3430,26 @@ private void InitializeTargetsData(List<string> defaultTargets,
private void CreateImportsSnapshot(IList<ResolvedImport> importClosure, IList<ResolvedImport> importClosureWithDuplicates)
{
var importPaths = new List<string>(Math.Max(0, importClosure.Count - 1) /* outer project */);
var importEdges = new List<ProjectImportEdge>(Math.Max(0, importClosure.Count - 1));

foreach (var resolvedImport in importClosure)
{
// Exclude outer project itself
if (resolvedImport.ImportingElement != null)
{
importPaths.Add(resolvedImport.ImportedProject.FullPath);
string importedPath = resolvedImport.ImportedProject.FullPath;
importPaths.Add(importedPath);

importEdges.Add(new ProjectImportEdge(
importedPath,
resolvedImport.ImportingElement.ContainingProject.FullPath,
resolvedImport.SdkResult?.SdkReference.Name));
}
}

_importPaths = importPaths;
ImportPaths = importPaths.AsReadOnly();
ImportEdges = importEdges.AsReadOnly();

var importPathsIncludingDuplicates = new List<string>(Math.Max(0, importClosureWithDuplicates.Count - 1) /* outer project */);
foreach (var resolvedImport in importClosureWithDuplicates)
Expand Down
7 changes: 7 additions & 0 deletions src/Framework/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ internal static class Constants

internal const string MSBuildAllProjectsPropertyName = "MSBuildAllProjects";

/// <summary>
/// Name of the MSBuild property that opts in to serializing the import graph
/// to out-of-process build nodes, making it available to tasks via
/// <see cref="EngineServices.ImportEdges"/>.
/// </summary>
internal const string MSBuildProvideImportGraphPropertyName = "MSBuildProvideImportGraph";

internal const string TaskHostExplicitlyRequested = "TaskHostExplicitlyRequested";
}
}
Loading
Loading