Bug 1173320 - patch 3/8 - Improve the Windows path management, r=smaug
☠☠ backed out by bd8284e36c7c ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 19 Mar 2016 14:33:27 +0100
changeset 313235 401c1e7df8eaef6c5801a84d1abf05fdf0e3d2be
parent 313234 bf65b38f759ede2f241278b83f3d46213c2ad302
child 313236 e94ced2f8e3cf5e639924b1800f93f1e79f4b1a9
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1173320
milestone48.0a1
Bug 1173320 - patch 3/8 - Improve the Windows path management, r=smaug
dom/devicestorage/test/test_fs_createDirectory.html
dom/filesystem/Directory.cpp
dom/filesystem/Directory.h
dom/filesystem/FileSystemBase.cpp
dom/filesystem/FileSystemBase.h
dom/filesystem/FileSystemUtils.cpp
dom/filesystem/FileSystemUtils.h
dom/filesystem/GetDirectoryListingTask.cpp
dom/filesystem/OSFileSystem.cpp
--- a/dom/devicestorage/test/test_fs_createDirectory.html
+++ b/dom/devicestorage/test/test_fs_createDirectory.html
@@ -29,24 +29,24 @@ var gTestCount = 0;
 var gPath = '';
 var gName = '';
 
 function testCreateDirectory(rootDir, path) {
   rootDir.createDirectory(path).then(createDirectorySuccess, cbError);
 }
 
 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;
       // Create a new directory under the root.
       gName = gPath = randomFilename(12);
       testCreateDirectory(gRoot, gName);
       break;
     case 1:
--- a/dom/filesystem/Directory.cpp
+++ b/dom/filesystem/Directory.cpp
@@ -30,16 +30,56 @@
 // by Directory#CreateFileW.
 #ifdef CreateFile
 #undef CreateFile
 #endif
 
 namespace mozilla {
 namespace dom {
 
+namespace {
+
+bool
+IsValidRelativeDOMPath(const nsString& aPath, nsTArray<nsString>& 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)
   if (tmp->mFileSystem) {
     tmp->mFileSystem->Unlink();
     tmp->mFileSystem = nullptr;
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
@@ -297,17 +337,17 @@ Directory::RemoveInternal(const StringOr
   FileSystemPermissionRequest::RequestForTask(task);
   return task->GetPromise();
 }
 
 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);
   }
 }
 
 nsresult
 Directory::GetFullRealPath(nsAString& aPath)
@@ -379,64 +419,32 @@ Directory::DOMPathToRealPath(const nsASt
 {
   nsString relativePath;
   relativePath = aPath;
 
   // Trim white spaces.
   static const char kWhitespace[] = "\b\t\r\n ";
   relativePath.Trim(kWhitespace);
 
-  if (!IsValidRelativePath(relativePath)) {
+  nsTArray<nsString> parts;
+  if (!IsValidRelativeDOMPath(relativePath, parts)) {
     return NS_ERROR_DOM_FILESYSTEM_INVALID_PATH_ERR;
   }
 
   nsCOMPtr<nsIFile> file;
   nsresult rv = mFile->Clone(getter_AddRefs(file));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     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
--- a/dom/filesystem/Directory.h
+++ b/dom/filesystem/Directory.h
@@ -142,19 +142,16 @@ public:
   GetFileSystem(ErrorResult& aRv);
 
 private:
   Directory(nsPIDOMWindowInner* aWindow,
             nsIFile* aFile, DirectoryType aType,
             FileSystemBase* aFileSystem = nullptr);
   ~Directory();
 
-  static bool
-  IsValidRelativePath(const nsString& aPath);
-
   /*
    * Convert relative DOM path to the absolute real path.
    */
   nsresult
   DOMPathToRealPath(const nsAString& aPath, nsIFile** aFile) const;
 
   already_AddRefed<Promise>
   RemoveInternal(const StringOrFileOrDirectory& aPath, bool aRecursive,
--- a/dom/filesystem/FileSystemBase.cpp
+++ b/dom/filesystem/FileSystemBase.cpp
@@ -59,49 +59,16 @@ FileSystemBase::Shutdown()
 }
 
 nsPIDOMWindowInner*
 FileSystemBase::GetWindow() const
 {
   return nullptr;
 }
 
-already_AddRefed<nsIFile>
-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<nsIFile> 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
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Should be on parent process!");
   MOZ_ASSERT(aFile, "aFile Should not be null.");
 
   nsAutoString filePath;
--- a/dom/filesystem/FileSystemBase.h
+++ b/dom/filesystem/FileSystemBase.h
@@ -34,22 +34,16 @@ public:
 
   // SerializeDOMPath the FileSystem to string.
   virtual void
   SerializeDOMPath(nsAString& aOutput) const = 0;
 
   virtual nsPIDOMWindowInner*
   GetWindow() const;
 
-  /**
-   * Create nsIFile object from the given real path (absolute DOM path).
-   */
-  already_AddRefed<nsIFile>
-  GetLocalFile(const nsAString& aRealPath) const;
-
   /*
    * Get the virtual name of the root directory. This name will be exposed to
    * the content page.
    */
   virtual void
   GetRootName(nsAString& aRetval) const = 0;
 
   const nsAString&
--- a/dom/filesystem/FileSystemUtils.cpp
+++ b/dom/filesystem/FileSystemUtils.cpp
@@ -49,17 +49,17 @@ FileSystemUtils::NormalizedPathToLocalPa
 
 // static
 bool
 FileSystemUtils::IsDescendantPath(const nsAString& aPath,
                                   const nsAString& aDescendantPath)
 {
   // 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() ||
       !StringBeginsWith(aDescendantPath, prefix)) {
     return false;
   }
 
   return true;
--- a/dom/filesystem/FileSystemUtils.h
+++ b/dom/filesystem/FileSystemUtils.h
@@ -7,17 +7,18 @@
 #ifndef mozilla_dom_FileSystemUtils_h
 #define mozilla_dom_FileSystemUtils_h
 
 #include "nsString.h"
 
 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.
  * All methods in this class are static.
  */
 class FileSystemUtils
 {
 public:
@@ -41,16 +42,14 @@ public:
   IsDescendantPath(nsIFile* aPath, nsIFile* aDescendantPath);
 
   /*
    * Return true if aDescendantPath is a descendant of aPath. Both aPath and
    * aDescendantPath are absolute DOM path.
    */
   static bool
   IsDescendantPath(const nsAString& aPath, const nsAString& aDescendantPath);
-
-  static const char16_t kSeparatorChar = char16_t('/');
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_FileSystemUtils_h
--- a/dom/filesystem/GetDirectoryListingTask.cpp
+++ b/dom/filesystem/GetDirectoryListingTask.cpp
@@ -338,61 +338,39 @@ GetDirectoryListingTask::HandlerCallback
   if (!listing.SetLength(count, mozilla::fallible_t())) {
     mPromise->MaybeReject(NS_ERROR_FAILURE);
     mPromise = nullptr;
     return;
   }
 
   for (unsigned i = 0; i < count; i++) {
     if (mTargetData[i].mType == Directory::BlobImplOrDirectoryPath::eDirectoryPath) {
-#ifdef DEBUG
-      if (XRE_IsParentProcess()) {
-        nsCOMPtr<nsIFile> 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<nsIFile> directoryPath;
       NS_ConvertUTF16toUTF8 path(mTargetData[i].mDirectoryPath);
       nsresult rv = NS_NewNativeLocalFile(path, true,
                                           getter_AddRefs(directoryPath));
       if (NS_WARN_IF(NS_FAILED(rv))) {
         mPromise->MaybeReject(rv);
         mPromise = nullptr;
         return;
       }
 
+#ifdef DEBUG
+      nsCOMPtr<nsIFile> 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 = Directory::Create(mFileSystem->GetWindow(),
                                                       directoryPath,
                                                       Directory::eNotDOMRootDirectory,
                                                       mFileSystem);
       MOZ_ASSERT(directory);
 
       // Propogate mFilter onto sub-Directory object:
       directory->SetContentFilters(mFilters);
--- a/dom/filesystem/OSFileSystem.cpp
+++ b/dom/filesystem/OSFileSystem.cpp
@@ -46,17 +46,17 @@ OSFileSystem::GetWindow() const
 {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
   return mWindow;
 }
 
 void
 OSFileSystem::GetRootName(nsAString& aRetval) const
 {
-  return aRetval.AssignLiteral("/");
+  aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL);
 }
 
 bool
 OSFileSystem::IsSafeFile(nsIFile* aFile) const
 {
   // The concept of "safe files" is specific to the Device Storage API where
   // files are only "safe" if they're of a type that is appropriate for the
   // area of device storage that is being used.