Bug 1196315 - Ensure MIME service is only accessed on the main thread. r=dhylands
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 16 Sep 2015 12:18:25 -0400
changeset 263867 672e0e69482e4c76da9252a8a88b1574fb8e7ca4
parent 263866 d1f16f3ff302a05c834d78fce3cb48ca4e31c745
child 263868 4f1a52c1ba8b9b83b287fe8d771514adf9055105
push id17650
push useraosmond@gmail.com
push dateWed, 23 Sep 2015 10:13:32 +0000
treeherderb2g-inbound@672e0e69482e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdhylands
bugs1196315
milestone44.0a1
Bug 1196315 - Ensure MIME service is only accessed on the main thread. r=dhylands
dom/devicestorage/DeviceStorage.h
dom/devicestorage/nsDeviceStorage.cpp
dom/devicestorage/nsDeviceStorage.h
--- a/dom/devicestorage/DeviceStorage.h
+++ b/dom/devicestorage/DeviceStorage.h
@@ -82,16 +82,23 @@ public:
   void SetPath(const nsAString& aPath);
   void SetEditable(bool aEditable);
 
   static already_AddRefed<DeviceStorageFile>
   CreateUnique(nsAString& aFileName,
                uint32_t aFileType,
                uint32_t aFileAttributes);
 
+  static already_AddRefed<DeviceStorageFile>
+  CreateUnique(const nsAString& aStorageType,
+               const nsAString& aStorageName,
+               nsAString& aFileName,
+               uint32_t aFileType,
+               uint32_t aFileAttributes);
+
   NS_DECL_THREADSAFE_ISUPPORTS
 
   bool IsAvailable();
   void GetFullPath(nsAString& aFullPath);
 
   // 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();
--- a/dom/devicestorage/nsDeviceStorage.cpp
+++ b/dom/devicestorage/nsDeviceStorage.cpp
@@ -688,18 +688,30 @@ DeviceStorageFile::CreateUnique(nsAStrin
   nsString storageName;
   nsString storagePath;
   if (!nsDOMDeviceStorage::ParseFullPath(aFileName, storageName, storagePath)) {
     return nullptr;
   }
   if (storageName.IsEmpty()) {
     nsDOMDeviceStorage::GetDefaultStorageName(storageType, storageName);
   }
+  return CreateUnique(storageType, storageName, storagePath,
+                      aFileType, aFileAttributes);
+}
+
+//static
+already_AddRefed<DeviceStorageFile>
+DeviceStorageFile::CreateUnique(const nsAString& aStorageType,
+                                const nsAString& aStorageName,
+                                nsAString& aFileName,
+                                uint32_t aFileType,
+                                uint32_t aFileAttributes)
+{
   nsRefPtr<DeviceStorageFile> dsf =
-    new DeviceStorageFile(storageType, storageName, storagePath);
+    new DeviceStorageFile(aStorageType, aStorageName, aFileName);
   if (!dsf->mFile) {
     return nullptr;
   }
 
   nsresult rv = dsf->mFile->CreateUnique(aFileType, aFileAttributes);
   NS_ENSURE_SUCCESS(rv, nullptr);
 
   // CreateUnique may cause the filename to change. So we need to update mPath
@@ -1494,16 +1506,17 @@ nsDOMDeviceStorageCursor::Continue(Error
   mOkToCallContinue = false;
   aRv = mRequest->Continue();
 }
 
 DeviceStorageRequest::DeviceStorageRequest()
   : mId(DeviceStorageRequestManager::INVALID_ID)
   , mAccess(DEVICE_STORAGE_ACCESS_UNDEFINED)
   , mSendToParent(true)
+  , mUseMainThread(false)
   , mUseStreamTransport(false)
   , mCheckFile(false)
   , mCheckBlob(false)
   , mMultipleResolve(false)
   , mPermissionCached(true)
 {
   DS_LOG_DEBUG("%p", this);
 }
@@ -1573,21 +1586,40 @@ nsresult
 DeviceStorageRequest::Cancel()
 {
   return Reject(POST_ERROR_EVENT_PERMISSION_DENIED);
 }
 
 nsresult
 DeviceStorageRequest::Allow()
 {
+  if (mUseMainThread && !NS_IsMainThread()) {
+    nsRefPtr<DeviceStorageRequest> self = this;
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([self] () -> void
+    {
+      self->Allow();
+    });
+    return NS_DispatchToMainThread(r);
+  }
+
   nsresult rv = AllowInternal();
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return Reject(rv == NS_ERROR_ILLEGAL_VALUE
-                  ? POST_ERROR_EVENT_ILLEGAL_TYPE
-                  : POST_ERROR_EVENT_UNKNOWN);
+    const char *reason;
+    switch (rv) {
+      case NS_ERROR_ILLEGAL_VALUE:
+        reason = POST_ERROR_EVENT_ILLEGAL_TYPE;
+        break;
+      case NS_ERROR_DOM_SECURITY_ERR:
+        reason = POST_ERROR_EVENT_PERMISSION_DENIED;
+        break;
+      default:
+        reason = POST_ERROR_EVENT_UNKNOWN;
+        break;
+    }
+    return Reject(reason);
   }
   return rv;
 }
 
 DeviceStorageFile*
 DeviceStorageRequest::GetFile() const
 {
   MOZ_ASSERT(mFile);
@@ -1602,33 +1634,38 @@ DeviceStorageRequest::GetFileDescriptor(
 }
 
 DeviceStorageRequestManager*
 DeviceStorageRequest::GetManager() const
 {
   return mManager;
 }
 
-void
+nsresult
 DeviceStorageRequest::Prepare()
 {
+  return NS_OK;
 }
 
 nsresult
 DeviceStorageRequest::CreateSendParams(DeviceStorageParams& aParams)
 {
   MOZ_ASSERT_UNREACHABLE("Cannot send to parent, missing param creator");
   return NS_ERROR_UNEXPECTED;
 }
 
 nsresult
 DeviceStorageRequest::AllowInternal()
 {
   MOZ_ASSERT(mManager->IsOwningThread() || NS_IsMainThread());
-  Prepare();
+
+  nsresult rv = Prepare();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   DeviceStorageTypeChecker* typeChecker
     = DeviceStorageTypeChecker::CreateOrGet();
   if (!typeChecker) {
     return NS_ERROR_UNEXPECTED;
   }
   if (mCheckBlob && (!mBlob ||
       !typeChecker->Check(mFile->mStorageType, mBlob))) {
@@ -1864,16 +1901,18 @@ protected:
     return NS_OK;
   }
 };
 
 class DeviceStorageCreateRequest final
   : public DeviceStorageRequest
 {
 public:
+  using DeviceStorageRequest::Initialize;
+
   DeviceStorageCreateRequest()
   {
     mAccess = DEVICE_STORAGE_ACCESS_CREATE;
     mUseStreamTransport = true;
     DS_LOG_INFO("");
   }
 
   NS_IMETHOD Run() override
@@ -1898,17 +1937,72 @@ public:
       return Reject(POST_ERROR_EVENT_UNKNOWN);
     }
 
     nsString fullPath;
     mFile->GetFullPath(fullPath);
     return Resolve(fullPath);
   }
 
+  void Initialize(DeviceStorageRequestManager* aManager,
+                  DeviceStorageFile* aFile,
+                  uint32_t aRequest) override
+  {
+    DeviceStorageRequest::Initialize(aManager, aFile, aRequest);
+    mUseMainThread = aFile->mPath.IsEmpty();
+  }
+
 protected:
+  nsresult Prepare() override
+  {
+    if (!mFile->mPath.IsEmpty()) {
+      // Checks have already been performed when request was created
+      return NS_OK;
+    }
+
+    MOZ_ASSERT(NS_IsMainThread());
+
+    nsCOMPtr<nsIMIMEService> mimeSvc = do_GetService(NS_MIMESERVICE_CONTRACTID);
+    if (!mimeSvc) {
+      return NS_ERROR_FAILURE;
+    }
+
+    // if mimeType or extension are null, the request will be rejected
+    // in DeviceStorageRequest::AllowInternal when the type checker
+    // verifies the file path
+    nsString mimeType;
+    mBlob->GetType(mimeType);
+
+    nsCString extension;
+    mimeSvc->GetPrimaryExtension(NS_LossyConvertUTF16toASCII(mimeType),
+                                 EmptyCString(), extension);
+
+    char buffer[32];
+    NS_MakeRandomString(buffer, ArrayLength(buffer) - 1);
+
+    nsAutoString path;
+    path.AssignLiteral(buffer);
+    path.Append('.');
+    path.AppendASCII(extension.get());
+
+    nsRefPtr<DeviceStorageFile> file
+      = DeviceStorageFile::CreateUnique(mFile->mStorageType,
+                                        mFile->mStorageName, path,
+                                        nsIFile::NORMAL_FILE_TYPE, 00600);
+    if (!file) {
+      return NS_ERROR_FAILURE;
+    }
+    if (!file->IsSafePath()) {
+      return NS_ERROR_DOM_SECURITY_ERR;
+    }
+
+    mFile = file.forget();
+    return NS_OK;
+  }
+
   nsresult CreateSendParams(DeviceStorageParams& aParams) override
   {
     BlobChild* actor
       = ContentChild::GetSingleton()->GetOrCreateActorForBlobImpl(mBlob);
     if (!actor) {
       return NS_ERROR_FAILURE;
     }
 
@@ -1981,16 +2075,17 @@ protected:
 class DeviceStorageOpenRequest final
   : public DeviceStorageRequest
 {
 public:
   using DeviceStorageRequest::Initialize;
 
   DeviceStorageOpenRequest()
   {
+    mUseMainThread = true;
     mUseStreamTransport = true;
     mCheckFile = true;
     DS_LOG_INFO("");
   }
 
   void Initialize(DeviceStorageRequestManager* aManager,
                   DeviceStorageFile* aFile,
                   uint32_t aRequest) override
@@ -2014,20 +2109,21 @@ public:
     if (NS_FAILED(rv)) {
       return Reject(POST_ERROR_EVENT_UNKNOWN);
     }
 
     return Resolve(mFile);
   }
 
 protected:
-  void Prepare() override
+  nsresult Prepare() override
   {
     MOZ_ASSERT(NS_IsMainThread());
     mFile->CalculateMimeType();
+    return NS_OK;
   }
 
   nsresult CreateSendParams(DeviceStorageParams& aParams) override
   {
     DeviceStorageGetParams params(mFile->mStorageType,
                                   mFile->mStorageName,
                                   mFile->mRootDir,
                                   mFile->mPath);
@@ -2921,62 +3017,39 @@ nsDOMDeviceStorage::IsAvailable()
 {
   nsRefPtr<DeviceStorageFile> dsf(new DeviceStorageFile(mStorageType, mStorageName));
   return dsf->IsAvailable();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::Add(Blob* aBlob, ErrorResult& aRv)
 {
-  if (!aBlob) {
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIMIMEService> mimeSvc = do_GetService(NS_MIMESERVICE_CONTRACTID);
-  if (!mimeSvc) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
-  // if mimeType isn't set, we will not get a correct
-  // extension, and AddNamed() will fail.  This will post an
-  // onerror to the requestee.
-  nsString mimeType;
-  aBlob->GetType(mimeType);
-
-  nsCString extension;
-  mimeSvc->GetPrimaryExtension(NS_LossyConvertUTF16toASCII(mimeType),
-                               EmptyCString(), extension);
-  // if extension is null here, we will ignore it for now.
-  // AddNamed() will check the file path and fail.  This
-  // will post an onerror to the requestee.
-
-  // possible race here w/ unique filename
-  char buffer[32];
-  NS_MakeRandomString(buffer, ArrayLength(buffer) - 1);
-
-  nsAutoCString path;
-  path.Assign(nsDependentCString(buffer));
-  path.Append('.');
-  path.Append(extension);
-
-  return AddNamed(aBlob, NS_ConvertASCIItoUTF16(path), aRv);
+  nsString path;
+  return AddOrAppendNamed(aBlob, path, true, aRv);
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::AddNamed(Blob* aBlob, const nsAString& aPath,
                              ErrorResult& aRv)
 {
+  if (aPath.IsEmpty()) {
+    aRv.Throw(NS_ERROR_ILLEGAL_VALUE);
+    return nullptr;
+  }
   return AddOrAppendNamed(aBlob, aPath, true, aRv);
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::AppendNamed(Blob* aBlob, const nsAString& aPath,
                                 ErrorResult& aRv)
 {
+  if (aPath.IsEmpty()) {
+    aRv.Throw(NS_ERROR_ILLEGAL_VALUE);
+    return nullptr;
+  }
   return AddOrAppendNamed(aBlob, aPath, false, aRv);
 }
 
 uint32_t
 nsDOMDeviceStorage::CreateDOMRequest(DOMRequest** aRequest, ErrorResult& aRv)
 {
   if (!mManager) {
     DS_LOG_WARN("shutdown");
@@ -3012,29 +3085,23 @@ nsDOMDeviceStorage::CreateAndRejectDOMRe
   return request.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::AddOrAppendNamed(Blob* aBlob, const nsAString& aPath,
                                      bool aCreate, ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
+  MOZ_ASSERT(aCreate || !aPath.IsEmpty());
 
   // if the blob is null here, bail
   if (!aBlob) {
     return nullptr;
   }
 
-  DeviceStorageTypeChecker* typeChecker
-    = DeviceStorageTypeChecker::CreateOrGet();
-  if (!typeChecker) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
   nsCOMPtr<nsIRunnable> r;
 
   if (IsFullPath(aPath)) {
     nsString storagePath;
     nsRefPtr<nsDOMDeviceStorage> ds = GetStorage(aPath, storagePath);
     if (!ds) {
       return CreateAndRejectDOMRequest(POST_ERROR_EVENT_UNKNOWN, aRv);
     }
@@ -3043,35 +3110,35 @@ nsDOMDeviceStorage::AddOrAppendNamed(Blo
   }
 
   nsRefPtr<DOMRequest> domRequest;
   uint32_t id = CreateDOMRequest(getter_AddRefs(domRequest), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
-  nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(mStorageType,
-                                                          mStorageName,
-                                                          aPath);
-  if (!dsf->IsSafePath()) {
-    aRv = mManager->Reject(id, POST_ERROR_EVENT_PERMISSION_DENIED);
-  } else if (!typeChecker->Check(mStorageType, dsf->mFile) ||
-      !typeChecker->Check(mStorageType, aBlob->Impl())) {
-    aRv = mManager->Reject(id, POST_ERROR_EVENT_ILLEGAL_TYPE);
+  nsRefPtr<DeviceStorageFile> dsf;
+  if (aPath.IsEmpty()) {
+    dsf = new DeviceStorageFile(mStorageType, mStorageName);
   } else {
-    nsRefPtr<DeviceStorageRequest> request;
-    if (aCreate) {
-      request = new DeviceStorageCreateRequest();
-    } else {
-      request = new DeviceStorageAppendRequest();
+    dsf = new DeviceStorageFile(mStorageType, mStorageName, aPath);
+    if (!dsf->IsSafePath()) {
+      aRv = mManager->Reject(id, POST_ERROR_EVENT_PERMISSION_DENIED);
+      return domRequest.forget();
     }
-    request->Initialize(mManager, dsf, id, aBlob->Impl());
-    aRv = CheckPermission(request);
-  }
-
+  }
+
+  nsRefPtr<DeviceStorageRequest> request;
+  if (aCreate) {
+    request = new DeviceStorageCreateRequest();
+  } else {
+    request = new DeviceStorageAppendRequest();
+  }
+  request->Initialize(mManager, dsf, id, aBlob->Impl());
+  aRv = CheckPermission(request);
   return domRequest.forget();
 }
 
 already_AddRefed<DOMRequest>
 nsDOMDeviceStorage::GetInternal(const nsAString& aPath, bool aEditable,
                                 ErrorResult& aRv)
 {
   MOZ_ASSERT(IsOwningThread());
--- a/dom/devicestorage/nsDeviceStorage.h
+++ b/dom/devicestorage/nsDeviceStorage.h
@@ -373,28 +373,29 @@ public:
 
 protected:
   bool ForceDispatch() const
   {
     return !mSendToParent && mPermissionCached;
   }
 
   virtual ~DeviceStorageRequest();
-  virtual void Prepare();
+  virtual nsresult Prepare();
   virtual nsresult CreateSendParams(mozilla::dom::DeviceStorageParams& aParams);
   nsresult AllowInternal();
   nsresult SendToParentProcess();
 
   nsRefPtr<DeviceStorageRequestManager> mManager;
   nsRefPtr<DeviceStorageFile> mFile;
   uint32_t mId;
   nsRefPtr<mozilla::dom::BlobImpl> mBlob;
   nsRefPtr<DeviceStorageFileDescriptor> mDSFileDescriptor;
   DeviceStorageAccessType mAccess;
   bool mSendToParent;
+  bool mUseMainThread;
   bool mUseStreamTransport;
   bool mCheckFile;
   bool mCheckBlob;
   bool mMultipleResolve;
   bool mPermissionCached;
 
 private:
   DeviceStorageRequest(const DeviceStorageRequest&) = delete;