Bug 1303999 - Check our kind after filling in external data in GetAsFile and GetAsString, r=baku
authorMichael Layzell <michael@thelayzells.com>
Fri, 23 Sep 2016 14:44:47 -0400
changeset 427714 71ee175c9c00efc3d59b80a871e4c8b38989d76a
parent 427713 a6787db4d131dce2eabe0fab90c0e03e531190b5
child 427715 3201c03e0c16fcba25c23af79cec8b08ea174ab7
push id33099
push userdholbert@mozilla.com
push dateThu, 20 Oct 2016 20:25:54 +0000
reviewersbaku
bugs1303999
milestone52.0a1
Bug 1303999 - Check our kind after filling in external data in GetAsFile and GetAsString, r=baku MozReview-Commit-ID: CbHWTDTY3Xs
dom/events/DataTransferItem.cpp
--- a/dom/events/DataTransferItem.cpp
+++ b/dom/events/DataTransferItem.cpp
@@ -229,27 +229,31 @@ DataTransferItem::FillInExternalData()
     mDataTransfer->TypesListMayHaveChanged();
   }
 }
 
 already_AddRefed<File>
 DataTransferItem::GetAsFile(nsIPrincipal& aSubjectPrincipal,
                             ErrorResult& aRv)
 {
-  if (mKind != KIND_FILE) {
-    return nullptr;
-  }
-
   // This is done even if we have an mCachedFile, as it performs the necessary
   // permissions checks to ensure that we are allowed to access this type.
   nsCOMPtr<nsIVariant> data = Data(&aSubjectPrincipal, aRv);
   if (NS_WARN_IF(!data || aRv.Failed())) {
     return nullptr;
   }
 
+  // We have to check our kind after getting the data, because if we have
+  // external data and the OS lied to us (which unfortunately does happen
+  // sometimes), then we might not have the same type of data as we did coming
+  // into this function.
+  if (NS_WARN_IF(mKind != KIND_FILE)) {
+    return nullptr;
+  }
+
   // Generate the dom::File from the stored data, caching it so that the
   // same object is returned in the future.
   if (!mCachedFile) {
     nsCOMPtr<nsISupports> supports;
     aRv = data->GetAsISupports(getter_AddRefs(supports));
     MOZ_ASSERT(!aRv.Failed() && supports,
                "File objects should be stored as nsISupports variants");
     if (aRv.Failed() || !supports) {
@@ -261,16 +265,17 @@ DataTransferItem::GetAsFile(nsIPrincipal
       mCachedFile = blob->ToFile();
     } else if (nsCOMPtr<BlobImpl> blobImpl = do_QueryInterface(supports)) {
       MOZ_ASSERT(blobImpl->IsFile());
       mCachedFile = File::Create(mDataTransfer, blobImpl);
     } else if (nsCOMPtr<nsIFile> ifile = do_QueryInterface(supports)) {
       mCachedFile = File::CreateFromFile(mDataTransfer, ifile);
     } else {
       MOZ_ASSERT(false, "One of the above code paths should be taken");
+      return nullptr;
     }
   }
 
   RefPtr<File> file = mCachedFile;
   return file.forget();
 }
 
 already_AddRefed<FileSystemEntry>
@@ -373,28 +378,36 @@ DataTransferItem::CreateFileFromInputStr
                                 mType, PR_Now());
 }
 
 void
 DataTransferItem::GetAsString(FunctionStringCallback* aCallback,
                               nsIPrincipal& aSubjectPrincipal,
                               ErrorResult& aRv)
 {
-  if (!aCallback || mKind != KIND_STRING) {
+  if (!aCallback) {
     return;
   }
 
   // Theoretically this should be done inside of the runnable, as it might be an
   // expensive operation on some systems, however we wouldn't get access to the
   // NS_ERROR_DOM_SECURITY_ERROR messages which may be raised by this method.
   nsCOMPtr<nsIVariant> data = Data(&aSubjectPrincipal, aRv);
   if (NS_WARN_IF(!data || aRv.Failed())) {
     return;
   }
 
+  // We have to check our kind after getting the data, because if we have
+  // external data and the OS lied to us (which unfortunately does happen
+  // sometimes), then we might not have the same type of data as we did coming
+  // into this function.
+  if (NS_WARN_IF(mKind != KIND_STRING)) {
+    return;
+  }
+
   nsAutoString stringData;
   nsresult rv = data->GetAsAString(stringData);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
   // Dispatch the callback to the main thread
   class GASRunnable final : public Runnable