From 6841a351b6c90f7285be24567978bc94682808aa Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 14 Sep 2015 11:05:35 -0400 Subject: [PATCH] Bug 1198544 - Prevent FetchDriver from creating multiple responses if OnStopRequest yields a failing status code. r=nsm --- dom/fetch/Fetch.cpp | 8 ++++---- dom/fetch/FetchDriver.cpp | 16 ++++++++++----- dom/fetch/FetchDriver.h | 15 +++++++++++++- .../test/serviceworkers/fetch/fetch_tests.js | 17 ++++++++++++++++ .../test/serviceworkers/fetch/interrupt.sjs | 20 +++++++++++++++++++ dom/workers/test/serviceworkers/mochitest.ini | 1 + 6 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 dom/workers/test/serviceworkers/fetch/interrupt.sjs diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp index 47c1c5e5166d..556bb1f31925 100644 --- a/dom/fetch/Fetch.cpp +++ b/dom/fetch/Fetch.cpp @@ -72,7 +72,7 @@ public: } void - OnResponseAvailable(InternalResponse* aResponse) override; + OnResponseAvailableInternal(InternalResponse* aResponse) override; void OnResponseEnd() override; @@ -99,7 +99,7 @@ public: explicit MainThreadFetchResolver(Promise* aPromise); void - OnResponseAvailable(InternalResponse* aResponse) override; + OnResponseAvailableInternal(InternalResponse* aResponse) override; private: ~MainThreadFetchResolver(); @@ -237,7 +237,7 @@ MainThreadFetchResolver::MainThreadFetchResolver(Promise* aPromise) } void -MainThreadFetchResolver::OnResponseAvailable(InternalResponse* aResponse) +MainThreadFetchResolver::OnResponseAvailableInternal(InternalResponse* aResponse) { NS_ASSERT_OWNINGTHREAD(MainThreadFetchResolver); AssertIsOnMainThread(); @@ -317,7 +317,7 @@ public: }; void -WorkerFetchResolver::OnResponseAvailable(InternalResponse* aResponse) +WorkerFetchResolver::OnResponseAvailableInternal(InternalResponse* aResponse) { AssertIsOnMainThread(); diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp index 4742c3003b6a..c323d42529fb 100644 --- a/dom/fetch/FetchDriver.cpp +++ b/dom/fetch/FetchDriver.cpp @@ -884,13 +884,19 @@ FetchDriver::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) { workers::AssertIsOnMainThread(); - if (mPipeOutputStream) { - mPipeOutputStream->Close(); + if (NS_FAILED(aStatusCode)) { + nsCOMPtr outputStream = do_QueryInterface(mPipeOutputStream); + if (outputStream) { + outputStream->CloseWithStatus(NS_BINDING_FAILED); + } + // We proceed as usual here, since we've already created a successful response + // from OnStartRequest. + SucceedWithResponse(); + return aStatusCode; } - if (NS_FAILED(aStatusCode)) { - FailWithNetworkError(); - return aStatusCode; + if (mPipeOutputStream) { + mPipeOutputStream->Close(); } ContinueHttpFetchAfterNetworkFetch(); diff --git a/dom/fetch/FetchDriver.h b/dom/fetch/FetchDriver.h index 96b8f848f200..9a5648579816 100644 --- a/dom/fetch/FetchDriver.h +++ b/dom/fetch/FetchDriver.h @@ -32,14 +32,27 @@ class InternalResponse; class FetchDriverObserver { public: + FetchDriverObserver() : mGotResponseAvailable(false) + { } + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FetchDriverObserver); - virtual void OnResponseAvailable(InternalResponse* aResponse) = 0; + void OnResponseAvailable(InternalResponse* aResponse) + { + MOZ_ASSERT(!mGotResponseAvailable); + mGotResponseAvailable = true; + OnResponseAvailableInternal(aResponse); + } virtual void OnResponseEnd() { }; protected: virtual ~FetchDriverObserver() { }; + + virtual void OnResponseAvailableInternal(InternalResponse* aResponse) = 0; + +private: + bool mGotResponseAvailable; }; class FetchDriver final : public nsIStreamListener, diff --git a/dom/workers/test/serviceworkers/fetch/fetch_tests.js b/dom/workers/test/serviceworkers/fetch/fetch_tests.js index c10fd1b87aa7..ec65f79bb5e0 100644 --- a/dom/workers/test/serviceworkers/fetch/fetch_tests.js +++ b/dom/workers/test/serviceworkers/fetch/fetch_tests.js @@ -319,6 +319,23 @@ fetch(new Request('body-blob', {method: 'POST', body: new Blob(new String('my bo finish(); }); +expectAsyncResult(); +fetch('interrupt.sjs') +.then(function(res) { + my_ok(true, "interrupted fetch succeeded"); + res.text().then(function(body) { + my_ok(false, "interrupted fetch shouldn't have complete body"); + finish(); + }, + function() { + my_ok(true, "interrupted fetch shouldn't have complete body"); + finish(); + }) +}, function(e) { + my_ok(false, "interrupted fetch failed"); + finish(); +}); + ['DELETE', 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT'].forEach(function(method) { fetchXHRWithMethod('xhr-method-test.txt', method, function(xhr) { my_ok(xhr.status == 200, method + " load should be successful"); diff --git a/dom/workers/test/serviceworkers/fetch/interrupt.sjs b/dom/workers/test/serviceworkers/fetch/interrupt.sjs new file mode 100644 index 000000000000..f6fe870efa29 --- /dev/null +++ b/dom/workers/test/serviceworkers/fetch/interrupt.sjs @@ -0,0 +1,20 @@ +function handleRequest(request, response) { + var body = "a"; + for (var i = 0; i < 20; i++) { + body += body; + } + + response.seizePower(); + response.write("HTTP/1.1 200 OK\r\n") + var count = 10; + response.write("Content-Length: " + body.length * count + "\r\n"); + response.write("Content-Type: text/plain; charset=utf-8\r\n"); + response.write("Cache-Control: no-cache, must-revalidate\r\n"); + response.write("\r\n"); + + for (var i = 0; i < count; i++) { + response.write(body); + } + + throw Components.results.NS_BINDING_ABORTED; +} diff --git a/dom/workers/test/serviceworkers/mochitest.ini b/dom/workers/test/serviceworkers/mochitest.ini index c89ca8c374f4..caa53aff9136 100644 --- a/dom/workers/test/serviceworkers/mochitest.ini +++ b/dom/workers/test/serviceworkers/mochitest.ini @@ -50,6 +50,7 @@ support-files = fetch/https/clonedresponse/register.html fetch/https/clonedresponse/unregister.html fetch/https/clonedresponse/https_test.js + fetch/interrupt.sjs fetch/origin/index.sjs fetch/origin/index-to-https.sjs fetch/origin/realindex.html