Bug 760643 - Device Storage - Delete isn't working. r=sicking
authorDoug Turner <dougt@dougt.org>
Fri, 01 Jun 2012 13:19:08 -0700
changeset 100575 efa54ecee724d349a5e00bc2b78f066022b551dc
parent 100574 2b2f7ab4304f63709dcdae388fce0e4ec4e42555
child 100576 1f4824e46f2391999a816f576469c4ebd7e6bdff
push idunknown
push userunknown
push dateunknown
reviewerssicking
bugs760643
milestone15.0a1
Bug 760643 - Device Storage - Delete isn't working. r=sicking
dom/devicestorage/nsDeviceStorage.cpp
--- a/dom/devicestorage/nsDeviceStorage.cpp
+++ b/dom/devicestorage/nsDeviceStorage.cpp
@@ -18,101 +18,117 @@
 #include "nsJSUtils.h"
 
 using namespace mozilla::dom;
 
 #include "nsDirectoryServiceDefs.h"
 
 class DeviceStorageFile : public nsISupports {
 public:
+
+  nsCOMPtr<nsIFile> mFile;
+  nsString mPath;
+
   DeviceStorageFile(nsIFile* aFile, const nsAString& aPath)
-    : mFile(aFile)
-    , mPath(aPath)
-    {
-      NS_ASSERTION(aFile, "Must not create a DeviceStorageFile with a null nsIFile");
-      NormalizeFilePath();
-    }
+  : mPath(aPath)
+  {
+    NS_ASSERTION(aFile, "Must not create a DeviceStorageFile with a null nsIFile");
+    // always take a clone
+    nsCOMPtr<nsIFile> file;
+    aFile->Clone(getter_AddRefs(mFile));
+
+    AppendRelativePath();
+
+    NormalizeFilePath();
+  }
+
   DeviceStorageFile(nsIFile* aFile)
-    : mFile(aFile)
-    {
-      NS_ASSERTION(aFile, "Must not create a DeviceStorageFile with a null nsIFile");
+  {
+    NS_ASSERTION(aFile, "Must not create a DeviceStorageFile with a null nsIFile");
+    // always take a clone
+    nsCOMPtr<nsIFile> file;
+    aFile->Clone(getter_AddRefs(mFile));
+  }
+
+  void
+  setPath(const nsAString& aPath) {
+    mPath.Assign(aPath);
+    NormalizeFilePath();
+  }
+
+  NS_DECL_ISUPPORTS
+
+  // we want to make sure that the names of file can't reach
+  // outside of the type of storage the user asked for.
+  bool
+  isSafePath()
+  {
+    nsAString::const_iterator start, end;
+    mPath.BeginReading(start);
+    mPath.EndReading(end);
+
+    // if the path has a ~ or \ in it, return false.
+    NS_NAMED_LITERAL_STRING(tilde, "~");
+    NS_NAMED_LITERAL_STRING(bslash, "\\");
+    if (FindInReadable(tilde, start, end) ||
+        FindInReadable(bslash, start, end)) {
+      return false;
     }
-    nsCOMPtr<nsIFile> mFile;
-    nsString mPath;
 
-    NS_DECL_ISUPPORTS
+    // split on /.  if any token is "", ., or .., return false.
+    NS_ConvertUTF16toUTF8 cname(mPath);
+    char* buffer = cname.BeginWriting();
+    const char* token;
+  
+    while ((token = nsCRT::strtok(buffer, "/", &buffer))) {
+      if (PL_strcmp(token, "") == 0 ||
+          PL_strcmp(token, ".") == 0 ||
+          PL_strcmp(token, "..") == 0 ) {
+            return false;
+      }
+    }
+    return true;
+  }
 
 private:
   void NormalizeFilePath() {
 #if defined(XP_WIN)
     PRUnichar* cur = mPath.BeginWriting();
     PRUnichar* end = mPath.EndWriting();
     for (; cur < end; ++cur) {
       if (PRUnichar('\\') == *cur)
 	*cur = PRUnichar('/');
     }
 #endif
   }
 
+  void AppendRelativePath() {
+#if defined(XP_WIN)
+    // replace forward slashes with backslashes,
+    // since nsLocalFileWin chokes on them
+    nsString temp;
+    temp.Assign(mPath);
+
+    PRUnichar* cur = temp.BeginWriting();
+    PRUnichar* end = temp.EndWriting();
+
+    for (; cur < end; ++cur) {
+      if (PRUnichar('/') == *cur)
+        *cur = PRUnichar('\\');
+    }
+    mFile->AppendRelativePath(temp);
+#else
+    mFile->AppendRelativePath(mPath);
+#endif
+  }
+
 };
 
 NS_IMPL_THREADSAFE_ISUPPORTS0(DeviceStorageFile)
 
-// we want to make sure that the names of file can't reach
-// outside of the type of storage the user asked for.
-bool
-isSafePath(const nsAString& aPath)
-{
-  nsAString::const_iterator start, end;
-  aPath.BeginReading(start);
-  aPath.EndReading(end);
-
-  // if the path has a ~ or \ in it, return false.
-  NS_NAMED_LITERAL_STRING(tilde, "~");
-  NS_NAMED_LITERAL_STRING(bslash, "\\");
-  if (FindInReadable(tilde, start, end) ||
-      FindInReadable(bslash, start, end)) {
-    return false;
-  }
-
-  // split on /.  if any token is "", ., or .., return false.
-  NS_ConvertUTF16toUTF8 cname(aPath);
-  char* buffer = cname.BeginWriting();
-  const char* token;
-
-  while ((token = nsCRT::strtok(buffer, "/", &buffer))) {
-    if (PL_strcmp(token, "") == 0 ||
-        PL_strcmp(token, ".") == 0 ||
-        PL_strcmp(token, "..") == 0 ) {
-          return false;
-    }
-  }
-  return true;
-}
-
-static void AppendRelativePath(nsIFile* file, const nsAString& aPath) {
-
-#if defined(XP_WIN)
-  // replace forward slashes with backslashes,
-  // since nsLocalFileWin chokes on them
-  nsString temp;
-  temp.Assign(aPath);
-
-  PRUnichar* cur = temp.BeginWriting();
-  PRUnichar* end = temp.EndWriting();
-
-  for (; cur < end; ++cur) {
-    if (PRUnichar('/') == *cur)
-      *cur = PRUnichar('\\');
-  }
-  file->AppendRelativePath(temp);
-#else
-  file->AppendRelativePath(aPath);
-#endif
-}
 
 // TODO - eventually, we will want to factor this method
 // out into different system specific subclasses (or
 // something)
 PRInt32
 nsDOMDeviceStorage::SetRootFileForType(const nsAString& aType, const PRInt32 aIndex)
 {
   PRInt32 typeResult = DEVICE_STORAGE_TYPE_DEFAULT;
@@ -466,27 +482,25 @@ public:
     if (!check) {
       nsCOMPtr<PostErrorEvent> event = new PostErrorEvent(mRequest,
                                                           POST_ERROR_EVENT_FILE_NOT_ENUMERABLE,
                                                           mFile);
       NS_DispatchToMainThread(event);
       return NS_OK;
     }
 
-    nsString fullpath;
-    mFile->mFile->GetPath(fullpath);
-    collectFiles(fullpath, mFile);
+    collectFiles(mFile);
 
     nsCOMPtr<ContinueCursorEvent> event = new ContinueCursorEvent(mRequest);
     NS_DispatchToMainThread(event);
 
     return NS_OK;
   }
 
-  void collectFiles(const nsString& aInitialFullPath, DeviceStorageFile* aFile)
+  void collectFiles(DeviceStorageFile* aFile)
   {
       // TODO - we may want to do this incrementally.
     if (!aFile)
       return;
 
     nsCOMPtr<nsISimpleEnumerator> e;
     aFile->mFile->GetDirectoryEntries(getter_AddRefs(e));
 
@@ -498,27 +512,31 @@ public:
       f->IsDirectory(&isDir);
 
       bool isFile;
       f->IsFile(&isFile);
 
       nsString fullpath;
       f->GetPath(fullpath);
 
-      nsAString::size_type len = aInitialFullPath.Length() + 1; // +1 for the trailing /
-      nsDependentSubstring newPath = Substring(fullpath, len);
+      nsString rootPath;
+      mFile->mFile->GetPath(rootPath);
 
-      if (!StringBeginsWith(fullpath, aInitialFullPath)) {
+      if (!StringBeginsWith(fullpath, rootPath)) {
 	NS_WARNING("collectFiles returned a path that does not belong!");
 	continue;
       }
 
-      nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(f, newPath);
+      nsAString::size_type len = rootPath.Length() + 1; // +1 for the trailing /
+      nsDependentSubstring newPath = Substring(fullpath, len);
+      nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(f);
+      dsf->setPath(newPath);
+
       if (isDir) {
-	 collectFiles(aInitialFullPath, dsf);
+        collectFiles(dsf);
       }
       else if (isFile) {
         nsDOMDeviceStorageCursor* cursor = static_cast<nsDOMDeviceStorageCursor*>(mRequest.get());
         cursor->mFiles.AppendElement(dsf);
       }
     }
   }
 
@@ -592,26 +610,24 @@ nsDOMDeviceStorageCursor::Cancel()
                                                       mFile);
   NS_DispatchToMainThread(event);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMDeviceStorageCursor::Allow()
 {
-  if (!isSafePath(mFile->mPath)) {
+  if (!mFile->isSafePath()) {
     nsCOMPtr<nsIRunnable> r = new PostErrorEvent(this,
                                                  POST_ERROR_EVENT_ILLEGAL_FILE_NAME,
                                                  mFile);
     NS_DispatchToMainThread(r);
     return NS_OK;
   }
 
-  AppendRelativePath(mFile->mFile, mFile->mPath);
-
   nsCOMPtr<nsIEventTarget> target = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
   NS_ASSERTION(target, "Must have stream transport service");
 
   nsCOMPtr<InitCursorEvent> event = new InitCursorEvent(this, mFile);
   target->Dispatch(event, NS_DISPATCH_NORMAL);
   return NS_OK;
 }
 
@@ -828,18 +844,28 @@ public:
 
   ~DeleteFileEvent() {}
     
   NS_IMETHOD Run() 
   {
     NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
 
     mFile->mFile->Remove(true);
-    nsCOMPtr<PostResultEvent> event = new PostResultEvent(mRequest, mFile->mPath);
-    NS_DispatchToMainThread(event);
+
+    nsRefPtr<nsRunnable> r;
+
+    bool check = false;
+    mFile->mFile->Exists(&check);
+    if (check) {
+      r = new PostErrorEvent(mRequest, POST_ERROR_EVENT_UNKNOWN, mFile);
+    }
+    else {
+      r = new PostResultEvent(mRequest, mFile->mPath);
+    }
+    NS_DispatchToMainThread(r);
     return NS_OK;
   }
 
 private:
   nsRefPtr<DeviceStorageFile> mFile;
   nsRefPtr<DOMRequest> mRequest;
 };
 
@@ -1108,23 +1134,19 @@ nsDOMDeviceStorage::AddNamed(nsIDOMBlob 
     return NS_ERROR_UNEXPECTED;
   }
   
   nsRefPtr<DOMRequest> request = new DOMRequest(win);
   NS_ADDREF(*_retval = request);
 
   nsCOMPtr<nsIRunnable> r;
 
-  nsCOMPtr<nsIFile> file;
-  mFile->Clone(getter_AddRefs(file));
-  AppendRelativePath(file, aPath);
+  nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile, aPath);
 
-  nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(file, aPath);
-
-  if (!isSafePath(aPath)) {
+  if (!dsf->isSafePath()) {
     r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf);
   }
   else {
     r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_WRITE,
                                  win, mURI, dsf, request, true, aBlob);
   }
   NS_DispatchToMainThread(r);
   return NS_OK;
@@ -1164,27 +1186,25 @@ nsDOMDeviceStorage::GetInternal(const JS
 
   JSString* jsstr = JS_ValueToString(aCx, aPath);
   nsDependentJSString path;
   if (!path.init(aCx, jsstr)) {
     nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile);
     r = new PostErrorEvent(request,
                            POST_ERROR_EVENT_NON_STRING_TYPE_UNSUPPORTED,
                            dsf);
-  } else if (!isSafePath(path)) {
-    nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile, path);
+    NS_DispatchToMainThread(r);
+    return NS_OK;
+  }
+
+  nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile, path);
+
+  if (!dsf->isSafePath()) {
     r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf);
   } else {
-
-    nsCOMPtr<nsIFile> file;
-    mFile->Clone(getter_AddRefs(file));
-    AppendRelativePath(file, path);
-
-    nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(file, path);
-
     r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_READ,
                                  win, mURI, dsf, request, aEditable);
   }
   NS_DispatchToMainThread(r);
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -1200,26 +1220,26 @@ nsDOMDeviceStorage::Delete(const JS::Val
   nsRefPtr<DOMRequest> request = new DOMRequest(win);
   NS_ADDREF(*_retval = request);
 
   JSString* jsstr = JS_ValueToString(aCx, aPath);
   nsDependentJSString path;
   if (!path.init(aCx, jsstr)) {
     nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile);
     r = new PostErrorEvent(request, POST_ERROR_EVENT_NON_STRING_TYPE_UNSUPPORTED, dsf);
-  } else if (!isSafePath(path)) {
-    nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile, path);
+    NS_DispatchToMainThread(r);
+    return NS_OK;
+  }
+
+  nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mFile, path);
+
+  if (!dsf->isSafePath()) {
     r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf);
   }
   else {
-    nsCOMPtr<nsIFile> file;
-    mFile->Clone(getter_AddRefs(file));
-    AppendRelativePath(file, path);
-
-    nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(file, path);
     r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_DELETE,
                                  win, mURI, dsf, request, true);
   }
   NS_DispatchToMainThread(r);
   return NS_OK;
 }
 
 NS_IMETHODIMP