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
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
70 changes: 52 additions & 18 deletions src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs
Expand Up @@ -208,25 +208,59 @@ 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.

string tempPath = Path.GetTempPath();
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

while (true)
{
string path = string.Create(tempPath.Length + 13 /* tmpXXXXXX.tmp */, (IntPtr)bytes, (span, state) =>
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
{
tempPath.TryCopyTo(span);

span = span.Slice(tempPath.Length);

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] = '.';
});

try
{
File.OpenHandle(path, FileMode.CreateNew, FileAccess.Write).Dispose();
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}
catch (IOException)
{
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
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 = "abcdefghijklmnopqrstuvwxyz012345";
for (int i = 3; i < 9; i++)
{
Assert.True(validChars.Contains(fileName[i]));
}

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