Skip to content

Commit

Permalink
Transferable Streams: Fix crash on deserialization error
Browse files Browse the repository at this point in the history
The transferable streams implementation would crash when it received a
chunk that wasn't deserializable, because it failed to enter a
ScriptScope before creating a V8 object.

Fix it.

Also fix the message on the DOMException that is created when
deserialization fails.

Add a test for deserialization failure. This is hard to trigger in a
cross-browser fashion. In this test, we use a WASM module, which can't
be transferred to a different site.

BUG=1122725

Change-Id: I6e49c69978388357a3c6006cc02dce8f0a949954
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379417
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803014}
  • Loading branch information
ricea authored and Commit Bot committed Aug 31, 2020
1 parent 87a891b commit 4dbcfb2
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 6 deletions.
17 changes: 11 additions & 6 deletions third_party/blink/renderer/core/streams/transferable_streams.cc
Expand Up @@ -11,6 +11,7 @@
#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_dom_exception.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_post_message_options.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/dom/events/native_event_listener.h"
#include "third_party/blink/renderer/core/events/message_event.h"
Expand Down Expand Up @@ -260,24 +261,28 @@ class CrossRealmTransformErrorListener final : public NativeEventListener {
void Invoke(ExecutionContext*, Event*) override {
ScriptState* script_state = target_->GetScriptState();

// Need to enter a script scope to manipulate JavaScript objects.
ScriptState::Scope scope(script_state);

// Common to
// https://streams.spec.whatwg.org/#abstract-opdef-setupcrossrealmtransformreadable
// and
// https://streams.spec.whatwg.org/#abstract-opdef-setupcrossrealmtransformwritable.

// 1. Let error be a new "DataCloneError" DOMException.
const auto* error =
DOMException::Create("chunk could not be cloned", "DataCloneError");
auto* message_port = target_->GetMessagePort();
v8::Local<v8::Value> error_value = ToV8(error, script_state);
v8::Local<v8::Value> error = V8ThrowDOMException::CreateOrEmpty(
script_state->GetIsolate(), DOMExceptionCode::kDataCloneError,
"chunk could not be cloned");

// 2. Perform ! CrossRealmTransformSendError(port, error).
CrossRealmTransformSendError(script_state, message_port, error_value);
auto* message_port = target_->GetMessagePort();
CrossRealmTransformSendError(script_state, message_port, error);

// 4. Disentangle port.
message_port->close();

target_->HandleError(error_value);
DVLOG(3) << "ErrorListener saw messageerror";
target_->HandleError(error);
}

void Trace(Visitor* visitor) const override {
Expand Down
@@ -0,0 +1,39 @@
// META: script=/common/get-host-info.sub.js
// META: script=resources/create-wasm-module.js
// META: timeout=long

const { HTTPS_NOTSAMESITE_ORIGIN } = get_host_info();
const iframe = document.createElement('iframe');
iframe.src = `${HTTPS_NOTSAMESITE_ORIGIN}/streams/transferable/resources/deserialize-error-frame.html`;

window.addEventListener('message', async evt => {
// Tests are serialized to make the results deterministic.
switch (evt.data) {
case 'init done': {
const ws = new WritableStream();
iframe.contentWindow.postMessage(ws, '*', [ws]);
return;
}

case 'ws done': {
const module = await createWasmModule();
const rs = new ReadableStream({
start(controller) {
controller.enqueue(module);
}
});
iframe.contentWindow.postMessage(rs, '*', [rs]);
return;
}

case 'rs done': {
iframe.remove();
}
}
});

// Need to do this after adding the listener to ensure we catch the first
// message.
document.body.appendChild(iframe);

fetch_tests_from_window(iframe.contentWindow);
@@ -0,0 +1,11 @@
// There aren't many cloneable types that will cause an error on
// deserialization. WASM modules have the property that it's an error to
// deserialize them cross-site, which works for our purposes.
async function createWasmModule() {
// It doesn't matter what the module is, so we use one from another
// test.
const response =
await fetch("/wasm/serialization/module/resources/incrementer.wasm");
const ab = await response.arrayBuffer();
return WebAssembly.compile(ab);
}
@@ -0,0 +1,39 @@
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="create-wasm-module.js"></script>
<script>
async_test(t => {
parent.postMessage('init done', '*');
window.addEventListener('message', async evt => {
if (evt.data.constructor.name !== 'WritableStream') {
return;
}
const ws = evt.data;
const writer = ws.getWriter();
const module = await createWasmModule();
writer.write(module);
await promise_rejects_dom(t, 'DataCloneError', writer.closed,
'should reject with a DataCloneError');
t.done();
// Signal that this test is done. When both tests are done the iframe will
// be removed.
parent.postMessage('ws done', '*');
});
}, 'a WritableStream deserialization failure should result in a DataCloneError');

async_test(t => {
window.addEventListener('message', async evt => {
if (evt.data.constructor.name !== 'ReadableStream') {
return;
}
const rs = evt.data;
const reader = rs.getReader();
await promise_rejects_dom(t, 'DataCloneError', reader.read(),
'should reject with a DataCloneError');
t.done();
// Signal that this test is done. When both tests are done the iframe will
// be removed.
parent.postMessage('rs done', '*');
});
}, 'a ReadableStream deserialization failure should result in a DataCloneError');
</script>

0 comments on commit 4dbcfb2

Please sign in to comment.