Bug 1405286: Part 4 - Don't overwrite existing state with finishedtransferringdata. r?mixedpuppy
In cases where data transfer finishes immediately after we close a request, we
can sometimes wind up overwriting that state information with
"finishedtransferringdata", which allows scripted callers to break certain
invariants and cause crashes.
MozReview-Commit-ID: Do3GttF3M9S
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_responseBody.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_responseBody.html
@@ -443,16 +443,65 @@ add_task(async function test_cachedRespo
await fetch("file_sample.html");
await fetch("file_sample.html");
await extension.awaitMessage("from-cache");
await extension.unload();
});
+// Test that finishing transferring data doesn't overwrite an existing closing/closed state.
+add_task(async function test_late_close() {
+ let extension = ExtensionTestUtils.loadExtension({
+ background() {
+ browser.webRequest.onBeforeRequest.addListener(data => {
+ let filter = browser.webRequest.filterResponseData(data.requestId);
+
+ filter.onstop = event => {
+ browser.test.fail("Should not receive onstop after close()");
+ browser.test.assertEq("closed", filter.status,
+ "Fitler status should still be 'closed'");
+ browser.test.assertThrows(() => { filter.close(); });
+ };
+ filter.ondata = event => {
+ filter.write(event.data);
+ filter.close();
+
+ browser.test.sendMessage(`done-${data.url}`);
+ };
+ }, {
+ urls: ["http://mochi.test/*/file_sample.html?*"],
+ },
+ ["blocking"]);
+ },
+
+ manifest: {
+ permissions: [
+ "webRequest",
+ "webRequestBlocking",
+ "http://mochi.test/",
+ ],
+ },
+ });
+
+ await extension.startup();
+
+ // This issue involves a race, so several requests in parallel to increase
+ // the chances of triggering it.
+ let urls = [];
+ for (let i = 0; i < 32; i++) {
+ urls.push(new URL(`file_sample.html?r=${Math.random()}`, location).href);
+ }
+
+ await Promise.all(urls.map(url => fetch(url)));
+ await Promise.all(urls.map(url => extension.awaitMessage(`done-${url}`)));
+
+ await extension.unload();
+});
+
add_task(async function test_permissions() {
let extension = ExtensionTestUtils.loadExtension({
background() {
browser.test.assertEq(
undefined, browser.webRequest.filterResponseData,
"filterResponseData is undefined without blocking permissions");
},
--- a/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
@@ -262,16 +262,18 @@ StreamFilterChild::MaybeStopRequest()
switch (mState) {
case State::Suspending:
case State::Resuming:
mNextState = State::FinishedTransferringData;
return;
case State::Disconnecting:
+ case State::Closing:
+ case State::Closed:
break;
default:
mState = State::FinishedTransferringData;
if (mStreamFilter) {
mStreamFilter->FireEvent(NS_LITERAL_STRING("stop"));
// We don't need access to the stream filter after this point, so break our
// reference cycle, so that it can be collected if we're the last reference.