diff --git a/src/Framework.UnitTests/AbsolutePath_Tests.cs b/src/Framework.UnitTests/AbsolutePath_Tests.cs index 09a87b60b09..70ad9d2732d 100644 --- a/src/Framework.UnitTests/AbsolutePath_Tests.cs +++ b/src/Framework.UnitTests/AbsolutePath_Tests.cs @@ -308,5 +308,23 @@ public void AbsolutePath_NotRooted_ShouldThrowWithLocalizedMessage() var exception = Should.Throw(() => new AbsolutePath("relative/path")); exception.Message.ShouldContain("Path must be rooted"); } + + [WindowsOnlyFact] + public void AbsolutePath_WhitespacePath_ShouldThrowOnWindows() + { + var basePath = GetTestBasePath(); + + Should.Throw(() => new AbsolutePath(" ", basePath)); + } + + [UnixOnlyFact] + public void AbsolutePath_WhitespacePath_ShouldNotThrowOnUnix() + { + var basePath = GetTestBasePath(); + + // Whitespace is a valid filename character on Unix — should not throw. + var absolutePath = new AbsolutePath(" ", basePath); + absolutePath.Value.ShouldNotBeNullOrEmpty(); + } } } diff --git a/src/Framework/PathHelpers/AbsolutePath.cs b/src/Framework/PathHelpers/AbsolutePath.cs index 59d08dc102f..f2c741cb44f 100644 --- a/src/Framework/PathHelpers/AbsolutePath.cs +++ b/src/Framework/PathHelpers/AbsolutePath.cs @@ -106,6 +106,16 @@ public AbsolutePath(string path, AbsolutePath basePath) { ArgumentException.ThrowIfNullOrEmpty(path); + // Before MSBuild's migration from multi-process to multi-threaded, Path.GetFullPath(" ") threw + // ArgumentException on Windows because whitespace-only is not a legal path. After combining with + // a rooted base directory, Path.GetFullPath silently trims the trailing whitespace, masking the + // error. Preserve the historical Windows contract by explicitly rejecting whitespace-only input. + // Unix retains the historical accepting behavior because whitespace is a valid filename character there. + if (NativeMethods.IsWindows && string.IsNullOrWhiteSpace(path)) + { + throw new ArgumentException(SR.WhitespacePathNotAllowedOnWindows, nameof(path)); + } + // This function should not throw when path has illegal characters. // For .NET Framework, Microsoft.IO.Path.Combine should be used instead of System.IO.Path.Combine to achieve it. // For .NET Core, System.IO.Path.Combine already does not throw in this case. diff --git a/src/Framework/Resources/SR.resx b/src/Framework/Resources/SR.resx index ebcc88a74d5..a7c9a068529 100644 --- a/src/Framework/Resources/SR.resx +++ b/src/Framework/Resources/SR.resx @@ -153,6 +153,9 @@ Path must be rooted. + + Path cannot be null or whitespace on Windows. + Path: {0} exceeds the OS max path limit. The fully qualified file name must be less than {1} characters. diff --git a/src/Framework/Resources/xlf/SR.cs.xlf b/src/Framework/Resources/xlf/SR.cs.xlf index 5c8295c4621..11bd107ecbe 100644 --- a/src/Framework/Resources/xlf/SR.cs.xlf +++ b/src/Framework/Resources/xlf/SR.cs.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Verze {0} sady Visual Studio není podporovaná. Zadejte hodnotu z výčtu Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.de.xlf b/src/Framework/Resources/xlf/SR.de.xlf index 2e0c24d2abf..a6c2041d927 100644 --- a/src/Framework/Resources/xlf/SR.de.xlf +++ b/src/Framework/Resources/xlf/SR.de.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Visual Studio-Version "{0}" wird nicht unterstützt. Geben Sie einen Wert aus der Enumeration "Microsoft.Build.Utilities.VisualStudioVersion" an. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.es.xlf b/src/Framework/Resources/xlf/SR.es.xlf index 2f1056cd32c..dbf7ad15b3f 100644 --- a/src/Framework/Resources/xlf/SR.es.xlf +++ b/src/Framework/Resources/xlf/SR.es.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ La versión "{0}" de Visual Studio no es compatible. Especifique un valor de la enumeración Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.fr.xlf b/src/Framework/Resources/xlf/SR.fr.xlf index f28cf91886d..ab42514cc83 100644 --- a/src/Framework/Resources/xlf/SR.fr.xlf +++ b/src/Framework/Resources/xlf/SR.fr.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ La version "{0}" de Visual Studio n'est pas prise en charge. Spécifiez une valeur de l'énumération Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.it.xlf b/src/Framework/Resources/xlf/SR.it.xlf index 16498bce12b..57f2c802c8a 100644 --- a/src/Framework/Resources/xlf/SR.it.xlf +++ b/src/Framework/Resources/xlf/SR.it.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ La versione "{0}" di Visual Studio non è supportata. Specificare un valore dall'enumerazione Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.ja.xlf b/src/Framework/Resources/xlf/SR.ja.xlf index 668cf563a91..c2df347e8b4 100644 --- a/src/Framework/Resources/xlf/SR.ja.xlf +++ b/src/Framework/Resources/xlf/SR.ja.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Visual Studio のバージョン "{0}" はサポートされていません。列挙 Microsoft.Build.Utilities.VisualStudioVersion から値を指定してください。 + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.ko.xlf b/src/Framework/Resources/xlf/SR.ko.xlf index 91aa56c2e76..29dac1ad0a9 100644 --- a/src/Framework/Resources/xlf/SR.ko.xlf +++ b/src/Framework/Resources/xlf/SR.ko.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Visual Studio 버전 "{0}"이(가) 지원되지 않습니다. Microsoft.Build.Utilities.VisualStudioVersion 열거형에서 값을 지정하세요. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.pl.xlf b/src/Framework/Resources/xlf/SR.pl.xlf index b5e5451c0d4..22417a32907 100644 --- a/src/Framework/Resources/xlf/SR.pl.xlf +++ b/src/Framework/Resources/xlf/SR.pl.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Program Visual Studio w wersji „{0}” nie jest obsługiwany. Podaj wartość z wyliczenia Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.pt-BR.xlf b/src/Framework/Resources/xlf/SR.pt-BR.xlf index dac25278675..1b007819213 100644 --- a/src/Framework/Resources/xlf/SR.pt-BR.xlf +++ b/src/Framework/Resources/xlf/SR.pt-BR.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ O Visual Studio versão "{0}" não tem suporte. Especifique um valor da enumeração Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.ru.xlf b/src/Framework/Resources/xlf/SR.ru.xlf index 8e1be411dd9..250763098a9 100644 --- a/src/Framework/Resources/xlf/SR.ru.xlf +++ b/src/Framework/Resources/xlf/SR.ru.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Visual Studio версии "{0}" не поддерживается. Укажите значение из перечисления Microsoft.Build.Utilities.VisualStudioVersion. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.tr.xlf b/src/Framework/Resources/xlf/SR.tr.xlf index 5a2098ed474..48d73bd6c08 100644 --- a/src/Framework/Resources/xlf/SR.tr.xlf +++ b/src/Framework/Resources/xlf/SR.tr.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ Visual Studio "{0}" sürümü desteklenmiyor. Lütfen Microsoft.Build.Utilities.VisualStudioVersion sabit listesinden bir değer belirtin. + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.zh-Hans.xlf b/src/Framework/Resources/xlf/SR.zh-Hans.xlf index 4e83c50e0d7..2c1440bd40f 100644 --- a/src/Framework/Resources/xlf/SR.zh-Hans.xlf +++ b/src/Framework/Resources/xlf/SR.zh-Hans.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ 不支持 Visual Studio 版本“{0}”。请指定枚举 Microsoft.Build.Utilities.VisualStudioVersion 中的某个值。 + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Framework/Resources/xlf/SR.zh-Hant.xlf b/src/Framework/Resources/xlf/SR.zh-Hant.xlf index bcba22030a4..a7c0cdd7664 100644 --- a/src/Framework/Resources/xlf/SR.zh-Hant.xlf +++ b/src/Framework/Resources/xlf/SR.zh-Hant.xlf @@ -1,4 +1,4 @@ - + @@ -127,6 +127,11 @@ 不支援 Visual Studio 版本 "{0}"。請從列舉 Microsoft.Build.Utilities.VisualStudioVersion 中指定值。 + + Path cannot be null or whitespace on Windows. + Path cannot be null or whitespace on Windows. + + '{0}' contains no elements. '{0}' contains no elements. diff --git a/src/Tasks.UnitTests/FormatUrl_Tests.cs b/src/Tasks.UnitTests/FormatUrl_Tests.cs index 14d738e5e09..8bd42d90ecb 100644 --- a/src/Tasks.UnitTests/FormatUrl_Tests.cs +++ b/src/Tasks.UnitTests/FormatUrl_Tests.cs @@ -1,8 +1,9 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; using System.IO; +using Microsoft.Build.Framework; using Microsoft.Build.Tasks; using Shouldly; using Xunit; @@ -18,14 +19,18 @@ public FormatUrl_Tests(ITestOutputHelper testOutputHelper) _out = testOutputHelper; } + private FormatUrl GetFormatUrlUnderTest() => new FormatUrl + { + BuildEngine = new MockEngine(_out), + }; + /// /// The URL to format is null. /// [Fact] public void NullTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = null; t.Execute().ShouldBeTrue(); @@ -38,8 +43,7 @@ public void NullTest() [Fact] public void EmptyTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = string.Empty; t.Execute().ShouldBeTrue(); @@ -52,8 +56,7 @@ public void EmptyTest() [Fact] public void NoInputTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.Execute().ShouldBeTrue(); t.OutputUrl.ShouldBe(string.Empty); @@ -68,8 +71,7 @@ public void NoInputTest() [UnixOnlyFact] public void WhitespaceTestOnUnix() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = " "; t.Execute().ShouldBeTrue(); @@ -78,12 +80,14 @@ public void WhitespaceTestOnUnix() /// /// The URL to format is white space. + /// PathUtil.Resolve explicitly rejects whitespace-only paths to preserve the historical contract + /// from Path.GetFullPath(" "), which threw on Windows + /// before MSBuild's migration from multi-process to multi-threaded execution. /// [WindowsOnlyFact] public void WhitespaceTestOnWindows() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = " "; Should.Throw(() => t.Execute()); @@ -95,8 +99,7 @@ public void WhitespaceTestOnWindows() [Fact] public void UncPathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"\\server\filename.ext"; t.Execute().ShouldBeTrue(); @@ -110,8 +113,7 @@ public void UncPathTest() [Fact] public void LocalAbsolutePathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = Environment.CurrentDirectory; t.Execute().ShouldBeTrue(); @@ -125,8 +127,7 @@ public void LocalAbsolutePathTest() [Fact] public void LocalRelativePathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"."; t.Execute().ShouldBeTrue(); @@ -139,8 +140,7 @@ public void LocalRelativePathTest() [UnixOnlyFact] public void LocalUnixAbsolutePathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"/usr/local/share"; t.Execute().ShouldBeTrue(); @@ -153,8 +153,7 @@ public void LocalUnixAbsolutePathTest() [WindowsOnlyFact] public void LocalWindowsAbsolutePathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"c:\folder\filename.ext"; t.Execute().ShouldBeTrue(); @@ -167,8 +166,7 @@ public void LocalWindowsAbsolutePathTest() [Fact] public void UrlLocalHostTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"https://localhost/Example/Path"; t.Execute().ShouldBeTrue(); @@ -181,8 +179,7 @@ public void UrlLocalHostTest() [Fact] public void UrlTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"https://example.com/Example/Path"; t.Execute().ShouldBeTrue(); @@ -195,12 +192,49 @@ public void UrlTest() [Fact] public void UrlParentPathTest() { - var t = new FormatUrl(); - t.BuildEngine = new MockEngine(_out); + var t = GetFormatUrlUnderTest(); t.InputUrl = @"https://example.com/Example/../Path"; t.Execute().ShouldBeTrue(); t.OutputUrl.ShouldBe(@"https://example.com/Path"); } + + /// + /// A relative input URL is resolved against the task's , + /// not the process current working directory. This documents the intentional semantic change + /// introduced when migrating the task to multithreaded execution. + /// + /// + /// Concrete example. Suppose the process is launched from D:\src\microsoft\dotnet\msbuild + /// and the engine has loaded a project located in + /// C:\Users\you\AppData\Local\Temp\msbuild_test_abc123\. In multithreaded mode the engine + /// hands the task a whose ProjectDirectory is the project's + /// folder, *not* the process cwd. Feeding InputUrl = "." must therefore produce + /// file:///C:/Users/you/AppData/Local/Temp/msbuild_test_abc123/ — *not* + /// file:///D:/src/microsoft/dotnet/msbuild, which is what the multi-process implementation + /// (rooted in against the process cwd) would have + /// produced. The other tests in this file use the Fallback environment where ProjectDirectory == cwd, + /// so they cannot detect a regression to cwd-based resolution; this test specifically exists to catch that. + /// + [Fact] + public void RelativePathResolvesAgainstProjectDirectory() + { + using TestEnvironment env = TestEnvironment.Create(_out); + TransientTestFolder projectFolder = env.CreateFolder(createFolder: true); + + // Sanity check: project directory must differ from the process current directory + // for the assertion below to be meaningful. + projectFolder.Path.ShouldNotBe(Environment.CurrentDirectory); + + var t = new FormatUrl + { + TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectFolder.Path), + BuildEngine = new MockEngine(_out), + InputUrl = @".", + }; + + t.Execute().ShouldBeTrue(); + t.OutputUrl.ShouldBe(new Uri(projectFolder.Path).AbsoluteUri); + } } } diff --git a/src/Tasks/FormatUrl.cs b/src/Tasks/FormatUrl.cs index 8dc8abab226..5918dc06b3e 100644 --- a/src/Tasks/FormatUrl.cs +++ b/src/Tasks/FormatUrl.cs @@ -12,8 +12,14 @@ namespace Microsoft.Build.Tasks /// /// Formats a url by canonicalizing it (i.e. " " -> "%20") and transforming "localhost" to "machinename". /// - public sealed class FormatUrl : TaskExtension + [MSBuildMultiThreadableTask] + public sealed class FormatUrl : TaskExtension, IMultiThreadableTask { + /// + /// Gets or sets the task execution environment for thread-safe path resolution. + /// + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + public string InputUrl { get; set; } [Output] @@ -21,7 +27,7 @@ public sealed class FormatUrl : TaskExtension public override bool Execute() { - OutputUrl = InputUrl != null ? PathUtil.Format(InputUrl) : String.Empty; + OutputUrl = InputUrl != null ? PathUtil.Format(InputUrl, TaskEnvironment.ProjectDirectory) : String.Empty; return true; } } diff --git a/src/Tasks/ManifestUtil/PathUtil.cs b/src/Tasks/ManifestUtil/PathUtil.cs index 83184f8f618..151f527ec43 100644 --- a/src/Tasks/ManifestUtil/PathUtil.cs +++ b/src/Tasks/ManifestUtil/PathUtil.cs @@ -8,6 +8,7 @@ #else using System.Text; #endif +using Microsoft.Build.Framework; using Microsoft.Build.Shared; #nullable disable @@ -41,14 +42,14 @@ public static string[] GetPathSegments(string path) } // Resolves the path, and if path is a url also canonicalizes it. - public static string Format(string path) + public static string Format(string path, AbsolutePath baseDirectory) { if (String.IsNullOrEmpty(path)) { return path; } - string resolvedPath = Resolve(path); + string resolvedPath = Resolve(path, baseDirectory); Uri u = new Uri(resolvedPath); // // GB18030: Uri class does not correctly encode chars in the PUA range for implicit @@ -202,8 +203,8 @@ public static bool IsUrl(string path) } // If path is a url and starts with "localhost", resolves to machine name. - // If path is a relative path, resolves to a full path. - public static string Resolve(string path) + // If path is a relative path, resolves to a full path against . + public static string Resolve(string path, AbsolutePath baseDirectory) { if (String.IsNullOrEmpty(path)) { @@ -236,7 +237,10 @@ public static string Resolve(string path) } // if not unc or url then it must be a local disk path... - return Path.GetFullPath(path); // make sure it's a full path + // Use AbsolutePath to root the path against the project's base directory instead of the + // process current directory. GetCanonicalForm() preserves the canonicalization that + // Path.GetFullPath performed previously, including throwing on invalid path characters. + return new AbsolutePath(path, baseDirectory).GetCanonicalForm(); } private static bool IsAsciiString(string str) =>