From 9f3ffa3140abc66fbfffc8d07b4ab318a70ce937 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 22 Jul 2022 15:47:02 -0500 Subject: [PATCH 1/7] Add Directory.CreateTempSubdirectory CreateTempSubdirectory will create a new uniquely named directory under the temp directory. This allows applications to write temporary files in a co-located directory. Contributes to #72881 --- .../Unix/System.Native/Interop.MkdTemp.cs | 14 ++++ .../Kernel32/Interop.CreateDirectory.cs | 8 +- .../FileSystem.DirectoryCreation.Windows.cs | 2 +- .../src/System/IO/TempFileCollection.cs | 38 ++++++++- .../FunctionalTests/DisposableFileSystem.cs | 10 ++- .../tests/TestUtility/DisposableFileSystem.cs | 10 ++- .../System/CodeDom/TempFileCollectionTests.cs | 5 ++ .../DirectoryCatalogDebuggerProxyTests.cs | 12 +-- .../tests/Directory/CreateTempSubdirectory.cs | 78 +++++++++++++++++++ .../tests/System.IO.FileSystem.Tests.csproj | 1 + .../src/Resources/Strings.resx | 6 ++ .../System.Private.CoreLib.Shared.projitems | 3 + .../src/System/IO/Directory.Unix.cs | 24 ++++++ .../src/System/IO/Directory.Windows.cs | 57 ++++++++++++++ .../src/System/IO/Directory.cs | 24 ++++++ .../src/System/IO/Path.Windows.cs | 2 +- .../src/System/IO/Path.cs | 2 +- .../RefEmitLoadContextTest.cs | 5 +- .../tests/ResourceAssemblyLoadContext.cs | 5 +- .../System.Runtime/ref/System.Runtime.cs | 1 + src/native/libs/System.Native/entrypoints.c | 1 + src/native/libs/System.Native/pal_io.c | 7 ++ src/native/libs/System.Native/pal_io.h | 8 ++ 23 files changed, 292 insertions(+), 31 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs create mode 100644 src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs new file mode 100644 index 00000000000000..c41413264507a4 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs @@ -0,0 +1,14 @@ +// 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.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MkdTemp", SetLastError = true)] + internal static unsafe partial byte* MkdTemp(byte[] template); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs index c679fbbb023dbd..2b889d797a1033 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.CreateDirectory.cs @@ -14,15 +14,15 @@ internal static partial class Kernel32 /// [LibraryImport(Libraries.Kernel32, EntryPoint = "CreateDirectoryW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool CreateDirectoryPrivate( + private static unsafe partial bool CreateDirectoryPrivate( string path, - ref SECURITY_ATTRIBUTES lpSecurityAttributes); + SECURITY_ATTRIBUTES* lpSecurityAttributes); - internal static bool CreateDirectory(string path, ref SECURITY_ATTRIBUTES lpSecurityAttributes) + internal static unsafe bool CreateDirectory(string path, SECURITY_ATTRIBUTES* lpSecurityAttributes) { // We always want to add for CreateDirectory to get around the legacy 248 character limitation path = PathInternal.EnsureExtendedPrefix(path); - return CreateDirectoryPrivate(path, ref lpSecurityAttributes); + return CreateDirectoryPrivate(path, lpSecurityAttributes); } } } diff --git a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs index dba07a8fb41567..708714ab2401ff 100644 --- a/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs +++ b/src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs @@ -88,7 +88,7 @@ public static unsafe void CreateDirectory(string fullPath, byte[]? securityDescr string name = stackDir[stackDir.Count - 1]; stackDir.RemoveAt(stackDir.Count - 1); - r = Interop.Kernel32.CreateDirectory(name, ref secAttrs); + r = Interop.Kernel32.CreateDirectory(name, &secAttrs); if (!r && (firstError == 0)) { int currentError = Marshal.GetLastWin32Error(); diff --git a/src/libraries/Common/src/System/IO/TempFileCollection.cs b/src/libraries/Common/src/System/IO/TempFileCollection.cs index 0611b259485cb9..36db27afbcb7a0 100644 --- a/src/libraries/Common/src/System/IO/TempFileCollection.cs +++ b/src/libraries/Common/src/System/IO/TempFileCollection.cs @@ -24,6 +24,7 @@ class TempFileCollection : ICollection, IDisposable private string _basePath; private readonly string _tempDir; private readonly Hashtable _files; + private bool _createdTempDirectory; public TempFileCollection() : this(null, false) { @@ -127,7 +128,7 @@ private void EnsureTempNameCreated() do { _basePath = Path.Combine( - string.IsNullOrEmpty(TempDir) ? Path.GetTempPath() : TempDir, + string.IsNullOrEmpty(TempDir) ? GetTempDirectory() : TempDir, Path.GetFileNameWithoutExtension(Path.GetRandomFileName())); tempFileName = _basePath + ".tmp"; @@ -150,6 +151,19 @@ private void EnsureTempNameCreated() } } +#if NET7_0_OR_GREATER + private string GetTempDirectory() + { + _createdTempDirectory = true; + return Directory.CreateTempSubdirectory().FullName; + } +#else + private static string GetTempDirectory() + { + return Path.GetTempPath(); + } +#endif + public bool KeepFiles { get; set; } private bool KeepFile(string fileName) @@ -177,6 +191,8 @@ private static void Delete(string fileName) internal void SafeDelete() { + bool allFilesDeleted = true; + if (_files != null && _files.Count > 0) { string[] fileNames = new string[_files.Count]; @@ -188,8 +204,28 @@ internal void SafeDelete() Delete(fileName); _files.Remove(fileName); } + else + { + allFilesDeleted = false; + } } } + + // if we created a temp directory, delete it and clear the basePath, so a new directory will be created for the next request. + if (_createdTempDirectory && allFilesDeleted) + { + try + { + Directory.Delete(Path.GetDirectoryName(BasePath)); + } + catch + { + // Ignore all exceptions + } + + _createdTempDirectory = false; + _basePath = null; + } } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs index 73f131afb3fa32..fffca558b0c74d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/FunctionalTests/DisposableFileSystem.cs @@ -14,9 +14,13 @@ public class DisposableFileSystem : IDisposable public DisposableFileSystem() { - RootPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - CreateFolder(""); - DirectoryInfo = new DirectoryInfo(RootPath); +#if NETCOREAPP + DirectoryInfo = Directory.CreateTempSubdirectory(); +#else + DirectoryInfo = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName())); + DirectoryInfo.Create(); +#endif + RootPath = DirectoryInfo.FullName; } public string RootPath { get; } diff --git a/src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/TestUtility/DisposableFileSystem.cs b/src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/TestUtility/DisposableFileSystem.cs index 86baaa0b14a6ab..50abfa57af42e3 100644 --- a/src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/TestUtility/DisposableFileSystem.cs +++ b/src/libraries/Microsoft.Extensions.FileSystemGlobbing/tests/TestUtility/DisposableFileSystem.cs @@ -10,9 +10,13 @@ public class DisposableFileSystem : IDisposable { public DisposableFileSystem() { - RootPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(RootPath); - DirectoryInfo = new DirectoryInfo(RootPath); +#if NETCOREAPP + DirectoryInfo = Directory.CreateTempSubdirectory(); +#else + DirectoryInfo = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName())); + DirectoryInfo.Create(); +#endif + RootPath = DirectoryInfo.FullName; } public string RootPath { get; } diff --git a/src/libraries/System.CodeDom/tests/System/CodeDom/TempFileCollectionTests.cs b/src/libraries/System.CodeDom/tests/System/CodeDom/TempFileCollectionTests.cs index f79ef1bb39c497..7162c9358fdd3c 100644 --- a/src/libraries/System.CodeDom/tests/System/CodeDom/TempFileCollectionTests.cs +++ b/src/libraries/System.CodeDom/tests/System/CodeDom/TempFileCollectionTests.cs @@ -277,8 +277,13 @@ private static string TempDirectory() { if (s_tempDirectory == null) { +#if NETCOREAPP + string tempDirectory = Directory.CreateTempSubdirectory().FullName; +#else string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); Directory.CreateDirectory(tempDirectory); +#endif + s_tempDirectory = tempDirectory; } return s_tempDirectory; diff --git a/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/DirectoryCatalogDebuggerProxyTests.cs b/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/DirectoryCatalogDebuggerProxyTests.cs index b91366935d5c44..0a69ae32ae2114 100644 --- a/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/DirectoryCatalogDebuggerProxyTests.cs +++ b/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/DirectoryCatalogDebuggerProxyTests.cs @@ -164,9 +164,7 @@ private DirectoryCatalog CreateDirectoryCatalog(string path, string filter) private string GetTemporaryDirectory(string location = null) { - string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDirectory); - return tempDirectory; + return Directory.CreateTempSubdirectory().FullName; } } @@ -193,16 +191,12 @@ public static string GetRootTemporaryDirectory() public static string GetNewTemporaryDirectory() { - string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDirectory); - return tempDirectory; + return Directory.CreateTempSubdirectory().FullName; } public static string GetTemporaryDirectory() { - string tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - Directory.CreateDirectory(tempDirectory); - return tempDirectory; + return Directory.CreateTempSubdirectory().FullName; } } } diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs new file mode 100644 index 00000000000000..f20f7f1f735b60 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs @@ -0,0 +1,78 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; + +namespace System.IO.Tests +{ + public class Directory_CreateTempSubdirectory : FileSystemTest + { + public static TheoryData CreateTempSubdirectoryData + { + get + { + var result = new TheoryData() { null, "", "myDir", "my.Dir" }; + if (!OperatingSystem.IsWindows()) + { + // ensure we can use backslashes on Unix since that isn't a directory separator + result.Add(@"my\File"); + result.Add(@"\"); + } + return result; + } + } + + [Theory] + [MemberData(nameof(CreateTempSubdirectoryData))] + public void CreateTempSubdirectory(string prefix) + { + DirectoryInfo tmpDir = Directory.CreateTempSubdirectory(prefix); + try + { + Assert.True(tmpDir.Exists); + Assert.Equal(-1, tmpDir.FullName.IndexOfAny(Path.GetInvalidPathChars())); + Assert.Empty(Directory.GetFiles(tmpDir.FullName)); + Assert.Equal(Path.TrimEndingDirectorySeparator(Path.GetTempPath()), tmpDir.Parent.FullName); + + if (!string.IsNullOrEmpty(prefix)) + { + Assert.StartsWith(prefix, tmpDir.Name); + } + + // Ensure a file can be written to the directory + string tempFile = Path.Combine(tmpDir.FullName, "newFile"); + using (FileStream fs = File.Create(tempFile, bufferSize: 1024, FileOptions.DeleteOnClose)) + { + Assert.Equal(0, fs.Length); + } + } + finally + { + tmpDir.Delete(recursive: true); + } + } + + public static TheoryData InvalidPrefixData + { + get + { + var result = new TheoryData() { "/", "myDir/", "my/Dir" }; + if (OperatingSystem.IsWindows()) + { + result.Add(@"\"); + result.Add(@"myDir\"); + result.Add(@"my\Dir"); + } + return result; + } + } + + [Theory] + [MemberData(nameof(InvalidPrefixData))] + public void CreateTempSubdirectoryThrowsWithPrefixContainingDirectorySeparator(string prefix) + { + ArgumentException e = Assert.Throws(() => Directory.CreateTempSubdirectory(prefix)); + Assert.Equal("prefix", e.ParamName); + } + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index a548e0b7cc90e6..e386671171d19d 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -19,6 +19,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 3fc2d2014809e2..53ac7df9179cba 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2710,6 +2710,9 @@ [Unknown] + + Unable to complete the operation after the maximum number of attempts. + The lazily-initialized type does not have a public, parameterless constructor. @@ -3988,4 +3991,7 @@ Cannot load assembly '{0}'. No metadata found for this assembly. + + The value may not contain directory separator characters. + diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 11ca7cec9d6988..fefbe27ae67c68 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2093,6 +2093,9 @@ Common\Interop\Unix\System.Native\Interop.MkDir.cs + + Common\Interop\Unix\System.Native\Interop.MkdTemp.cs + Common\Interop\Unix\System.Native\Interop.MksTemps.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs index d874d1c8c9bc30..d00f3e3fd0aaf7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs @@ -1,10 +1,15 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; +using System.Text; + namespace System.IO { public static partial class Directory { + private static readonly char[] s_directorySeparators = new char[] { Path.DirectorySeparatorChar }; + private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixCreateMode) { ArgumentException.ThrowIfNullOrEmpty(path); @@ -20,5 +25,24 @@ private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixC return new DirectoryInfo(path, fullPath, isNormalized: true); } + + private static unsafe string CreateTempSubdirectoryCore(string? prefix) + { + // mkdtemp takes a char* and overwrites the XXXXXX with six characters + // that'll result in a unique file name. + string template = $"{Path.GetTempPath()}{prefix}XXXXXX\0"; + byte[] name = Encoding.UTF8.GetBytes(template); + + // Create the temp directory. + byte* result = Interop.Sys.MkdTemp(name); + if (result == null) + { + Interop.CheckIo(-1); + } + + // 'name' is now the name of the directory + Debug.Assert(name[name.Length - 1] == '\0'); + return Encoding.UTF8.GetString(name, 0, name.Length - 1); // trim off the trailing '\0' + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs index 0f96417902ffce..ff2b2b05658868 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs @@ -1,11 +1,68 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Runtime.InteropServices; +using System.Text; + namespace System.IO { public static partial class Directory { + private static readonly char[] s_directorySeparators = new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }; + private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixCreateMode) => throw new PlatformNotSupportedException(SR.PlatformNotSupported_UnixFileMode); + + private static unsafe string CreateTempSubdirectoryCore(string? prefix) + { + ValueStringBuilder builder = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]); + Path.GetTempPath(ref builder); + + // ensure the base TEMP directory exists + CreateDirectory(PathHelper.Normalize(ref builder)); + + builder.Append(prefix); + + const int RandomFileNameLength = 12; // 12 == 8 + 1 (for period) + 3 + int initialTempPathLength = builder.Length; + builder.EnsureCapacity(initialTempPathLength + RandomFileNameLength); + + // For generating random file names + // 8 random bytes provides 12 chars in our encoding for the 8.3 name. + const int RandomKeyLength = 8; + byte* pKey = stackalloc byte[RandomKeyLength]; + + // to avoid an infinite loop, only try as many as GetTempFileNameW will create + const int MaxAttempts = ushort.MaxValue; + int attempts = 0; + while (attempts < MaxAttempts) + { + Interop.GetRandomBytes(pKey, RandomKeyLength); + Path.Populate83FileNameFromRandomBytes(pKey, RandomKeyLength, builder.RawChars.Slice(builder.Length, RandomFileNameLength)); + builder.Length += RandomFileNameLength; + + string path = PathHelper.Normalize(ref builder); + + bool directoryCreated = Interop.Kernel32.CreateDirectory(path, null); + if (!directoryCreated) + { + // in the off-chance that the directory already exists, try again + int error = Marshal.GetLastWin32Error(); + if (error == Interop.Errors.ERROR_ALREADY_EXISTS) + { + builder.Length = initialTempPathLength; + attempts++; + continue; + } + + throw Win32Marshal.GetExceptionForWin32Error(error, path); + } + + builder.Dispose(); + return path; + } + + throw new IOException(SR.IO_MaxAttemptsReached); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs index 0087ecfef8f9bb..f55df6e622c59c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.IO.Enumeration; +using System.Runtime.CompilerServices; using System.Runtime.Versioning; namespace System.IO @@ -52,6 +53,29 @@ public static DirectoryInfo CreateDirectory(string path) public static DirectoryInfo CreateDirectory(string path, UnixFileMode unixCreateMode) => CreateDirectoryCore(path, unixCreateMode); + /// + /// Creates a uniquely-named, empty directory in the current user's temporary directory. + /// + /// An optional string to add to the beginning of the subdirectory name. + /// An object that represents the directory that was created. + /// contains a directory separator. + /// A new directory cannot be created. + public static unsafe DirectoryInfo CreateTempSubdirectory(string? prefix = null) + { + EnsureNoDirectorySeparators(prefix); + + string path = CreateTempSubdirectoryCore(prefix); + return new DirectoryInfo(path, isNormalized: true); + } + + private static void EnsureNoDirectorySeparators(string? value, [CallerArgumentExpression("value")] string? paramName = null) + { + if (!string.IsNullOrEmpty(value) && value.IndexOfAny(s_directorySeparators) != -1) + { + throw new ArgumentException(SR.Argument_DirectorySeparatorInvalid, paramName); + } + } + // Tests if the given path refers to an existing DirectoryInfo on disk. public static bool Exists([NotNullWhen(true)] string? path) { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs index 690f2782389a24..f3aea073194aec 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs @@ -151,7 +151,7 @@ public static string GetTempPath() return path; } - private static void GetTempPath(ref ValueStringBuilder builder) + internal static void GetTempPath(ref ValueStringBuilder builder) { uint result; while ((result = Interop.Kernel32.GetTempPathW(builder.Capacity, ref builder.GetPinnableReference())) > builder.Capacity) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs index b6edb311eac4cf..75ffa1698e719d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs @@ -818,7 +818,7 @@ private static unsafe string JoinInternal(ReadOnlySpan first, ReadOnlySpan private static ReadOnlySpan Base32Char => "abcdefghijklmnopqrstuvwxyz012345"u8; - private static unsafe void Populate83FileNameFromRandomBytes(byte* bytes, int byteCount, Span chars) + internal static unsafe void Populate83FileNameFromRandomBytes(byte* bytes, int byteCount, Span chars) { // This method requires bytes of length 8 and chars of length 12. Debug.Assert(bytes != null); diff --git a/src/libraries/System.Runtime.Loader/tests/RefEmitLoadContext/RefEmitLoadContextTest.cs b/src/libraries/System.Runtime.Loader/tests/RefEmitLoadContext/RefEmitLoadContextTest.cs index 1d0caac253140c..1863ad3767d1f1 100644 --- a/src/libraries/System.Runtime.Loader/tests/RefEmitLoadContext/RefEmitLoadContextTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/RefEmitLoadContext/RefEmitLoadContextTest.cs @@ -38,10 +38,7 @@ private static void Init() var assemblyFilename = "System.Runtime.Loader.Noop.Assembly.dll"; // Form the dynamic path that would not collide if another instance of this test is running. - s_loadFromPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); - - // Create the folder - Directory.CreateDirectory(s_loadFromPath); + s_loadFromPath = Directory.CreateTempSubdirectory().FullName; var targetPath = Path.Combine(s_loadFromPath, assemblyFilename); diff --git a/src/libraries/System.Runtime.Loader/tests/ResourceAssemblyLoadContext.cs b/src/libraries/System.Runtime.Loader/tests/ResourceAssemblyLoadContext.cs index a9fee0bef7acbb..84ad66d3433455 100644 --- a/src/libraries/System.Runtime.Loader/tests/ResourceAssemblyLoadContext.cs +++ b/src/libraries/System.Runtime.Loader/tests/ResourceAssemblyLoadContext.cs @@ -42,10 +42,7 @@ protected override Assembly Load(AssemblyName assemblyName) // This custom load context will extract that resource and store it at the %temp% path at runtime. // This prevents the corerun from adding the test assembly to the TPA list. // Once loaded it is not possible to unload the assembly, therefore it cannot be deleted. - var tempPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); - - // Create the folder since it will not exist already - Directory.CreateDirectory(tempPath); + var tempPath = Directory.CreateTempSubdirectory().FullName; string path = Path.Combine(tempPath, assembly); using (FileStream output = File.OpenWrite(path)) diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index f0425a2bf00c2e..94de712e795afe 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -9379,6 +9379,7 @@ public static partial class Directory [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")] public static System.IO.DirectoryInfo CreateDirectory(string path, System.IO.UnixFileMode unixCreateMode) { throw null; } public static System.IO.FileSystemInfo CreateSymbolicLink(string path, string pathToTarget) { throw null; } + public static System.IO.DirectoryInfo CreateTempSubdirectory(string? prefix = null) { throw null; } public static void Delete(string path) { } public static void Delete(string path, bool recursive) { } public static System.Collections.Generic.IEnumerable EnumerateDirectories(string path) { throw null; } diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index a6e406b4a54d2c..8357bcc66bc43c 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -86,6 +86,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_MkNod) DllImportEntry(SystemNative_GetDeviceIdentifiers) DllImportEntry(SystemNative_MkFifo) + DllImportEntry(SystemNative_MkdTemp) DllImportEntry(SystemNative_MksTemps) DllImportEntry(SystemNative_MMap) DllImportEntry(SystemNative_MUnmap) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index b31163a01e80be..4642b55707564b 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -794,6 +794,13 @@ int32_t SystemNative_MkFifo(const char* pathName, uint32_t mode) return result; } +char* SystemNative_MkdTemp(char* pathTemplate) +{ + char* result = NULL; + while ((result = mkdtemp(pathTemplate)) == NULL && errno == EINTR); + return result; +} + intptr_t SystemNative_MksTemps(char* pathTemplate, int32_t suffixLength) { intptr_t result; diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index ec3049f5b78ec4..f80b2bf7d56ee7 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -561,6 +561,14 @@ PALEXPORT int32_t SystemNative_MkNod(const char* pathName, uint32_t mode, uint32 */ PALEXPORT int32_t SystemNative_MkFifo(const char* pathName, uint32_t mode); +/** + * Creates a directory name that adheres to the specified template, creates the directory on disk with + * 0700 permissions, and returns the directory name. + * + * Returns returns a pointer to the modified template string on success; otherwise, returns NULL and errno is set. + */ +PALEXPORT char* SystemNative_MkdTemp(char* pathTemplate); + /** * Creates a file name that adheres to the specified template, creates the file on disk with * 0600 permissions, and returns an open r/w File Descriptor on the file. From 692c536d2b182cecadd38dfa82b149093fc03383 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 5 Aug 2022 14:58:57 -0500 Subject: [PATCH 2/7] PR feedback --- src/libraries/Common/src/System/IO/PathInternal.Unix.cs | 1 + src/libraries/Common/src/System/IO/PathInternal.Windows.cs | 1 + .../tests/Directory/CreateTempSubdirectory.cs | 4 +++- .../System.Private.CoreLib/src/System/IO/Directory.Unix.cs | 4 +--- .../System.Private.CoreLib/src/System/IO/Directory.Windows.cs | 2 -- .../System.Private.CoreLib/src/System/IO/Directory.cs | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs index 912db66d56d16c..cc26dd25b566a5 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs @@ -16,6 +16,7 @@ internal static partial class PathInternal internal const char PathSeparator = ':'; internal const string DirectorySeparatorCharAsString = "/"; internal const string ParentDirectoryPrefix = @"../"; + internal const string DirectorySeparators = DirectorySeparatorCharAsString; internal static int GetRootLength(ReadOnlySpan path) { diff --git a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs index 9015df805583e5..48c5509ef6126b 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs @@ -54,6 +54,7 @@ internal static partial class PathInternal internal const string UncNTPathPrefix = @"\??\UNC\"; internal const string DevicePathPrefix = @"\\.\"; internal const string ParentDirectoryPrefix = @"..\"; + internal const string DirectorySeparators = @"\/"; internal const int MaxShortPath = 260; internal const int MaxShortDirectoryPath = 248; diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs index f20f7f1f735b60..d12cb58cfd0451 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs @@ -31,12 +31,14 @@ public void CreateTempSubdirectory(string prefix) { Assert.True(tmpDir.Exists); Assert.Equal(-1, tmpDir.FullName.IndexOfAny(Path.GetInvalidPathChars())); - Assert.Empty(Directory.GetFiles(tmpDir.FullName)); + Assert.Empty(Directory.GetFileSystemEntries(tmpDir.FullName)); Assert.Equal(Path.TrimEndingDirectorySeparator(Path.GetTempPath()), tmpDir.Parent.FullName); if (!string.IsNullOrEmpty(prefix)) { Assert.StartsWith(prefix, tmpDir.Name); + int expectedNameLength = prefix.Length + (OperatingSystem.IsWindows() ? 12 : 6); + Assert.Equal(expectedNameLength, tmpDir.Name.Length); } // Ensure a file can be written to the directory diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs index d00f3e3fd0aaf7..391944d72a574b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs @@ -8,8 +8,6 @@ namespace System.IO { public static partial class Directory { - private static readonly char[] s_directorySeparators = new char[] { Path.DirectorySeparatorChar }; - private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixCreateMode) { ArgumentException.ThrowIfNullOrEmpty(path); @@ -41,7 +39,7 @@ private static unsafe string CreateTempSubdirectoryCore(string? prefix) } // 'name' is now the name of the directory - Debug.Assert(name[name.Length - 1] == '\0'); + Debug.Assert(name[^1] == '\0'); return Encoding.UTF8.GetString(name, 0, name.Length - 1); // trim off the trailing '\0' } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs index ff2b2b05658868..cc86edffa05ed9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs @@ -8,8 +8,6 @@ namespace System.IO { public static partial class Directory { - private static readonly char[] s_directorySeparators = new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }; - private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixCreateMode) => throw new PlatformNotSupportedException(SR.PlatformNotSupported_UnixFileMode); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs index f55df6e622c59c..372b120a9eafe5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs @@ -70,7 +70,7 @@ public static unsafe DirectoryInfo CreateTempSubdirectory(string? prefix = null) private static void EnsureNoDirectorySeparators(string? value, [CallerArgumentExpression("value")] string? paramName = null) { - if (!string.IsNullOrEmpty(value) && value.IndexOfAny(s_directorySeparators) != -1) + if (value is not null && value.AsSpan().IndexOfAny(PathInternal.DirectorySeparators) >= 0) { throw new ArgumentException(SR.Argument_DirectorySeparatorInvalid, paramName); } From 5b86fd34554a17bf0a05c342cc62e71dc7d5fe11 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 5 Aug 2022 16:47:47 -0500 Subject: [PATCH 3/7] PR feedback - Rewrite CreateTempSubdirectory and GetTempFileName on Unix to minimize allocations. --- .../Unix/System.Native/Interop.MkdTemp.cs | 2 +- .../Unix/System.Native/Interop.MksTemps.cs | 4 +-- .../tests/Directory/CreateTempSubdirectory.cs | 29 ++++++++++++++- .../src/System/IO/Directory.Unix.cs | 31 +++++++++++----- .../src/System/IO/Path.Unix.cs | 35 ++++++++++++------- .../tests/System/IO/PathTests.cs | 26 ++++++++++++++ 6 files changed, 102 insertions(+), 25 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs index c41413264507a4..59d6ace2dfa277 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MkdTemp.cs @@ -9,6 +9,6 @@ internal static partial class Interop internal static partial class Sys { [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MkdTemp", SetLastError = true)] - internal static unsafe partial byte* MkdTemp(byte[] template); + internal static unsafe partial byte* MkdTemp(byte* template); } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MksTemps.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MksTemps.cs index 95250e6c27aa0e..1ce54b66f560f6 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MksTemps.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.MksTemps.cs @@ -9,8 +9,8 @@ internal static partial class Interop internal static partial class Sys { [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_MksTemps", SetLastError = true)] - internal static partial IntPtr MksTemps( - byte[] template, + internal static unsafe partial IntPtr MksTemps( + byte* template, int suffixlen); } } diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs index d12cb58cfd0451..8fae0422653f69 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.IO.Tests @@ -11,7 +12,7 @@ public static TheoryData CreateTempSubdirectoryData { get { - var result = new TheoryData() { null, "", "myDir", "my.Dir" }; + var result = new TheoryData() { null, "", "myDir", "my.Dir", "H\u00EBllo" }; if (!OperatingSystem.IsWindows()) { // ensure we can use backslashes on Unix since that isn't a directory separator @@ -54,6 +55,32 @@ public void CreateTempSubdirectory(string prefix) } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void CreateTempSubdirectoryTempUnicode() + { + RemoteExecutor.Invoke(() => + { + DirectoryInfo tempPathWithUnicode = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + "\u00F6")); + tempPathWithUnicode.Create(); + + string tempEnvVar = OperatingSystem.IsWindows() ? "TEMP" : "TMPDIR"; + Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.FullName); + + try + { + DirectoryInfo tmpDir = Directory.CreateTempSubdirectory(); + Assert.True(tmpDir.Exists); + Assert.Equal(tempPathWithUnicode.FullName, tmpDir.Parent.FullName); + + Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.Parent.FullName); + } + finally + { + tempPathWithUnicode.Delete(recursive: true); + } + }).Dispose(); + } + public static TheoryData InvalidPrefixData { get diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs index 391944d72a574b..110985c11b56d5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs @@ -27,20 +27,33 @@ private static DirectoryInfo CreateDirectoryCore(string path, UnixFileMode unixC private static unsafe string CreateTempSubdirectoryCore(string? prefix) { // mkdtemp takes a char* and overwrites the XXXXXX with six characters - // that'll result in a unique file name. - string template = $"{Path.GetTempPath()}{prefix}XXXXXX\0"; - byte[] name = Encoding.UTF8.GetBytes(template); + // that'll result in a unique directory name. + string tempPath = Path.GetTempPath(); + int tempPathByteCount = Encoding.UTF8.GetByteCount(tempPath); + int prefixByteCount = prefix is not null ? Encoding.UTF8.GetByteCount(prefix) : 0; + int totalByteCount = tempPathByteCount + prefixByteCount + 6 + 1; + + Span path = totalByteCount <= 256 ? stackalloc byte[256].Slice(0, totalByteCount) : new byte[totalByteCount]; + int pos = Encoding.UTF8.GetBytes(tempPath, path); + if (prefix is not null) + { + pos += Encoding.UTF8.GetBytes(prefix, path.Slice(pos)); + } + path.Slice(pos, 6).Fill((byte)'X'); + path[pos + 6] = 0; // Create the temp directory. - byte* result = Interop.Sys.MkdTemp(name); - if (result == null) + fixed (byte* pPath = path) { - Interop.CheckIo(-1); + if (Interop.Sys.MkdTemp(pPath) is null) + { + Interop.CheckIo(-1); + } } - // 'name' is now the name of the directory - Debug.Assert(name[^1] == '\0'); - return Encoding.UTF8.GetString(name, 0, name.Length - 1); // trim off the trailing '\0' + // 'path' is now the name of the directory + Debug.Assert(path[^1] == 0); + return Encoding.UTF8.GetString(path.Slice(0, path.Length - 1)); // trim off the trailing '\0' } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs index 899dc764f3e3d3..89c4f2e276daaa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs @@ -93,23 +93,34 @@ public static string GetTempPath() path + PathInternal.DirectorySeparatorChar; } - public static string GetTempFileName() + public static unsafe string GetTempFileName() { - const string Suffix = ".tmp"; - const int SuffixByteLength = 4; + const int SuffixByteLength = 4; // ".tmp" // mkstemps takes a char* and overwrites the XXXXXX with six characters - // that'll result in a unique file name. - string template = GetTempPath() + "tmpXXXXXX" + Suffix + "\0"; - byte[] name = Encoding.UTF8.GetBytes(template); + // that'll result in a unique file name. The file template we use is "tmpXXXXXX.tmp". + string tempPath = Path.GetTempPath(); + int tempPathByteCount = Encoding.UTF8.GetByteCount(tempPath); + int totalByteCount = tempPathByteCount + 3 + 6 + SuffixByteLength + 1; + + Span path = totalByteCount <= 256 ? stackalloc byte[256].Slice(0, totalByteCount) : new byte[totalByteCount]; + int pos = Encoding.UTF8.GetBytes(tempPath, path); + "tmp"u8.CopyTo(path.Slice(pos)); + pos += 3; + path.Slice(pos, 6).Fill((byte)'X'); + pos += 6; + ".tmp"u8.CopyTo(path.Slice(pos)); + path[pos + SuffixByteLength] = 0; // Create, open, and close the temp file. - IntPtr fd = Interop.CheckIo(Interop.Sys.MksTemps(name, SuffixByteLength)); - Interop.Sys.Close(fd); // ignore any errors from close; nothing to do if cleanup isn't possible - - // 'name' is now the name of the file - Debug.Assert(name[name.Length - 1] == '\0'); - return Encoding.UTF8.GetString(name, 0, name.Length - 1); // trim off the trailing '\0' + fixed (byte* pPath = path) + { + IntPtr fd = Interop.CheckIo(Interop.Sys.MksTemps(pPath, SuffixByteLength)); + Interop.Sys.Close(fd); // ignore any errors from close; nothing to do if cleanup isn't possible + } + // 'path' is now the name of the file + Debug.Assert(path[^1] == 0); + return Encoding.UTF8.GetString(path.Slice(0, path.Length - 1)); // trim off the trailing '\0' } public static bool IsPathRooted([NotNullWhen(true)] string? path) diff --git a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs index 45f3b048c92a39..999863fc26d455 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs @@ -195,6 +195,32 @@ public void GetTempFileName() } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void GetTempFileNameTempUnicode() + { + RemoteExecutor.Invoke(() => + { + DirectoryInfo tempPathWithUnicode = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + "\u00F6")); + tempPathWithUnicode.Create(); + + string tempEnvVar = OperatingSystem.IsWindows() ? "TEMP" : "TMPDIR"; + Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.FullName); + + try + { + string tmpFile = Path.GetTempFileName(); + Assert.True(File.Exists(tmpFile)); + Assert.Equal(tempPathWithUnicode.FullName, Path.GetDirectoryName(tmpFile)); + + Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.Parent.FullName); + } + finally + { + tempPathWithUnicode.Delete(recursive: true); + } + }).Dispose(); + } + [Fact] public void GetFullPath_InvalidArgs() { From d744ac935c9f35f5810265c4f5c2440e750912eb Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 5 Aug 2022 17:08:58 -0500 Subject: [PATCH 4/7] Fix tests on Windows. --- .../tests/Directory/CreateTempSubdirectory.cs | 5 ++--- .../System.Runtime.Extensions/tests/System/IO/PathTests.cs | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs index 8fae0422653f69..0b190cd3dbe76a 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs @@ -63,7 +63,7 @@ public void CreateTempSubdirectoryTempUnicode() DirectoryInfo tempPathWithUnicode = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + "\u00F6")); tempPathWithUnicode.Create(); - string tempEnvVar = OperatingSystem.IsWindows() ? "TEMP" : "TMPDIR"; + string tempEnvVar = OperatingSystem.IsWindows() ? "TMP" : "TMPDIR"; Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.FullName); try @@ -100,8 +100,7 @@ public static TheoryData InvalidPrefixData [MemberData(nameof(InvalidPrefixData))] public void CreateTempSubdirectoryThrowsWithPrefixContainingDirectorySeparator(string prefix) { - ArgumentException e = Assert.Throws(() => Directory.CreateTempSubdirectory(prefix)); - Assert.Equal("prefix", e.ParamName); + AssertExtensions.Throws("prefix", () => Directory.CreateTempSubdirectory(prefix)); } } } diff --git a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs index 999863fc26d455..aa1709024868b7 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.DotNet.RemoteExecutor; using System.Collections.Generic; using System.Text; using Xunit; @@ -203,7 +204,7 @@ public void GetTempFileNameTempUnicode() DirectoryInfo tempPathWithUnicode = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + "\u00F6")); tempPathWithUnicode.Create(); - string tempEnvVar = OperatingSystem.IsWindows() ? "TEMP" : "TMPDIR"; + string tempEnvVar = OperatingSystem.IsWindows() ? "TMP" : "TMPDIR"; Environment.SetEnvironmentVariable(tempEnvVar, tempPathWithUnicode.FullName); try From dc910fd3ba3e0d9d8a673b026f62967d72e7b0ed Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 8 Aug 2022 11:38:43 -0500 Subject: [PATCH 5/7] Assert UnixFileMode is 700 for created temp directory. --- .../tests/Directory/CreateTempSubdirectory.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs index 0b190cd3dbe76a..2195693da87122 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/CreateTempSubdirectory.cs @@ -42,6 +42,12 @@ public void CreateTempSubdirectory(string prefix) Assert.Equal(expectedNameLength, tmpDir.Name.Length); } + if (!OperatingSystem.IsWindows()) + { + UnixFileMode userRWX = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute; + Assert.Equal(userRWX, tmpDir.UnixFileMode); + } + // Ensure a file can be written to the directory string tempFile = Path.Combine(tmpDir.FullName, "newFile"); using (FileStream fs = File.Create(tempFile, bufferSize: 1024, FileOptions.DeleteOnClose)) From 5fd8d41ab6c7f4c2b4f2af60e9eedee1436fb677 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 9 Aug 2022 15:01:22 -0500 Subject: [PATCH 6/7] PR feedback --- .../Common/src/Interop/Unix/Interop.IOErrors.cs | 8 ++++++++ .../src/System/IO/Directory.Unix.cs | 2 +- .../src/System/IO/Directory.Windows.cs | 1 + .../src/System/IO/Path.Unix.cs | 14 ++++++-------- .../PosixSignalRegistration.Unix.cs | 4 ++-- src/native/libs/System.Native/pal_io.h | 2 +- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 137b39405962ed..e0f01863c8010c 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -45,6 +45,14 @@ internal static long CheckIo(long result, string? path = null, bool isDirectory return result; } + /// + /// Throws an exception using the last error info (errno). + /// + internal static void ThrowExceptionForLastError() + { + ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path: null, isDirectory: false); + } + /// /// Validates the result of system call that returns greater than or equal to 0 on success /// and less than 0 on failure, with errno set to the error code. diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs index 110985c11b56d5..13088e6bff4117 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs @@ -47,7 +47,7 @@ private static unsafe string CreateTempSubdirectoryCore(string? prefix) { if (Interop.Sys.MkdTemp(pPath) is null) { - Interop.CheckIo(-1); + Interop.ThrowExceptionForLastError(); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs index cc86edffa05ed9..8861c97ca1d3d3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Windows.cs @@ -35,6 +35,7 @@ private static unsafe string CreateTempSubdirectoryCore(string? prefix) int attempts = 0; while (attempts < MaxAttempts) { + // simulate a call to Path.GetRandomFileName() without allocating an intermediate string Interop.GetRandomBytes(pKey, RandomKeyLength); Path.Populate83FileNameFromRandomBytes(pKey, RandomKeyLength, builder.RawChars.Slice(builder.Length, RandomFileNameLength)); builder.Length += RandomFileNameLength; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs index 89c4f2e276daaa..bc967e329950ca 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs @@ -96,21 +96,18 @@ public static string GetTempPath() public static unsafe string GetTempFileName() { const int SuffixByteLength = 4; // ".tmp" + ReadOnlySpan fileTemplate = "tmpXXXXXX.tmp"u8; // mkstemps takes a char* and overwrites the XXXXXX with six characters - // that'll result in a unique file name. The file template we use is "tmpXXXXXX.tmp". + // that'll result in a unique file name. string tempPath = Path.GetTempPath(); int tempPathByteCount = Encoding.UTF8.GetByteCount(tempPath); - int totalByteCount = tempPathByteCount + 3 + 6 + SuffixByteLength + 1; + int totalByteCount = tempPathByteCount + fileTemplate.Length + 1; Span path = totalByteCount <= 256 ? stackalloc byte[256].Slice(0, totalByteCount) : new byte[totalByteCount]; int pos = Encoding.UTF8.GetBytes(tempPath, path); - "tmp"u8.CopyTo(path.Slice(pos)); - pos += 3; - path.Slice(pos, 6).Fill((byte)'X'); - pos += 6; - ".tmp"u8.CopyTo(path.Slice(pos)); - path[pos + SuffixByteLength] = 0; + fileTemplate.CopyTo(path.Slice(pos)); + path[^1] = 0; // Create, open, and close the temp file. fixed (byte* pPath = path) @@ -118,6 +115,7 @@ public static unsafe string GetTempFileName() IntPtr fd = Interop.CheckIo(Interop.Sys.MksTemps(pPath, SuffixByteLength)); Interop.Sys.Close(fd); // ignore any errors from close; nothing to do if cleanup isn't possible } + // 'path' is now the name of the file Debug.Assert(path[^1] == 0); return Encoding.UTF8.GetString(path.Slice(0, path.Length - 1)); // trim off the trailing '\0' diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index d91dd1c3a8dc75..10a961ed9da480 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -15,7 +15,7 @@ private static unsafe Dictionary> Initialize() { if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { - Interop.CheckIo(-1); + Interop.ThrowExceptionForLastError(); } Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); @@ -44,7 +44,7 @@ private static PosixSignalRegistration Register(PosixSignal signal, Action Date: Tue, 9 Aug 2022 15:18:46 -0500 Subject: [PATCH 7/7] Rename ThrowExceptionForLastError to ThrowIOExceptionForLastError --- src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs | 4 ++-- .../System.Private.CoreLib/src/System/IO/Directory.Unix.cs | 2 +- .../Runtime/InteropServices/PosixSignalRegistration.Unix.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index e0f01863c8010c..f0dee88a4b61dc 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -46,9 +46,9 @@ internal static long CheckIo(long result, string? path = null, bool isDirectory } /// - /// Throws an exception using the last error info (errno). + /// Throws an IOException using the last error info (errno). /// - internal static void ThrowExceptionForLastError() + internal static void ThrowIOExceptionForLastError() { ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path: null, isDirectory: false); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs index 13088e6bff4117..26116cd19e4f78 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.Unix.cs @@ -47,7 +47,7 @@ private static unsafe string CreateTempSubdirectoryCore(string? prefix) { if (Interop.Sys.MkdTemp(pPath) is null) { - Interop.ThrowExceptionForLastError(); + Interop.ThrowIOExceptionForLastError(); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs index 10a961ed9da480..89c2eb45ac852f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/PosixSignalRegistration.Unix.cs @@ -15,7 +15,7 @@ private static unsafe Dictionary> Initialize() { if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { - Interop.ThrowExceptionForLastError(); + Interop.ThrowIOExceptionForLastError(); } Interop.Sys.SetPosixSignalHandler(&OnPosixSignal); @@ -44,7 +44,7 @@ private static PosixSignalRegistration Register(PosixSignal signal, Action