From 146c01ec983614a7c26578fb0408ee91d9b4ab69 Mon Sep 17 00:00:00 2001 From: Jeremy Kuhne Date: Thu, 27 Aug 2015 16:10:47 -0700 Subject: [PATCH] Unblock paths > MAX_PATH without extended syntax. Also allow directories up to max component length (255) as opening does not require extended syntax. --- .../mincore/Interop.CreateDirectory.cs | 3 +- .../mincore/Interop.GetFullPathNameW.cs | 25 +++++++++++- .../mincore/Interop.GetLongPathNameW.cs | 25 +++++++++++- .../Windows/mincore/Interop.GetTempPathW.cs | 1 + .../src/System/IO/PathInternal.Windows.cs | 40 ++++++++++--------- .../tests/Directory/CreateDirectory.cs | 35 +++++++++++++--- .../tests/Directory/Exists.cs | 4 +- .../tests/Directory/Move.cs | 22 +++++++--- src/System.IO.FileSystem/tests/File/Move.cs | 18 +++++++++ .../tests/PortedCommon/IOInputs.cs | 19 +++++---- .../src/System/IO/Path.Unix.cs | 1 + .../src/System/IO/Path.Windows.cs | 2 +- .../src/System/IO/Path.cs | 12 ++---- .../tests/System/IO/PathTests.cs | 4 +- 14 files changed, 158 insertions(+), 53 deletions(-) diff --git a/src/Common/src/Interop/Windows/mincore/Interop.CreateDirectory.cs b/src/Common/src/Interop/Windows/mincore/Interop.CreateDirectory.cs index 3b603dadd8b1..1432d339f51a 100644 --- a/src/Common/src/Interop/Windows/mincore/Interop.CreateDirectory.cs +++ b/src/Common/src/Interop/Windows/mincore/Interop.CreateDirectory.cs @@ -14,7 +14,8 @@ internal partial class mincore internal static bool CreateDirectory(string path, ref SECURITY_ATTRIBUTES lpSecurityAttributes) { - path = PathInternal.AddExtendedPathPrefixForLongPaths(path); + // We always want to add for CreateDirectory to get around the legacy 248 character limitation + path = PathInternal.AddExtendedPathPrefix(path); return CreateDirectoryPrivate(path, ref lpSecurityAttributes); } } diff --git a/src/Common/src/Interop/Windows/mincore/Interop.GetFullPathNameW.cs b/src/Common/src/Interop/Windows/mincore/Interop.GetFullPathNameW.cs index 2435e69694d7..0035120bcfaa 100644 --- a/src/Common/src/Interop/Windows/mincore/Interop.GetFullPathNameW.cs +++ b/src/Common/src/Interop/Windows/mincore/Interop.GetFullPathNameW.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.IO; using System.Runtime.InteropServices; using System.Text; @@ -9,10 +10,30 @@ partial class Interop { partial class mincore { + /// + /// WARNING: This overload does not implicitly handle long paths. + /// [DllImport(Libraries.CoreFile_L1, SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] internal unsafe static extern int GetFullPathNameW(char* path, int numBufferChars, char* buffer, IntPtr mustBeZero); - [DllImport(Libraries.CoreFile_L1, SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern int GetFullPathNameW(string path, int numBufferChars, [Out]StringBuilder buffer, IntPtr mustBeZero); + [DllImport(Libraries.CoreFile_L1, EntryPoint = "GetFullPathNameW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + private static extern int GetFullPathNameWPrivate(string path, int numBufferChars, [Out]StringBuilder buffer, IntPtr mustBeZero); + + internal static int GetFullPathNameW(string path, int numBufferChars, [Out]StringBuilder buffer, IntPtr mustBeZero) + { + bool wasExtended = PathInternal.IsExtended(path); + if (!wasExtended) + { + path = PathInternal.AddExtendedPathPrefixForLongPaths(path); + } + int result = GetFullPathNameWPrivate(path, numBufferChars, buffer, mustBeZero); + + if (!wasExtended) + { + // We don't want to give back \\?\ if we possibly added it ourselves + PathInternal.RemoveExtendedPathPrefix(buffer); + } + return result; + } } } diff --git a/src/Common/src/Interop/Windows/mincore/Interop.GetLongPathNameW.cs b/src/Common/src/Interop/Windows/mincore/Interop.GetLongPathNameW.cs index 41078977242a..d56cf95316e2 100644 --- a/src/Common/src/Interop/Windows/mincore/Interop.GetLongPathNameW.cs +++ b/src/Common/src/Interop/Windows/mincore/Interop.GetLongPathNameW.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.IO; using System.Text; using System.Runtime.InteropServices; @@ -8,10 +9,30 @@ partial class Interop { partial class mincore { + /// + /// WARNING: This overload does not implicitly handle long paths. + /// [DllImport(Libraries.CoreFile_L1, SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] internal unsafe static extern int GetLongPathNameW(char* path, char* longPathBuffer, int bufferLength); - [DllImport(Libraries.CoreFile_L1, SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] - internal static extern int GetLongPathNameW(string path, [Out]StringBuilder longPathBuffer, int bufferLength); + [DllImport(Libraries.CoreFile_L1, EntryPoint = "GetLongPathNameW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false)] + private static extern int GetLongPathNameWPrivate(string path, [Out]StringBuilder longPathBuffer, int bufferLength); + + internal static int GetLongPathNameW(string path, [Out]StringBuilder longPathBuffer, int bufferLength) + { + bool wasExtended = PathInternal.IsExtended(path); + if (!wasExtended) + { + path = PathInternal.AddExtendedPathPrefixForLongPaths(path); + } + int result = GetLongPathNameWPrivate(path, longPathBuffer, bufferLength); + + if (!wasExtended) + { + // We don't want to give back \\?\ if we possibly added it ourselves + PathInternal.RemoveExtendedPathPrefix(longPathBuffer); + } + return result; + } } } diff --git a/src/Common/src/Interop/Windows/mincore/Interop.GetTempPathW.cs b/src/Common/src/Interop/Windows/mincore/Interop.GetTempPathW.cs index ec2b41794324..4f80fc7ce328 100644 --- a/src/Common/src/Interop/Windows/mincore/Interop.GetTempPathW.cs +++ b/src/Common/src/Interop/Windows/mincore/Interop.GetTempPathW.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.IO; using System.Text; using System.Runtime.InteropServices; diff --git a/src/Common/src/System/IO/PathInternal.Windows.cs b/src/Common/src/System/IO/PathInternal.Windows.cs index f30d5c3ca316..590d223c862d 100644 --- a/src/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/Common/src/System/IO/PathInternal.Windows.cs @@ -41,16 +41,29 @@ internal static partial class PathInternal /// internal static bool IsPathTooLong(string fullPath) { + // We'll never know precisely what will fail as paths get changed internally in Windows and + // may grow to exceed MaxExtendedPath. We'll only try to catch ones we know will absolutely + // fail. + + if (fullPath.Length < MaxExtendedPath - UncExtendedPathPrefix.Length) + { + // For sure is ok + return false; + } + + // We need to check if we have a prefix to account for one being implicitly added. if (IsExtended(fullPath)) { + // We won't prepend, just check return fullPath.Length >= MaxExtendedPath; } - else + + if (fullPath.StartsWith(UncPathPrefix, StringComparison.Ordinal)) { - // Will need to be updated with #2581 to allow all paths to MaxExtendedPath - // minus legth of extended local or UNC prefix. - return fullPath.Length >= MaxShortPath; + return fullPath.Length + UncExtendedPrefixToInsert.Length >= MaxExtendedPath; } + + return fullPath.Length + ExtendedPathPrefix.Length >= MaxExtendedPath; } /// @@ -58,16 +71,7 @@ internal static bool IsPathTooLong(string fullPath) /// internal static bool IsDirectoryTooLong(string fullPath) { - if (IsExtended(fullPath)) - { - return fullPath.Length >= MaxExtendedPath; - } - else - { - // Will need to be updated with #2581 to allow all paths to MaxExtendedPath - // minus legth of extended local or UNC prefix. - return fullPath.Length >= MaxShortDirectoryPath; - } + return IsPathTooLong(fullPath); } /// @@ -272,17 +276,17 @@ internal static bool IsPathRelative(string path) return true; } - if ((path[0] == '\\') || (path[0] == '/')) + if ((path[0] == Path.DirectorySeparatorChar) || (path[0] == Path.AltDirectorySeparatorChar)) { // There is no valid way to specify a relative path with two initial slashes - return !((path[1] == '\\') || (path[1] == '/')); + return !((path[1] == Path.DirectorySeparatorChar) || (path[1] == Path.AltDirectorySeparatorChar)); } // The only way to specify a fixed path that doesn't begin with two slashes // is the drive, colon, slash format- i.e. C:\ return !((path.Length >= 3) - && (path[1] == ':') - && ((path[2] == '\\') || (path[2] == '/'))); + && (path[1] == Path.VolumeSeparatorChar) + && ((path[2] == Path.DirectorySeparatorChar) || (path[2] == Path.AltDirectorySeparatorChar))); } internal static bool IsDirectorySeparator(char c) diff --git a/src/System.IO.FileSystem/tests/Directory/CreateDirectory.cs b/src/System.IO.FileSystem/tests/Directory/CreateDirectory.cs index c7d6eb6df2e5..421ee187e031 100644 --- a/src/System.IO.FileSystem/tests/Directory/CreateDirectory.cs +++ b/src/System.IO.FileSystem/tests/Directory/CreateDirectory.cs @@ -210,20 +210,44 @@ public void DirectoryWithComponentLongerThanMaxComponentAsPath_ThrowsPathTooLong [Fact] [PlatformSpecific(PlatformID.Windows)] - public void DirectoryLongerThanMaxPathAsPath_ThrowsPathTooLongException() + public void DirectoryLongerThanMaxPath_Succeeds() { var paths = IOInputs.GetPathsLongerThanMaxPath(); Assert.All(paths, (path) => + { + DirectoryInfo result = Create(path); + Assert.True(Directory.Exists(result.FullName)); + }); + } + + [Fact] + [PlatformSpecific(PlatformID.Windows)] + public void DirectoryLongerThanMaxLongPath_ThrowsPathTooLongException() + { + var paths = IOInputs.GetPathsLongerThanMaxLongPath(); + Assert.All(paths, (path) => + { + Assert.Throws(() => Create(path)); + }); + } + + [Fact] + [PlatformSpecific(PlatformID.Windows)] + public void DirectoryLongerThanMaxLongPathWithExtendedSyntax_ThrowsPathTooLongException() + { + var paths = IOInputs.GetPathsLongerThanMaxLongPath(useExtendedSyntax: true); + Assert.All(paths, (path) => { Assert.Throws(() => Create(path)); }); } + [Fact] [PlatformSpecific(PlatformID.Windows)] - public void ExtendedDirectoryLongerThanLegacyMaxPathSucceeds() + public void ExtendedDirectoryLongerThanLegacyMaxPath_Succeeds() { - var paths = IOInputs.GetPathsLongerThanMaxPath(useExtendedSyntax: true, includeExtendedMaxPath: false); + var paths = IOInputs.GetPathsLongerThanMaxPath(useExtendedSyntax: true); Assert.All(paths, (path) => { Assert.True(Create(path).Exists); @@ -232,12 +256,13 @@ public void ExtendedDirectoryLongerThanLegacyMaxPathSucceeds() [Fact] [PlatformSpecific(PlatformID.Windows)] - public void DirectoryLongerThanMaxDirectoryAsPath_ThrowsPathTooLongException() + public void DirectoryLongerThanMaxDirectoryAsPath_Succeeds() { var paths = IOInputs.GetPathsLongerThanMaxDirectory(); Assert.All(paths, (path) => { - Assert.Throws(() => Create(path)); + var result = Create(path); + Assert.True(Directory.Exists(result.FullName)); }); } diff --git a/src/System.IO.FileSystem/tests/Directory/Exists.cs b/src/System.IO.FileSystem/tests/Directory/Exists.cs index f01bd4a89c0a..42e03c760c80 100644 --- a/src/System.IO.FileSystem/tests/Directory/Exists.cs +++ b/src/System.IO.FileSystem/tests/Directory/Exists.cs @@ -105,9 +105,9 @@ public void DotDotAsPath_ReturnsTrue() } [Fact] - public void DirectoryLongerThanMaxPathAsPath_DoesntThrow() + public void DirectoryLongerThanMaxLongPath_DoesntThrow() { - Assert.All((IOInputs.GetPathsLongerThanMaxPath()), (path) => + Assert.All((IOInputs.GetPathsLongerThanMaxLongPath()), (path) => { Assert.False(Exists(path), path); }); diff --git a/src/System.IO.FileSystem/tests/Directory/Move.cs b/src/System.IO.FileSystem/tests/Directory/Move.cs index f2b010b9652d..9914d14ec435 100644 --- a/src/System.IO.FileSystem/tests/Directory/Move.cs +++ b/src/System.IO.FileSystem/tests/Directory/Move.cs @@ -127,11 +127,11 @@ public void IncludeSubdirectories() } [Fact] - public void Path_Longer_Than_MaxPath_Throws_Exception() + public void Path_Longer_Than_MaxLongPath_Throws_Exception() { string testDir = GetTestFilePath(); Directory.CreateDirectory(testDir); - Assert.All((IOInputs.GetPathsLongerThanMaxPath()), (path) => + Assert.All((IOInputs.GetPathsLongerThanMaxLongPath()), (path) => { Assert.Throws(() => Move(testDir, path)); Assert.Throws(() => Move(path, testDir)); @@ -144,14 +144,26 @@ public void Path_Longer_Than_MaxPath_Throws_Exception() [Fact] [PlatformSpecific(PlatformID.Windows)] - public void Path_With_Longer_Than_MaxDirectory_Throws_Exception() + public void Path_With_Longer_Than_MaxDirectory_Succeeds() { string testDir = GetTestFilePath(); Directory.CreateDirectory(testDir); + Assert.True(Directory.Exists(testDir), "test directory should exist"); Assert.All((IOInputs.GetPathsLongerThanMaxDirectory()), (path) => { - Assert.Throws(() => Move(testDir, path)); - Assert.Throws(() => Move(path, testDir)); + string baseDestinationPath = Path.GetDirectoryName(path); + if (!Directory.Exists(baseDestinationPath)) + { + Directory.CreateDirectory(baseDestinationPath); + } + Assert.True(Directory.Exists(baseDestinationPath), "base destination path should exist"); + + Move(testDir, path); + Assert.False(Directory.Exists(testDir), "source directory should exist"); + Assert.True(Directory.Exists(path), "destination directory should exist"); + Move(path, testDir); + Assert.False(Directory.Exists(path), "source directory should exist"); + Assert.True(Directory.Exists(testDir), "destination directory should exist"); }); } diff --git a/src/System.IO.FileSystem/tests/File/Move.cs b/src/System.IO.FileSystem/tests/File/Move.cs index ca4d72650d00..bd3aae805863 100644 --- a/src/System.IO.FileSystem/tests/File/Move.cs +++ b/src/System.IO.FileSystem/tests/File/Move.cs @@ -149,8 +149,26 @@ public void LongPath() //Create a destination path longer than the traditional Windows limit of 256 characters string testFileSource = Path.Combine(TestDirectory, GetTestFileName()); File.Create(testFileSource).Dispose(); + Assert.True(File.Exists(testFileSource), "test file should exist"); Assert.All(IOInputs.GetPathsLongerThanMaxPath(), (path) => + { + string baseDestinationPath = Path.GetDirectoryName(path); + if (!Directory.Exists(baseDestinationPath)) + { + Directory.CreateDirectory(baseDestinationPath); + } + Assert.True(Directory.Exists(baseDestinationPath), "base destination path should exist"); + + Move(testFileSource, path); + Assert.True(File.Exists(path), "moved test file should exist"); + File.Delete(testFileSource); + Assert.False(File.Exists(testFileSource), "source test file should not exist"); + Move(path, testFileSource); + Assert.True(File.Exists(testFileSource), "restored test file should exist"); + }); + + Assert.All(IOInputs.GetPathsLongerThanMaxLongPath(), (path) => { Assert.Throws(() => Move(testFileSource, path)); File.Delete(testFileSource); diff --git a/src/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs b/src/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs index 0591a514dbf0..b1e828e4ed07 100644 --- a/src/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs +++ b/src/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs @@ -21,13 +21,17 @@ internal static class IOInputs // Max path length (minus trailing \0). Unix values vary system to system; just using really long values here likely to be more than on the average system. public static readonly int MaxPath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 259 : 10000; + // Same as MaxPath on Unix + public static readonly int MaxLongPath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? MaxExtendedPath : 10000; + // Windows specific, this is the maximum length that can be passed to APIs taking directory names, such as Directory.CreateDirectory & Directory.Move. // Does not include the trailing \0. - public static readonly int MaxDirectory = 247; + public static readonly int MaxDirectory = MaxComponent - 1; // Windows specific, this is the maximum length that can be passed using extended syntax. Does not include the trailing \0. public static readonly int MaxExtendedPath = short.MaxValue - 1; + public const int MaxComponent = 255; public const string ExtendedPrefix = @"\\?\"; @@ -218,16 +222,17 @@ public static IEnumerable GetPathsLongerThanMaxDirectory() yield return GetLongPath(MaxDirectory + 3); } - public static IEnumerable GetPathsLongerThanMaxPath(bool useExtendedSyntax = false, bool includeExtendedMaxPath = true) + public static IEnumerable GetPathsLongerThanMaxPath(bool useExtendedSyntax = false) { yield return GetLongPath(MaxPath + 1, useExtendedSyntax); yield return GetLongPath(MaxPath + 2, useExtendedSyntax); yield return GetLongPath(MaxPath + 3, useExtendedSyntax); - if (includeExtendedMaxPath) - { - yield return GetLongPath(MaxExtendedPath + 1, useExtendedSyntax); - yield return GetLongPath(MaxExtendedPath + 2, useExtendedSyntax); - } + } + + public static IEnumerable GetPathsLongerThanMaxLongPath(bool useExtendedSyntax = false) + { + yield return GetLongPath(MaxExtendedPath + 1 - (useExtendedSyntax ? 0 : ExtendedPrefix.Length), useExtendedSyntax); + yield return GetLongPath(MaxExtendedPath + 2 - (useExtendedSyntax ? 0 : ExtendedPrefix.Length), useExtendedSyntax); } private static string GetLongPath(int characterCount, bool extended = false) diff --git a/src/System.Runtime.Extensions/src/System/IO/Path.Unix.cs b/src/System.Runtime.Extensions/src/System/IO/Path.Unix.cs index e3e25fbc77ca..1e7cc016a727 100644 --- a/src/System.Runtime.Extensions/src/System/IO/Path.Unix.cs +++ b/src/System.Runtime.Extensions/src/System/IO/Path.Unix.cs @@ -18,6 +18,7 @@ public static partial class Path private static readonly char[] InvalidFileNameChars = { '\0', '/' }; private static readonly int MaxPath = Interop.libc.MaxPath; + private static readonly int MaxLongPath = MaxPath; private static readonly int MaxComponentLength = Interop.libc.MaxName; private static bool IsDirectoryOrVolumeSeparator(char c) diff --git a/src/System.Runtime.Extensions/src/System/IO/Path.Windows.cs b/src/System.Runtime.Extensions/src/System/IO/Path.Windows.cs index a65e1b7eb545..1c7f8898ca7c 100644 --- a/src/System.Runtime.Extensions/src/System/IO/Path.Windows.cs +++ b/src/System.Runtime.Extensions/src/System/IO/Path.Windows.cs @@ -33,7 +33,7 @@ public static partial class Path // For example, D:\<256 char file name> isn't legal, even though it's under 260 chars. internal static readonly int MaxPath = 260; private static readonly int MaxComponentLength = 255; - private const int MaxLongPath = 32000; + internal static readonly int MaxLongPath = short.MaxValue; private static bool IsDirectoryOrVolumeSeparator(char c) { diff --git a/src/System.Runtime.Extensions/src/System/IO/Path.cs b/src/System.Runtime.Extensions/src/System/IO/Path.cs index 7779628931b8..73628bf2e231 100644 --- a/src/System.Runtime.Extensions/src/System/IO/Path.cs +++ b/src/System.Runtime.Extensions/src/System/IO/Path.cs @@ -134,21 +134,15 @@ private static string GetFullPathInternal(string path) [System.Security.SecuritySafeCritical] // auto-generated private static string NormalizePath(string path, bool fullCheck) { - return NormalizePath(path, fullCheck, MaxPath); + return NormalizePath(path, fullCheck, MaxLongPath, expandShortPaths: true); } [System.Security.SecuritySafeCritical] // auto-generated private static string NormalizePath(string path, bool fullCheck, bool expandShortPaths) { - return NormalizePath(path, fullCheck, MaxPath, expandShortPaths); + return NormalizePath(path, fullCheck, MaxLongPath, expandShortPaths); } - - [System.Security.SecuritySafeCritical] // auto-generated - private static string NormalizePath(string path, bool fullCheck, int maxPathLength) - { - return NormalizePath(path, fullCheck, maxPathLength, expandShortPaths: true); - } - + // Returns the name and extension parts of the given path. The resulting // string contains the characters of path that follow the last // separator in path. The resulting string is null if path is null. diff --git a/src/System.Runtime.Extensions/tests/System/IO/PathTests.cs b/src/System.Runtime.Extensions/tests/System/IO/PathTests.cs index 0172718f283b..d7458f150538 100644 --- a/src/System.Runtime.Extensions/tests/System/IO/PathTests.cs +++ b/src/System.Runtime.Extensions/tests/System/IO/PathTests.cs @@ -408,7 +408,9 @@ public static void GetFullPath_Windows_NormalizedLongPathTooLong() { longPath.Append(Path.DirectorySeparatorChar).Append('a').Append(Path.DirectorySeparatorChar).Append('.'); } - Assert.Throws(() => Path.GetFullPath(longPath.ToString())); + + // Now no longer throws unless over ~32K + Assert.NotNull(Path.GetFullPath(longPath.ToString())); } [PlatformSpecific(PlatformID.Windows)]