From ebf242be0d8255a0f3c1270d3cd0925434e48052 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 6 Nov 2018 14:48:07 +0100 Subject: [PATCH] Bug 1500879 - Fetch should not consume Request/Response with null-body, r=asuth The fetch spec treats null bodies specially. Their Body cannot become disturbed or locked and a fresh empty ReadableStream is returned whenever an attempt is made to consume the body. We currently fail a number of WPT tests in this area because we do mark the body consumed as exposed via bodyUsed. --- dom/fetch/EmptyBody.cpp | 82 +++++++++++++++++ dom/fetch/EmptyBody.h | 89 +++++++++++++++++++ dom/fetch/Fetch.cpp | 46 +++++++++- dom/fetch/Fetch.h | 9 +- dom/fetch/FetchConsumer.cpp | 13 +-- dom/fetch/FetchConsumer.h | 1 + dom/fetch/moz.build | 1 + .../request/request-consume-empty.html.ini | 28 ------ .../api/request/request-disturbed.html.ini | 3 - .../response/response-consume-empty.html.ini | 29 ------ .../serviceworker/cache-add.https.html.ini | 4 - .../window/cache-add.https.html.ini | 4 - .../worker/cache-add.https.html.ini | 4 - 13 files changed, 227 insertions(+), 86 deletions(-) create mode 100644 dom/fetch/EmptyBody.cpp create mode 100644 dom/fetch/EmptyBody.h delete mode 100644 testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-add.https.html.ini delete mode 100644 testing/web-platform/meta/service-workers/cache-storage/window/cache-add.https.html.ini delete mode 100644 testing/web-platform/meta/service-workers/cache-storage/worker/cache-add.https.html.ini diff --git a/dom/fetch/EmptyBody.cpp b/dom/fetch/EmptyBody.cpp new file mode 100644 index 000000000000..26ad0c4a7375 --- /dev/null +++ b/dom/fetch/EmptyBody.cpp @@ -0,0 +1,82 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "EmptyBody.h" + +namespace mozilla { +namespace dom { + +NS_IMPL_CYCLE_COLLECTING_ADDREF(EmptyBody) +NS_IMPL_CYCLE_COLLECTING_RELEASE(EmptyBody) + +NS_IMPL_CYCLE_COLLECTION_CLASS(EmptyBody) + +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(EmptyBody) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mOwner) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mAbortSignalImpl) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mFetchStreamReader) +NS_IMPL_CYCLE_COLLECTION_UNLINK_END + +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(EmptyBody) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOwner) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAbortSignalImpl) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFetchStreamReader) +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END + +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(EmptyBody) +NS_IMPL_CYCLE_COLLECTION_TRACE_END + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(EmptyBody) + NS_INTERFACE_MAP_ENTRY(nsISupports) +NS_INTERFACE_MAP_END + +EmptyBody::EmptyBody(nsIGlobalObject* aGlobal, + mozilla::ipc::PrincipalInfo* aPrincipalInfo, + AbortSignalImpl* aAbortSignalImpl, + already_AddRefed aBodyStream) + : FetchBody(aGlobal) + , mPrincipalInfo(aPrincipalInfo) + , mAbortSignalImpl(aAbortSignalImpl) + , mBodyStream(std::move(aBodyStream)) +{} + +EmptyBody::~EmptyBody() = default; + +/* static */ already_AddRefed +EmptyBody::Create(nsIGlobalObject* aGlobal, + mozilla::ipc::PrincipalInfo* aPrincipalInfo, + AbortSignalImpl* aAbortSignalImpl, + const nsACString& aMimeType, + ErrorResult& aRv) +{ + nsCOMPtr bodyStream; + aRv = NS_NewCStringInputStream(getter_AddRefs(bodyStream), EmptyCString()); + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + + RefPtr emptyBody = new EmptyBody(aGlobal, aPrincipalInfo, + aAbortSignalImpl, + bodyStream.forget()); + emptyBody->OverrideMimeType(aMimeType); + return emptyBody.forget(); +} + +void +EmptyBody::GetBody(nsIInputStream** aStream, int64_t* aBodyLength) +{ + MOZ_ASSERT(aStream); + + if (aBodyLength) { + *aBodyLength = 0; + } + + nsCOMPtr bodyStream = mBodyStream; + bodyStream.forget(aStream); +} + +} // dom namespace +} // mozilla namespace diff --git a/dom/fetch/EmptyBody.h b/dom/fetch/EmptyBody.h new file mode 100644 index 000000000000..b08de813dbd9 --- /dev/null +++ b/dom/fetch/EmptyBody.h @@ -0,0 +1,89 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_dom_EmptyBody_h +#define mozilla_dom_EmptyBody_h + +#include "nsISupportsImpl.h" + +#include "mozilla/dom/Fetch.h" + +namespace mozilla { + +namespace ipc { +class PrincipalInfo; +} // namespace ipc + +namespace dom { + +class EmptyBody final : public nsISupports + , public FetchBody +{ + NS_DECL_CYCLE_COLLECTING_ISUPPORTS + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(EmptyBody) + +public: + static already_AddRefed + Create(nsIGlobalObject* aGlobal, + mozilla::ipc::PrincipalInfo* aPrincipalInfo, + AbortSignalImpl* aAbortSignalImpl, + const nsACString& aMimeType, + ErrorResult& aRv); + + nsIGlobalObject* + GetParentObject() const + { + return mOwner; + } + + AbortSignalImpl* + GetSignalImpl() const override + { + return mAbortSignalImpl; + } + + const UniquePtr& + GetPrincipalInfo() const + { + return mPrincipalInfo; + } + + void + GetBody(nsIInputStream** aStream, int64_t* aBodyLength = nullptr); + + using FetchBody::BodyBlobURISpec; + + const nsACString& + BodyBlobURISpec() const + { + return EmptyCString(); + } + + using FetchBody::BodyLocalPath; + + const nsAString& + BodyLocalPath() const + { + return EmptyString(); + } + +private: + EmptyBody(nsIGlobalObject* aGlobal, + mozilla::ipc::PrincipalInfo* aPrincipalInfo, + AbortSignalImpl* aAbortSignalImpl, + already_AddRefed mBodyStream); + + ~EmptyBody(); + + UniquePtr mPrincipalInfo; + RefPtr mAbortSignalImpl; + nsCOMPtr mBodyStream; +}; + +} // namespace dom +} // namespace mozilla + +#endif // mozilla_dom_EmptyBody_h diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp index 837cc2ad6e3e..3e9ccbf999ea 100644 --- a/dom/fetch/Fetch.cpp +++ b/dom/fetch/Fetch.cpp @@ -40,6 +40,7 @@ #include "mozilla/Telemetry.h" #include "BodyExtractor.h" +#include "EmptyBody.h" #include "FetchObserver.h" #include "InternalRequest.h" #include "InternalResponse.h" @@ -1242,6 +1243,29 @@ FetchBody::ConsumeBody(JSContext* aCx, FetchConsumeType aType, return nullptr; } + // Null bodies are a special-case in the fetch spec. The Body mix-in can only + // be "disturbed" or "locked" if its associated "body" is non-null. + // Additionally, the Body min-in's "consume body" algorithm explicitly creates + // a fresh empty ReadableStream object in step 2. This means that `bodyUsed` + // will never return true for a null body. + // + // To this end, we create a fresh (empty) body every time a request is made + // and consume its body here, without marking this FetchBody consumed via + // SetBodyUsed. + nsCOMPtr bodyStream; + DerivedClass()->GetBody(getter_AddRefs(bodyStream)); + if (!bodyStream) { + RefPtr emptyBody = + EmptyBody::Create(DerivedClass()->GetParentObject(), + DerivedClass()->GetPrincipalInfo().get(), + signalImpl, mMimeType, aRv); + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + + return emptyBody->ConsumeBody(aCx, aType, aRv); + } + SetBodyUsed(aCx, aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; @@ -1251,7 +1275,7 @@ FetchBody::ConsumeBody(JSContext* aCx, FetchConsumeType aType, RefPtr promise = FetchBodyConsumer::Create(global, mMainThreadEventTarget, this, - signalImpl, aType, aRv); + bodyStream, signalImpl, aType, aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -1269,6 +1293,11 @@ already_AddRefed FetchBody::ConsumeBody(JSContext* aCx, FetchConsumeType aType, ErrorResult& aRv); +template +already_AddRefed +FetchBody::ConsumeBody(JSContext* aCx, FetchConsumeType aType, + ErrorResult& aRv); + template void FetchBody::SetMimeType() @@ -1297,6 +1326,13 @@ template void FetchBody::SetMimeType(); +template +void +FetchBody::OverrideMimeType(const nsACString& aMimeType) +{ + mMimeType = aMimeType; +} + template const nsACString& FetchBody::BodyBlobURISpec() const @@ -1312,6 +1348,10 @@ template const nsACString& FetchBody::BodyBlobURISpec() const; +template +const nsACString& +FetchBody::BodyBlobURISpec() const; + template const nsAString& FetchBody::BodyLocalPath() const @@ -1327,6 +1367,10 @@ template const nsAString& FetchBody::BodyLocalPath() const; +template +const nsAString& +FetchBody::BodyLocalPath() const; + template void FetchBody::SetReadableStreamBody(JSContext* aCx, JSObject* aBody) diff --git a/dom/fetch/Fetch.h b/dom/fetch/Fetch.h index afdfddeefe25..bf3720cd61f6 100644 --- a/dom/fetch/Fetch.h +++ b/dom/fetch/Fetch.h @@ -267,6 +267,9 @@ public: void Abort() override; + already_AddRefed + ConsumeBody(JSContext* aCx, FetchConsumeType aType, ErrorResult& aRv); + protected: nsCOMPtr mOwner; @@ -288,6 +291,9 @@ protected: void SetMimeType(); + void + OverrideMimeType(const nsACString& aMimeType); + void SetReadableStreamBody(JSContext* aCx, JSObject* aBody); @@ -298,9 +304,6 @@ private: return static_cast(const_cast(this)); } - already_AddRefed - ConsumeBody(JSContext* aCx, FetchConsumeType aType, ErrorResult& aRv); - void LockStream(JSContext* aCx, JS::HandleObject aStream, ErrorResult& aRv); diff --git a/dom/fetch/FetchConsumer.cpp b/dom/fetch/FetchConsumer.cpp index 247e1da7e9c5..697066191827 100644 --- a/dom/fetch/FetchConsumer.cpp +++ b/dom/fetch/FetchConsumer.cpp @@ -323,22 +323,15 @@ template FetchBodyConsumer::Create(nsIGlobalObject* aGlobal, nsIEventTarget* aMainThreadEventTarget, FetchBody* aBody, + nsIInputStream* aBodyStream, AbortSignalImpl* aSignalImpl, FetchConsumeType aType, ErrorResult& aRv) { MOZ_ASSERT(aBody); + MOZ_ASSERT(aBodyStream); MOZ_ASSERT(aMainThreadEventTarget); - nsCOMPtr bodyStream; - aBody->DerivedClass()->GetBody(getter_AddRefs(bodyStream)); - if (!bodyStream) { - aRv = NS_NewCStringInputStream(getter_AddRefs(bodyStream), EmptyCString()); - if (NS_WARN_IF(aRv.Failed())) { - return nullptr; - } - } - RefPtr promise = Promise::Create(aGlobal, aRv); if (aRv.Failed()) { return nullptr; @@ -346,7 +339,7 @@ FetchBodyConsumer::Create(nsIGlobalObject* aGlobal, RefPtr> consumer = new FetchBodyConsumer(aMainThreadEventTarget, aGlobal, - aBody, bodyStream, promise, + aBody, aBodyStream, promise, aType); RefPtr workerRef; diff --git a/dom/fetch/FetchConsumer.h b/dom/fetch/FetchConsumer.h index 8a2d8fabacd8..875b12dffdc8 100644 --- a/dom/fetch/FetchConsumer.h +++ b/dom/fetch/FetchConsumer.h @@ -39,6 +39,7 @@ public: Create(nsIGlobalObject* aGlobal, nsIEventTarget* aMainThreadEventTarget, FetchBody* aBody, + nsIInputStream* aBodyStream, AbortSignalImpl* aSignalImpl, FetchConsumeType aType, ErrorResult& aRv); diff --git a/dom/fetch/moz.build b/dom/fetch/moz.build index 18a11a2e937d..4378c16fe128 100644 --- a/dom/fetch/moz.build +++ b/dom/fetch/moz.build @@ -27,6 +27,7 @@ EXPORTS.mozilla.dom += [ UNIFIED_SOURCES += [ 'BodyExtractor.cpp', 'ChannelInfo.cpp', + 'EmptyBody.cpp', 'Fetch.cpp', 'FetchConsumer.cpp', 'FetchDriver.cpp', diff --git a/testing/web-platform/meta/fetch/api/request/request-consume-empty.html.ini b/testing/web-platform/meta/fetch/api/request/request-consume-empty.html.ini index 1aac745a150b..0ab1faa7c227 100644 --- a/testing/web-platform/meta/fetch/api/request/request-consume-empty.html.ini +++ b/testing/web-platform/meta/fetch/api/request/request-consume-empty.html.ini @@ -1,32 +1,4 @@ [request-consume-empty.html] - max-asserts: 2 - [Consume request's body as text] - expected: FAIL - - [Consume request's body as blob] - expected: FAIL - - [Consume request's body as arrayBuffer] - expected: FAIL - - [Consume request's body as json] - expected: FAIL - - [Consume request's body as formData] - expected: FAIL - [Consume empty FormData request body as text] expected: FAIL - [Consume request's body as json (error case)] - expected: FAIL - - [Consume request's body as formData with correct multipart type (error case)] - expected: FAIL - - [Consume request's body as formData with correct urlencoded type] - expected: FAIL - - [Consume request's body as formData without correct type (error case)] - expected: FAIL - diff --git a/testing/web-platform/meta/fetch/api/request/request-disturbed.html.ini b/testing/web-platform/meta/fetch/api/request/request-disturbed.html.ini index e3c2659a31ef..a82f0c86d789 100644 --- a/testing/web-platform/meta/fetch/api/request/request-disturbed.html.ini +++ b/testing/web-platform/meta/fetch/api/request/request-disturbed.html.ini @@ -1,7 +1,4 @@ [request-disturbed.html] - [Request without body cannot be disturbed] - expected: FAIL - [Request's body: initial state] expected: FAIL diff --git a/testing/web-platform/meta/fetch/api/response/response-consume-empty.html.ini b/testing/web-platform/meta/fetch/api/response/response-consume-empty.html.ini index ac7db196a7ab..532d2c9089fa 100644 --- a/testing/web-platform/meta/fetch/api/response/response-consume-empty.html.ini +++ b/testing/web-platform/meta/fetch/api/response/response-consume-empty.html.ini @@ -1,32 +1,3 @@ [response-consume-empty.html] - max-asserts: 2 - [Consume response's body as text] - expected: FAIL - - [Consume response's body as blob] - expected: FAIL - - [Consume response's body as arrayBuffer] - expected: FAIL - - [Consume response's body as json] - expected: FAIL - - [Consume response's body as formData] - expected: FAIL - [Consume empty FormData response body as text] expected: FAIL - - [Consume response's body as json (error case)] - expected: FAIL - - [Consume response's body as formData with correct multipart type (error case)] - expected: FAIL - - [Consume response's body as formData with correct urlencoded type] - expected: FAIL - - [Consume response's body as formData without correct type (error case)] - expected: FAIL - diff --git a/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-add.https.html.ini b/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-add.https.html.ini deleted file mode 100644 index b0a127627597..000000000000 --- a/testing/web-platform/meta/service-workers/cache-storage/serviceworker/cache-add.https.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[cache-add.https.html] - [Cache.add with request with null body (not consumed)] - expected: FAIL - diff --git a/testing/web-platform/meta/service-workers/cache-storage/window/cache-add.https.html.ini b/testing/web-platform/meta/service-workers/cache-storage/window/cache-add.https.html.ini deleted file mode 100644 index b0a127627597..000000000000 --- a/testing/web-platform/meta/service-workers/cache-storage/window/cache-add.https.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[cache-add.https.html] - [Cache.add with request with null body (not consumed)] - expected: FAIL - diff --git a/testing/web-platform/meta/service-workers/cache-storage/worker/cache-add.https.html.ini b/testing/web-platform/meta/service-workers/cache-storage/worker/cache-add.https.html.ini deleted file mode 100644 index b0a127627597..000000000000 --- a/testing/web-platform/meta/service-workers/cache-storage/worker/cache-add.https.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[cache-add.https.html] - [Cache.add with request with null body (not consumed)] - expected: FAIL -