Changeset 221441 in webkit
- Timestamp:
- Aug 31, 2017 1:53:38 PM (7 years ago)
- Location:
- trunk
- Files:
-
- 14 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r221438 r221441 1 2017-08-31 Chris Dumez <cdumez@apple.com> 2 3 getFileMetadata() does not work as expected for symbolic links 4 https://bugs.webkit.org/show_bug.cgi?id=176143 5 6 Reviewed by Andreas Kling. 7 8 getFileMetadata() does not work as expected for symbolic links: 9 On POSIX, getFileMetadata() always followed symlinks, which meant that FileMetadata.type could 10 never be TypeSymbolicLink. On Windows, the function properly did not follow symlinks but failed to set 11 FileMetadata.type to TypeSymbolicLink when the file was a symbolic link. 12 13 This patch adds a new ShouldFollowSymbolicLinks parameter to getFileMetadata() so that 14 the call site can decide the behavior it wants. If getFileMetadata() is called on a 15 symbolic link with ShouldFollowSymbolicLinks::No as parameter, FileMetadata.type is now 16 properly set to TypeSymbolicLink. 17 18 No new tests, covered by new API test. 19 20 * Modules/entriesapi/DOMFileSystem.cpp: 21 (WebCore::listDirectoryWithMetadata): 22 It is important we do not follow symlinks here since the code wants to discard them 23 and does so by checking FileMetadata.type. 24 25 * WebCore.xcodeproj/project.pbxproj: 26 * fileapi/File.cpp: 27 (WebCore::File::isDirectory const): 28 29 * html/FileListCreator.cpp: 30 (WebCore::appendDirectoryFiles): 31 (WebCore::FileListCreator::createFileList): 32 It is important we do not follow symlinks here since the code wants to discard them 33 and does so by checking FileMetadata.type. 34 35 * platform/FileSystem.cpp: 36 (WebCore::fileIsDirectory): 37 * platform/FileSystem.h: 38 * platform/glib/FileSystemGlib.cpp: 39 (WebCore::getFileLStat): 40 (WebCore::getFileMetadata): 41 * platform/posix/FileSystemPOSIX.cpp: 42 (WebCore::getFileMetadata): 43 (WebCore::createSymbolicLink): 44 * platform/win/FileSystemWin.cpp: 45 (WebCore::getFinalPathName): 46 (WebCore::getFileMetadata): 47 (WebCore::createSymbolicLink): 48 - Add new createSymbolicLink() function for testing purposes. 49 - On Posix, call lstat() instead of stat if ShouldFollowSymbolicLinks::No. 50 - On Windows, since FindFirstFileW() does not follow symlinks, resolve 51 final path using GetFinalPathNameByHandleW() if ShouldFollowSymbolicLinks::Yes. 52 - On Windows, properly set FileMetadata.type to TypeSymbolicLink if the file 53 is a symbolic link. 54 1 55 2017-08-31 Michael Catanzaro <mcatanzaro@igalia.com> 2 56 -
trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp
r221414 r221441 47 47 { 48 48 ASSERT(!isMainThread()); 49 if (!fileIsDirectory(fullPath ))49 if (!fileIsDirectory(fullPath, ShouldFollowSymbolicLinks::No)) 50 50 return Exception { NotFoundError, "Path no longer exists or is no longer a directory" }; 51 51 … … 55 55 for (auto& childPath : childPaths) { 56 56 FileMetadata metadata; 57 if (!getFileMetadata(childPath, metadata ))57 if (!getFileMetadata(childPath, metadata, ShouldFollowSymbolicLinks::No)) 58 58 continue; 59 59 listedChildren.uncheckedAppend(ListedChild { pathGetFileName(childPath), metadata.type }); -
trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj
r221437 r221441 1940 1940 468344DF1EDDFAAA00B7795B /* DOMRectList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 468344DD1EDDFA5F00B7795B /* DOMRectList.cpp */; }; 1941 1941 468344E01EDDFAAA00B7795B /* DOMRectList.h in Headers */ = {isa = PBXBuildFile; fileRef = 468344DE1EDDFA5F00B7795B /* DOMRectList.h */; settings = {ATTRIBUTES = (Private, ); }; }; 1942 4689F1AF1267BAE100E8D380 /* FileMetadata.h in Headers */ = {isa = PBXBuildFile; fileRef = 4689F1AE1267BAE100E8D380 /* FileMetadata.h */; };1942 4689F1AF1267BAE100E8D380 /* FileMetadata.h in Headers */ = {isa = PBXBuildFile; fileRef = 4689F1AE1267BAE100E8D380 /* FileMetadata.h */; settings = {ATTRIBUTES = (Private, ); }; }; 1943 1943 46B63F6C1C6E8D19002E914B /* JSEventTargetCustom.h in Headers */ = {isa = PBXBuildFile; fileRef = 46B63F6B1C6E8CDF002E914B /* JSEventTargetCustom.h */; settings = {ATTRIBUTES = (Private, ); }; }; 1944 1944 46C696CB1E7205F700597937 /* CPUMonitor.h in Headers */ = {isa = PBXBuildFile; fileRef = 46C696C91E7205E400597937 /* CPUMonitor.h */; settings = {ATTRIBUTES = (Private, ); }; }; -
trunk/Source/WebCore/fileapi/File.cpp
r221302 r221441 131 131 { 132 132 if (!m_isDirectory) 133 m_isDirectory = fileIsDirectory(m_path );133 m_isDirectory = fileIsDirectory(m_path, ShouldFollowSymbolicLinks::Yes); 134 134 return *m_isDirectory; 135 135 } -
trunk/Source/WebCore/html/FileListCreator.cpp
r221414 r221441 44 44 for (auto& childPath : listDirectory(directory, "*")) { 45 45 FileMetadata metadata; 46 if (!getFileMetadata(childPath, metadata ))46 if (!getFileMetadata(childPath, metadata, ShouldFollowSymbolicLinks::No)) 47 47 continue; 48 48 … … 81 81 Vector<RefPtr<File>> fileObjects; 82 82 for (auto& info : paths) { 83 if (shouldResolveDirectories == ShouldResolveDirectories::Yes && fileIsDirectory(info.path ))83 if (shouldResolveDirectories == ShouldResolveDirectories::Yes && fileIsDirectory(info.path, ShouldFollowSymbolicLinks::No)) 84 84 appendDirectoryFiles(info.path, pathGetFileName(info.path), fileObjects); 85 85 else -
trunk/Source/WebCore/platform/FileSystem.cpp
r221291 r221441 352 352 } 353 353 354 bool fileIsDirectory(const String& path )354 bool fileIsDirectory(const String& path, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks) 355 355 { 356 356 FileMetadata metadata; 357 if (!getFileMetadata(path, metadata ))357 if (!getFileMetadata(path, metadata, shouldFollowSymbolicLinks)) 358 358 return false; 359 359 return metadata.type == FileMetadata::TypeDirectory; -
trunk/Source/WebCore/platform/FileSystem.h
r221414 r221441 89 89 }; 90 90 91 enum class ShouldFollowSymbolicLinks { No, Yes }; 92 91 93 struct FileMetadata; 92 94 … … 99 101 WEBCORE_EXPORT bool getFileModificationTime(const String&, time_t& result); 100 102 WEBCORE_EXPORT bool getFileCreationTime(const String&, time_t& result); // Not all platforms store file creation time. 101 bool getFileMetadata(const String&, FileMetadata&);102 bool fileIsDirectory(const String& );103 WEBCORE_EXPORT bool getFileMetadata(const String&, FileMetadata&, ShouldFollowSymbolicLinks); 104 bool fileIsDirectory(const String&, ShouldFollowSymbolicLinks); 103 105 WEBCORE_EXPORT String pathByAppendingComponent(const String& path, const String& component); 104 106 String pathByAppendingComponents(const String& path, const Vector<String>& components); … … 110 112 WEBCORE_EXPORT bool getVolumeFreeSpace(const String&, uint64_t&); 111 113 WEBCORE_EXPORT std::optional<int32_t> getFileDeviceId(const CString&); 114 WEBCORE_EXPORT bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath); 112 115 113 116 WEBCORE_EXPORT void setMetadataURL(const String& path, const String& urlString, const String& referrer = { }); -
trunk/Source/WebCore/platform/glib/FileSystemGlib.cpp
r221414 r221441 124 124 } 125 125 126 static bool getFileLStat(const String& path, GStatBuf* statBuffer) 127 { 128 GUniquePtr<gchar> filename = unescapedFilename(path); 129 if (!filename) 130 return false; 131 132 return g_lstat(filename.get(), statBuffer) != -1; 133 } 134 126 135 bool getFileSize(const String& path, long long& resultSize) 127 136 { … … 156 165 } 157 166 158 bool getFileMetadata(const String& path, FileMetadata& metadata )167 bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks) 159 168 { 160 169 GStatBuf statResult; 161 if (!getFileStat(path, &statResult)) 162 return false; 170 171 if (shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) { 172 if (!getFileStat(path, &statResult)) 173 return false; 174 } else { 175 if (!getFileLStat(path, &statResult)) 176 return false; 177 } 163 178 164 179 String filename = pathGetFileName(path); … … 202 217 { 203 218 return stringFromFileSystemRepresentation(g_get_home_dir()); 219 } 220 221 bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath) 222 { 223 CString targetPathFSRep = fileSystemRepresentation(targetPath); 224 if (!targetPathFSRep.data() || targetPathFSRep.data()[0] == '\0') 225 return false; 226 227 CString symbolicLinkPathFSRep = fileSystemRepresentation(symbolicLinkPath); 228 if (!symbolicLinkPathFSRep.data() || symbolicLinkPathFSRep.data()[0] == '\0') 229 return false; 230 231 return !symlink(targetPathFSRep.data(), symbolicLinkPathFSRep.data()); 204 232 } 205 233 -
trunk/Source/WebCore/platform/network/BlobDataFileReference.cpp
r202414 r221441 97 97 // FIXME: Some platforms provide better ways to listen for file system object changes, consider using these. 98 98 FileMetadata metadata; 99 if (!getFileMetadata(m_path, metadata ))99 if (!getFileMetadata(m_path, metadata, ShouldFollowSymbolicLinks::Yes)) 100 100 return; 101 101 -
trunk/Source/WebCore/platform/network/mac/BlobDataFileReferenceMac.mm
r219191 r221441 81 81 if (!m_replacementPath.isNull()) { 82 82 FileMetadata metadata; 83 if (getFileMetadata(m_replacementPath, metadata ))83 if (getFileMetadata(m_replacementPath, metadata, ShouldFollowSymbolicLinks::Yes)) 84 84 m_size = metadata.length; 85 85 } -
trunk/Source/WebCore/platform/posix/FileSystemPOSIX.cpp
r221414 r221441 240 240 } 241 241 242 bool getFileMetadata(const String& path, FileMetadata& metadata) 243 { 244 CString fsRep = fileSystemRepresentation(path); 245 246 if (!fsRep.data() || fsRep.data()[0] == '\0') 247 return false; 248 249 struct stat fileInfo; 250 if (stat(fsRep.data(), &fileInfo)) 251 return false; 242 bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks) 243 { 244 CString fsRep = fileSystemRepresentation(path); 245 246 if (!fsRep.data() || fsRep.data()[0] == '\0') 247 return false; 248 249 struct stat fileInfo; 250 251 if (shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) { 252 if (stat(fsRep.data(), &fileInfo)) 253 return false; 254 } else { 255 if (lstat(fsRep.data(), &fileInfo)) 256 return false; 257 } 252 258 253 259 String filename = pathGetFileName(path); … … 263 269 metadata.type = FileMetadata::TypeFile; 264 270 return true; 271 } 272 273 bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath) 274 { 275 CString targetPathFSRep = fileSystemRepresentation(targetPath); 276 if (!targetPathFSRep.data() || targetPathFSRep.data()[0] == '\0') 277 return false; 278 279 CString symbolicLinkPathFSRep = fileSystemRepresentation(symbolicLinkPath); 280 if (!symbolicLinkPathFSRep.data() || symbolicLinkPathFSRep.data()[0] == '\0') 281 return false; 282 283 return !symlink(targetPathFSRep.data(), symbolicLinkPathFSRep.data()); 265 284 } 266 285 -
trunk/Source/WebCore/platform/win/FileSystemWin.cpp
r221422 r221441 142 142 } 143 143 144 bool getFileMetadata(const String& path, FileMetadata& metadata) 144 static String getFinalPathName(const String& path) 145 { 146 auto handle = openFile(path, OpenForRead); 147 if (!isHandleValid(handle)) 148 return String(); 149 150 StringVector<UChar> buffer(MAX_PATH); 151 if (::GetFinalPathNameByHandleW(handle, buffer.data(), buffer.size(), VOLUME_NAME_NT) >= MAX_PATH) { 152 closeFile(handle); 153 return String(); 154 } 155 closeFile(handle); 156 157 buffer.shrink(wcslen(buffer.data())); 158 return String::adopt(WTFMove(buffer)); 159 } 160 161 bool getFileMetadata(const String& path, FileMetadata& metadata, ShouldFollowSymbolicLinks shouldFollowSymbolicLinks) 145 162 { 146 163 WIN32_FIND_DATAW findData; 147 164 if (!getFindData(path, findData)) 148 165 return false; 166 167 bool isSymbolicLink = findData.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT && findData.dwReserved0 == IO_REPARSE_TAG_SYMLINK; 168 if (isSymbolicLink && shouldFollowSymbolicLinks == ShouldFollowSymbolicLinks::Yes) { 169 String targetPath = getFinalPathName(path); 170 if (targetPath.isNull()) 171 return false; 172 if (!getFindData(targetPath, findData)) 173 return false; 174 isSymbolicLink = false; 175 } 149 176 150 177 if (!getFileSizeFromFindData(findData, metadata.length)) … … 155 182 metadata.modificationTime = modificationTime; 156 183 metadata.isHidden = findData.dwFileAttributes & FILE_ATTRIBUTE_HIDDEN; 157 metadata.type = (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? FileMetadata::TypeDirectory : FileMetadata::TypeFile; 158 159 return true; 184 if (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) 185 metadata.type = FileMetadata::TypeDirectory; 186 else if (isSymbolicLink) 187 metadata.type = FileMetadata::TypeSymbolicLink; 188 else 189 metadata.type = FileMetadata::TypeFile; 190 191 return true; 192 } 193 194 bool createSymbolicLink(const String& targetPath, const String& symbolicLinkPath) 195 { 196 return !::CreateSymbolicLinkW(symbolicLinkPath.charactersWithNullTermination().data(), targetPath.charactersWithNullTermination().data(), 0); 160 197 } 161 198 -
trunk/Tools/ChangeLog
r221436 r221441 1 2017-08-31 Chris Dumez <cdumez@apple.com> 2 3 getFileMetadata() does not work as expected for symbolic links 4 https://bugs.webkit.org/show_bug.cgi?id=176143 5 6 Reviewed by Andreas Kling. 7 8 Add API test coverage. 9 10 * TestWebKitAPI/Tests/WebCore/FileSystem.cpp: 11 (TestWebKitAPI::TEST_F): 12 1 13 2017-08-31 Filip Pizlo <fpizlo@apple.com> 2 14 -
trunk/Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp
r211763 r221441 28 28 29 29 #include "Test.h" 30 #include <WebCore/FileMetadata.h> 30 31 #include <WebCore/FileSystem.h> 31 32 #include <wtf/MainThread.h> … … 49 50 m_tempFilePath = openTemporaryFile("tempTestFile", handle); 50 51 writeToFile(handle, FileSystemTestData, strlen(FileSystemTestData)); 51 closeFile(handle); 52 closeFile(handle); 53 54 m_tempFileSymlinkPath = m_tempFilePath + "-symlink"; 55 createSymbolicLink(m_tempFilePath, m_tempFileSymlinkPath); 52 56 53 57 m_tempEmptyFilePath = openTemporaryFile("tempEmptyTestFile", handle); … … 67 71 { 68 72 deleteFile(m_tempFilePath); 73 deleteFile(m_tempFileSymlinkPath); 69 74 deleteFile(m_tempEmptyFilePath); 70 75 deleteFile(m_spaceContainingFilePath); … … 74 79 75 80 const String& tempFilePath() { return m_tempFilePath; } 81 const String& tempFileSymlinkPath() { return m_tempFileSymlinkPath; } 76 82 const String& tempEmptyFilePath() { return m_tempEmptyFilePath; } 77 83 const String& spaceContainingFilePath() { return m_spaceContainingFilePath; } … … 81 87 private: 82 88 String m_tempFilePath; 89 String m_tempFileSymlinkPath; 83 90 String m_tempEmptyFilePath; 84 91 String m_spaceContainingFilePath; … … 120 127 } 121 128 129 TEST_F(FileSystemTest, GetFileMetadataSymlink) 130 { 131 FileMetadata symlinkMetadata; 132 EXPECT_TRUE(getFileMetadata(tempFileSymlinkPath(), symlinkMetadata, ShouldFollowSymbolicLinks::No)); 133 EXPECT_TRUE(symlinkMetadata.type == FileMetadata::TypeSymbolicLink); 134 EXPECT_FALSE(static_cast<size_t>(symlinkMetadata.length) == strlen(FileSystemTestData)); 135 136 FileMetadata targetMetadata; 137 EXPECT_TRUE(getFileMetadata(tempFileSymlinkPath(), targetMetadata, ShouldFollowSymbolicLinks::Yes)); 138 EXPECT_TRUE(targetMetadata.type == FileMetadata::TypeFile); 139 EXPECT_EQ(strlen(FileSystemTestData), static_cast<size_t>(targetMetadata.length)); 122 140 } 141 142 }
Note: See TracChangeset
for help on using the changeset viewer.