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 264064 672e0e69482e4c76da9252a8a88b1574fb8e7ca4
parent 264063 d1f16f3ff302a05c834d78fce3cb48ca4e31c745
child 264065 4f1a52c1ba8b9b83b287fe8d771514adf9055105
push id65512
push userkwierso@gmail.com
push dateWed, 23 Sep 2015 20:23:51 +0000
treeherdermozilla-inbound@b00078e693a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdhylands
bugs1196315
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;