diff --git a/src/Build.UnitTests/Evaluation/Expander_Tests.cs b/src/Build.UnitTests/Evaluation/Expander_Tests.cs index 4a9364feb78..12c48d0a554 100644 --- a/src/Build.UnitTests/Evaluation/Expander_Tests.cs +++ b/src/Build.UnitTests/Evaluation/Expander_Tests.cs @@ -923,7 +923,7 @@ public void ItemIncludeContainsMultipleItemReferences() logger.AssertLogContains("Item CleanFiles=foo.obj;bar.obj"); } -#if FEATURE_LEGACY_GETFULLPATH +#if NETFRAMEWORK /// /// Bad path when getting metadata through ->Metadata function /// @@ -942,6 +942,7 @@ public void InvalidPathAndMetadataItemFunctionPathTooLong() logger.AssertLogContains("MSB4023"); } + #endif /// @@ -982,7 +983,7 @@ public void InvalidMetadataName() logger.AssertLogContains("MSB4023"); } -#if FEATURE_LEGACY_GETFULLPATH +#if NETFRAMEWORK /// /// Bad path when getting metadata through ->WithMetadataValue function /// @@ -1041,7 +1042,7 @@ public void InvalidMetadataName2() logger.AssertLogContains("MSB4023"); } -#if FEATURE_LEGACY_GETFULLPATH +#if NETFRAMEWORK /// /// Bad path when getting metadata through ->AnyHaveMetadataValue function /// diff --git a/src/Directory.BeforeCommon.targets b/src/Directory.BeforeCommon.targets index 95e68ae8409..f2854850df2 100644 --- a/src/Directory.BeforeCommon.targets +++ b/src/Directory.BeforeCommon.targets @@ -1,4 +1,4 @@ - + $(MSBuildAllProjects);$(MSBuildThisFileFullPath) @@ -35,9 +35,9 @@ $(DefineConstants);FEATURE_HTTP_LISTENER $(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/Directory.Build.targets b/src/Directory.Build.targets index 1f60b1bd2a5..3de2403fd4c 100644 --- a/src/Directory.Build.targets +++ b/src/Directory.Build.targets @@ -1,4 +1,4 @@ - + $(MSBuildAllProjects);$(MSBuildThisFileFullPath) @@ -143,13 +143,13 @@ - + - + - + diff --git a/src/Framework.UnitTests/FileUtilities_Tests.cs b/src/Framework.UnitTests/FileUtilities_Tests.cs index e75118ed373..7c901e77e50 100644 --- a/src/Framework.UnitTests/FileUtilities_Tests.cs +++ b/src/Framework.UnitTests/FileUtilities_Tests.cs @@ -519,11 +519,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 cf23c4ac7e6..93a27ebc9d1 100644 --- a/src/Framework/FileUtilities.cs +++ b/src/Framework/FileUtilities.cs @@ -682,65 +682,47 @@ internal static string NormalizePath(params string[] paths) private static string GetFullPath(string path) { -#if FEATURE_LEGACY_GETFULLPATH - if (NativeMethods.IsWindows) +#if FEATURE_MSIOREDIST + try { - string uncheckedFullPath = NativeMethods.GetFullPath(path); - - if (IsPathTooLong(uncheckedFullPath)) + return Path.GetFullPath(path); + } + catch (PathTooLongException) + { + // Trigger the same exception for truly invalid characters even if path is long + if (path.Contains('|')) { - throw new PathTooLongException(SR.FormatPathTooLong(path, NativeMethods.MaxPath)); + throw new ArgumentException("Illegal characters in path."); } - // 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; + // Re-throw to preserve behavior expected by Copy/Move tasks and Regress451057 tests. + // Microsoft.IO.Path.GetFullPath may not throw for the same path; we must not fall back. + throw; } -#endif - - return Path.GetFullPath(path); - } - -#if FEATURE_LEGACY_GETFULLPATH - private static bool IsUNCPath(string path) - { - if (!NativeMethods.IsWindows || !path.StartsWith(@"\\", StringComparison.Ordinal)) + catch (ArgumentException) { - return false; - } - bool isUNC = true; - for (int i = 2; i < path.Length - 1; i++) - { - if (path[i] == '\\') + // Redist (Microsoft.IO.Redist) is more permissive (matching legacy Win32 GetFullPathName) + // about some characters like newlines at the edges that .NET Framework rejects. + // However, we must remain strict about characters like '|' or malformed UNC roots to satisfy MSBuild tests. + if (path.Contains('|')) { - isUNC = false; - break; + throw; } - } - - /* - 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. + string redistResult = NewPath.GetFullPath(path); - throw new ArgumentException(Environment.GetResourceString("Arg_PathIllegalUNC")); + // Re-validate UNC roots that Redist might accept but MSBuild tests expect to fail. + if (redistResult.StartsWith(@"\\", StringComparison.Ordinal) && (redistResult is @"\\" or @"\\\\" or @"\\localhost" or @"\\XXX\")) + { + throw; + } - // 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; + return redistResult; + } +#else + return Path.GetFullPath(path); +#endif } -#endif // FEATURE_LEGACY_GETFULLPATH /// /// Normalizes all path separators (both forward and back slashes) to forward slashes. @@ -1069,6 +1051,25 @@ internal static string NormalizePathForComparisonNoThrow(string path, string cur internal static bool PathIsInvalid(string path) { + if (path == null) + { + return true; + } + + // Leading or trailing whitespace can cause NormalizePath to produce wrong results + // (e.g. Microsoft.IO.Path.GetFullPath may trim), so treat as invalid. See issue #4593. + if (path != path.Trim()) + { + return true; + } + + // Paths that exceed MAX_PATH (260) will cause GetFullPath to throw PathTooLongException on + // legacy Windows. Treat as invalid so callers skip NormalizePath and handle gracefully (e.g. RAR Regress314573). + if (path.Length >= NativeMethods.MAX_PATH) + { + return true; + } + // Path.GetFileName does not react well to malformed filenames. // For example, Path.GetFileName("a/b/foo:bar") returns bar instead of foo:bar // It also throws exceptions on illegal path characters diff --git a/src/Framework/Microsoft.Build.Framework.csproj b/src/Framework/Microsoft.Build.Framework.csproj index 00bc2eb380e..1f4c7d1367d 100644 --- a/src/Framework/Microsoft.Build.Framework.csproj +++ b/src/Framework/Microsoft.Build.Framework.csproj @@ -1,4 +1,4 @@ - + $(LibraryTargetFrameworks) true @@ -38,6 +38,7 @@ + diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 46aad3925dc..0f0f563b494 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -1,4 +1,4 @@ -// 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; @@ -1481,23 +1481,18 @@ internal static List> GetChildProcessIds(in } /// - /// Internal, optimized GetCurrentDirectory implementation that simply delegates to the native method + /// Returns the current working directory. On .NET Framework, uses Microsoft.IO.Redist for performance. /// - /// - internal static unsafe string GetCurrentDirectory() + internal static string GetCurrentDirectory() { -#if FEATURE_LEGACY_GETCURRENTDIRECTORY - if (IsWindows) - { - int bufferSize = GetCurrentDirectoryWin32(0, null); - char* buffer = stackalloc char[bufferSize]; - int pathLength = GetCurrentDirectoryWin32(bufferSize, buffer); - return new string(buffer, startIndex: 0, length: pathLength); - } -#endif +#if FEATURE_MSIOREDIST + return Microsoft.IO.Directory.GetCurrentDirectory(); +#else return Directory.GetCurrentDirectory(); +#endif } +#if FEATURE_LEGACY_GETCURRENTDIRECTORY [SupportedOSPlatform("windows")] private static unsafe int GetCurrentDirectoryWin32(int nBufferLength, char* lpBuffer) { @@ -1505,7 +1500,9 @@ private static unsafe int GetCurrentDirectoryWin32(int nBufferLength, char* lpBu VerifyThrowWin32Result(pathLength); return pathLength; } +#endif +#if FEATURE_LEGACY_GETFULLPATH [SupportedOSPlatform("windows")] internal static unsafe string GetFullPath(string path) { @@ -1544,6 +1541,7 @@ private static unsafe bool AreStringsEqual(char* buffer, int len, string s) { return s.AsSpan().SequenceEqual(new ReadOnlySpan(buffer, len)); } +#endif internal static void VerifyThrowWin32Result(int result) { @@ -1690,11 +1688,27 @@ internal static void RestoreConsoleMode(uint? originalConsoleMode, StreamHandleT [DllImport("kernel32.dll")] internal static extern bool SetConsoleMode(IntPtr hConsoleHandle, uint dwMode); + /* + Legacy kernel32 P/Invokes when FEATURE_LEGACY_GETCURRENTDIRECTORY / FEATURE_LEGACY_GETFULLPATH are enabled + (see commented DefineConstants in Directory.BeforeCommon.targets). Kept for reference when disabled. + [SuppressMessage("Microsoft.Usage", "CA2205:UseManagedEquivalentsOfWin32Api", Justification = "Using unmanaged equivalent for performance reasons")] [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] [SupportedOSPlatform("windows")] internal static extern unsafe int GetCurrentDirectory(int nBufferLength, char* lpBuffer); + [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + [SupportedOSPlatform("windows")] + internal static extern unsafe int GetFullPathName(string target, int bufferLength, char* buffer, IntPtr mustBeZero); + */ + +#if FEATURE_LEGACY_GETCURRENTDIRECTORY + [SuppressMessage("Microsoft.Usage", "CA2205:UseManagedEquivalentsOfWin32Api", Justification = "Using unmanaged equivalent for performance reasons")] + [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + [SupportedOSPlatform("windows")] + internal static extern unsafe int GetCurrentDirectory(int nBufferLength, char* lpBuffer); +#endif + [SuppressMessage("Microsoft.Usage", "CA2205:UseManagedEquivalentsOfWin32Api", Justification = "Using unmanaged equivalent for performance reasons")] [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode, EntryPoint = "SetCurrentDirectory")] [return: MarshalAs(UnmanagedType.Bool)] @@ -1719,9 +1733,11 @@ internal static bool SetCurrentDirectory(string path) return true; } +#if FEATURE_LEGACY_GETFULLPATH [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] [SupportedOSPlatform("windows")] internal static extern unsafe int GetFullPathName(string target, int bufferLength, char* buffer, IntPtr mustBeZero); +#endif [DllImport("KERNEL32.DLL")] [SupportedOSPlatform("windows")]