Commit 1977b793 authored by Adam Rice's avatar Adam Rice Committed by Commit Bot

Remove blink::CallExtraOrCrash

CallExtraOrCrash is a function to call into V8 Extras and crash if it
fails. Unfortunately, calling into V8 Extras can fail under normal
conditions such as stack overflow and Worker destruction, causing a lot
of unnecessary crashes. Remove CallExtraOrCrash.

This is a stopgap change designed to be mergable to version 68. It
avoids major code restructuring but as a result lacks robust
encapsulation.

The general strategy is to pass a boolean pointer to functions in
ReadableStreamOperations that call into V8 Extras. If the internal call
fails, the boolean is set to true, and the call returns a default value.

BodyStreamBuffer keeps the boolean as member variable. Once a call to a
V8 extra function fails, the boolean is set and no more calls will be
made. This is to avoid us taking action based on information that is
incorrect.

For Tee() and the from-stream version of the BodyStreamBuffer
constructor, we use an ExceptionState object instead. In these cases, it
is straightforward to pass through ExceptionState object from the Blink
bindings. This allows the failure to be propagated safely back to
Javascript.

Conversely, DefaultReaderRead() has been switched from using
ExceptionState to using an error-signalling bool. This is because its
callers do not have access to a real ExceptionState object. Previously
callers used a NonThrowableExceptionState, but this was not suitable as
DefaultReaderRead() can throw.

BodyStreamBuffer mostly wraps access to ReadableStreamOperations, so
some degree of encapsulation has been retained.

7 new layout tests exercise each of the deterministically reachable
crashes, using stack overflows. Because they depend on stack layout and
whether DCHECKs are enabled, they only hit the crashes in particular
environments.

Bug: 829790, 849312
Change-Id: I47481b33a47b418dc6916e3b4311e60b5fd89e3d
Reviewed-on: https://chromium-review.googlesource.com/1097047
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568009}
parent 49a65ae5
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
test(() => {
const rs = new ReadableStream();
fillStackAndRun(() => new Response(rs));
}, 'stack overflow in Response construction from stream should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
test(() => {
fillStackAndRun(() => new Response('hi'));
}, 'stack overflow in Response constructor should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
test(() => {
const rs = new ReadableStream();
const response = new Response(rs);
fillStackAndRun(() => response.bodyUsed);
}, 'stack overflow in bodyUsed should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
// Running put several times is necessary to trigger the crash, but running too
// many times causes the test to timeout on windows.
let putRunsLeft = 64;
// fillStackAndRun() doesn't not cause a stack overflow in this test. This is
// probably because put() takes two arguments.
function fillStackThenCallPut(foo, request, response) {
try {
fillStackThenCallPut(foo, request, response);
} finally {
// This runs thousands of times, causing the console to spew "Uncaught
// (in promise) TypeError: Response body is already used" messages, but
// it's harmless.
if (putRunsLeft > 0) {
--putRunsLeft;
foo.put(request, response);
}
}
}
promise_test(async t => {
const request = new Request('/');
const response = new Response('foo');
const foo = await caches.open('foo');
t.add_cleanup(() => caches.delete('foo'));
await foo.put(request, response);
assert_throws(
new RangeError(), () => fillStackThenCallPut(foo, request, response),
'fillStackThenCallPut should throw');
}, 'stack overflow in IsLocked() should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
test(() => {
const request = new Request('/', {method: 'POST', body: 'hi'});
fillStackAndRun(() => fetch(request));
}, 'stack overflow during fetch should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
// This test unhelpfully crashes in IsReadableStream when compiled with DCHECKs
// enabled. For best coverage, it should be compiled without DCHECKs.
test(() => {
const rs = new ReadableStream();
const response = new Response(rs);
try {
// This throws different errors depending on how Blink is built, so
// assert_throws() cannot be used.
fillStackAndRun(() => response.clone(), 784);
assert_unreached('stack should overflow');
} catch (e) {
}
}, 'stack overflow in clone() should not crash the browser');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/stack-overflow.js"></script>
<script>
// Trigger the following crash:
// #0 0x7ffff7d8092c base::debug::StackTrace::StackTrace()
// #1 0x7ffff7d80491 base::debug::(anonymous namespace)::StackDumpSignalHandler()
// #2 0x7ffff00130c0 <unknown>
// #3 0x7ffff41c113d v8::internal::GlobalHandles::MakeWeak()
// #4 0x7ffff308198e blink::ReadableStreamBytesConsumer::ReadableStreamBytesConsumer()
// #5 0x7ffff306bc6f blink::BodyStreamBuffer::ReleaseHandle()
// #6 0x7ffff306b883 blink::BodyStreamBuffer::StartLoading()
// #7 0x7ffff306914d blink::Body::blob()
// #8 0x7ffff37a9d20 blink::V8Response::blobMethodCallback()
// #9 0x7ffff3f02097 v8::internal::FunctionCallbackArguments::Call()
// #10 0x7ffff3f015a3 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
// #11 0x7ffff3f00ca6 v8::internal::Builtin_Impl_HandleApiCall()
// This happens on Linux release build at 70ddf623, no DCHECK. It may be
// possible to reproduce in other environments by bisecting the second argument
// to fillStackAndRun() until you get the above crash.
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
test(() => {
const rs = new ReadableStream();
const response = new Response(rs);
try {
// This throws an exception in debug builds but not in release builds.
// If the process doesn't crash, the test passed.
fillStackAndRun(() => response.blob(), 784);
} catch (e) {
}
}, 'stack overflow in response.blob() should not crash the browser');
</script>
// The bugs in this file are necessary to reproduce the crashes. Don't fix them.
// Run |func| with a full stack, so that any Javascript running inside it is hit
// with a stack overflow exception. |extraPadLevels| can be set to a non-zero
// value to add a bit more stack space. Sometimes this makes it possible to
// avoid crashing inside a small uninteresting function and instead crash inside
// the function of interest which is called later and requires more stack
// space. Useful |extraPadLevels| values are not portable and can only be
// determined by experiment.
function fillStackAndRun(func, extraPadLevels = 0) {
try {
padStack(extraPadLevels);
fillStackAndRun(func, extraPadLevels);
} catch (e) {
return func();
}
}
// Recurse |n| levels and then return. If it doesn't throw then an amount of
// stack space proportional to |n| is available.
function padStack(n) {
if (n > 0) padStack(n - 1);
}
......@@ -95,8 +95,7 @@ class CORE_EXPORT V8ScriptRunner final {
const String& file_name,
const TextPosition&);
// Utilities for calling functions added to the V8 extras binding object.
// Calls a function on the V8 extras binding object.
template <size_t N>
static v8::MaybeLocal<v8::Value> CallExtra(ScriptState* script_state,
const char* name,
......@@ -104,14 +103,6 @@ class CORE_EXPORT V8ScriptRunner final {
return CallExtraHelper(script_state, name, N, args);
}
template <size_t N>
static v8::Local<v8::Value> CallExtraOrCrash(
ScriptState* script_state,
const char* name,
v8::Local<v8::Value> (&args)[N]) {
return CallExtraHelper(script_state, name, N, args).ToLocalChecked();
}
// Reports an exception to the message handler, as if it were an uncaught
// exception. Can only be called on the main thread.
//
......
......@@ -13,6 +13,7 @@
#include "third_party/blink/renderer/core/streams/readable_stream_operations.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_typed_array.h"
#include "third_party/blink/renderer/platform/bindings/exception_code.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h"
#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
......@@ -103,12 +104,27 @@ BodyStreamBuffer::BodyStreamBuffer(ScriptState* script_state,
DCHECK(body_value->IsObject());
v8::Local<v8::Object> body = body_value.As<v8::Object>();
ScriptValue readable_stream = ReadableStreamOperations::CreateReadableStream(
script_state, this,
ReadableStreamOperations::CreateCountQueuingStrategy(script_state, 0));
DCHECK(!readable_stream.IsEmpty());
V8PrivateProperty::GetInternalBodyStream(script_state->GetIsolate())
.Set(body, readable_stream.V8Value());
{
// Leaving an exception pending will cause Blink to crash in the bindings
// code later, so catch instead.
v8::TryCatch try_catch(script_state->GetIsolate());
ScriptValue strategy =
ReadableStreamOperations::CreateCountQueuingStrategy(script_state, 0);
if (!strategy.IsEmpty()) {
ScriptValue readable_stream =
ReadableStreamOperations::CreateReadableStream(script_state, this,
strategy);
if (!readable_stream.IsEmpty()) {
V8PrivateProperty::GetInternalBodyStream(script_state->GetIsolate())
.Set(body, readable_stream.V8Value());
} else {
stream_broken_ = true;
}
} else {
stream_broken_ = true;
}
DCHECK_EQ(stream_broken_, try_catch.HasCaught());
}
consumer_->SetClient(this);
if (signal) {
if (signal->aborted()) {
......@@ -122,12 +138,19 @@ BodyStreamBuffer::BodyStreamBuffer(ScriptState* script_state,
}
BodyStreamBuffer::BodyStreamBuffer(ScriptState* script_state,
ScriptValue stream)
ScriptValue stream,
ExceptionState& exception_state)
: UnderlyingSourceBase(script_state),
script_state_(script_state),
signal_(nullptr),
made_from_readable_stream_(true) {
DCHECK(ReadableStreamOperations::IsReadableStream(script_state, stream));
DCHECK(ReadableStreamOperations::IsReadableStream(script_state, stream,
exception_state)
.value_or(true));
if (exception_state.HadException())
return;
v8::Local<v8::Value> body_value = ToV8(this, script_state);
DCHECK(!body_value.IsEmpty());
DCHECK(body_value->IsObject());
......@@ -138,6 +161,10 @@ BodyStreamBuffer::BodyStreamBuffer(ScriptState* script_state,
}
ScriptValue BodyStreamBuffer::Stream() {
// Since this is the implementation of response.body, we return the stream
// even if |stream_broken_| is true, so that expected Javascript attribute
// behaviour is not changed. User code is still permitted to access the
// stream even when it has thrown an exception.
ScriptState::Scope scope(script_state_.get());
v8::Local<v8::Value> body_value = ToV8(this, script_state_.get());
DCHECK(!body_value.IsEmpty());
......@@ -204,18 +231,42 @@ void BodyStreamBuffer::StartLoading(FetchDataLoader* loader,
}
void BodyStreamBuffer::Tee(BodyStreamBuffer** branch1,
BodyStreamBuffer** branch2) {
BodyStreamBuffer** branch2,
ExceptionState& exception_state) {
DCHECK(!IsStreamLocked());
DCHECK(!IsStreamDisturbed());
*branch1 = nullptr;
*branch2 = nullptr;
if (made_from_readable_stream_) {
if (stream_broken_) {
// We don't really know what state the stream is in, so throw an exception
// rather than making things worse.
exception_state.ThrowDOMException(
DOMExceptionCode::kInvalidStateError,
"Unsafe to tee stream in unknown state");
return;
}
ScriptValue stream1, stream2;
ReadableStreamOperations::Tee(script_state_.get(), Stream(), &stream1,
&stream2);
*branch1 = new BodyStreamBuffer(script_state_.get(), stream1);
*branch2 = new BodyStreamBuffer(script_state_.get(), stream2);
&stream2, exception_state);
if (exception_state.HadException()) {
stream_broken_ = true;
return;
}
// Exceptions here imply that |stream1| and/or |stream2| are broken, not the
// stream owned by this object, so we shouldn't set |stream_broken_|.
auto* tmp1 =
new BodyStreamBuffer(script_state_.get(), stream1, exception_state);
if (exception_state.HadException())
return;
auto* tmp2 =
new BodyStreamBuffer(script_state_.get(), stream2, exception_state);
if (exception_state.HadException())
return;
*branch1 = tmp1;
*branch2 = tmp2;
return;
}
BytesConsumer* dest1 = nullptr;
......@@ -282,28 +333,28 @@ void BodyStreamBuffer::ContextDestroyed(ExecutionContext* destroyed_context) {
}
bool BodyStreamBuffer::IsStreamReadable() {
ScriptState::Scope scope(script_state_.get());
return ReadableStreamOperations::IsReadable(script_state_.get(), Stream());
return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsReadable,
false);
}
bool BodyStreamBuffer::IsStreamClosed() {
ScriptState::Scope scope(script_state_.get());
return ReadableStreamOperations::IsClosed(script_state_.get(), Stream());
return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsClosed,
true);
}
bool BodyStreamBuffer::IsStreamErrored() {
ScriptState::Scope scope(script_state_.get());
return ReadableStreamOperations::IsErrored(script_state_.get(), Stream());
return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsErrored,
true);
}
bool BodyStreamBuffer::IsStreamLocked() {
ScriptState::Scope scope(script_state_.get());
return ReadableStreamOperations::IsLocked(script_state_.get(), Stream());
return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsLocked,
true);
}
bool BodyStreamBuffer::IsStreamDisturbed() {
ScriptState::Scope scope(script_state_.get());
return ReadableStreamOperations::IsDisturbed(script_state_.get(), Stream());
return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsDisturbed,
true);
}
void BodyStreamBuffer::CloseAndLockAndDisturb() {
......@@ -312,11 +363,14 @@ void BodyStreamBuffer::CloseAndLockAndDisturb() {
// the internal buffer.
Close();
}
if (stream_broken_)
return;
ScriptState::Scope scope(script_state_.get());
NonThrowableExceptionState exception_state;
ScriptValue reader = ReadableStreamOperations::GetReader(
script_state_.get(), Stream(), exception_state);
ScriptValue reader =
ReadableStreamOperations::GetReader(script_state_.get(), Stream());
if (reader.IsEmpty())
return;
ReadableStreamOperations::DefaultReaderRead(script_state_.get(), reader);
}
......@@ -409,10 +463,29 @@ void BodyStreamBuffer::StopLoading() {
loader_ = nullptr;
}
bool BodyStreamBuffer::BooleanStreamOperationOrFallback(
base::Optional<bool> (*predicate)(ScriptState*, ScriptValue),
bool fallback_value) {
if (stream_broken_)
return fallback_value;
ScriptState::Scope scope(script_state_.get());
const base::Optional<bool> result = predicate(script_state_.get(), Stream());
if (!result.has_value()) {
stream_broken_ = true;
return fallback_value;
}
return result.value();
}
BytesConsumer* BodyStreamBuffer::ReleaseHandle() {
DCHECK(!IsStreamLocked());
DCHECK(!IsStreamDisturbed());
if (stream_broken_) {
return BytesConsumer::CreateErrored(
BytesConsumer::Error("ReleaseHandle called with broken stream"));
}
if (made_from_readable_stream_) {
ScriptState::Scope scope(script_state_.get());
// We need to have |reader| alive by some means (as written in
......@@ -421,9 +494,13 @@ BytesConsumer* BodyStreamBuffer::ReleaseHandle() {
// - This branch cannot be taken when called from tee.
// - startLoading makes hasPendingActivity return true while loading.
// , we don't need to keep the reader explicitly.
NonThrowableExceptionState exception_state;
ScriptValue reader = ReadableStreamOperations::GetReader(
script_state_.get(), Stream(), exception_state);
ScriptValue reader =
ReadableStreamOperations::GetReader(script_state_.get(), Stream());
if (reader.IsEmpty()) {
stream_broken_ = true;
return BytesConsumer::CreateErrored(
BytesConsumer::Error("Failed to GetReader in ReleaseHandle"));
}
return new ReadableStreamBytesConsumer(script_state_.get(), reader);
}
// We need to call these before calling closeAndLockAndDisturb.
......
......@@ -6,6 +6,7 @@
#define THIRD_PARTY_BLINK_RENDERER_CORE_FETCH_BODY_STREAM_BUFFER_H_
#include <memory>
#include "base/optional.h"
#include "third_party/blink/public/platform/web_data_consumer_handle.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
......@@ -20,6 +21,7 @@
namespace blink {
class EncodedFormData;
class ExceptionState;
class ScriptState;
class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
......@@ -36,7 +38,7 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
AbortSignal* /* signal */);
// |ReadableStreamOperations::isReadableStream(stream)| must hold.
// This function must be called with entering an appropriate V8 context.
BodyStreamBuffer(ScriptState*, ScriptValue stream);
BodyStreamBuffer(ScriptState*, ScriptValue stream, ExceptionState&);
ScriptValue Stream();
......@@ -45,7 +47,7 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
BytesConsumer::BlobSizePolicy);
scoped_refptr<EncodedFormData> DrainAsFormData();
void StartLoading(FetchDataLoader*, FetchDataLoader::Client* /* client */);
void Tee(BodyStreamBuffer**, BodyStreamBuffer**);
void Tee(BodyStreamBuffer**, BodyStreamBuffer**, ExceptionState&);
// UnderlyingSourceBase
ScriptPromise pull(ScriptState*) override;
......@@ -86,6 +88,13 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
void EndLoading();
void StopLoading();
// Implementation of IsStream*() methods. Sets |stream_broken_| if |predicate|
// returns empty. Returns |fallback_value| if |stream_broken_| is or becomes
// true.
bool BooleanStreamOperationOrFallback(
base::Optional<bool> (*predicate)(ScriptState*, ScriptValue),
bool fallback_value);
scoped_refptr<ScriptState> script_state_;
Member<BytesConsumer> consumer_;
// We need this member to keep it alive while loading.
......@@ -96,6 +105,7 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
bool stream_needs_more_ = false;
bool made_from_readable_stream_;
bool in_process_data_ = false;
bool stream_broken_ = false;
DISALLOW_COPY_AND_ASSIGN(BodyStreamBuffer);
};
......
......@@ -14,6 +14,7 @@
#include "third_party/blink/renderer/core/fetch/bytes_consumer_test_util.h"
#include "third_party/blink/renderer/core/fetch/form_data_bytes_consumer.h"
#include "third_party/blink/renderer/core/html/forms/form_data.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/blob/blob_data.h"
#include "third_party/blink/renderer/platform/blob/blob_url.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
......@@ -87,6 +88,7 @@ class MockFetchDataLoader : public FetchDataLoader {
TEST_F(BodyStreamBufferTest, Tee) {
V8TestingScope scope;
NonThrowableExceptionState exception_state;
Checkpoint checkpoint;
MockFetchDataLoaderClient* client1 = MockFetchDataLoaderClient::Create();
MockFetchDataLoaderClient* client2 = MockFetchDataLoaderClient::Create();
......@@ -110,7 +112,7 @@ TEST_F(BodyStreamBufferTest, Tee) {
BodyStreamBuffer* new1;
BodyStreamBuffer* new2;
buffer->Tee(&new1, &new2);
buffer->Tee(&new1, &new2, exception_state);
EXPECT_TRUE(buffer->IsStreamLocked());
EXPECT_TRUE(buffer->IsStreamDisturbed());
......@@ -130,6 +132,7 @@ TEST_F(BodyStreamBufferTest, Tee) {
TEST_F(BodyStreamBufferTest, TeeFromHandleMadeFromStream) {
V8TestingScope scope;
NonThrowableExceptionState exception_state;
ScriptValue stream = EvalWithPrintingError(
scope.GetScriptState(),
"stream = new ReadableStream({start: c => controller = c});"
......@@ -150,11 +153,11 @@ TEST_F(BodyStreamBufferTest, TeeFromHandleMadeFromStream) {
EXPECT_CALL(checkpoint, Call(4));
BodyStreamBuffer* buffer =
new BodyStreamBuffer(scope.GetScriptState(), stream);
new BodyStreamBuffer(scope.GetScriptState(), stream, exception_state);
BodyStreamBuffer* new1;
BodyStreamBuffer* new2;
buffer->Tee(&new1, &new2);
buffer->Tee(&new1, &new2, exception_state);
EXPECT_TRUE(buffer->IsStreamLocked());
// Note that this behavior is slightly different from for the behavior of
......@@ -228,10 +231,11 @@ TEST_F(BodyStreamBufferTest, DrainAsBlobDataHandleReturnsNull) {
TEST_F(BodyStreamBufferTest,
DrainAsBlobFromBufferMadeFromBufferMadeFromStream) {
V8TestingScope scope;
NonThrowableExceptionState exception_state;
ScriptValue stream =
EvalWithPrintingError(scope.GetScriptState(), "new ReadableStream()");
BodyStreamBuffer* buffer =
new BodyStreamBuffer(scope.GetScriptState(), stream);
new BodyStreamBuffer(scope.GetScriptState(), stream, exception_state);
EXPECT_FALSE(buffer->HasPendingActivity());
EXPECT_FALSE(buffer->IsStreamLocked());
......@@ -293,10 +297,11 @@ TEST_F(BodyStreamBufferTest, DrainAsFormDataReturnsNull) {
TEST_F(BodyStreamBufferTest,
DrainAsFormDataFromBufferMadeFromBufferMadeFromStream) {
V8TestingScope scope;
NonThrowableExceptionState exception_state;
ScriptValue stream =
EvalWithPrintingError(scope.GetScriptState(), "new ReadableStream()");
BodyStreamBuffer* buffer =
new BodyStreamBuffer(scope.GetScriptState(), stream);
new BodyStreamBuffer(scope.GetScriptState(), stream, exception_state);
EXPECT_FALSE(buffer->HasPendingActivity());
EXPECT_FALSE(buffer->IsStreamLocked());
......
......@@ -13,6 +13,7 @@
#include "third_party/blink/renderer/core/fetch/fetch_header_list.h"
#include "third_party/blink/renderer/core/fetch/form_data_bytes_consumer.h"
#include "third_party/blink/renderer/core/loader/threadable_loader.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_loader_options.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
......@@ -83,12 +84,15 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
return request;
}
FetchRequestData* FetchRequestData::Clone(ScriptState* script_state) {
FetchRequestData* FetchRequestData::Clone(ScriptState* script_state,
ExceptionState& exception_state) {
FetchRequestData* request = FetchRequestData::CloneExceptBody();
if (buffer_) {
BodyStreamBuffer* new1 = nullptr;
BodyStreamBuffer* new2 = nullptr;
buffer_->Tee(&new1, &new2);
buffer_->Tee(&new1, &new2, exception_state);
if (exception_state.HadException())
return nullptr;
buffer_ = new1;
request->buffer_ = new2;
}
......
......@@ -22,6 +22,7 @@
namespace blink {
class BodyStreamBuffer;
class ExceptionState;
class FetchHeaderList;
class SecurityOrigin;
class ScriptState;
......@@ -35,7 +36,7 @@ class FetchRequestData final
static FetchRequestData* Create();
static FetchRequestData* Create(ScriptState*, const WebServiceWorkerRequest&);
// Call Request::refreshBody() after calling clone() or pass().
FetchRequestData* Clone(ScriptState*);
FetchRequestData* Clone(ScriptState*, ExceptionState&);
FetchRequestData* Pass(ScriptState*);
~FetchRequestData();
......
......@@ -8,6 +8,7 @@
#include "third_party/blink/renderer/core/fetch/body_stream_buffer.h"
#include "third_party/blink/renderer/core/fetch/fetch_header_list.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h"
#include "third_party/blink/renderer/platform/loader/fetch/fetch_utils.h"
#include "third_party/blink/renderer/platform/network/http_names.h"
......@@ -171,7 +172,8 @@ const Vector<KURL>& FetchResponseData::InternalURLList() const {
return url_list_;
}
FetchResponseData* FetchResponseData::Clone(ScriptState* script_state) {
FetchResponseData* FetchResponseData::Clone(ScriptState* script_state,
ExceptionState& exception_state) {
FetchResponseData* new_response = Create();
new_response->type_ = type_;
if (termination_reason_) {
......@@ -194,7 +196,9 @@ FetchResponseData* FetchResponseData::Clone(ScriptState* script_state) {
DCHECK_EQ(buffer_, internal_response_->buffer_);
DCHECK_EQ(internal_response_->type_, Type::kDefault);
new_response->internal_response_ =
internal_response_->Clone(script_state);
internal_response_->Clone(script_state, exception_state);
if (exception_state.HadException())
return nullptr;
buffer_ = internal_response_->buffer_;
new_response->buffer_ = new_response->internal_response_->buffer_;
break;
......@@ -203,7 +207,9 @@ FetchResponseData* FetchResponseData::Clone(ScriptState* script_state) {
if (buffer_) {
BodyStreamBuffer* new1 = nullptr;
BodyStreamBuffer* new2 = nullptr;
buffer_->Tee(&new1, &new2);
buffer_->Tee(&new1, &new2, exception_state);
if (exception_state.HadException())
return nullptr;
buffer_ = new1;
new_response->buffer_ = new2;
}
......@@ -219,7 +225,9 @@ FetchResponseData* FetchResponseData::Clone(ScriptState* script_state) {
DCHECK(!buffer_);
DCHECK_EQ(internal_response_->type_, Type::kDefault);
new_response->internal_response_ =
internal_response_->Clone(script_state);
internal_response_->Clone(script_state, exception_state);
if (exception_state.HadException())
return nullptr;
break;
}
return new_response;
......
......@@ -24,6 +24,7 @@
namespace blink {
class BodyStreamBuffer;
class ExceptionState;
class FetchHeaderList;
class ScriptState;
class WebServiceWorkerResponse;
......@@ -54,7 +55,7 @@ class CORE_EXPORT FetchResponseData final
return internal_response_;
}
FetchResponseData* Clone(ScriptState*);
FetchResponseData* Clone(ScriptState*, ExceptionState& exception_state);
network::mojom::FetchResponseType GetType() const { return type_; }
const KURL* Url() const;
......
......@@ -83,9 +83,8 @@ class ReadableStreamBytesConsumerTest : public testing::Test {
}
ReadableStreamBytesConsumer* CreateConsumer(ScriptValue stream) {
NonThrowableExceptionState es;
ScriptValue reader =
ReadableStreamOperations::GetReader(GetScriptState(), stream, es);
ReadableStreamOperations::GetReader(GetScriptState(), stream);
DCHECK(!reader.IsEmpty());
DCHECK(reader.V8Value()->IsObject());
return new ReadableStreamBytesConsumer(GetScriptState(), reader);
......
......@@ -26,6 +26,7 @@
#include "third_party/blink/renderer/core/html/forms/form_data.h"
#include "third_party/blink/renderer/core/loader/threadable_loader.h"
#include "third_party/blink/renderer/core/url/url_search_params.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
#include "third_party/blink/renderer/platform/blob/blob_data.h"
#include "third_party/blink/renderer/platform/loader/cors/cors.h"
......@@ -830,7 +831,9 @@ Request* Request::clone(ScriptState* script_state,
return nullptr;
}
FetchRequestData* request = request_->Clone(script_state);
FetchRequestData* request = request_->Clone(script_state, exception_state);
if (exception_state.HadException())
return nullptr;
RefreshBody(script_state);
Headers* headers = Headers::Create(request->HeaderList());
headers->SetGuard(headers_->GetGuard());
......
......@@ -22,6 +22,7 @@ namespace blink {
class AbortSignal;
class BodyStreamBuffer;
class ExceptionState;
class RequestInit;
class WebServiceWorkerRequest;
......
......@@ -236,11 +236,17 @@ Response* Response::Create(ScriptState* script_state,
new FormDataBytesConsumer(execution_context, std::move(form_data)),
nullptr /* AbortSignal */);
content_type = "application/x-www-form-urlencoded;charset=UTF-8";
} else if (ReadableStreamOperations::IsReadableStream(script_state,
body_value)) {
} else if (ReadableStreamOperations::IsReadableStream(
script_state, body_value, exception_state)
.value_or(true)) {
if (exception_state.HadException())
return nullptr;
UseCounter::Count(execution_context,
WebFeature::kFetchResponseConstructionWithStream);
body_buffer = new BodyStreamBuffer(script_state, body_value);
body_buffer =
new BodyStreamBuffer(script_state, body_value, exception_state);
if (exception_state.HadException())
return nullptr;
} else {
String string = NativeValueTraits<IDLUSVString>::NativeValue(
isolate, body, exception_state);
......@@ -450,7 +456,9 @@ Response* Response::clone(ScriptState* script_state,
return nullptr;
}
FetchResponseData* response = response_->Clone(script_state);
FetchResponseData* response = response_->Clone(script_state, exception_state);
if (exception_state.HadException())
return nullptr;
RefreshBody(script_state);
Headers* headers = Headers::Create(response->HeaderList());
headers->SetGuard(headers_->GetGuard());
......
......@@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_READABLE_STREAM_OPERATIONS_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_READABLE_STREAM_OPERATIONS_H_
#include "base/optional.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
#include "third_party/blink/renderer/core/core_export.h"
......@@ -20,65 +21,82 @@ class ScriptState;
// V8 Extras.
// All methods should be called in an appropriate V8 context. All ScriptValue
// arguments must not be empty.
//
// Boolean methods return an optional bool, where an empty value indicates that
// Javascript failed to return a value (ie. an exception occurred). Exceptions
// are not caught, so that they can be handled by user Javascript. This implicit
// exception passing is error-prone and bad.
//
// TODO(ricea): Add ExceptionState arguments and make exception passing
// explicit. https://crbug.com/853189.
class CORE_EXPORT ReadableStreamOperations {
STATIC_ONLY(ReadableStreamOperations);
public:
// createReadableStreamWithExternalController
// If the caller supplies an invalid strategy (e.g. one that returns
// negative sizes, or doesn't have appropriate properties), this will crash.
// negative sizes, or doesn't have appropriate properties), or an exception
// occurs for another reason, this will return an empty value.
static ScriptValue CreateReadableStream(ScriptState*,
UnderlyingSourceBase*,
ScriptValue strategy);
// createBuiltInCountQueuingStrategy
// If the constructor throws, this will return an empty value.
static ScriptValue CreateCountQueuingStrategy(ScriptState*,
size_t high_water_mark);
// AcquireReadableStreamDefaultReader
// This function assumes |isReadableStream(stream)|.
// Returns an empty value and throws an error via the ExceptionState when
// errored.
static ScriptValue GetReader(ScriptState*,
ScriptValue stream,
ExceptionState&);
// This function assumes |IsReadableStream(stream)|.
static ScriptValue GetReader(ScriptState*, ScriptValue stream);
// IsReadableStream
static bool IsReadableStream(ScriptState*, ScriptValue);
// IsReadableStream. Exceptions are not caught.
static base::Optional<bool> IsReadableStream(ScriptState*, ScriptValue);
// IsReadableStreamDisturbed
// This function assumes |isReadableStream(stream)|.
static bool IsDisturbed(ScriptState*, ScriptValue stream);
// IsReadableStream, exception-catching version. Exceptions will be passed to
// |exception_state|.
static base::Optional<bool> IsReadableStream(ScriptState*,
ScriptValue,
ExceptionState& exception_state);
// IsReadableStreamLocked
// This function assumes |isReadableStream(stream)|.
static bool IsLocked(ScriptState*, ScriptValue stream);
// IsReadableStreamDisturbed.
// This function assumes |IsReadableStream(stream)|.
static base::Optional<bool> IsDisturbed(ScriptState*, ScriptValue stream);
// IsReadableStreamReadable
// This function assumes |isReadableStream(stream)|.
static bool IsReadable(ScriptState*, ScriptValue stream);
// IsReadableStreamLocked.
// This function assumes |IsReadableStream(stream)|.
static base::Optional<bool> IsLocked(ScriptState*, ScriptValue stream);
// IsReadableStreamClosed
// This function assumes |isReadableStream(stream)|.
static bool IsClosed(ScriptState*, ScriptValue stream);
// IsReadableStreamReadable.
// This function assumes |IsReadableStream(stream)|.
static base::Optional<bool> IsReadable(ScriptState*, ScriptValue stream);
// IsReadableStreamErrored
// This function assumes |isReadableStream(stream)|.
static bool IsErrored(ScriptState*, ScriptValue stream);
// IsReadableStreamClosed.
// This function assumes |IsReadableStream(stream)|.
static base::Optional<bool> IsClosed(ScriptState*, ScriptValue stream);
// IsReadableStreamDefaultReader
static bool IsReadableStreamDefaultReader(ScriptState*, ScriptValue);
// IsReadableStreamErrored.
// This function assumes |IsReadableStream(stream)|.
static base::Optional<bool> IsErrored(ScriptState*, ScriptValue stream);
// IsReadableStreamDefaultReader.
static base::Optional<bool> IsReadableStreamDefaultReader(ScriptState*,
ScriptValue);
// ReadableStreamDefaultReaderRead
// This function assumes |isReadableStreamDefaultReader(reader)|.
// This function assumes |IsReadableStreamDefaultReader(reader)|.
// If an exception occurs, returns a rejected promise.
static ScriptPromise DefaultReaderRead(ScriptState*, ScriptValue reader);
// ReadableStreamTee
// This function assumes |isReadableStream(stream)| and |!isLocked(stream)|
// This function assumes |IsReadableStream(stream)| and |!IsLocked(stream)|
// Returns without setting new_stream1 or new_stream2 if an error occurs.
// Exceptions are caught and rethrown on |exception_state|.
static void Tee(ScriptState*,
ScriptValue stream,
ScriptValue* new_stream1,
ScriptValue* new_stream2);
ScriptValue* new_stream2,
ExceptionState&);
};
} // namespace blink
......
......@@ -120,37 +120,49 @@ TEST(ReadableStreamOperationsTest, IsReadableStream) {
V8TestingScope scope;
TryCatchScope try_catch_scope(scope.GetIsolate());
EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(), v8::Undefined(scope.GetIsolate()))));
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Undefined(scope.GetIsolate())))
.value_or(true));
EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
scope.GetScriptState(), ScriptValue::CreateNull(scope.GetScriptState())));
scope.GetScriptState(),
ScriptValue::CreateNull(scope.GetScriptState()))
.value_or(true));
EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Object::New(scope.GetIsolate()))));
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Object::New(scope.GetIsolate())))
.value_or(true));
ScriptValue stream = EvalWithPrintingError(&scope, "new ReadableStream()");
EXPECT_FALSE(stream.IsEmpty());
EXPECT_TRUE(ReadableStreamOperations::IsReadableStream(scope.GetScriptState(),
stream));
EXPECT_TRUE(
ReadableStreamOperations::IsReadableStream(scope.GetScriptState(), stream)
.value_or(false));
}
TEST(ReadableStreamOperationsTest, IsReadableStreamDefaultReaderInvalid) {
V8TestingScope scope;
TryCatchScope try_catch_scope(scope.GetIsolate());
EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(), v8::Undefined(scope.GetIsolate()))));
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Undefined(scope.GetIsolate())))
.value_or(true));
EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(), ScriptValue::CreateNull(scope.GetScriptState())));
scope.GetScriptState(),
ScriptValue::CreateNull(scope.GetScriptState()))
.value_or(true));
EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Object::New(scope.GetIsolate()))));
scope.GetScriptState(),
ScriptValue(scope.GetScriptState(),
v8::Object::New(scope.GetIsolate())))
.value_or(true));
ScriptValue stream = EvalWithPrintingError(&scope, "new ReadableStream()");
EXPECT_FALSE(stream.IsEmpty());
ASSERT_FALSE(stream.IsEmpty());
EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(), stream));
scope.GetScriptState(), stream)
.value_or(true));
}
TEST(ReadableStreamOperationsTest, GetReader) {
......@@ -160,31 +172,24 @@ TEST(ReadableStreamOperationsTest, GetReader) {
EXPECT_FALSE(stream.IsEmpty());
EXPECT_FALSE(
ReadableStreamOperations::IsLocked(scope.GetScriptState(), stream));
ReadableStreamOperations::IsLocked(scope.GetScriptState(), stream)
.value_or(true));
ScriptValue reader;
{
DummyExceptionStateForTesting es;
reader =
ReadableStreamOperations::GetReader(scope.GetScriptState(), stream, es);
ASSERT_FALSE(es.HadException());
}
EXPECT_TRUE(
ReadableStreamOperations::IsLocked(scope.GetScriptState(), stream));
reader = ReadableStreamOperations::GetReader(scope.GetScriptState(), stream);
EXPECT_TRUE(ReadableStreamOperations::IsLocked(scope.GetScriptState(), stream)
.value_or(false));
ASSERT_FALSE(reader.IsEmpty());
EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
scope.GetScriptState(), reader));
EXPECT_FALSE(
ReadableStreamOperations::IsReadableStream(scope.GetScriptState(), reader)
.value_or(true));
EXPECT_TRUE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(), reader));
scope.GetScriptState(), reader)
.value_or(false));
// Already locked!
{
DummyExceptionStateForTesting es;
reader =
ReadableStreamOperations::GetReader(scope.GetScriptState(), stream, es);
ASSERT_TRUE(es.HadException());
}
ASSERT_TRUE(reader.IsEmpty());
reader = ReadableStreamOperations::GetReader(scope.GetScriptState(), stream);
EXPECT_TRUE(reader.IsEmpty());
}
TEST(ReadableStreamOperationsTest, IsDisturbed) {
......@@ -195,12 +200,14 @@ TEST(ReadableStreamOperationsTest, IsDisturbed) {
EXPECT_FALSE(stream.IsEmpty());
EXPECT_FALSE(
ReadableStreamOperations::IsDisturbed(scope.GetScriptState(), stream));
ReadableStreamOperations::IsDisturbed(scope.GetScriptState(), stream)
.value_or(true));
ASSERT_FALSE(EvalWithPrintingError(&scope, "stream.cancel()").IsEmpty());
EXPECT_TRUE(
ReadableStreamOperations::IsDisturbed(scope.GetScriptState(), stream));
ReadableStreamOperations::IsDisturbed(scope.GetScriptState(), stream)
.value_or(false));
}
TEST(ReadableStreamOperationsTest, Read) {
......@@ -213,7 +220,8 @@ TEST(ReadableStreamOperationsTest, Read) {
"new ReadableStream({start}).getReader()");
EXPECT_FALSE(reader.IsEmpty());
ASSERT_TRUE(ReadableStreamOperations::IsReadableStreamDefaultReader(
scope.GetScriptState(), reader));
scope.GetScriptState(), reader)
.value_or(false));
Iteration* it1 = new Iteration();
Iteration* it2 = new Iteration();
......@@ -273,12 +281,7 @@ TEST(ReadableStreamOperationsTest,
EXPECT_EQ(8, underlying_source->DesiredSize());
ScriptValue reader;
{
DummyExceptionStateForTesting es;
reader =
ReadableStreamOperations::GetReader(scope.GetScriptState(), stream, es);
ASSERT_FALSE(es.HadException());
}
reader = ReadableStreamOperations::GetReader(scope.GetScriptState(), stream);
ASSERT_FALSE(reader.IsEmpty());
Iteration* it1 = new Iteration();
......@@ -368,11 +371,14 @@ TEST(ReadableStreamOperationsTest, IsReadable) {
ASSERT_FALSE(errored.IsEmpty());
EXPECT_TRUE(
ReadableStreamOperations::IsReadable(scope.GetScriptState(), readable));
ReadableStreamOperations::IsReadable(scope.GetScriptState(), readable)
.value_or(false));
EXPECT_FALSE(
ReadableStreamOperations::IsReadable(scope.GetScriptState(), closed));
ReadableStreamOperations::IsReadable(scope.GetScriptState(), closed)
.value_or(true));
EXPECT_FALSE(
ReadableStreamOperations::IsReadable(scope.GetScriptState(), errored));
ReadableStreamOperations::IsReadable(scope.GetScriptState(), errored)
.value_or(true));
}
TEST(ReadableStreamOperationsTest, IsClosed) {
......@@ -388,11 +394,13 @@ TEST(ReadableStreamOperationsTest, IsClosed) {
ASSERT_FALSE(errored.IsEmpty());
EXPECT_FALSE(
ReadableStreamOperations::IsClosed(scope.GetScriptState(), readable));
EXPECT_TRUE(
ReadableStreamOperations::IsClosed(scope.GetScriptState(), closed));
ReadableStreamOperations::IsClosed(scope.GetScriptState(), readable)
.value_or(true));
EXPECT_TRUE(ReadableStreamOperations::IsClosed(scope.GetScriptState(), closed)
.value_or(false));
EXPECT_FALSE(
ReadableStreamOperations::IsClosed(scope.GetScriptState(), errored));
ReadableStreamOperations::IsClosed(scope.GetScriptState(), errored)
.value_or(true));
}
TEST(ReadableStreamOperationsTest, IsErrored) {
......@@ -408,29 +416,39 @@ TEST(ReadableStreamOperationsTest, IsErrored) {
ASSERT_FALSE(errored.IsEmpty());
EXPECT_FALSE(
ReadableStreamOperations::IsErrored(scope.GetScriptState(), readable));
ReadableStreamOperations::IsErrored(scope.GetScriptState(), readable)
.value_or(true));
EXPECT_FALSE(
ReadableStreamOperations::IsErrored(scope.GetScriptState(), closed));
ReadableStreamOperations::IsErrored(scope.GetScriptState(), closed)
.value_or(true));
EXPECT_TRUE(
ReadableStreamOperations::IsErrored(scope.GetScriptState(), errored));
ReadableStreamOperations::IsErrored(scope.GetScriptState(), errored)
.value_or(false));
}
TEST(ReadableStreamOperationsTest, Tee) {
V8TestingScope scope;
TryCatchScope try_catch_scope(scope.GetIsolate());
NonThrowableExceptionState exception_state;
ScriptValue original =
EvalWithPrintingError(&scope,
"var controller;"
"new ReadableStream({start: c => controller = c})");
ASSERT_FALSE(original.IsEmpty());
ScriptValue new1, new2;
ReadableStreamOperations::Tee(scope.GetScriptState(), original, &new1, &new2);
ReadableStreamOperations::Tee(scope.GetScriptState(), original, &new1, &new2,
exception_state);
ASSERT_FALSE(new1.IsEmpty());
ASSERT_FALSE(new2.IsEmpty());
NonThrowableExceptionState ec;
ScriptValue reader1 =
ReadableStreamOperations::GetReader(scope.GetScriptState(), new1, ec);
ReadableStreamOperations::GetReader(scope.GetScriptState(), new1);
ScriptValue reader2 =
ReadableStreamOperations::GetReader(scope.GetScriptState(), new2, ec);
ReadableStreamOperations::GetReader(scope.GetScriptState(), new2);
ASSERT_FALSE(reader1.IsEmpty());
ASSERT_FALSE(reader2.IsEmpty());
Iteration* it1 = new Iteration();
Iteration* it2 = new Iteration();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment