diff --git a/dom/filesystem/GetDirectoryListingTask.cpp b/dom/filesystem/GetDirectoryListingTask.cpp index 1d4f77b3b8..8fdfb4a5bb 100644 --- a/dom/filesystem/GetDirectoryListingTask.cpp +++ b/dom/filesystem/GetDirectoryListingTask.cpp @@ -340,9 +340,14 @@ GetDirectoryListingTaskParent::IOWork() nsCOMPtr currFile = do_QueryInterface(supp); MOZ_ASSERT(currFile); - bool isSpecial, isFile; - if (NS_WARN_IF(NS_FAILED(currFile->IsSpecial(&isSpecial))) || - isSpecial) { + bool isLink, isSpecial, isFile; + if (NS_WARN_IF(NS_FAILED(currFile->IsSymlink(&isLink)) || + NS_FAILED(currFile->IsSpecial(&isSpecial))) || + // Although we allow explicit individual selection of symlinks via the + // file picker, we do not process symlinks in directory traversal. Our + // specific policy decision is documented at + // https://bugzilla.mozilla.org/show_bug.cgi?id=1813299#c20 + isLink || isSpecial) { continue; } if (NS_WARN_IF(NS_FAILED(currFile->IsFile(&isFile)) || diff --git a/dom/filesystem/GetFilesHelper.cpp b/dom/filesystem/GetFilesHelper.cpp index 563ef60b4a..fb3cf7a24d 100644 --- a/dom/filesystem/GetFilesHelper.cpp +++ b/dom/filesystem/GetFilesHelper.cpp @@ -333,13 +333,8 @@ GetFilesHelperBase::ExploreDirectory(const nsAString& aDOMPath, nsIFile* aFile) return NS_OK; } - nsresult rv = AddExploredDirectory(aFile); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - nsCOMPtr entries; - rv = aFile->GetDirectoryEntries(getter_AddRefs(entries)); + nsresult rv = aFile->GetDirectoryEntries(getter_AddRefs(entries)); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -361,7 +356,12 @@ GetFilesHelperBase::ExploreDirectory(const nsAString& aDOMPath, nsIFile* aFile) bool isLink, isSpecial, isFile, isDir; if (NS_WARN_IF(NS_FAILED(currFile->IsSymlink(&isLink)) || NS_FAILED(currFile->IsSpecial(&isSpecial))) || - isSpecial) { + isSpecial || + // Although we allow explicit individual selection of symlinks via the + // file picker, we do not process symlinks in directory traversal. Our + // specific policy decision is documented at + // https://bugzilla.mozilla.org/show_bug.cgi?id=1813299#c20 + isLink) { continue; } @@ -371,11 +371,6 @@ GetFilesHelperBase::ExploreDirectory(const nsAString& aDOMPath, nsIFile* aFile) continue; } - // We don't want to explore loops of links. - if (isDir && isLink && !ShouldFollowSymLink(currFile)) { - continue; - } - // The new domPath nsAutoString domPath; domPath.Assign(aDOMPath); @@ -415,69 +410,6 @@ GetFilesHelperBase::ExploreDirectory(const nsAString& aDOMPath, nsIFile* aFile) return NS_OK; } -nsresult -GetFilesHelperBase::AddExploredDirectory(nsIFile* aDir) -{ - nsresult rv; - -#ifdef DEBUG - bool isDir; - rv = aDir->IsDirectory(&isDir); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - MOZ_ASSERT(isDir, "Why are we here?"); -#endif - - bool isLink; - rv = aDir->IsSymlink(&isLink); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - nsAutoCString path; - - if (!isLink) { - nsAutoString path16; - rv = aDir->GetPath(path16); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - path = NS_ConvertUTF16toUTF8(path16); - } else { - rv = aDir->GetNativeTarget(path); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - } - - mExploredDirectories.PutEntry(path); - return NS_OK; -} - -bool -GetFilesHelperBase::ShouldFollowSymLink(nsIFile* aDir) -{ -#ifdef DEBUG - bool isLink, isDir; - if (NS_WARN_IF(NS_FAILED(aDir->IsSymlink(&isLink)) || - NS_FAILED(aDir->IsDirectory(&isDir)))) { - return false; - } - - MOZ_ASSERT(isLink && isDir, "Why are we here?"); -#endif - - nsAutoCString targetPath; - if (NS_WARN_IF(NS_FAILED(aDir->GetNativeTarget(targetPath)))) { - return false; - } - - return !mExploredDirectories.Contains(targetPath); -} - void GetFilesHelper::ResolveOrRejectPromise(Promise* aPromise) { diff --git a/dom/filesystem/GetFilesHelper.h b/dom/filesystem/GetFilesHelper.h index 4afd41d7e0..b6c58ef396 100644 --- a/dom/filesystem/GetFilesHelper.h +++ b/dom/filesystem/GetFilesHelper.h @@ -9,10 +9,8 @@ #include "mozilla/Mutex.h" #include "mozilla/RefPtr.h" #include "mozilla/dom/File.h" -#include "nsClassHashtable.h" #include "nsCycleCollectionTraversalCallback.h" #include "nsTArray.h" -#include "nsTHashtable.h" #include "nsThreadUtils.h" class nsIGlobalObject; @@ -57,17 +55,10 @@ protected: nsresult ExploreDirectory(const nsAString& aDOMPath, nsIFile* aFile); - nsresult - AddExploredDirectory(nsIFile* aDirectory); - - bool - ShouldFollowSymLink(nsIFile* aDirectory); - bool mRecursiveFlag; // We populate this array in the I/O thread with the BlobImpl. FallibleTArray> mTargetBlobImplArray; - nsTHashtable mExploredDirectories; }; // Retrieving the list of files can be very time/IO consuming. We use this diff --git a/dom/filesystem/tests/filesystem_commons.js b/dom/filesystem/tests/filesystem_commons.js index 4f7234121e..c8bd9ac9fc 100644 --- a/dom/filesystem/tests/filesystem_commons.js +++ b/dom/filesystem/tests/filesystem_commons.js @@ -38,6 +38,10 @@ function test_getFilesAndDirectories(aDirectory, aRecursive, aNext) { if (data[i] instanceof File) { is(data[i].webkitRelativePath, createRelativePath(dir, data[i]), "File.webkitRelativePath should be called: parentdir.path + '/' + file.name: " + data[i].webkitRelativePath); } + ok( + !data[i].webkitRelativePath.endsWith("symlink.txt"), + "We should never see a path ending with symlink.txt, our symlink sentinel." + ); } } ); diff --git a/dom/filesystem/tests/script_fileList.js b/dom/filesystem/tests/script_fileList.js index 89fd04cabe..dedc61a448 100644 --- a/dom/filesystem/tests/script_fileList.js +++ b/dom/filesystem/tests/script_fileList.js @@ -36,6 +36,15 @@ function createTreeFile(depth, parent) { if (depth == 0) { nextFile.append('file.txt'); nextFile.create(Components.interfaces.nsIFile.NORMAL_FILE_TYPE, 0o600); + +#ifdef XP_UNIX + // It's not possible to create symlinks on windows by default or on our + // Android platforms, so we can't create the symlink file there. Our + // callers that care are aware of this. + var linkFile = parent.clone(); + linkFile.append("symlink.txt"); + createSymLink(nextFile.path, linkFile.path); +#endif } else { nextFile.append('subdir' + depth); nextFile.createUnique(Components.interfaces.nsIFile.DIRECTORY_TYPE, 0o700); @@ -84,6 +93,15 @@ function createTestFile() { file2.append('bar.txt'); file2.create(Components.interfaces.nsIFile.NORMAL_FILE_TYPE, 0o600); +#ifdef XP_UNIX + // It's not possible to create symlinks on windows by default or on our + // Android platforms, so we can't create the symlink file there. Our + // callers that care are aware of this. + var linkFile = dir.clone(); + linkFile.append("symlink.txt"); + createSymLink(file1.path, linkFile.path); +#endif + return tmpFile; } @@ -127,3 +145,14 @@ addMessageListener("file.open", function (e) { file: File.createFromNsIFile(testFile) }); }); + +addMessageListener("symlink.open", function (e) { + let testDir = createTestFile(); + let testFile = testDir.clone(); + testFile.append("subdir"); + testFile.append("symlink.txt"); + + File.createFromNsIFile(testFile).then(function (file) { + sendAsyncMessage("symlink.opened", { dir: testDir.path, file }); + }); +}); diff --git a/dom/filesystem/tests/test_webkitdirectory.html b/dom/filesystem/tests/test_webkitdirectory.html index 591619e45b..46c728718f 100644 --- a/dom/filesystem/tests/test_webkitdirectory.html +++ b/dom/filesystem/tests/test_webkitdirectory.html @@ -9,11 +9,26 @@ +