Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GetTempFileName() on Windows #74855

Merged
merged 14 commits into from
Sep 2, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@
<value>Invalid type for ParameterInfo member in Attribute class.</value>
</data>
<data name="Argument_InvalidPathChars" xml:space="preserve">
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
<value>Illegal characters in path.</value>
<value>Null character in path.</value>
</data>
<data name="Argument_InvalidResourceCultureName" xml:space="preserve">
<value>The given culture name '{0}' cannot be used to locate a resource file. Resource filenames must consist of only letters, numbers, hyphens or underscores.</value>
Expand Down
60 changes: 46 additions & 14 deletions src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,25 +208,57 @@ static uint GetTempPathW(int bufferLen, ref char buffer)
// name on disk.
public static string GetTempFileName()
{
var tempPathBuilder = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]);

GetTempPath(ref tempPathBuilder);

var builder = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]);
// Avoid GetTempFileNameW because it is limited to 0xFFFF possibilities, which both
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
// means that it may have to make many attempts to create the file before
// finding an unused name, and also that if an app "leaks" such temp files,
// it can prevent GetTempFileNameW succeeding at all.
//
// To make this a little more robust, generate our own name with more
// entropy. We could use GetRandomFileName() here, but for consistency
// with Unix and to retain the ".tmp" extension we will use the "tmpXXXXXX.tmp" pattern.
// Using 32 characters for convenience, that gives us 32^^6 ~= 10^^9 possibilities,
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
// but we'll still loop to handle the unlikely case the file already exists.

const int KeyLength = 4;
byte* bytes = stackalloc byte[KeyLength];
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
danmoseley marked this conversation as resolved.
Show resolved Hide resolved

Span<char> span = stackalloc char[13]; // tmpXXXXXX.tmp
span[0] = span[10] = 't';
span[1] = span[11] = 'm';
span[2] = span[12] = 'p';
span[9] = '.';

int i = 0;
while (true)
{
Interop.GetRandomBytes(bytes, KeyLength); // 4 bytes = more than 6 x 5 bits

uint result = Interop.Kernel32.GetTempFileNameW(
ref tempPathBuilder.GetPinnableReference(), "tmp", 0, ref builder.GetPinnableReference());
byte b0 = bytes[0];
byte b1 = bytes[1];
byte b2 = bytes[2];
byte b3 = bytes[3];

tempPathBuilder.Dispose();
span[3] = (char)Base32Char[b0 & 0b0001_1111];
span[4] = (char)Base32Char[b1 & 0b0001_1111];
span[5] = (char)Base32Char[b2 & 0b0001_1111];
span[6] = (char)Base32Char[b3 & 0b0001_1111];
span[7] = (char)Base32Char[((b0 & 0b1110_0000) >> 5) | ((b1 & 0b1100_0000) >> 3)];
span[8] = (char)Base32Char[((b2 & 0b1110_0000) >> 5) | ((b3 & 0b1100_0000) >> 3)];

if (result == 0)
throw Win32Marshal.GetExceptionForLastWin32Error();
string path = string.Concat(Path.GetTempPath(), span);
danmoseley marked this conversation as resolved.
Show resolved Hide resolved

builder.Length = builder.RawChars.IndexOf('\0');
try
{
File.OpenHandle(path, FileMode.CreateNew, FileAccess.Write).Dispose();
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}
catch (IOException ex) when (i < 100 && Win32Marshal.TryMakeWin32ErrorCodeFromHR(ex.HResult) == Interop.Errors.ERROR_FILE_EXISTS)
{
i++; // If there's a read-only temp volume, don't loop forever
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
continue; // File already exists: very, very unlikely
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}

string path = PathHelper.Normalize(ref builder);
builder.Dispose();
return path;
return path;
}
}

// Tests if the given path contains a root. A path is considered rooted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,17 @@ public void GetTempFileName()
try
{
Assert.True(File.Exists(tmpFile));
Assert.Equal(".tmp", Path.GetExtension(tmpFile), ignoreCase: true, ignoreLineEndingDifferences: false, ignoreWhiteSpaceDifferences: false);
string fileName = Path.GetFileName(tmpFile);
Assert.StartsWith("tmp", fileName);
Assert.Equal("tmpXXXXXX.tmp".Length, fileName.Length);
Assert.EndsWith(".tmp", fileName);

const string validChars = "abcdefghijklmnopqrstuvwxyz0123456789";
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
for (int i = 3; i < 9; i++)
{
Assert.True(validChars.Contains(char.ToLowerInvariant(fileName[i])));
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}

Assert.Equal(-1, tmpFile.IndexOfAny(Path.GetInvalidPathChars()));
using (FileStream fs = File.OpenRead(tmpFile))
{
Expand Down