Fetch: Handle ArrayBuffer detach during ReadableStream reading.

Previously ReadableStreamByteConsumer assumed that each UInt8Array data
chunk was immutable.  While its generally true a UInt8Array is immutable
it can still be invalidated by transferring it via postMessage().  This
will cause any references to the chunk to suddenly become length zero.
If this happens to chunks being processed by the consumer then it will
get confused and cause a renderer crash.  This CL causes the consumer to
error out instead.

Bug: 1161236
Change-Id: I44c7ad9115b05e321ba7cacaf28863f88ab14492
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615675Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843005}

third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer.cc

@@ -112,6 +112,14 @@ BytesConsumer::Result ReadableStreamBytesConsumer::BeginRead(
     return Result::kDone;
 
   if (pending_buffer_) {
+    // The UInt8Array has become detached due to, for example, the site
+    // transferring it away via postMessage().  Since we were in the middle
+    // of reading the array we must error out.
+    if (pending_buffer_->IsDetached()) {
+      SetErrored();
+      return Result::kError;
+    }
+
     DCHECK_LE(pending_offset_, pending_buffer_->length());
     *buffer = reinterpret_cast<const char*>(pending_buffer_->Data()) +
               pending_offset_;
@@ -141,6 +149,15 @@ BytesConsumer::Result ReadableStreamBytesConsumer::BeginRead(
 
 BytesConsumer::Result ReadableStreamBytesConsumer::EndRead(size_t read_size) {
   DCHECK(pending_buffer_);
+
+  // While the buffer size is immutable once constructed, the buffer can be
+  // detached if the site does something like transfer it away using
+  // postMessage().  Since we were in the middle of a read we must error out.
+  if (pending_buffer_->IsDetached()) {
+    SetErrored();
+    return Result::kError;
+  }
+
   DCHECK_LE(pending_offset_ + read_size, pending_buffer_->length());
   pending_offset_ += read_size;
   if (pending_offset_ >= pending_buffer_->length()) {
@@ -220,12 +237,18 @@ void ReadableStreamBytesConsumer::OnRejected() {
   if (state_ == PublicState::kClosed)
     return;
   DCHECK_EQ(state_, PublicState::kReadableOrWaiting);
-  state_ = PublicState::kErrored;
-  reader_ = nullptr;
   Client* client = client_;
-  ClearClient();
+  SetErrored();
   if (client)
     client->OnStateChange();
 }
 
+void ReadableStreamBytesConsumer::SetErrored() {
+  DCHECK_NE(state_, PublicState::kClosed);
+  DCHECK_NE(state_, PublicState::kErrored);
+  state_ = PublicState::kErrored;
+  ClearClient();
+  reader_ = nullptr;
+}
+
 }  // namespace blink

third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer.h

@@ -47,6 +47,7 @@ class CORE_EXPORT ReadableStreamBytesConsumer final : public BytesConsumer {
   void OnRead(DOMUint8Array*);
   void OnReadDone();
   void OnRejected();
+  void SetErrored();
   void Notify();
 
   Member<ReadableStreamDefaultReader> reader_;

third_party/blink/renderer/core/fetch/readable_stream_bytes_consumer_test.cc

@@ -239,6 +239,109 @@ TEST(ReadableStreamBytesConsumerTest, TwoPhaseRead) {
   EXPECT_EQ(Result::kDone, consumer->BeginRead(&buffer, &available));
 }
 
+TEST(ReadableStreamBytesConsumerTest, TwoPhaseReadDetachedDuringRead) {
+  V8TestingScope scope;
+  ScriptState* script_state = scope.GetScriptState();
+
+  auto* underlying_source =
+      MakeGarbageCollected<TestUnderlyingSource>(script_state);
+  auto* stream = ReadableStream::CreateWithCountQueueingStrategy(
+      script_state, underlying_source, 0);
+
+  auto* chunk = DOMUint8Array::Create(4);
+  chunk->Data()[0] = 0x43;
+  chunk->Data()[1] = 0x44;
+  chunk->Data()[2] = 0x45;
+  chunk->Data()[3] = 0x46;
+  underlying_source->Enqueue(
+      ScriptValue(script_state->GetIsolate(), ToV8(chunk, script_state)));
+  underlying_source->Close();
+
+  Persistent<BytesConsumer> consumer =
+      MakeGarbageCollected<ReadableStreamBytesConsumer>(script_state, stream);
+  Persistent<MockClient> client = MakeGarbageCollected<MockClient>();
+  consumer->SetClient(client);
+  Checkpoint checkpoint;
+
+  InSequence s;
+  EXPECT_CALL(checkpoint, Call(1));
+  EXPECT_CALL(checkpoint, Call(2));
+  EXPECT_CALL(checkpoint, Call(3));
+  EXPECT_CALL(*client, OnStateChange());
+  EXPECT_CALL(checkpoint, Call(4));
+
+  const char* buffer = nullptr;
+  size_t available = 0;
+  checkpoint.Call(1);
+  test::RunPendingTasks();
+  checkpoint.Call(2);
+  EXPECT_EQ(Result::kShouldWait, consumer->BeginRead(&buffer, &available));
+  checkpoint.Call(3);
+  test::RunPendingTasks();
+  checkpoint.Call(4);
+  EXPECT_EQ(Result::kOk, consumer->BeginRead(&buffer, &available));
+  ASSERT_EQ(4u, available);
+  EXPECT_EQ(0x43, buffer[0]);
+  EXPECT_EQ(0x44, buffer[1]);
+  EXPECT_EQ(0x45, buffer[2]);
+  EXPECT_EQ(0x46, buffer[3]);
+  chunk->DetachForTesting();
+  EXPECT_EQ(Result::kError, consumer->EndRead(4));
+  EXPECT_EQ(PublicState::kErrored, consumer->GetPublicState());
+}
+
+TEST(ReadableStreamBytesConsumerTest, TwoPhaseReadDetachedBetweenReads) {
+  V8TestingScope scope;
+  ScriptState* script_state = scope.GetScriptState();
+
+  auto* underlying_source =
+      MakeGarbageCollected<TestUnderlyingSource>(script_state);
+  auto* stream = ReadableStream::CreateWithCountQueueingStrategy(
+      script_state, underlying_source, 0);
+
+  auto* chunk = DOMUint8Array::Create(4);
+  chunk->Data()[0] = 0x43;
+  chunk->Data()[1] = 0x44;
+  chunk->Data()[2] = 0x45;
+  chunk->Data()[3] = 0x46;
+  underlying_source->Enqueue(
+      ScriptValue(script_state->GetIsolate(), ToV8(chunk, script_state)));
+  underlying_source->Close();
+
+  Persistent<BytesConsumer> consumer =
+      MakeGarbageCollected<ReadableStreamBytesConsumer>(script_state, stream);
+  Persistent<MockClient> client = MakeGarbageCollected<MockClient>();
+  consumer->SetClient(client);
+  Checkpoint checkpoint;
+
+  InSequence s;
+  EXPECT_CALL(checkpoint, Call(1));
+  EXPECT_CALL(checkpoint, Call(2));
+  EXPECT_CALL(checkpoint, Call(3));
+  EXPECT_CALL(*client, OnStateChange());
+  EXPECT_CALL(checkpoint, Call(4));
+
+  const char* buffer = nullptr;
+  size_t available = 0;
+  checkpoint.Call(1);
+  test::RunPendingTasks();
+  checkpoint.Call(2);
+  EXPECT_EQ(Result::kShouldWait, consumer->BeginRead(&buffer, &available));
+  checkpoint.Call(3);
+  test::RunPendingTasks();
+  checkpoint.Call(4);
+  EXPECT_EQ(Result::kOk, consumer->BeginRead(&buffer, &available));
+  ASSERT_EQ(4u, available);
+  EXPECT_EQ(0x43, buffer[0]);
+  EXPECT_EQ(0x44, buffer[1]);
+  EXPECT_EQ(0x45, buffer[2]);
+  EXPECT_EQ(0x46, buffer[3]);
+  EXPECT_EQ(Result::kOk, consumer->EndRead(1));
+  chunk->DetachForTesting();
+  EXPECT_EQ(Result::kError, consumer->BeginRead(&buffer, &available));
+  EXPECT_EQ(PublicState::kErrored, consumer->GetPublicState());
+}
+
 TEST(ReadableStreamBytesConsumerTest, EnqueueUndefined) {
   V8TestingScope scope;
   ScriptState* script_state = scope.GetScriptState();

third_party/blink/renderer/core/typed_arrays/dom_array_buffer_view.h

@@ -122,6 +122,10 @@ class CORE_EXPORT DOMArrayBufferView : public ScriptWrappable {
     ScriptWrappable::Trace(visitor);
   }
 
+  void DetachForTesting() { dom_array_buffer_->Detach(); }
+
+  bool IsDetached() const { return dom_array_buffer_->IsDetached(); }
+
  protected:
   DOMArrayBufferView(DOMArrayBufferBase* dom_array_buffer, size_t byte_offset)
       : raw_byte_offset_(byte_offset), dom_array_buffer_(dom_array_buffer) {
@@ -130,8 +134,6 @@ class CORE_EXPORT DOMArrayBufferView : public ScriptWrappable {
         static_cast<char*>(dom_array_buffer_->DataMaybeShared()) + byte_offset;
   }
 
-  bool IsDetached() const { return dom_array_buffer_->IsDetached(); }
-
  private:
   // The raw_* fields may be stale after Detach. Use getters instead.
   // This is the address of the ArrayBuffer's storage, plus the byte offset.