From 24ba0054315e3a21e3509f68ef931ee1b7de3dfd Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Fri, 15 May 2026 05:22:30 +0300 Subject: [PATCH 1/2] Remove `FEATURE_LEGACY_GETFULLPATH` and use `Microsoft.IO.Path.GetFullPath` in .NET Framework. The use of the new `Path.GetFullPath` in .NET Framework This is expected to improve performance by forgoing path validation, and improve consistency with modern .NET. --- .../Evaluation/Expander_Tests.cs | 63 ----------------- src/Directory.BeforeCommon.targets | 2 - .../FileUtilities_Tests.cs | 4 -- src/Framework/FileUtilities.cs | 70 ++----------------- src/Framework/NativeMethods.cs | 26 ------- 5 files changed, 4 insertions(+), 161 deletions(-) diff --git a/src/Build.UnitTests/Evaluation/Expander_Tests.cs b/src/Build.UnitTests/Evaluation/Expander_Tests.cs index cbd2af5c2c4..c70724faa68 100644 --- a/src/Build.UnitTests/Evaluation/Expander_Tests.cs +++ b/src/Build.UnitTests/Evaluation/Expander_Tests.cs @@ -922,27 +922,6 @@ public void ItemIncludeContainsMultipleItemReferences() logger.AssertLogContains("Item CleanFiles=foo.obj;bar.obj"); } -#if FEATURE_LEGACY_GETFULLPATH - /// - /// Bad path when getting metadata through ->Metadata function - /// - [LongPathSupportDisabledFact] - public void InvalidPathAndMetadataItemFunctionPathTooLong() - { - MockLogger logger = Helpers.BuildProjectWithNewOMExpectFailure(@" - - - - - - Metadata('FullPath'))"" /> - - ", false); - - logger.AssertLogContains("MSB4023"); - } -#endif - /// /// Bad path with illegal windows chars when getting metadata through ->Metadata function /// @@ -981,27 +960,6 @@ public void InvalidMetadataName() logger.AssertLogContains("MSB4023"); } -#if FEATURE_LEGACY_GETFULLPATH - /// - /// Bad path when getting metadata through ->WithMetadataValue function - /// - [LongPathSupportDisabledFact] - public void InvalidPathAndMetadataItemFunctionPathTooLong2() - { - MockLogger logger = Helpers.BuildProjectWithNewOMExpectFailure(@" - - - - - - WithMetadataValue('FullPath', 'x'))"" /> - - ", false); - - logger.AssertLogContains("MSB4023"); - } -#endif - /// /// Bad path with illegal windows chars when getting metadata through ->WithMetadataValue function /// @@ -1040,27 +998,6 @@ public void InvalidMetadataName2() logger.AssertLogContains("MSB4023"); } -#if FEATURE_LEGACY_GETFULLPATH - /// - /// Bad path when getting metadata through ->AnyHaveMetadataValue function - /// - [LongPathSupportDisabledFact] - public void InvalidPathAndMetadataItemFunctionPathTooLong3() - { - MockLogger logger = Helpers.BuildProjectWithNewOMExpectFailure(@" - - - - - - AnyHaveMetadataValue('FullPath', 'x'))"" /> - - ", false); - - logger.AssertLogContains("MSB4023"); - } -#endif - /// /// Bad path with illegal windows chars when getting metadata through ->AnyHaveMetadataValue function /// diff --git a/src/Directory.BeforeCommon.targets b/src/Directory.BeforeCommon.targets index 133fde5ff07..1c79f42e05c 100644 --- a/src/Directory.BeforeCommon.targets +++ b/src/Directory.BeforeCommon.targets @@ -36,8 +36,6 @@ $(DefineConstants);FEATURE_INSTALLED_MSBUILD $(DefineConstants);FEATURE_LEGACY_GETCURRENTDIRECTORY - - $(DefineConstants);FEATURE_LEGACY_GETFULLPATH $(DefineConstants);FEATURE_NAMED_PIPE_SECURITY_CONSTRUCTOR $(DefineConstants);FEATURE_PERFORMANCE_COUNTERS $(DefineConstants);FEATURE_PIPE_SECURITY diff --git a/src/Framework.UnitTests/FileUtilities_Tests.cs b/src/Framework.UnitTests/FileUtilities_Tests.cs index 4160ab4551a..2b3d43d9a08 100644 --- a/src/Framework.UnitTests/FileUtilities_Tests.cs +++ b/src/Framework.UnitTests/FileUtilities_Tests.cs @@ -518,11 +518,7 @@ public void CannotNormalizePathWithNewLineAndSpace() { string filePath = "\r\n C:\\work\\sdk3\\artifacts\\tmp\\Debug\\SimpleNamesWi---6143883E\\NETFrameworkLibrary\\bin\\Debug\\net462\\NETFrameworkLibrary.dll\r\n "; -#if FEATURE_LEGACY_GETFULLPATH - Assert.Throws(() => FileUtilities.NormalizePath(filePath)); -#else Assert.NotEqual("C:\\work\\sdk3\\artifacts\\tmp\\Debug\\SimpleNamesWi---6143883E\\NETFrameworkLibrary\\bin\\Debug\\net462\\NETFrameworkLibrary.dll", FileUtilities.NormalizePath(filePath)); -#endif } [Fact] diff --git a/src/Framework/FileUtilities.cs b/src/Framework/FileUtilities.cs index f44c6b515dd..e86a66c6983 100644 --- a/src/Framework/FileUtilities.cs +++ b/src/Framework/FileUtilities.cs @@ -666,7 +666,7 @@ private static bool IsValidRelativePathBound(char? c) internal static string NormalizePath(string path) { ArgumentException.ThrowIfNullOrEmpty(path); - string fullPath = GetFullPath(path); + string fullPath = NewPath.GetFullPath(path); return FixFilePath(fullPath); } @@ -680,68 +680,6 @@ internal static string NormalizePath(params string[] paths) return NormalizePath(Path.Combine(paths)); } - private static string GetFullPath(string path) - { -#if FEATURE_LEGACY_GETFULLPATH - if (NativeMethods.IsWindows) - { - string uncheckedFullPath = NativeMethods.GetFullPath(path); - - if (IsPathTooLong(uncheckedFullPath)) - { - throw new PathTooLongException(SR.FormatPathTooLong(path, NativeMethods.MaxPath)); - } - - // We really don't care about extensions here, but Path.HasExtension provides a great way to - // invoke the CLR's invalid path checks (these are independent of path length) - Path.HasExtension(uncheckedFullPath); - - // If we detect we are a UNC path then we need to use the regular get full path in order to do the correct checks for UNC formatting - // and security checks for strings like \\?\GlobalRoot - return IsUNCPath(uncheckedFullPath) ? Path.GetFullPath(uncheckedFullPath) : uncheckedFullPath; - } -#endif - - return Path.GetFullPath(path); - } - -#if FEATURE_LEGACY_GETFULLPATH - private static bool IsUNCPath(string path) - { - if (!NativeMethods.IsWindows || !path.StartsWith(@"\\", StringComparison.Ordinal)) - { - return false; - } - bool isUNC = true; - for (int i = 2; i < path.Length - 1; i++) - { - if (path[i] == '\\') - { - isUNC = false; - break; - } - } - - /* - From Path.cs in the CLR - - Throw an ArgumentException for paths like \\, \\server, \\server\ - This check can only be properly done after normalizing, so - \\foo\.. will be properly rejected. Also, reject \\?\GLOBALROOT\ - (an internal kernel path) because it provides aliases for drives. - - throw new ArgumentException(Environment.GetResourceString("Arg_PathIllegalUNC")); - - // Check for \\?\Globalroot, an internal mechanism to the kernel - // that provides aliases for drives and other undocumented stuff. - // The kernel team won't even describe the full set of what - // is available here - we don't want managed apps mucking - // with this for security reasons. - */ - return isUNC || path.IndexOf(@"\\?\globalroot", StringComparison.OrdinalIgnoreCase) != -1; - } -#endif // FEATURE_LEGACY_GETFULLPATH - /// /// Normalizes all path separators (both forward and back slashes) to forward slashes. /// This is platform-independent, unlike FrameworkFileUtilities.FixFilePath which only normalizes on non-Windows platforms. @@ -1379,8 +1317,8 @@ internal static string MakeRelative(string basePath, string path) ArgumentNullException.ThrowIfNull(basePath); ArgumentException.ThrowIfNullOrEmpty(path); - string fullBase = GetFullPath(basePath); - string fullPath = GetFullPath(path); + string fullBase = NewPath.GetFullPath(basePath); + string fullPath = NewPath.GetFullPath(path); string[] splitBase = fullBase.Split(MSBuildConstants.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries); string[] splitPath = fullPath.Split(MSBuildConstants.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries); @@ -1678,7 +1616,7 @@ internal static string GetDirectoryNameOfFileAbove(string startingDirectory, str fileSystem ??= DefaultFileSystem; // Canonicalize our starting location - string? lookInDirectory = GetFullPath(startingDirectory); + string? lookInDirectory = NewPath.GetFullPath(startingDirectory); do { diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 915cf0673e4..9aaaa58acbb 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -1217,32 +1217,6 @@ internal static bool SetCurrentDirectory(string path) return true; } -#if FEATURE_WINDOWSINTEROP - [SupportedOSPlatform("windows6.1")] - internal static unsafe string GetFullPath(string path) - { - using BufferScope buffer = new(stackalloc char[(int)PInvoke.MAX_PATH]); - int fullPathLength = (int)PInvoke.GetFullPathName(path, buffer, out _); - - // If user is using long paths we could need to allocate a larger buffer - if (fullPathLength > buffer.Length) - { - buffer.EnsureCapacity(fullPathLength); - fullPathLength = (int)PInvoke.GetFullPathName(path, buffer, out _); - } - - if (fullPathLength == 0) - { - HRESULT.FromLastError().ThrowOnFailure(); - } - - // Avoid creating new strings unnecessarily - ReadOnlySpan result = buffer.AsSpan().Slice(0, fullPathLength); - return result.SequenceEqual(path.AsSpan()) ? path : result.ToString(); - } - -#endif - internal static (bool acceptAnsiColorCodes, bool outputIsScreen, uint? originalConsoleMode) QueryIsScreenAndTryEnableAnsiColorCodes(bool useStandardError = false) { if (Console.IsOutputRedirected) From 505d24e0e15ecefb60b5679e3dabce3b84afb2e7 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Sat, 16 May 2026 01:30:38 +0300 Subject: [PATCH 2/2] Update tests to account for new behavior. --- .../ImportFromMSBuildExtensionsPath_Tests.cs | 4 +- .../FileUtilities_Tests.cs | 38 ---------- .../TrackedDependenciesTests.cs | 76 ------------------- 3 files changed, 1 insertion(+), 117 deletions(-) diff --git a/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs b/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs index 421611fef76..6f3b5d951b6 100644 --- a/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs +++ b/src/Build.UnitTests/Evaluation/ImportFromMSBuildExtensionsPath_Tests.cs @@ -876,9 +876,7 @@ public void FallbackImportWithInvalidProjectValue(string projectValue) } else { -#if NETFRAMEWORK - logger.AssertLogContains("MSB4102"); -#endif + logger.AssertLogDoesntContain("MSB4102"); } } diff --git a/src/Framework.UnitTests/FileUtilities_Tests.cs b/src/Framework.UnitTests/FileUtilities_Tests.cs index 2b3d43d9a08..e694db0e384 100644 --- a/src/Framework.UnitTests/FileUtilities_Tests.cs +++ b/src/Framework.UnitTests/FileUtilities_Tests.cs @@ -462,33 +462,6 @@ public void NormalizePathEmpty() }); } - [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486.")] - public void NormalizePathBadUNC1() - { - Assert.Throws(() => - { - Assert.Null(FileUtilities.NormalizePath(@"\\")); - }); - } - - [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486.")] - public void NormalizePathBadUNC2() - { - Assert.Throws(() => - { - Assert.Null(FileUtilities.NormalizePath(@"\\XXX\")); - }); - } - - [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486.")] - public void NormalizePathBadUNC3() - { - Assert.Throws(() => - { - Assert.Equal(@"\\localhost", FileUtilities.NormalizePath(@"\\localhost")); - }); - } - [WindowsOnlyFact] public void NormalizePathGoodUNC() { @@ -502,17 +475,6 @@ public void NormalizePathTooLongWithDots() Assert.Equal(@"c:\abc\def", FileUtilities.NormalizePath(@"c:\abc\" + longPart + @"\..\def")); } - [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486.")] - public void NormalizePathInvalid() - { - string filePath = @"c:\aardvark\|||"; - - Assert.Throws(() => - { - FileUtilities.NormalizePath(filePath); - }); - } - [WindowsOnlyFact] public void CannotNormalizePathWithNewLineAndSpace() { diff --git a/src/Utilities.UnitTests/TrackedDependencies/TrackedDependenciesTests.cs b/src/Utilities.UnitTests/TrackedDependencies/TrackedDependenciesTests.cs index 0ba128f9f33..d15e4baf1ab 100644 --- a/src/Utilities.UnitTests/TrackedDependencies/TrackedDependenciesTests.cs +++ b/src/Utilities.UnitTests/TrackedDependencies/TrackedDependenciesTests.cs @@ -260,18 +260,6 @@ public void FormatNormalizedRootingMarkerTests() { Assert.Equal(test.Value, DependencyTableCache.FormatNormalizedTlogRootingMarker(test.Key)); // "Incorrectly formatted rooting marker" } - - bool exceptionCaught = false; - try - { - DependencyTableCache.FormatNormalizedTlogRootingMarker(new ITaskItem[] { new TaskItem("\\\\") }); - } - catch (ArgumentException) - { - exceptionCaught = true; - } - - Assert.True(exceptionCaught); // "Should have failed to format a rooting marker from a malformed UNC path" } [Fact] @@ -378,34 +366,6 @@ public void EmptyTLog() Assert.Equal(outofdate[0].ItemSpec, Path.Combine("TestFiles", "one.cpp")); } - [Fact] - public void InvalidReadTLogName() - { - Console.WriteLine("Test: InvalidReadTLogName"); - - // Prepare files - DependencyTestHelper.WriteAll("TestFiles\\one.h", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.cpp", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.obj", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.tlog", ""); - - MockTask task = DependencyTestHelper.MockTask; - - CanonicalTrackedInputFiles d = new CanonicalTrackedInputFiles( - task, - DependencyTestHelper.ItemArray(new TaskItem("TestFiles\\|one|.tlog")), - DependencyTestHelper.ItemArray(new TaskItem("TestFiles\\one.cpp")), - null, - DependencyTestHelper.ItemArray(new TaskItem("TestFiles\\one.obj")), - false, /* no minimal rebuild optimization */ - false); /* shred composite rooting markers */ - - d.ComputeSourcesNeedingCompilation(); - - Assert.Equal(1, ((MockEngine)task.BuildEngine).Warnings); // "Should have an error." - Assert.Empty(d.DependencyTable); // "DependencyTable should be empty." - } - [Fact] public void ReadTLogWithInitialEmptyLine() { @@ -537,21 +497,6 @@ public void ReadTLogWithDuplicateInRoot() Assert.NotEmpty(d.DependencyTable); // "Dependency Table should not be empty." } - [Fact] - public void InvalidWriteTLogName() - { - Console.WriteLine("Test: InvalidWriteTLogName"); - - MockTask task = DependencyTestHelper.MockTask; - - CanonicalTrackedOutputFiles d = new CanonicalTrackedOutputFiles( - task, - DependencyTestHelper.ItemArray(new TaskItem("TestFiles\\|one|.write.tlog"))); - - Assert.Equal(1, ((MockEngine)task.BuildEngine).Warnings); // "Should have an error." - Assert.Empty(d.DependencyTable); // "DependencyTable should be empty." - } - [Fact] public void WriteTLogWithInitialEmptyLine() { @@ -2970,27 +2915,6 @@ public void SaveCompactedReadTlog_MaintainCompositeRootingMarkers() Assert.True(d2.DependencyTable[Path.GetFullPath(Path.Combine("TestFiles", "three.cpp")) + "|" + Path.GetFullPath(Path.Combine("TestFiles", "two.cpp"))].Values.Count == 4); } - [Fact] - public void InvalidFlatTrackingTLogName() - { - Console.WriteLine("Test: InvalidFlatTrackingTLogName"); - - // Prepare files - DependencyTestHelper.WriteAll("TestFiles\\one.h", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.cpp", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.obj", ""); - DependencyTestHelper.WriteAll("TestFiles\\one.tlog", ""); - - MockTask task = DependencyTestHelper.MockTask; - FlatTrackingData data = new FlatTrackingData( - task, - DependencyTestHelper.ItemArray(new TaskItem("TestFiles\\|one|.write.tlog")), - false); /* don't skip missing files */ - - Assert.Equal(1, ((MockEngine)task.BuildEngine).Warnings); // "Should have a warning." - Assert.Empty(data.DependencyTable); // "DependencyTable should be empty." - } - [Fact] public void FlatTrackingTLogWithInitialEmptyLine() {