Skip to content

Commit 0a4e6af

Browse files
aperezdcaperezdc
committed
Content rule list store may end up copying compiled bytecode files
https://bugs.webkit.org/show_bug.cgi?id=315236 Reviewed by Chris Dumez and Michael Catanzaro. Add a third parameter to WTF::openTemporaryFile() that allows overriding the temporary directory where to create the file. Then the APIContentRuleListStore can take advantage of this avoid hitting the case in which the temporary directory is different from that of the resulting file, thus avoiding the file copy fallback when calling WTF::moveFile() down the line to place the compiled bytecode file into its final destination. * Source/WTF/wtf/FileSystem.h: (WTF::FileSystemImpl::openTemporaryFile): * Source/WTF/wtf/cocoa/FileSystemCocoa.mm: (WTF::FileSystemImpl::openTemporaryFile): * Source/WTF/wtf/posix/FileSystemPOSIX.cpp: (WTF::FileSystemImpl::openTemporaryFile): * Source/WTF/wtf/win/FileSystemWin.cpp: (WTF::FileSystemImpl::openTemporaryFile): * Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp: (API::compiledToFile): * Tools/TestWebKitAPI/Tests/WTF/FileSystem.cpp: (TestWebKitAPI::TEST_F(FileSystemTest, createTemporaryFileInDirectory)): Canonical link: https://commits.webkit.org/313681@main
1 parent 3aac827 commit 0a4e6af

6 files changed

Lines changed: 32 additions & 11 deletions

File tree

‎Source/WTF/wtf/FileSystem.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ WTF_EXPORT_PRIVATE std::optional<Vector<uint8_t>> readEntireFile(const String& p
143143
WTF_EXPORT_PRIVATE std::optional<uint64_t> overwriteEntireFile(const String& path, std::span<const uint8_t>);
144144

145145
// Prefix is what the filename should be prefixed with, not the full path.
146-
WTF_EXPORT_PRIVATE std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix = { });
146+
WTF_EXPORT_PRIVATE std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix = { }, const String& temporaryDirectory = { });
147147
WTF_EXPORT_PRIVATE String createTemporaryFile(StringView prefix, StringView suffix = { });
148148
#if PLATFORM(COCOA)
149149
WTF_EXPORT_PRIVATE std::pair<FileHandle, CString> createTemporaryFileInDirectory(const String& directory, const String& suffix);

‎Source/WTF/wtf/cocoa/FileSystemCocoa.mm‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,22 @@ String extractTemporaryZipArchive(const String& path)
132132
return temporaryDirectory;
133133
}
134134

135-
std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix)
135+
std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix, const String& temporaryDirectory)
136136
{
137137
Vector<char> temporaryFilePath(PATH_MAX);
138-
if (!confstr(_CS_DARWIN_USER_TEMP_DIR, temporaryFilePath.mutableSpan().data(), temporaryFilePath.size()))
139-
return { String(), FileHandle() };
140-
141-
// Shrink the vector.
142-
temporaryFilePath.shrink(strlenSpan(temporaryFilePath.span()));
138+
if (temporaryDirectory.isEmpty()) {
139+
if (!confstr(_CS_DARWIN_USER_TEMP_DIR, temporaryFilePath.mutableSpan().data(), temporaryFilePath.size()))
140+
return { String(), FileHandle() };
141+
// Shrink the vector.
142+
temporaryFilePath.shrink(strlenSpan(temporaryFilePath.span()));
143+
} else {
144+
const auto temporaryDirectoryUtf8 = temporaryDirectory.utf8();
145+
memcpySpan(temporaryFilePath.mutableSpan(), temporaryDirectoryUtf8.span());
146+
// Shrink the vector.
147+
temporaryFilePath.shrink(temporaryDirectoryUtf8.length());
148+
if (temporaryDirectoryUtf8.span().back() != '/')
149+
temporaryFilePath.append('/');
150+
}
143151

144152
ASSERT(temporaryFilePath.last() == '/');
145153

‎Source/WTF/wtf/posix/FileSystemPOSIX.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,14 +173,15 @@ static const char* temporaryFileDirectory()
173173
#endif
174174
}
175175

176-
std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix)
176+
std::pair<String, FileHandle> openTemporaryFile(StringView prefix, StringView suffix, const String& temporaryDirectory)
177177
{
178178
// Suffix is not supported because that's incompatible with mkostemp, mkostemps would be needed for that.
179179
// This is OK for now since the code using it is built on macOS only.
180180
ASSERT_UNUSED(suffix, suffix.isEmpty());
181181

182+
const auto temporaryDirectoryUtf8 = temporaryDirectory.utf8();
182183
IGNORE_CLANG_WARNINGS_BEGIN("unsafe-buffer-usage-in-libc-call")
183-
const char* directory = temporaryFileDirectory();
184+
const char* directory = !temporaryDirectory.isEmpty() ? temporaryDirectoryUtf8.data() : temporaryFileDirectory();
184185
CString prefixUTF8 = prefix.utf8();
185186
size_t length = strlen(directory) + 1 + prefixUTF8.length() + 1 + 6 + 1;
186187
auto buffer = MallocSpan<char>::malloc(length);

‎Source/WTF/wtf/win/FileSystemWin.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ static String generateTemporaryPath(const Function<bool(const String&)>& action)
159159
return proposedPath;
160160
}
161161

162-
std::pair<String, FileHandle> openTemporaryFile(StringView, StringView suffix)
162+
std::pair<String, FileHandle> openTemporaryFile(StringView, StringView suffix, const String&)
163163
{
164164
// FIXME: Suffix is not supported, but OK for now since the code using it is macOS-port-only.
165165
ASSERT_UNUSED(suffix, suffix.isEmpty());

‎Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ static Expected<MappedData, std::error_code> compiledToFile(WTF::String&& json,
424424
bool m_hadFileError { false };
425425
};
426426

427-
auto [temporaryFilePath, temporaryFileHandle] = openTemporaryFile("ContentRuleList"_s);
427+
auto [temporaryFilePath, temporaryFileHandle] = openTemporaryFile("ContentRuleList"_s, { }, parentPath(finalFilePath));
428428
if (!temporaryFileHandle) {
429429
RELEASE_LOG_ERROR(ContentRuleLists, "Content Rule List compiling failed: Opening temporary file failed.");
430430
return makeUnexpected(ContentRuleListStore::Error::CompileFailed);

‎Tools/TestWebKitAPI/Tests/WTF/FileSystem.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,4 +1008,16 @@ TEST_F(FileSystemTest, isAncestor)
10081008
);
10091009
}
10101010

1011+
#if !OS(WINDOWS)
1012+
// The third parameter to WTF::openTemporaryFile() is ignored on Windows.
1013+
TEST_F(FileSystemTest, createTemporaryFileInDirectory)
1014+
{
1015+
auto [filePath, fileHandle] = FileSystem::openTemporaryFile("tempTestFile"_s, { }, tempEmptyFolderPath());
1016+
EXPECT_TRUE(!!fileHandle);
1017+
EXPECT_TRUE(FileSystem::fileType(filePath) == FileSystem::FileType::Regular);
1018+
EXPECT_TRUE(FileSystem::isAncestor(tempEmptyFolderPath(), filePath));
1019+
EXPECT_TRUE(FileSystem::parentPath(filePath) == tempEmptyFolderPath());
1020+
}
1021+
#endif
1022+
10111023
} // namespace TestWebKitAPI

0 commit comments

Comments
 (0)