Diff
Modified: trunk/Source/WebCore/ChangeLog (221440 => 221441)
--- trunk/Source/WebCore/ChangeLog 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/ChangeLog 2017-08-31 20:53:38 UTC (rev 221441)
@@ -1,3 +1,57 @@
+2017-08-31 Chris Dumez <[email protected]>
+
+ getFileMetadata() does not work as expected for symbolic links
+ https://bugs.webkit.org/show_bug.cgi?id=176143
+
+ Reviewed by Andreas Kling.
+
+ getFileMetadata() does not work as expected for symbolic links:
+ On POSIX, getFileMetadata() always followed symlinks, which meant that FileMetadata.type could
+ never be TypeSymbolicLink. On Windows, the function properly did not follow symlinks but failed to set
+ FileMetadata.type to TypeSymbolicLink when the file was a symbolic link.
+
+ This patch adds a new ShouldFollowSymbolicLinks parameter to getFileMetadata() so that
+ the call site can decide the behavior it wants. If getFileMetadata() is called on a
+ symbolic link with ShouldFollowSymbolicLinks::No as parameter, FileMetadata.type is now
+ properly set to TypeSymbolicLink.
+
+ No new tests, covered by new API test.
+
+ * Modules/entriesapi/DOMFileSystem.cpp:
+ (WebCore::listDirectoryWithMetadata):
+ It is important we do not follow symlinks here since the code wants to discard them
+ and does so by checking FileMetadata.type.
+
+ * WebCore.xcodeproj/project.pbxproj:
+ * fileapi/File.cpp:
+ (WebCore::File::isDirectory const):
+
+ * html/FileListCreator.cpp:
+ (WebCore::appendDirectoryFiles):
+ (WebCore::FileListCreator::createFileList):
+ It is important we do not follow symlinks here since the code wants to discard them
+ and does so by checking FileMetadata.type.
+
+ * platform/FileSystem.cpp:
+ (WebCore::fileIsDirectory):
+ * platform/FileSystem.h:
+ * platform/glib/FileSystemGlib.cpp:
+ (WebCore::getFileLStat):
+ (WebCore::getFileMetadata):
+ * platform/posix/FileSystemPOSIX.cpp:
+ (WebCore::getFileMetadata):
+ (WebCore::createSymbolicLink):
+ * platform/win/FileSystemWin.cpp:
+ (WebCore::getFinalPathName):
+ (WebCore::getFileMetadata):
+ (WebCore::createSymbolicLink):
+ - Add new createSymbolicLink() function for testing purposes.
+ - On Posix, call lstat() instead of stat if ShouldFollowSymbolicLinks::No.
+ - On Windows, since FindFirstFileW() does not follow symlinks, resolve
+ final path using GetFinalPathNameByHandleW() if ShouldFollowSymbolicLinks::Yes.
+ - On Windows, properly set FileMetadata.type to TypeSymbolicLink if the file
+ is a symbolic link.
+
2017-08-31 Michael Catanzaro <[email protected]>
REGRESSION(r221226): [SOUP] libsoup-CRITICAL **: soup_cookies_to_cookie_header: assertion 'cookies != NULL' failed
Modified: trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp (221440 => 221441)
--- trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -46,7 +46,7 @@
static ExceptionOr<Vector<ListedChild>> listDirectoryWithMetadata(const String& fullPath)
{
ASSERT(!isMainThread());
- if (!fileIsDirectory(fullPath))
+ if (!fileIsDirectory(fullPath, ShouldFollowSymbolicLinks::No))
return Exception { NotFoundError, "Path no longer exists or is no longer a directory" };
auto childPaths = listDirectory(fullPath, "*");
@@ -54,7 +54,7 @@
listedChildren.reserveInitialCapacity(childPaths.size());
for (auto& childPath : childPaths) {
FileMetadata metadata;
- if (!getFileMetadata(childPath, metadata))
+ if (!getFileMetadata(childPath, metadata, ShouldFollowSymbolicLinks::No))
continue;
listedChildren.uncheckedAppend(ListedChild { pathGetFileName(childPath), metadata.type });
}
Modified: trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj (221440 => 221441)
--- trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj 2017-08-31 20:53:38 UTC (rev 221441)
@@ -1939,7 +1939,7 @@
467302021C4EFE7800BCB357 /* IgnoreOpensDuringUnloadCountIncrementer.h in Headers */ = {isa = PBXBuildFile; fileRef = 467302011C4EFE6600BCB357 /* IgnoreOpensDuringUnloadCountIncrementer.h */; };
468344DF1EDDFAAA00B7795B /* DOMRectList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 468344DD1EDDFA5F00B7795B /* DOMRectList.cpp */; };
468344E01EDDFAAA00B7795B /* DOMRectList.h in Headers */ = {isa = PBXBuildFile; fileRef = 468344DE1EDDFA5F00B7795B /* DOMRectList.h */; settings = {ATTRIBUTES = (Private, ); }; };
- 4689F1AF1267BAE100E8D380 /* FileMetadata.h in Headers */ = {isa = PBXBuildFile; fileRef = 4689F1AE1267BAE100E8D380 /* FileMetadata.h */; };
+ 4689F1AF1267BAE100E8D380 /* FileMetadata.h in Headers */ = {isa = PBXBuildFile; fileRef = 4689F1AE1267BAE100E8D380 /* FileMetadata.h */; settings = {ATTRIBUTES = (Private, ); }; };
46B63F6C1C6E8D19002E914B /* JSEventTargetCustom.h in Headers */ = {isa = PBXBuildFile; fileRef = 46B63F6B1C6E8CDF002E914B /* JSEventTargetCustom.h */; settings = {ATTRIBUTES = (Private, ); }; };
46C696CB1E7205F700597937 /* CPUMonitor.h in Headers */ = {isa = PBXBuildFile; fileRef = 46C696C91E7205E400597937 /* CPUMonitor.h */; settings = {ATTRIBUTES = (Private, ); }; };
46C696CC1E7205FC00597937 /* CPUMonitor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 46C696CA1E7205E400597937 /* CPUMonitor.cpp */; };
Modified: trunk/Source/WebCore/fileapi/File.cpp (221440 => 221441)
--- trunk/Source/WebCore/fileapi/File.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/fileapi/File.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -130,7 +130,7 @@
bool File::isDirectory() const
{
if (!m_isDirectory)
- m_isDirectory = fileIsDirectory(m_path);
+ m_isDirectory = fileIsDirectory(m_path, ShouldFollowSymbolicLinks::Yes);
return *m_isDirectory;
}
Modified: trunk/Source/WebCore/html/FileListCreator.cpp (221440 => 221441)
--- trunk/Source/WebCore/html/FileListCreator.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/html/FileListCreator.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -43,7 +43,7 @@
{
for (auto& childPath : listDirectory(directory, "*")) {
FileMetadata metadata;
- if (!getFileMetadata(childPath, metadata))
+ if (!getFileMetadata(childPath, metadata, ShouldFollowSymbolicLinks::No))
continue;
if (metadata.isHidden)
@@ -80,7 +80,7 @@
{
Vector<RefPtr<File>> fileObjects;
for (auto& info : paths) {
- if (shouldResolveDirectories == ShouldResolveDirectories::Yes && fileIsDirectory(info.path))
+ if (shouldResolveDirectories == ShouldResolveDirectories::Yes && fileIsDirectory(info.path, ShouldFollowSymbolicLinks::No))
appendDirectoryFiles(info.path, pathGetFileName(info.path), fileObjects);
else
fileObjects.append(File::createWithName(info.path, info.displayName));
Modified: trunk/Source/WebCore/platform/FileSystem.cpp (221440 => 221441)
--- trunk/Source/WebCore/platform/FileSystem.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/FileSystem.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -351,10 +351,10 @@
closeFile(handle);
}
-bool fileIsDirectory(const String& path)
+bool fileIsDirectory(const String& path, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks)
{
FileMetadata metadata;
- if (!getFileMetadata(path, metadata))
+ if (!getFileMetadata(path, metadata, shouldFollowSymbolicLinks))
return false;
return metadata.type == FileMetadata::TypeDirectory;
}
Modified: trunk/Source/WebCore/platform/FileSystem.h (221440 => 221441)
--- trunk/Source/WebCore/platform/FileSystem.h 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/FileSystem.h 2017-08-31 20:53:38 UTC (rev 221441)
@@ -88,6 +88,8 @@
LockNonBlocking = 4
};
+enum class ShouldFollowSymbolicLinks { No, Yes };
+
struct FileMetadata;
WEBCORE_EXPORT bool fileExists(const String&);
@@ -98,8 +100,8 @@
WEBCORE_EXPORT bool getFileSize(PlatformFileHandle, long long& result);
WEBCORE_EXPORT bool getFileModificationTime(const String&, time_t& result);
WEBCORE_EXPORT bool getFileCreationTime(const String&, time_t& result); // Not all platforms store file creation time.
-bool getFileMetadata(const String&, FileMetadata&);
-bool fileIsDirectory(const String&);
+WEBCORE_EXPORT bool getFileMetadata(const String&, FileMetadata&, ShouldFollowSymbolicLinks);
+bool fileIsDirectory(const String&, ShouldFollowSymbolicLinks);
WEBCORE_EXPORT String pathByAppendingComponent(const String& path, const String& component);
String pathByAppendingComponents(const String& path, const Vector<String>& components);
String lastComponentOfPathIgnoringTrailingSlash(const String& path);
@@ -109,6 +111,7 @@
WEBCORE_EXPORT String directoryName(const String&);
WEBCORE_EXPORT bool getVolumeFreeSpace(const String&, uint64_t&);
WEBCORE_EXPORT std::optional<int32_t> getFileDeviceId(const CString&);
+WEBCORE_EXPORT bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath);
WEBCORE_EXPORT void setMetadataURL(const String& path, const String& urlString, const String& referrer = { });
Modified: trunk/Source/WebCore/platform/glib/FileSystemGlib.cpp (221440 => 221441)
--- trunk/Source/WebCore/platform/glib/FileSystemGlib.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/glib/FileSystemGlib.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -123,6 +123,15 @@
return g_stat(filename.get(), statBuffer) != -1;
}
+static bool getFileLStat(const String& path, GStatBuf* statBuffer)
+{
+ GUniquePtr<gchar> filename = unescapedFilename(path);
+ if (!filename)
+ return false;
+
+ return g_lstat(filename.get(), statBuffer) != -1;
+}
+
bool getFileSize(const String& path, long long& resultSize)
{
GStatBuf statResult;
@@ -155,12 +164,18 @@
return true;
}
-bool getFileMetadata(const String& path, FileMetadata& metadata)
+bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks)
{
GStatBuf statResult;
- if (!getFileStat(path, &statResult))
- return false;
+ if (shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) {
+ if (!getFileStat(path, &statResult))
+ return false;
+ } else {
+ if (!getFileLStat(path, &statResult))
+ return false;
+ }
+
String filename = pathGetFileName(path);
metadata.isHidden = !filename.isEmpty() && filename[0] == '.';
metadata.modificationTime = statResult.st_mtime;
@@ -203,6 +218,19 @@
return stringFromFileSystemRepresentation(g_get_home_dir());
}
+bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath)
+{
+ CString targetPathFSRep = fileSystemRepresentation(targetPath);
+ if (!targetPathFSRep.data() || targetPathFSRep.data()[0] == '\0')
+ return false;
+
+ CString symbolicLinkPathFSRep = fileSystemRepresentation(symbolicLinkPath);
+ if (!symbolicLinkPathFSRep.data() || symbolicLinkPathFSRep.data()[0] == '\0')
+ return false;
+
+ return !symlink(targetPathFSRep.data(), symbolicLinkPathFSRep.data());
+}
+
String pathGetFileName(const String& pathName)
{
GUniquePtr<gchar> tmpFilename = unescapedFilename(pathName);
Modified: trunk/Source/WebCore/platform/network/BlobDataFileReference.cpp (221440 => 221441)
--- trunk/Source/WebCore/platform/network/BlobDataFileReference.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/network/BlobDataFileReference.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -96,7 +96,7 @@
// FIXME: Some platforms provide better ways to listen for file system object changes, consider using these.
FileMetadata metadata;
- if (!getFileMetadata(m_path, metadata))
+ if (!getFileMetadata(m_path, metadata, ShouldFollowSymbolicLinks::Yes))
return;
m_expectedModificationTime = metadata.modificationTime;
Modified: trunk/Source/WebCore/platform/network/mac/BlobDataFileReferenceMac.mm (221440 => 221441)
--- trunk/Source/WebCore/platform/network/mac/BlobDataFileReferenceMac.mm 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/network/mac/BlobDataFileReferenceMac.mm 2017-08-31 20:53:38 UTC (rev 221441)
@@ -80,7 +80,7 @@
m_replacementShouldBeGenerated = false;
if (!m_replacementPath.isNull()) {
FileMetadata metadata;
- if (getFileMetadata(m_replacementPath, metadata))
+ if (getFileMetadata(m_replacementPath, metadata, ShouldFollowSymbolicLinks::Yes))
m_size = metadata.length;
}
Modified: trunk/Source/WebCore/platform/posix/FileSystemPOSIX.cpp (221440 => 221441)
--- trunk/Source/WebCore/platform/posix/FileSystemPOSIX.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/posix/FileSystemPOSIX.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -239,7 +239,7 @@
return true;
}
-bool getFileMetadata(const String& path, FileMetadata& metadata)
+bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks)
{
CString fsRep = fileSystemRepresentation(path);
@@ -247,9 +247,15 @@
return false;
struct stat fileInfo;
- if (stat(fsRep.data(), &fileInfo))
- return false;
+ if (shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) {
+ if (stat(fsRep.data(), &fileInfo))
+ return false;
+ } else {
+ if (lstat(fsRep.data(), &fileInfo))
+ return false;
+ }
+
String filename = pathGetFileName(path);
metadata.modificationTime = fileInfo.st_mtime;
@@ -264,6 +270,19 @@
return true;
}
+bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath)
+{
+ CString targetPathFSRep = fileSystemRepresentation(targetPath);
+ if (!targetPathFSRep.data() || targetPathFSRep.data()[0] == '\0')
+ return false;
+
+ CString symbolicLinkPathFSRep = fileSystemRepresentation(symbolicLinkPath);
+ if (!symbolicLinkPathFSRep.data() || symbolicLinkPathFSRep.data()[0] == '\0')
+ return false;
+
+ return !symlink(targetPathFSRep.data(), symbolicLinkPathFSRep.data());
+}
+
String pathByAppendingComponent(const String& path, const String& component)
{
if (path.endsWith('/'))
Modified: trunk/Source/WebCore/platform/win/FileSystemWin.cpp (221440 => 221441)
--- trunk/Source/WebCore/platform/win/FileSystemWin.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Source/WebCore/platform/win/FileSystemWin.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -141,12 +141,39 @@
return true;
}
-bool getFileMetadata(const String& path, FileMetadata& metadata)
+static String getFinalPathName(const String& path)
{
+ auto handle = openFile(path, OpenForRead);
+ if (!isHandleValid(handle))
+ return String();
+
+ StringVector<UChar> buffer(MAX_PATH);
+ if (::GetFinalPathNameByHandleW(handle, buffer.data(), buffer.size(), VOLUME_NAME_NT) >= MAX_PATH) {
+ closeFile(handle);
+ return String();
+ }
+ closeFile(handle);
+
+ buffer.shrink(wcslen(buffer.data()));
+ return String::adopt(WTFMove(buffer));
+}
+
+bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks)
+{
WIN32_FIND_DATAW findData;
if (!getFindData(path, findData))
return false;
+ bool isSymbolicLink = findData.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT && findData.dwReserved0 == IO_REPARSE_TAG_SYMLINK;
+ if (isSymbolicLink && shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) {
+ String targetPath = getFinalPathName(path);
+ if (targetPath.isNull())
+ return false;
+ if (!getFindData(targetPath, findData))
+ return false;
+ isSymbolicLink = false;
+ }
+
if (!getFileSizeFromFindData(findData, metadata.length))
return false;
@@ -154,11 +181,21 @@
getFileModificationTimeFromFindData(findData, modificationTime);
metadata.modificationTime = modificationTime;
metadata.isHidden = findData.dwFileAttributes & FILE_ATTRIBUTE_HIDDEN;
- metadata.type = (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? FileMetadata::TypeDirectory : FileMetadata::TypeFile;
+ if (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+ metadata.type = FileMetadata::TypeDirectory;
+ else if (isSymbolicLink)
+ metadata.type = FileMetadata::TypeSymbolicLink;
+ else
+ metadata.type = FileMetadata::TypeFile;
return true;
}
+bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath)
+{
+ return !::CreateSymbolicLinkW(symbolicLinkPath.charactersWithNullTermination().data(), targetPath.charactersWithNullTermination().data(), 0);
+}
+
bool fileExists(const String& path)
{
WIN32_FIND_DATAW findData;
Modified: trunk/Tools/ChangeLog (221440 => 221441)
--- trunk/Tools/ChangeLog 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Tools/ChangeLog 2017-08-31 20:53:38 UTC (rev 221441)
@@ -1,3 +1,15 @@
+2017-08-31 Chris Dumez <[email protected]>
+
+ getFileMetadata() does not work as expected for symbolic links
+ https://bugs.webkit.org/show_bug.cgi?id=176143
+
+ Reviewed by Andreas Kling.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebCore/FileSystem.cpp:
+ (TestWebKitAPI::TEST_F):
+
2017-08-31 Filip Pizlo <[email protected]>
WSL parser should pass the token as the origin to the AST
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp (221440 => 221441)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp 2017-08-31 20:50:55 UTC (rev 221440)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp 2017-08-31 20:53:38 UTC (rev 221441)
@@ -27,6 +27,7 @@
#include "config.h"
#include "Test.h"
+#include <WebCore/FileMetadata.h>
#include <WebCore/FileSystem.h>
#include <wtf/MainThread.h>
#include <wtf/StringExtras.h>
@@ -48,8 +49,11 @@
PlatformFileHandle handle;
m_tempFilePath = openTemporaryFile("tempTestFile", handle);
writeToFile(handle, FileSystemTestData, strlen(FileSystemTestData));
- closeFile(handle);
+ closeFile(handle);
+ m_tempFileSymlinkPath = m_tempFilePath + "-symlink";
+ createSymbolicLink(m_tempFilePath, m_tempFileSymlinkPath);
+
m_tempEmptyFilePath = openTemporaryFile("tempEmptyTestFile", handle);
closeFile(handle);
@@ -66,6 +70,7 @@
void TearDown() override
{
deleteFile(m_tempFilePath);
+ deleteFile(m_tempFileSymlinkPath);
deleteFile(m_tempEmptyFilePath);
deleteFile(m_spaceContainingFilePath);
deleteFile(m_bangContainingFilePath);
@@ -73,6 +78,7 @@
}
const String& tempFilePath() { return m_tempFilePath; }
+ const String& tempFileSymlinkPath() { return m_tempFileSymlinkPath; }
const String& tempEmptyFilePath() { return m_tempEmptyFilePath; }
const String& spaceContainingFilePath() { return m_spaceContainingFilePath; }
const String& bangContainingFilePath() { return m_bangContainingFilePath; }
@@ -80,6 +86,7 @@
private:
String m_tempFilePath;
+ String m_tempFileSymlinkPath;
String m_tempEmptyFilePath;
String m_spaceContainingFilePath;
String m_bangContainingFilePath;
@@ -119,4 +126,17 @@
EXPECT_TRUE(filesHaveSameVolume(bangContainingFilePath(), quoteContainingFilePath()));
}
+TEST_F(FileSystemTest, GetFileMetadataSymlink)
+{
+ FileMetadata symlinkMetadata;
+ EXPECT_TRUE(getFileMetadata(tempFileSymlinkPath(), symlinkMetadata, ShouldFollowSymbolicLinks::No));
+ EXPECT_TRUE(symlinkMetadata.type == FileMetadata::TypeSymbolicLink);
+ EXPECT_FALSE(static_cast<size_t>(symlinkMetadata.length) == strlen(FileSystemTestData));
+
+ FileMetadata targetMetadata;
+ EXPECT_TRUE(getFileMetadata(tempFileSymlinkPath(), targetMetadata, ShouldFollowSymbolicLinks::Yes));
+ EXPECT_TRUE(targetMetadata.type == FileMetadata::TypeFile);
+ EXPECT_EQ(strlen(FileSystemTestData), static_cast<size_t>(targetMetadata.length));
}
+
+}
