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.
This commit is contained in:
Andrea Marchesini 2018-11-06 14:48:07 +01:00
parent f985d9152e
commit ebf242be0d
13 changed files with 227 additions and 86 deletions

82
dom/fetch/EmptyBody.cpp Normal file
View File

@ -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<nsIInputStream> aBodyStream)
: FetchBody<EmptyBody>(aGlobal)
, mPrincipalInfo(aPrincipalInfo)
, mAbortSignalImpl(aAbortSignalImpl)
, mBodyStream(std::move(aBodyStream))
{}
EmptyBody::~EmptyBody() = default;
/* static */ already_AddRefed<EmptyBody>
EmptyBody::Create(nsIGlobalObject* aGlobal,
mozilla::ipc::PrincipalInfo* aPrincipalInfo,
AbortSignalImpl* aAbortSignalImpl,
const nsACString& aMimeType,
ErrorResult& aRv)
{
nsCOMPtr<nsIInputStream> bodyStream;
aRv = NS_NewCStringInputStream(getter_AddRefs(bodyStream), EmptyCString());
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
RefPtr<EmptyBody> 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<nsIInputStream> bodyStream = mBodyStream;
bodyStream.forget(aStream);
}
} // dom namespace
} // mozilla namespace

89
dom/fetch/EmptyBody.h Normal file
View File

@ -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<EmptyBody>
{
NS_DECL_CYCLE_COLLECTING_ISUPPORTS
NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(EmptyBody)
public:
static already_AddRefed<EmptyBody>
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<mozilla::ipc::PrincipalInfo>&
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<nsIInputStream> mBodyStream);
~EmptyBody();
UniquePtr<mozilla::ipc::PrincipalInfo> mPrincipalInfo;
RefPtr<AbortSignalImpl> mAbortSignalImpl;
nsCOMPtr<nsIInputStream> mBodyStream;
};
} // namespace dom
} // namespace mozilla
#endif // mozilla_dom_EmptyBody_h

View File

@ -40,6 +40,7 @@
#include "mozilla/Telemetry.h" #include "mozilla/Telemetry.h"
#include "BodyExtractor.h" #include "BodyExtractor.h"
#include "EmptyBody.h"
#include "FetchObserver.h" #include "FetchObserver.h"
#include "InternalRequest.h" #include "InternalRequest.h"
#include "InternalResponse.h" #include "InternalResponse.h"
@ -1242,6 +1243,29 @@ FetchBody<Derived>::ConsumeBody(JSContext* aCx, FetchConsumeType aType,
return nullptr; 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<nsIInputStream> bodyStream;
DerivedClass()->GetBody(getter_AddRefs(bodyStream));
if (!bodyStream) {
RefPtr<EmptyBody> 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); SetBodyUsed(aCx, aRv);
if (NS_WARN_IF(aRv.Failed())) { if (NS_WARN_IF(aRv.Failed())) {
return nullptr; return nullptr;
@ -1251,7 +1275,7 @@ FetchBody<Derived>::ConsumeBody(JSContext* aCx, FetchConsumeType aType,
RefPtr<Promise> promise = RefPtr<Promise> promise =
FetchBodyConsumer<Derived>::Create(global, mMainThreadEventTarget, this, FetchBodyConsumer<Derived>::Create(global, mMainThreadEventTarget, this,
signalImpl, aType, aRv); bodyStream, signalImpl, aType, aRv);
if (NS_WARN_IF(aRv.Failed())) { if (NS_WARN_IF(aRv.Failed())) {
return nullptr; return nullptr;
} }
@ -1269,6 +1293,11 @@ already_AddRefed<Promise>
FetchBody<Response>::ConsumeBody(JSContext* aCx, FetchConsumeType aType, FetchBody<Response>::ConsumeBody(JSContext* aCx, FetchConsumeType aType,
ErrorResult& aRv); ErrorResult& aRv);
template
already_AddRefed<Promise>
FetchBody<EmptyBody>::ConsumeBody(JSContext* aCx, FetchConsumeType aType,
ErrorResult& aRv);
template <class Derived> template <class Derived>
void void
FetchBody<Derived>::SetMimeType() FetchBody<Derived>::SetMimeType()
@ -1297,6 +1326,13 @@ template
void void
FetchBody<Response>::SetMimeType(); FetchBody<Response>::SetMimeType();
template <class Derived>
void
FetchBody<Derived>::OverrideMimeType(const nsACString& aMimeType)
{
mMimeType = aMimeType;
}
template <class Derived> template <class Derived>
const nsACString& const nsACString&
FetchBody<Derived>::BodyBlobURISpec() const FetchBody<Derived>::BodyBlobURISpec() const
@ -1312,6 +1348,10 @@ template
const nsACString& const nsACString&
FetchBody<Response>::BodyBlobURISpec() const; FetchBody<Response>::BodyBlobURISpec() const;
template
const nsACString&
FetchBody<EmptyBody>::BodyBlobURISpec() const;
template <class Derived> template <class Derived>
const nsAString& const nsAString&
FetchBody<Derived>::BodyLocalPath() const FetchBody<Derived>::BodyLocalPath() const
@ -1327,6 +1367,10 @@ template
const nsAString& const nsAString&
FetchBody<Response>::BodyLocalPath() const; FetchBody<Response>::BodyLocalPath() const;
template
const nsAString&
FetchBody<EmptyBody>::BodyLocalPath() const;
template <class Derived> template <class Derived>
void void
FetchBody<Derived>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody) FetchBody<Derived>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody)

View File

@ -267,6 +267,9 @@ public:
void void
Abort() override; Abort() override;
already_AddRefed<Promise>
ConsumeBody(JSContext* aCx, FetchConsumeType aType, ErrorResult& aRv);
protected: protected:
nsCOMPtr<nsIGlobalObject> mOwner; nsCOMPtr<nsIGlobalObject> mOwner;
@ -288,6 +291,9 @@ protected:
void void
SetMimeType(); SetMimeType();
void
OverrideMimeType(const nsACString& aMimeType);
void void
SetReadableStreamBody(JSContext* aCx, JSObject* aBody); SetReadableStreamBody(JSContext* aCx, JSObject* aBody);
@ -298,9 +304,6 @@ private:
return static_cast<Derived*>(const_cast<FetchBody*>(this)); return static_cast<Derived*>(const_cast<FetchBody*>(this));
} }
already_AddRefed<Promise>
ConsumeBody(JSContext* aCx, FetchConsumeType aType, ErrorResult& aRv);
void void
LockStream(JSContext* aCx, JS::HandleObject aStream, ErrorResult& aRv); LockStream(JSContext* aCx, JS::HandleObject aStream, ErrorResult& aRv);

View File

@ -323,22 +323,15 @@ template <class Derived>
FetchBodyConsumer<Derived>::Create(nsIGlobalObject* aGlobal, FetchBodyConsumer<Derived>::Create(nsIGlobalObject* aGlobal,
nsIEventTarget* aMainThreadEventTarget, nsIEventTarget* aMainThreadEventTarget,
FetchBody<Derived>* aBody, FetchBody<Derived>* aBody,
nsIInputStream* aBodyStream,
AbortSignalImpl* aSignalImpl, AbortSignalImpl* aSignalImpl,
FetchConsumeType aType, FetchConsumeType aType,
ErrorResult& aRv) ErrorResult& aRv)
{ {
MOZ_ASSERT(aBody); MOZ_ASSERT(aBody);
MOZ_ASSERT(aBodyStream);
MOZ_ASSERT(aMainThreadEventTarget); MOZ_ASSERT(aMainThreadEventTarget);
nsCOMPtr<nsIInputStream> 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 = Promise::Create(aGlobal, aRv); RefPtr<Promise> promise = Promise::Create(aGlobal, aRv);
if (aRv.Failed()) { if (aRv.Failed()) {
return nullptr; return nullptr;
@ -346,7 +339,7 @@ FetchBodyConsumer<Derived>::Create(nsIGlobalObject* aGlobal,
RefPtr<FetchBodyConsumer<Derived>> consumer = RefPtr<FetchBodyConsumer<Derived>> consumer =
new FetchBodyConsumer<Derived>(aMainThreadEventTarget, aGlobal, new FetchBodyConsumer<Derived>(aMainThreadEventTarget, aGlobal,
aBody, bodyStream, promise, aBody, aBodyStream, promise,
aType); aType);
RefPtr<ThreadSafeWorkerRef> workerRef; RefPtr<ThreadSafeWorkerRef> workerRef;

View File

@ -39,6 +39,7 @@ public:
Create(nsIGlobalObject* aGlobal, Create(nsIGlobalObject* aGlobal,
nsIEventTarget* aMainThreadEventTarget, nsIEventTarget* aMainThreadEventTarget,
FetchBody<Derived>* aBody, FetchBody<Derived>* aBody,
nsIInputStream* aBodyStream,
AbortSignalImpl* aSignalImpl, AbortSignalImpl* aSignalImpl,
FetchConsumeType aType, FetchConsumeType aType,
ErrorResult& aRv); ErrorResult& aRv);

View File

@ -27,6 +27,7 @@ EXPORTS.mozilla.dom += [
UNIFIED_SOURCES += [ UNIFIED_SOURCES += [
'BodyExtractor.cpp', 'BodyExtractor.cpp',
'ChannelInfo.cpp', 'ChannelInfo.cpp',
'EmptyBody.cpp',
'Fetch.cpp', 'Fetch.cpp',
'FetchConsumer.cpp', 'FetchConsumer.cpp',
'FetchDriver.cpp', 'FetchDriver.cpp',

View File

@ -1,32 +1,4 @@
[request-consume-empty.html] [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] [Consume empty FormData request body as text]
expected: FAIL 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

View File

@ -1,7 +1,4 @@
[request-disturbed.html] [request-disturbed.html]
[Request without body cannot be disturbed]
expected: FAIL
[Request's body: initial state] [Request's body: initial state]
expected: FAIL expected: FAIL

View File

@ -1,32 +1,3 @@
[response-consume-empty.html] [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] [Consume empty FormData response body as text]
expected: FAIL 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

View File

@ -1,4 +0,0 @@
[cache-add.https.html]
[Cache.add with request with null body (not consumed)]
expected: FAIL

View File

@ -1,4 +0,0 @@
[cache-add.https.html]
[Cache.add with request with null body (not consumed)]
expected: FAIL

View File

@ -1,4 +0,0 @@
[cache-add.https.html]
[Cache.add with request with null body (not consumed)]
expected: FAIL