From f11df067d2a76b13338c922041fcd274ee5d5f6d Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Sun, 20 Mar 2016 11:56:10 +0100 Subject: [PATCH] Bug 1173320 - patch 3/8 - Improve the Windows path management, r=smaug --- .../test/test_fs_createDirectory.html | 4 +- dom/filesystem/Directory.cpp | 88 ++++++++++--------- dom/filesystem/Directory.h | 3 - dom/filesystem/FileSystemBase.cpp | 33 ------- dom/filesystem/FileSystemBase.h | 6 -- dom/filesystem/FileSystemUtils.cpp | 2 +- dom/filesystem/FileSystemUtils.h | 5 +- dom/filesystem/GetDirectoryListingTask.cpp | 48 +++------- dom/filesystem/OSFileSystem.cpp | 2 +- 9 files changed, 67 insertions(+), 124 deletions(-) diff --git a/dom/devicestorage/test/test_fs_createDirectory.html b/dom/devicestorage/test/test_fs_createDirectory.html index d34905140a96..3b573b086f42 100644 --- a/dom/devicestorage/test/test_fs_createDirectory.html +++ b/dom/devicestorage/test/test_fs_createDirectory.html @@ -34,14 +34,14 @@ function testCreateDirectory(rootDir, path) { } function createDirectorySuccess(d) { - ok(d.name === gName, "Failed to create directory: name mismatch."); + is(d.name, gName, "Failed to create directory: name mismatch."); // Get the new created directory from the root. gRoot.get(gPath).then(getSuccess, cbError); } function getSuccess(d) { - ok(d.name === gName, "Should get directory - " + (gPath || "[root]") + "."); + is(d.name, gName, "Should get directory - " + (gPath || "[root]") + "."); switch (gTestCount) { case 0: gRoot = d; diff --git a/dom/filesystem/Directory.cpp b/dom/filesystem/Directory.cpp index a3ff29b14e6d..28d1ed170a46 100644 --- a/dom/filesystem/Directory.cpp +++ b/dom/filesystem/Directory.cpp @@ -35,6 +35,46 @@ namespace mozilla { namespace dom { +namespace { + +bool +IsValidRelativeDOMPath(const nsString& aPath, nsTArray& aParts) +{ + // We don't allow empty relative path to access the root. + if (aPath.IsEmpty()) { + return false; + } + + // Leading and trailing "/" are not allowed. + if (aPath.First() == FILESYSTEM_DOM_PATH_SEPARATOR_CHAR || + aPath.Last() == FILESYSTEM_DOM_PATH_SEPARATOR_CHAR) { + return false; + } + + NS_NAMED_LITERAL_STRING(kCurrentDir, "."); + NS_NAMED_LITERAL_STRING(kParentDir, ".."); + + // Split path and check each path component. + nsCharSeparatedTokenizer tokenizer(aPath, FILESYSTEM_DOM_PATH_SEPARATOR_CHAR); + while (tokenizer.hasMoreTokens()) { + nsDependentSubstring pathComponent = tokenizer.nextToken(); + // The path containing empty components, such as "foo//bar", is invalid. + // We don't allow paths, such as "../foo", "foo/./bar" and "foo/../bar", + // to walk up the directory. + if (pathComponent.IsEmpty() || + pathComponent.Equals(kCurrentDir) || + pathComponent.Equals(kParentDir)) { + return false; + } + + aParts.AppendElement(pathComponent); + } + + return true; +} + +} // anonymous namespace + NS_IMPL_CYCLE_COLLECTION_CLASS(Directory) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Directory) @@ -302,7 +342,7 @@ void Directory::GetPath(nsAString& aRetval, ErrorResult& aRv) { if (mType == eDOMRootDirectory) { - aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR); + aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL); } else { // TODO: this should be a bit different... GetName(aRetval, aRv); @@ -384,7 +424,8 @@ Directory::DOMPathToRealPath(const nsAString& aPath, nsIFile** aFile) const static const char kWhitespace[] = "\b\t\r\n "; relativePath.Trim(kWhitespace); - if (!IsValidRelativePath(relativePath)) { + nsTArray parts; + if (!IsValidRelativeDOMPath(relativePath, parts)) { return NS_ERROR_DOM_FILESYSTEM_INVALID_PATH_ERR; } @@ -394,49 +435,16 @@ Directory::DOMPathToRealPath(const nsAString& aPath, nsIFile** aFile) const return rv; } - rv = file->AppendRelativePath(relativePath); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; + for (uint32_t i = 0; i < parts.Length(); ++i) { + rv = file->AppendRelativePath(parts[i]); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } file.forget(aFile); return NS_OK; } -// static -bool -Directory::IsValidRelativePath(const nsString& aPath) -{ - // We don't allow empty relative path to access the root. - if (aPath.IsEmpty()) { - return false; - } - - // Leading and trailing "/" are not allowed. - if (aPath.First() == FileSystemUtils::kSeparatorChar || - aPath.Last() == FileSystemUtils::kSeparatorChar) { - return false; - } - - NS_NAMED_LITERAL_STRING(kCurrentDir, "."); - NS_NAMED_LITERAL_STRING(kParentDir, ".."); - - // Split path and check each path component. - nsCharSeparatedTokenizer tokenizer(aPath, FileSystemUtils::kSeparatorChar); - while (tokenizer.hasMoreTokens()) { - nsDependentSubstring pathComponent = tokenizer.nextToken(); - // The path containing empty components, such as "foo//bar", is invalid. - // We don't allow paths, such as "../foo", "foo/./bar" and "foo/../bar", - // to walk up the directory. - if (pathComponent.IsEmpty() || - pathComponent.Equals(kCurrentDir) || - pathComponent.Equals(kParentDir)) { - return false; - } - } - - return true; -} - } // namespace dom } // namespace mozilla diff --git a/dom/filesystem/Directory.h b/dom/filesystem/Directory.h index 3d8a420723e5..a71f02930302 100644 --- a/dom/filesystem/Directory.h +++ b/dom/filesystem/Directory.h @@ -147,9 +147,6 @@ private: FileSystemBase* aFileSystem = nullptr); ~Directory(); - static bool - IsValidRelativePath(const nsString& aPath); - /* * Convert relative DOM path to the absolute real path. */ diff --git a/dom/filesystem/FileSystemBase.cpp b/dom/filesystem/FileSystemBase.cpp index a750fa7839db..2b1d52608ebc 100644 --- a/dom/filesystem/FileSystemBase.cpp +++ b/dom/filesystem/FileSystemBase.cpp @@ -64,39 +64,6 @@ FileSystemBase::GetWindow() const return nullptr; } -already_AddRefed -FileSystemBase::GetLocalFile(const nsAString& aRealPath) const -{ - MOZ_ASSERT(XRE_IsParentProcess(), - "Should be on parent process!"); - - // Let's convert the input path to /path - nsAutoString localPath; - if (!aRealPath.IsEmpty() && - !StringBeginsWith(aRealPath, - NS_LITERAL_STRING(FILESYSTEM_DOM_PATH_SEPARATOR))) { - localPath.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR); - } - localPath.Append(aRealPath); - - // We have to normalize the path string in order to follow the separator - // schema of this OS. - nsAutoString normalizedPath; - FileSystemUtils::NormalizedPathToLocalPath(localPath, normalizedPath); - - // The full path is mLocalRootPath + normalizedPath. - nsAutoString path(mLocalRootPath); - path.Append(normalizedPath); - - nsCOMPtr file; - nsresult rv = NS_NewLocalFile(path, false, getter_AddRefs(file)); - if (NS_WARN_IF(NS_FAILED(rv))) { - return nullptr; - } - - return file.forget(); -} - bool FileSystemBase::GetRealPath(BlobImpl* aFile, nsIFile** aPath) const { diff --git a/dom/filesystem/FileSystemBase.h b/dom/filesystem/FileSystemBase.h index 0ac36f009d5f..6a715dee9741 100644 --- a/dom/filesystem/FileSystemBase.h +++ b/dom/filesystem/FileSystemBase.h @@ -39,12 +39,6 @@ public: virtual nsPIDOMWindowInner* GetWindow() const; - /** - * Create nsIFile object from the given real path (absolute DOM path). - */ - already_AddRefed - GetLocalFile(const nsAString& aRealPath) const; - /* * Get the virtual name of the root directory. This name will be exposed to * the content page. diff --git a/dom/filesystem/FileSystemUtils.cpp b/dom/filesystem/FileSystemUtils.cpp index 1e62e4c56bce..c5a8817f7cd5 100644 --- a/dom/filesystem/FileSystemUtils.cpp +++ b/dom/filesystem/FileSystemUtils.cpp @@ -54,7 +54,7 @@ FileSystemUtils::IsDescendantPath(const nsAString& aPath, { // The descendant path should begin with its ancestor path. nsAutoString prefix; - prefix = aPath + NS_LITERAL_STRING(FILESYSTEM_DOM_PATH_SEPARATOR); + prefix = aPath + NS_LITERAL_STRING(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL); // Check the sub-directory path to see if it has the parent path as prefix. if (aDescendantPath.Length() < prefix.Length() || diff --git a/dom/filesystem/FileSystemUtils.h b/dom/filesystem/FileSystemUtils.h index cadead05abea..a2572bbf1775 100644 --- a/dom/filesystem/FileSystemUtils.h +++ b/dom/filesystem/FileSystemUtils.h @@ -12,7 +12,8 @@ namespace mozilla { namespace dom { -#define FILESYSTEM_DOM_PATH_SEPARATOR "/" +#define FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL "/" +#define FILESYSTEM_DOM_PATH_SEPARATOR_CHAR '/' /* * This class is for error handling. @@ -46,8 +47,6 @@ public: */ static bool IsDescendantPath(const nsAString& aPath, const nsAString& aDescendantPath); - - static const char16_t kSeparatorChar = char16_t('/'); }; } // namespace dom diff --git a/dom/filesystem/GetDirectoryListingTask.cpp b/dom/filesystem/GetDirectoryListingTask.cpp index ee65e54324a9..f80a3b7eea36 100644 --- a/dom/filesystem/GetDirectoryListingTask.cpp +++ b/dom/filesystem/GetDirectoryListingTask.cpp @@ -343,41 +343,6 @@ GetDirectoryListingTask::HandlerCallback() for (unsigned i = 0; i < count; i++) { if (mTargetData[i].mType == Directory::BlobImplOrDirectoryPath::eDirectoryPath) { -#ifdef DEBUG - if (XRE_IsParentProcess()) { - nsCOMPtr file; - nsresult rv = NS_NewLocalFile(mTargetData[i].mDirectoryPath, false, - getter_AddRefs(file)); - if (NS_WARN_IF(NS_FAILED(rv))) { - mPromise->MaybeReject(rv); - mPromise = nullptr; - return; - } - - bool exist; - rv = file->Exists(&exist); - if (NS_WARN_IF(NS_FAILED(rv))) { - mPromise->MaybeReject(rv); - mPromise = nullptr; - return; - } - - // We cannot assert here because the file can be done in the meantime. - NS_ASSERTION(exist, "The file doesn't exist anymore?!?"); - - nsAutoString normalizedLocalRootPath; - FileSystemUtils::NormalizedPathToLocalPath(mFileSystem->GetLocalRootPath(), - normalizedLocalRootPath); - - nsAutoString directoryPath; - FileSystemUtils::NormalizedPathToLocalPath(mTargetData[i].mDirectoryPath, - directoryPath); - - MOZ_ASSERT(FileSystemUtils::IsDescendantPath(normalizedLocalRootPath, - directoryPath)); - } -#endif - nsCOMPtr directoryPath; NS_ConvertUTF16toUTF8 path(mTargetData[i].mDirectoryPath); nsresult rv = NS_NewNativeLocalFile(path, true, @@ -388,6 +353,19 @@ GetDirectoryListingTask::HandlerCallback() return; } +#ifdef DEBUG + nsCOMPtr rootPath; + rv = NS_NewLocalFile(mFileSystem->GetLocalRootPath(), false, + getter_AddRefs(rootPath)); + if (NS_WARN_IF(NS_FAILED(rv))) { + mPromise->MaybeReject(rv); + mPromise = nullptr; + return; + } + + MOZ_ASSERT(FileSystemUtils::IsDescendantPath(rootPath, directoryPath)); +#endif + RefPtr directory = Directory::Create(mFileSystem->GetWindow(), directoryPath, Directory::eNotDOMRootDirectory, diff --git a/dom/filesystem/OSFileSystem.cpp b/dom/filesystem/OSFileSystem.cpp index 8fc9e88cf6fe..9ce96b0e0f04 100644 --- a/dom/filesystem/OSFileSystem.cpp +++ b/dom/filesystem/OSFileSystem.cpp @@ -51,7 +51,7 @@ OSFileSystem::GetWindow() const void OSFileSystem::GetRootName(nsAString& aRetval) const { - return aRetval.AssignLiteral("/"); + aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL); } bool