From 2d7f3c0fb08fd5c5905c39acd44adf4a6f8ca763 Mon Sep 17 00:00:00 2001
From: Mayur Patil <[email protected]>
Date: Tue, 09 Jun 2026 08:17:40 -0700
Subject: [PATCH] [WebSocket/H3] Handle QUIC stream close during write

WebSocketQuicStreamAdapter::Write() can trigger a QUIC stream close
while WriteOrBufferBody() is running. If the close path clears or
destroys the adapter, Write() must not continue using the old stream
pointer after the call returns.

Bug: 515438256
Change-Id: Id02d2c75a99543d868c438dcbd29065644062449
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7874191
Reviewed-by: Nidhi Jaju <[email protected]>
Commit-Queue: Mayur Patil <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1643981}
---

diff --git a/net/websockets/websocket_basic_stream_adapters.cc b/net/websockets/websocket_basic_stream_adapters.cc
index 12e769bb..707b04a 100644
--- a/net/websockets/websocket_basic_stream_adapters.cc
+++ b/net/websockets/websocket_basic_stream_adapters.cc
@@ -284,7 +284,7 @@
                                      int buf_len,
                                      CompletionOnceCallback callback) {
   if (!websocket_quic_spdy_stream_) {
-    return ERR_UNEXPECTED;
+    return stream_error_;
   }
 
   int rv = websocket_quic_spdy_stream_->Read(buf, buf_len);
@@ -304,15 +304,29 @@
     CompletionOnceCallback callback,
     const NetworkTrafficAnnotationTag& traffic_annotation) {
   DCHECK(!write_callback_);
-  DCHECK(websocket_quic_spdy_stream_);
   CHECK_GT(buf_len, 0);
   DCHECK(callback);
 
+  if (!websocket_quic_spdy_stream_) {
+    return stream_error_;
+  }
+
   // Queue data to the QUIC stream. WriteOrBufferBody() either sends the data
   // immediately if flow control allows, or buffers it internally.
+  // It can also synchronously close the connection on socket write errors.
+  base::WeakPtr<WebSocketQuicStreamAdapter> weak_this =
+      weak_factory_.GetWeakPtr();
   websocket_quic_spdy_stream_->WriteOrBufferBody(
       {buf->data(), static_cast<size_t>(buf_len)},
       /*fin=*/false);
+  // If the adapter was destroyed by a callback during the write, return
+  // safely without accessing member variables.
+  if (!weak_this) {
+    return ERR_CONNECTION_CLOSED;
+  }
+  if (!websocket_quic_spdy_stream_) {
+    return stream_error_;
+  }
 
   // Check CanWriteNewData() after queuing rather than before. This is necessary
   // because WriteOrBufferBody() may have caused the send buffer to cross its
@@ -403,21 +417,27 @@
 
 void WebSocketQuicStreamAdapter::OnClose(int status) {
   CHECK_LE(status, 0);
-  auto self = weak_factory_.GetWeakPtr();
-  ClearStream();
-
   if (status == OK) {
     status = ERR_CONNECTION_CLOSED;
   }
+  stream_error_ = status;
+
+  base::WeakPtr<WebSocketQuicStreamAdapter> weak_this =
+      weak_factory_.GetWeakPtr();
+  ClearStream();
+
+  // Running a completion callback can delete the current
+  // WebSocketQuicStreamAdapter. In that case, `weak_this` becomes invalid and
+  // OnClose() must return before accessing more member variables.
   if (read_callback_) {
     std::move(read_callback_).Run(status);
-    if (!self) {  // |this| might have been destroyed.
+    if (!weak_this) {
       return;
     }
   }
   if (write_callback_) {
     std::move(write_callback_).Run(status);
-    if (!self) {  // |this| might have been destroyed.
+    if (!weak_this) {
       return;
     }
   }
diff --git a/net/websockets/websocket_basic_stream_adapters.h b/net/websockets/websocket_basic_stream_adapters.h
index 8dfc9d5..0fa7882 100644
--- a/net/websockets/websocket_basic_stream_adapters.h
+++ b/net/websockets/websocket_basic_stream_adapters.h
@@ -239,6 +239,9 @@
   //  because they may be destroyed in any order.
   raw_ptr<WebSocketQuicSpdyStream> websocket_quic_spdy_stream_;
 
+  // Close error returned by Read() and Write() after the stream is cleared.
+  int stream_error_ = ERR_UNEXPECTED;
+
   raw_ptr<Delegate> delegate_;
 
   // Read buffer, length and callback used for asynchronous read operations.
diff --git a/net/websockets/websocket_basic_stream_adapters_test.cc b/net/websockets/websocket_basic_stream_adapters_test.cc
index b0929d0..d084be6 100644
--- a/net/websockets/websocket_basic_stream_adapters_test.cc
+++ b/net/websockets/websocket_basic_stream_adapters_test.cc
@@ -2227,6 +2227,57 @@
   EXPECT_TRUE(mock_quic_data_.AllWriteDataConsumed());
 }
 
+// Verifies that WebSocketQuicStreamAdapter::Write() safely returns when
+// QuicSpdyStream::WriteOrBufferBody() synchronously closes the QUIC stream and
+// a pending read callback deletes the adapter before Write() continues.
+TEST_P(WebSocketQuicStreamAdapterTest,
+       WriteSyncSocketErrorDestroysAdapterWithPendingRead) {
+  int packet_number = 1;
+
+  mock_quic_data_.AddWrite(SYNCHRONOUS,
+                           ConstructSettingsPacket(packet_number++));
+  mock_quic_data_.AddWrite(
+      SYNCHRONOUS, client_maker_.MakeRequestHeadersPacket(
+                       packet_number++, client_data_stream_id1_, /*fin=*/false,
+                       ConvertRequestPriorityToQuicPriority(LOWEST),
+                       RequestHeaders(), nullptr));
+  // This write is the DATA packet from WebSocketQuicStreamAdapter::Write().
+  // Fail it synchronously to close the connection inside WriteOrBufferBody().
+  mock_quic_data_.AddWrite(SYNCHRONOUS, ERR_CONNECTION_REFUSED);
+
+  Initialize();
+
+  net::QuicChromiumClientSession::Handle* session_handle =
+      GetQuicSessionHandle();
+  ASSERT_TRUE(session_handle);
+
+  TestWebSocketQuicStreamAdapterCompletionCallback creation_callback;
+  auto adapter = session_handle->CreateWebSocketQuicStreamAdapter(
+      &mock_delegate_, creation_callback.callback(),
+      TRAFFIC_ANNOTATION_FOR_TESTS);
+  ASSERT_TRUE(adapter);
+  adapter->WriteHeaders(RequestHeaders(), false);
+
+  // Start a Read() that will finish later. DeleterCallback owns the adapter and
+  // deletes it when the read callback runs.
+  DeleterCallback callback(std::move(adapter));
+  constexpr int kReadBufSize = 1024;
+  auto read_buf = base::MakeRefCounted<IOBufferWithSize>(kReadBufSize);
+  int rv = callback.adapter()->Read(read_buf.get(), kReadBufSize,
+                                    callback.callback());
+  EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+
+  // This WebSocketQuicStreamAdapter::Write() triggers the mocked socket error.
+  // Closing the stream runs the pending read callback, which deletes the
+  // adapter. Write() must return without using the deleted adapter.
+  auto write_buf = base::MakeRefCounted<StringIOBuffer>("test data");
+  rv = callback.adapter()->Write(write_buf.get(), write_buf->size(),
+                                 base::DoNothing(),
+                                 TRAFFIC_ANNOTATION_FOR_TESTS);
+  EXPECT_THAT(rv, IsError(ERR_CONNECTION_CLOSED));
+  EXPECT_THAT(callback.WaitForResult(), IsError(ERR_QUIC_PROTOCOL_ERROR));
+}
+
 // Tests that the adapter correctly handles being destroyed from within its own
 // OnClose() delegate method, when there are no pending read or write
 // callbacks.

diff --git a/net/websockets/websocket_basic_stream_adapters_test.cc b/net/websockets/websocket_basic_stream_adapters_test.cc
index b0929d0..d084be6 100644
--- a/net/websockets/websocket_basic_stream_adapters_test.cc
+++ b/net/websockets/websocket_basic_stream_adapters_test.cc
@@ -2227,6 +2227,57 @@
   EXPECT_TRUE(mock_quic_data_.AllWriteDataConsumed());
 }
 
+// Verifies that WebSocketQuicStreamAdapter::Write() safely returns when
+// QuicSpdyStream::WriteOrBufferBody() synchronously closes the QUIC stream and
+// a pending read callback deletes the adapter before Write() continues.
+TEST_P(WebSocketQuicStreamAdapterTest,
+       WriteSyncSocketErrorDestroysAdapterWithPendingRead) {
+  int packet_number = 1;
+
+  mock_quic_data_.AddWrite(SYNCHRONOUS,
+                           ConstructSettingsPacket(packet_number++));
+  mock_quic_data_.AddWrite(
+      SYNCHRONOUS, client_maker_.MakeRequestHeadersPacket(
+                       packet_number++, client_data_stream_id1_, /*fin=*/false,
+                       ConvertRequestPriorityToQuicPriority(LOWEST),
+                       RequestHeaders(), nullptr));
+  // This write is the DATA packet from WebSocketQuicStreamAdapter::Write().
+  // Fail it synchronously to close the connection inside WriteOrBufferBody().
+  mock_quic_data_.AddWrite(SYNCHRONOUS, ERR_CONNECTION_REFUSED);
+
+  Initialize();
+
+  net::QuicChromiumClientSession::Handle* session_handle =
+      GetQuicSessionHandle();
+  ASSERT_TRUE(session_handle);
+
+  TestWebSocketQuicStreamAdapterCompletionCallback creation_callback;
+  auto adapter = session_handle->CreateWebSocketQuicStreamAdapter(
+      &mock_delegate_, creation_callback.callback(),
+      TRAFFIC_ANNOTATION_FOR_TESTS);
+  ASSERT_TRUE(adapter);
+  adapter->WriteHeaders(RequestHeaders(), false);
+
+  // Start a Read() that will finish later. DeleterCallback owns the adapter and
+  // deletes it when the read callback runs.
+  DeleterCallback callback(std::move(adapter));
+  constexpr int kReadBufSize = 1024;
+  auto read_buf = base::MakeRefCounted<IOBufferWithSize>(kReadBufSize);
+  int rv = callback.adapter()->Read(read_buf.get(), kReadBufSize,
+                                    callback.callback());
+  EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+
+  // This WebSocketQuicStreamAdapter::Write() triggers the mocked socket error.
+  // Closing the stream runs the pending read callback, which deletes the
+  // adapter. Write() must return without using the deleted adapter.
+  auto write_buf = base::MakeRefCounted<StringIOBuffer>("test data");
+  rv = callback.adapter()->Write(write_buf.get(), write_buf->size(),
+                                 base::DoNothing(),
+                                 TRAFFIC_ANNOTATION_FOR_TESTS);
+  EXPECT_THAT(rv, IsError(ERR_CONNECTION_CLOSED));
+  EXPECT_THAT(callback.WaitForResult(), IsError(ERR_QUIC_PROTOCOL_ERROR));
+}
+
 // Tests that the adapter correctly handles being destroyed from within its own
 // OnClose() delegate method, when there are no pending read or write
 // callbacks.