Skip to content

Commit c3ccdfa

Browse files
committed
C#: Guard against cyclic inclusions in project files
1 parent e4f68ae commit c3ccdfa

File tree

3 files changed

+52
-17
lines changed

3 files changed

+52
-17
lines changed

csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,5 +1020,27 @@ public void TestDirsProjLinux()
10201020
var autobuilder = CreateAutoBuilder("csharp", false);
10211021
TestAutobuilderScript(autobuilder, 0, 4);
10221022
}
1023+
1024+
[Fact]
1025+
public void TestCyclicDirsProj()
1026+
{
1027+
Actions.FileExists["dirs.proj"] = true;
1028+
Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null;
1029+
Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null;
1030+
Actions.FileExists["csharp.log"] = false;
1031+
Actions.EnumerateFiles[@"C:\Project"] = "dirs.proj";
1032+
Actions.EnumerateDirectories[@"C:\Project"] = "";
1033+
1034+
var dirsproj1 = new XmlDocument();
1035+
dirsproj1.LoadXml(@"<Project DefaultTargets=""Build"" xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"" ToolsVersion=""3.5"">
1036+
<ItemGroup>
1037+
<ProjectFiles Include=""dirs.proj"" />
1038+
</ItemGroup>
1039+
</Project>");
1040+
Actions.LoadXml["dirs.proj"] = dirsproj1;
1041+
1042+
var autobuilder = CreateAutoBuilder("csharp", false);
1043+
TestAutobuilderScript(autobuilder, 1, 0);
1044+
}
10231045
}
10241046
}

csharp/autobuilder/Semmle.Autobuild/Project.cs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ public class Project : ProjectOrSolution
2323

2424
public Version ToolsVersion { get; private set; }
2525

26-
readonly List<Project> includedProjects = new List<Project>();
27-
public override IEnumerable<IProjectOrSolution> IncludedProjects =>
28-
includedProjects.Concat(includedProjects.SelectMany(s => s.IncludedProjects));
26+
readonly Lazy<List<Project>> includedProjectsLazy;
27+
public override IEnumerable<IProjectOrSolution> IncludedProjects => includedProjectsLazy.Value;
2928

3029
public Project(Autobuilder builder, string path) : base(builder, path)
3130
{
3231
ToolsVersion = new Version();
32+
includedProjectsLazy = new Lazy<List<Project>>(() => new List<Project>());
3333

3434
if (!builder.Actions.FileExists(FullPath))
3535
return;
@@ -69,17 +69,22 @@ public Project(Autobuilder builder, string path) : base(builder, path)
6969
}
7070
}
7171

72-
// The documentation on `.proj` files is very limited, but it appears that both
73-
// `<ProjectFile Include="X"/>` and `<ProjectFiles Include="X"/>` is valid
74-
var mgr = new XmlNamespaceManager(projFile.NameTable);
75-
mgr.AddNamespace("msbuild", "http://schemas.microsoft.com/developer/msbuild/2003");
76-
var projectFileIncludes = root.SelectNodes("//msbuild:Project/msbuild:ItemGroup/msbuild:ProjectFile/@Include", mgr).OfType<XmlNode>();
77-
var projectFilesIncludes = root.SelectNodes("//msbuild:Project/msbuild:ItemGroup/msbuild:ProjectFiles/@Include", mgr).OfType<XmlNode>();
78-
foreach (var include in projectFileIncludes.Concat(projectFilesIncludes))
72+
includedProjectsLazy = new Lazy<List<Project>>(() =>
7973
{
80-
var includePath = builder.Actions.IsWindows() ? include.Value : include.Value.Replace("\\", "/");
81-
includedProjects.Add(new Project(builder, builder.Actions.PathCombine(Path.GetDirectoryName(this.FullPath), includePath)));
82-
}
74+
var ret = new List<Project>();
75+
// The documentation on `.proj` files is very limited, but it appears that both
76+
// `<ProjectFile Include="X"/>` and `<ProjectFiles Include="X"/>` is valid
77+
var mgr = new XmlNamespaceManager(projFile.NameTable);
78+
mgr.AddNamespace("msbuild", "http://schemas.microsoft.com/developer/msbuild/2003");
79+
var projectFileIncludes = root.SelectNodes("//msbuild:Project/msbuild:ItemGroup/msbuild:ProjectFile/@Include", mgr).OfType<XmlNode>();
80+
var projectFilesIncludes = root.SelectNodes("//msbuild:Project/msbuild:ItemGroup/msbuild:ProjectFiles/@Include", mgr).OfType<XmlNode>();
81+
foreach (var include in projectFileIncludes.Concat(projectFilesIncludes))
82+
{
83+
var includePath = builder.Actions.IsWindows() ? include.Value : include.Value.Replace("\\", "/");
84+
ret.Add(new Project(builder, builder.Actions.PathCombine(Path.GetDirectoryName(this.FullPath), includePath)));
85+
}
86+
return ret;
87+
});
8388
}
8489
}
8590
}

csharp/autobuilder/Semmle.Autobuild/ProjectOrSolution.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public interface IProjectOrSolution
1515
string FullPath { get; }
1616

1717
/// <summary>
18-
/// Gets a list of other projects included by this file.
18+
/// Gets a list of other projects directly included by this file.
1919
/// </summary>
2020
IEnumerable<IProjectOrSolution> IncludedProjects { get; }
2121
}
@@ -39,8 +39,16 @@ public static class IProjectOrSolutionExtensions
3939
/// <summary>
4040
/// Holds if this file includes a project with code from language <paramref name="l"/>.
4141
/// </summary>
42-
public static bool HasLanguage(this IProjectOrSolution p, Language l) =>
43-
l.ProjectFileHasThisLanguage(p.FullPath) ||
44-
p.IncludedProjects.Any(p0 => l.ProjectFileHasThisLanguage(p0.FullPath));
42+
public static bool HasLanguage(this IProjectOrSolution p, Language l)
43+
{
44+
bool HasLanguage(IProjectOrSolution p0, HashSet<string> seen)
45+
{
46+
if (seen.Contains(p0.FullPath))
47+
return false;
48+
seen.Add(p0.FullPath); // guard against cyclic includes
49+
return l.ProjectFileHasThisLanguage(p0.FullPath) || p0.IncludedProjects.Any(p1 => HasLanguage(p1, seen));
50+
}
51+
return HasLanguage(p, new HashSet<string>());
52+
}
4553
}
4654
}

0 commit comments

Comments
 (0)