Bug 1198095 - FileReader should dispatch an error if the blob changes size in the meantime the read is executed, r=bz
☠☠ backed out by e257e66b682b ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 02 Dec 2015 18:46:23 +0000
changeset 309441 269290441727275e48387abe813e473a25dd4d87
parent 309440 8042563db419d9566f28aede0cfcbef8be61707d
child 309442 87d9fe68abf24bd062bc8ef140c3fed161ec6182
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1198095
milestone45.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 1198095 - FileReader should dispatch an error if the blob changes size in the meantime the read is executed, r=bz
dom/base/FileIOObject.h
dom/base/nsDOMFileReader.cpp
dom/base/test/file_bug1198095.js
dom/base/test/mochitest.ini
dom/base/test/test_bug1198095.html
--- a/dom/base/FileIOObject.h
+++ b/dom/base/FileIOObject.h
@@ -75,17 +75,23 @@ protected:
   virtual nsresult DoOnLoadEnd(nsresult aStatus, nsAString& aSuccessEvent,
                                nsAString& aTerminationEvent) = 0;
 
   nsresult OnLoadEnd(nsresult aStatus);
   nsresult DoAsyncWait(nsIAsyncInputStream* aStream);
 
   void StartProgressEventTimer();
   void ClearProgressEventTimer();
+
+  // This method generates and dispatches:
+  // - NotFoundError if rv is NS_ERROR_FILE_NOT_FOUND
+  // - SecurityError if NS_ERROR_FILE_ACCESS_DENIED
+  // - NotReadableError with any other rv value.
   void DispatchError(nsresult rv, nsAString& finalEvent);
+
   nsresult DispatchProgressEvent(const nsAString& aType);
 
   nsCOMPtr<nsITimer> mProgressNotifier;
   bool mProgressEventWasDelayed;
   bool mTimerIsActive;
 
   nsCOMPtr<nsIAsyncInputStream> mAsyncStream;
 
--- a/dom/base/nsDOMFileReader.cpp
+++ b/dom/base/nsDOMFileReader.cpp
@@ -270,43 +270,53 @@ ReadFuncBinaryString(nsIInputStream* in,
   return NS_OK;
 }
 
 nsresult
 nsDOMFileReader::DoOnLoadEnd(nsresult aStatus,
                              nsAString& aSuccessEvent,
                              nsAString& aTerminationEvent)
 {
-
   // Make sure we drop all the objects that could hold files open now.
   nsCOMPtr<nsIAsyncInputStream> stream;
   mAsyncStream.swap(stream);
 
   RefPtr<Blob> blob;
   mBlob.swap(blob);
 
+  // In case we read a different number of bytes, we can assume that the
+  // underlying storage has changed. We should not continue.
+  if (mDataLen != mTotal) {
+    // We use NS_ERROR_FAILURE because that will dispatch a NotReadableError in
+    // FileIOObject::DispatchError.
+    DispatchError(NS_ERROR_FAILURE, aTerminationEvent);
+    FreeFileData();
+    return NS_ERROR_FAILURE;
+  }
+
   aSuccessEvent = NS_LITERAL_STRING(LOAD_STR);
   aTerminationEvent = NS_LITERAL_STRING(LOADEND_STR);
 
   // Clear out the data if necessary
   if (NS_FAILED(aStatus)) {
     FreeFileData();
     return NS_OK;
   }
 
   nsresult rv = NS_OK;
   switch (mDataFormat) {
     case FILE_AS_ARRAYBUFFER: {
       AutoJSAPI jsapi;
       if (NS_WARN_IF(!jsapi.Init(mozilla::DOMEventTargetHelper::GetParentObject()))) {
+        FreeFileData();
         return NS_ERROR_FAILURE;
       }
 
       RootResultArrayBuffer();
-      mResultArrayBuffer = JS_NewArrayBufferWithContents(jsapi.cx(), mTotal, mFileData);
+      mResultArrayBuffer = JS_NewArrayBufferWithContents(jsapi.cx(), mDataLen, mFileData);
       if (!mResultArrayBuffer) {
         JS_ClearPendingException(jsapi.cx());
         rv = NS_ERROR_OUT_OF_MEMORY;
       } else {
         mFileData = nullptr; // Transfer ownership
       }
       break;
     }
@@ -338,18 +348,17 @@ nsDOMFileReader::DoOnLoadEnd(nsresult aS
 nsresult
 nsDOMFileReader::DoReadData(nsIAsyncInputStream* aStream, uint64_t aCount)
 {
   MOZ_ASSERT(aStream);
 
   if (mDataFormat == FILE_AS_BINARY) {
     //Continuously update our binary string as data comes in
     uint32_t oldLen = mResult.Length();
-    NS_ASSERTION(mResult.Length() == mDataLen,
-                 "unexpected mResult length");
+    NS_ASSERTION(mResult.Length() == mDataLen, "unexpected mResult length");
     if (uint64_t(oldLen) + aCount > UINT32_MAX)
       return NS_ERROR_OUT_OF_MEMORY;
     char16_t *buf = nullptr;
     mResult.GetMutableData(&buf, oldLen + aCount, fallible);
     NS_ENSURE_TRUE(buf, NS_ERROR_OUT_OF_MEMORY);
 
     uint32_t bytesRead = 0;
     aStream->ReadSegments(ReadFuncBinaryString, buf + oldLen, aCount,
new file mode 100644
--- /dev/null
+++ b/dom/base/test/file_bug1198095.js
@@ -0,0 +1,26 @@
+var { classes: Cc, interfaces: Ci, utils: Cu } = Components;
+Cu.importGlobalProperties(["File"]);
+
+function createFileWithData(message) {
+  var dirSvc = Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties);
+  var testFile = dirSvc.get("ProfD", Ci.nsIFile);
+  testFile.append("fileAPItestfileBug1198095");
+
+  var outStream = Cc["@mozilla.org/network/file-output-stream;1"].createInstance(Ci.nsIFileOutputStream);
+  outStream.init(testFile, 0x02 | 0x08 | 0x20, // write, create, truncate
+                 0666, 0);
+
+  outStream.write(message, message.length);
+  outStream.close();
+
+  var domFile = new File(testFile);
+  return domFile;
+}
+
+addMessageListener("file.open", function (message) {
+  sendAsyncMessage("file.opened", createFileWithData(message));
+});
+
+addMessageListener("file.modify", function (message) {
+  sendAsyncMessage("file.modified", createFileWithData(message));
+});
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -252,16 +252,17 @@ support-files =
   iframe_postMessages.html
   test_performance_observer.js
   performance_observer.html
   test_anonymousContent_style_csp.html^headers^
   file_explicit_user_agent.sjs
   referrer_change_server.sjs
   file_change_policy_redirect.html
   empty_worker.js
+  file_bug1198095.js
 
 [test_anonymousContent_api.html]
 [test_anonymousContent_append_after_reflow.html]
 [test_anonymousContent_canvas.html]
 skip-if = buildapp == 'b2g' # Requires webgl support
 [test_anonymousContent_insert.html]
 [test_anonymousContent_manipulate_content.html]
 [test_anonymousContent_style_csp.html]
@@ -855,8 +856,9 @@ support-files = worker_postMessages.js
 [test_window_proto.html]
 [test_frameLoader_switchProcess.html]
 skip-if = e10s || os != 'linux' || buildapp != 'browser'
 [test_explicit_user_agent.html]
 [test_change_policy.html]
 skip-if = buildapp == 'b2g' #no ssl support
 [test_document.all_iteration.html]
 [test_performance_translate.html]
+[test_bug1198095.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_bug1198095.html
@@ -0,0 +1,77 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1198095
+-->
+  <title>Test for Bug 1198095</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1198095">Mozilla Bug 1198095</a>
+
+<pre id="test">
+<script class="testbody" type="text/javascript;version=1.7">
+
+var fileData1 = '1234567890';
+var fileData2 = '43210';
+var r, firstBlob;
+
+var openerURL = SimpleTest.getTestFileURL("file_bug1198095.js");
+
+var opener = SpecialPowers.loadChromeScript(openerURL);
+opener.addMessageListener("file.opened", onFileOpened);
+opener.addMessageListener("file.modified", onFileModified);
+opener.sendAsyncMessage("file.open", fileData1);
+
+function onLoadEnd1(e) {
+  e.target.removeEventListener('loadend', onLoadEnd1);
+
+  is(e.target, r, "Target and r are ok");
+  ok(e.target.readyState, FileReader.DONE, "The file has been read.");
+  ok(e.target.result instanceof ArrayBuffer, "The result is an ArrayBuffer");
+
+  var view = new Uint8Array(e.target.result);
+  is(view.length, fileData1.length, "File data length matches");
+  for (var i = 0; i < fileData1.length; ++i) {
+    is(String.fromCharCode(view[i]), fileData1[i], "Byte matches");
+  }
+
+  opener.sendAsyncMessage("file.modify", fileData2);
+}
+
+function onLoadEnd2(e) {
+  e.target.removeEventListener('loadend', onLoadEnd2);
+  ok(false, "This method should not be called - loadEnd2!");
+}
+
+function onError1(e) {
+  ok(false, "This method should not be called - error1!");
+}
+
+function onError2(e) {
+  e.target.removeEventListener('error', onError2);
+  SimpleTest.finish();
+}
+
+function onFileOpened(blob) {
+  firstBlob = blob;
+  r = new FileReader();
+  r.addEventListener("loadend", onLoadEnd1, false);
+  r.addEventListener("error", onError1, false);
+  r.readAsArrayBuffer(firstBlob);
+}
+
+function onFileModified(blob) {
+  r.addEventListener("loadend", onLoadEnd2, false);
+  r.removeEventListener('error', onError1);
+  r.addEventListener("error", onError2, false);
+  r.readAsArrayBuffer(firstBlob);
+}
+
+SimpleTest.waitForExplicitFinish();
+</script>
+</pre>
+</body> </html>