Use Microsoft.IO.Redist in Framework FileUtilities/NativeMethods on net472 (#13078)

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
         /// <summary>
         /// Bad path when getting metadata through ->Metadata function
         /// </summary>
@@ -942,6 +942,7 @@ public void InvalidPathAndMetadataItemFunctionPathTooLong()
 
             logger.AssertLogContains("MSB4023");
         }
+
 #endif
 
         /// <summary>
@@ -982,7 +983,7 @@ public void InvalidMetadataName()
             logger.AssertLogContains("MSB4023");
         }
 
-#if FEATURE_LEGACY_GETFULLPATH
+#if NETFRAMEWORK
         /// <summary>
         /// Bad path when getting metadata through ->WithMetadataValue function
         /// </summary>
@@ -1041,7 +1042,7 @@ public void InvalidMetadataName2()
             logger.AssertLogContains("MSB4023");
         }
 
-#if FEATURE_LEGACY_GETFULLPATH
+#if NETFRAMEWORK
         /// <summary>
         /// Bad path when getting metadata through ->AnyHaveMetadataValue function
         /// </summary>

src/Directory.BeforeCommon.targets

@@ -1,4 +1,4 @@
-<Project>
+<Project>
 
   <PropertyGroup>
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
@@ -35,9 +35,9 @@
     <DefineConstants>$(DefineConstants);FEATURE_HTTP_LISTENER</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_INSTALLED_MSBUILD</DefineConstants>
     <!-- Directory.GetCurrentDirectory The pre .Net 4.6.2 implementation of Directory.GetCurrentDirectory is slow and creates strings in its work. -->
-    <DefineConstants>$(DefineConstants);FEATURE_LEGACY_GETCURRENTDIRECTORY</DefineConstants>
+    <!-- <DefineConstants>$(DefineConstants);FEATURE_LEGACY_GETCURRENTDIRECTORY</DefineConstants> -->
     <!-- Path.GetFullPath The pre .Net 4.6.2 implementation of Path.GetFullPath is slow and creates strings in its work. -->
-    <DefineConstants>$(DefineConstants);FEATURE_LEGACY_GETFULLPATH</DefineConstants>
+    <!-- <DefineConstants>$(DefineConstants);FEATURE_LEGACY_GETFULLPATH</DefineConstants> -->
     <DefineConstants>$(DefineConstants);FEATURE_NAMED_PIPE_SECURITY_CONSTRUCTOR</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_PERFORMANCE_COUNTERS</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_PIPE_SECURITY</DefineConstants>

src/Directory.Build.targets

@@ -1,4 +1,4 @@
-<Project DefaultTargets="Build">
+<Project DefaultTargets="Build">
 
   <PropertyGroup>
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
@@ -143,13 +143,13 @@
       <TlbExpAssemblyPaths Include="@(_TlbExpAssemblyPaths->'%(SlashlessPath)')" />
     </ItemGroup>
 
-    <Error Condition="!Exists('$(TlbExpPath)')"
-           Text="TlbExp was not found at '$(TlbExpPath)'. Ensure the .NET Framework SDK tools are installed." />
+    <Warning Condition="!Exists('$(TlbExpPath)')"
+             Text="TlbExp was not found at '$(TlbExpPath)'. Type library generation will be skipped. Ensure the .NET Framework SDK tools are installed if you need the .tlb file." />
 
     <!-- Generate x86 type library -->
-    <Exec Command="&quot;$(TlbExpPath)&quot; $(TlbExpVerbosity) /NOLOGO @(TlbExpAssemblyPaths->'/asmpath:&quot;%(Identity)&quot;', ' ') &quot;$(TargetPath)&quot; /out:&quot;$(TargetDir)$(TargetName).tlb&quot;" />
+    <Exec Condition="Exists('$(TlbExpPath)')" Command="&quot;$(TlbExpPath)&quot; $(TlbExpVerbosity) /NOLOGO @(TlbExpAssemblyPaths->'/asmpath:&quot;%(Identity)&quot;', ' ') &quot;$(TargetPath)&quot; /out:&quot;$(TargetDir)$(TargetName).tlb&quot;" />
     <!-- Generate x64 type library -->
-    <Exec Command="&quot;$(TlbExpPath)&quot; $(TlbExpVerbosity) /NOLOGO @(TlbExpAssemblyPaths->'/asmpath:&quot;%(Identity)&quot;', ' ') &quot;$(TargetPath)&quot; /out:&quot;$(TargetDir)x64\$(TargetName).tlb&quot; /win64" />
+    <Exec Condition="Exists('$(TlbExpPath)')" Command="&quot;$(TlbExpPath)&quot; $(TlbExpVerbosity) /NOLOGO @(TlbExpAssemblyPaths->'/asmpath:&quot;%(Identity)&quot;', ' ') &quot;$(TargetPath)&quot; /out:&quot;$(TargetDir)x64\$(TargetName).tlb&quot; /win64" />
   </Target>
 
   <Import Project="$(BUILD_STAGINGDIRECTORY)\MicroBuild\Plugins\MicroBuild.Plugins.IBCMerge.*\**\build\MicroBuild.Plugins.*.targets" Condition="'$(BUILD_STAGINGDIRECTORY)' != '' and $(TargetFramework.StartsWith('net4')) and '$(MicroBuild_EnablePGO)' != 'false'" />

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<ArgumentException>(() => 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]

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
 
         /// <summary>
         /// 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

src/Framework/Microsoft.Build.Framework.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFrameworks>$(LibraryTargetFrameworks)</TargetFrameworks>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -38,6 +38,7 @@
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
+    <PackageReference Include="Microsoft.IO.Redist" Condition="'$(FeatureMSIORedist)' == 'true'" />
     <!-- Promote CompilerServices.Unsafe from the old version we get from System.Memory on net472. -->
     <PackageReference Include="System.Runtime.CompilerServices.Unsafe" />
     <PackageReference Include="Microsoft.VisualStudio.Setup.Configuration.Interop" PrivateAssets="all" />

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,31 +1481,28 @@ internal static List<KeyValuePair<int, SafeProcessHandle>> GetChildProcessIds(in
     }
 
     /// <summary>
-    /// 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.
     /// </summary>
-    /// <returns></returns>
-    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)
     {
         int pathLength = GetCurrentDirectory(nBufferLength, lpBuffer);
         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<char>(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")]