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
67 changes: 49 additions & 18 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,56 @@ 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]);

uint result = Interop.Kernel32.GetTempFileNameW(
ref tempPathBuilder.GetPinnableReference(), "tmp", 0, ref builder.GetPinnableReference());

tempPathBuilder.Dispose();

if (result == 0)
throw Win32Marshal.GetExceptionForLastWin32Error();

builder.Length = builder.RawChars.IndexOf('\0');
// 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.

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

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

byte b0 = bytes[0];
byte b1 = bytes[1];
byte b2 = bytes[2];
byte b3 = bytes[3];

span[0] = span[10] = 't';
span[1] = span[11] = 'm';
span[2] = span[12] = 'p';
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)];
span[9] = '.';

string path = string.Concat(Path.GetTempPath(), span);
danmoseley marked this conversation as resolved.
Show resolved Hide resolved

try
{
File.OpenHandle(path, FileMode.CreateNew, FileAccess.Write).Dispose();
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}
catch (IOException) when (i < 100)
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
{
i++; // If TMP is invalid, don't loop forever
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