Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unsupported paths in ProjectInSolution.AbsolutePath #6238

Merged
merged 3 commits into from
Mar 12, 2021
Merged

Handle unsupported paths in ProjectInSolution.AbsolutePath #6238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97a9a16
Handle unsupported paths in ProjectInSolution.AbsolutePath
jeffkl Mar 10, 2021
0fadc38
Call Uri.TryCreate() to see if the relative path is a URI before tryi…
jeffkl Mar 10, 2021
c2b0410
Reduce complexity of the solution file used in the unit test
jeffkl Mar 10, 2021
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
61 changes: 61 additions & 0 deletions src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2477,6 +2477,67 @@ public void DirectorySolutionPropsTest(string projectName, bool enable)
}
}

/// <summary>
/// Regression test for https://github.com/dotnet/msbuild/issues/6236
/// </summary>
[Theory]
[InlineData("http://localhost:8080")]
[InlineData("a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-a-really-long-string-")]
public void AbsolutePathWorksForUnsupportedPaths(string relativePath)
{
string solutionFileContents =
$@"
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
VisualStudioVersion = 16.0.31025.194
MinimumVisualStudioVersion = 10.0.40219.1
Project(""{{E24C65DC-7377-472B-9ABA-BC803B73C61A}}"") = ""WebSite1"", ""{relativePath}"", ""{{{{96E0707C-2E9C-4704-946F-FA583147737F}}}}""
ProjectSection(WebsiteProperties) = preProject
UseIISExpress = ""true""
TargetFrameworkMoniker = "".NETFramework,Version%3Dv4.0""
Debug.AspNetCompiler.VirtualPath = ""/localhost_8080""
Debug.AspNetCompiler.PhysicalPath = ""C:\Users\UserA\Documents\My Web Sites\WebSite1\""
Debug.AspNetCompiler.TargetPath = ""PrecompiledWeb\localhost_8080\""
Debug.AspNetCompiler.Updateable = ""true""
Debug.AspNetCompiler.ForceOverwrite = ""true""
Debug.AspNetCompiler.FixedNames = ""false""
Debug.AspNetCompiler.Debug = ""True""
Release.AspNetCompiler.VirtualPath = ""/localhost_8080""
Release.AspNetCompiler.PhysicalPath = ""C:\Users\UserA\Documents\My Web Sites\WebSite1\""
Release.AspNetCompiler.TargetPath = ""PrecompiledWeb\localhost_8080\""
Release.AspNetCompiler.Updateable = ""true""
Release.AspNetCompiler.ForceOverwrite = ""true""
Release.AspNetCompiler.FixedNames = ""false""
Release.AspNetCompiler.Debug = ""False""
SlnRelativePath = ""C:\Users\UserA\Documents\My Web Sites\WebSite1\""
EndProjectSection
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Release|Any CPU = Release|Any CPU
jeffkl marked this conversation as resolved.
Show resolved Hide resolved
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{{{{96E0707C-2E9C-4704-946F-FA583147737F}}}}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{{{{96E0707C-2E9C-4704-946F-FA583147737F}}}}.Debug|Any CPU.Build.0 = Debug|Any CPU
{{{{96E0707C-2E9C-4704-946F-FA583147737F}}}}.Release|Any CPU.ActiveCfg = Debug|Any CPU
{{{{96E0707C-2E9C-4704-946F-FA583147737F}}}}.Release|Any CPU.Build.0 = Debug|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {{{{87CFA95B-DF37-4C4B-B629-44656ED585AB}}}}
EndGlobalSection
EndGlobal";

SolutionFile solution = SolutionFile_Tests.ParseSolutionHelper(solutionFileContents);

ProjectInSolution projectInSolution = solution.ProjectsInOrder.ShouldHaveSingleItem();

projectInSolution.AbsolutePath.ShouldBe(Path.Combine(solution.SolutionFileDirectory, projectInSolution.RelativePath));
jeffkl marked this conversation as resolved.
Show resolved Hide resolved
}

#region Helper Functions

/// <summary>
Expand Down
24 changes: 17 additions & 7 deletions src/Build/Construction/Solution/ProjectInSolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,21 @@ public string AbsolutePath
{
if (_absolutePath == null)
{
_absolutePath = Path.Combine(ParentSolution.SolutionFileDirectory, _relativePath);
jeffkl marked this conversation as resolved.
Show resolved Hide resolved

try
{
#if NETFRAMEWORK && !MONO
_absolutePath = Path.GetFullPath(Path.Combine(ParentSolution.SolutionFileDirectory, _relativePath));
_absolutePath = Path.GetFullPath(_absolutePath);
#else
_absolutePath = FileUtilities.NormalizePath(Path.Combine(ParentSolution.SolutionFileDirectory, _relativePath));
_absolutePath = FileUtilities.NormalizePath(_absolutePath);
#endif
}
catch (Exception)
{
// The call to GetFullPath can throw if the relative path is a URL or the paths are too long for the current file system
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a simpler path normalization here like (look for \..\) -> remove along with the prior path segment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd hate to get the logic wrong, I think the new code is pretty good to handle the situation more correctly.

// This falls back to previous behavior of returning a path that may not be correct but at least returns some value
}
}

return _absolutePath;
Expand Down Expand Up @@ -229,9 +239,9 @@ public IReadOnlyDictionary<string, ProjectConfigurationInSolution> ProjectConfig

internal string TargetFrameworkMoniker { get; set; }

#endregion
#endregion

#region Methods
#region Methods

private bool _checkedIfCanBeMSBuildProjectFile;
private bool _canBeMSBuildProjectFile;
Expand Down Expand Up @@ -529,13 +539,13 @@ private static bool ElementContainsInvalidNamespaceDefitions(XmlElement mainProj
return false;
}

#endregion
#endregion

#region Constants
#region Constants

internal const int DependencyLevelUnknown = -1;
internal const int DependencyLevelBeingDetermined = -2;

#endregion
#endregion
}
}