Skip to content

Commit

Permalink
Abort WebSocket upgrade if stream is closed
Browse files Browse the repository at this point in the history
Previously, WebSocketHttp2HandshakeStream::Upgrade could crash if the
HTTP2 stream had been closed after receiving headers.

Check the handshake stream is still open and cleanly error the handshake
if it is not.

BUG=1215989

(cherry picked from commit 4714554)

Change-Id: Ia8707e7861d658c806a3d7fc66c6709680009882
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4589429
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1154845}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4611439
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#725}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
ricea authored and Chromium LUCI CQ committed Jun 14, 2023
1 parent f9b78cf commit 2c58623
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 0 deletions.
2 changes: 2 additions & 0 deletions net/http/http_stream_factory_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ class MockWebSocketHandshakeStream : public WebSocketHandshakeStreamBase {

std::unique_ptr<WebSocketStream> Upgrade() override { return nullptr; }

bool CanReadFromStream() const override { return true; }

base::WeakPtr<WebSocketHandshakeStreamBase> GetWeakPtr() override {
return weak_ptr_factory_.GetWeakPtr();
}
Expand Down
8 changes: 8 additions & 0 deletions net/websockets/websocket_basic_handshake_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,14 @@ std::unique_ptr<WebSocketStream> WebSocketBasicHandshakeStream::Upgrade() {
return basic_stream;
}

bool WebSocketBasicHandshakeStream::CanReadFromStream() const {
auto* connection = state_.connection();
if (!connection) {
return false;
}
return connection->socket();
}

base::WeakPtr<WebSocketHandshakeStreamBase>
WebSocketBasicHandshakeStream::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
Expand Down
2 changes: 2 additions & 0 deletions net/websockets/websocket_basic_handshake_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class NET_EXPORT_PRIVATE WebSocketBasicHandshakeStream final
// Upgrade() has been called and should be disposed of as soon as possible.
std::unique_ptr<WebSocketStream> Upgrade() override;

bool CanReadFromStream() const override;

base::WeakPtr<WebSocketHandshakeStreamBase> GetWeakPtr() override;

// Set the value used for the next Sec-WebSocket-Key header
Expand Down
4 changes: 4 additions & 0 deletions net/websockets/websocket_handshake_stream_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ class NET_EXPORT WebSocketHandshakeStreamBase : public HttpStream {
// been called.
virtual std::unique_ptr<WebSocketStream> Upgrade() = 0;

// Returns true if a read from the stream will succeed. This should be true
// even if the stream is at EOF.
virtual bool CanReadFromStream() const = 0;

void SetRequestHeadersCallback(RequestHeadersCallback callback) override {}

static std::string MultipleHeaderValuesMessage(
Expand Down
4 changes: 4 additions & 0 deletions net/websockets/websocket_http2_handshake_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ std::unique_ptr<WebSocketStream> WebSocketHttp2HandshakeStream::Upgrade() {
std::make_unique<WebSocketDeflatePredictorImpl>());
}

bool WebSocketHttp2HandshakeStream::CanReadFromStream() const {
return stream_adapter_ && stream_adapter_->is_initialized();
}

base::WeakPtr<WebSocketHandshakeStreamBase>
WebSocketHttp2HandshakeStream::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
Expand Down
2 changes: 2 additions & 0 deletions net/websockets/websocket_http2_handshake_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class NET_EXPORT_PRIVATE WebSocketHttp2HandshakeStream
// Upgrade() has been called and should be disposed of as soon as possible.
std::unique_ptr<WebSocketStream> Upgrade() override;

bool CanReadFromStream() const override;

base::WeakPtr<WebSocketHandshakeStreamBase> GetWeakPtr() override;

// WebSocketSpdyStreamAdapter::Delegate methods.
Expand Down
4 changes: 4 additions & 0 deletions net/websockets/websocket_http3_handshake_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ std::unique_ptr<WebSocketStream> WebSocketHttp3HandshakeStream::Upgrade() {
std::make_unique<WebSocketDeflatePredictorImpl>());
}

bool WebSocketHttp3HandshakeStream::CanReadFromStream() const {
return stream_adapter_ && stream_adapter_->is_initialized();
}

base::WeakPtr<WebSocketHandshakeStreamBase>
WebSocketHttp3HandshakeStream::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
Expand Down
2 changes: 2 additions & 0 deletions net/websockets/websocket_http3_handshake_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ class NET_EXPORT_PRIVATE WebSocketHttp3HandshakeStream final
// Upgrade() has been called and should be disposed of as soon as possible.
std::unique_ptr<WebSocketStream> Upgrade() override;

bool CanReadFromStream() const override;

base::WeakPtr<WebSocketHandshakeStreamBase> GetWeakPtr() override;

// WebSocketQuicStreamAdapter::Delegate methods.
Expand Down
6 changes: 6 additions & 0 deletions net/websockets/websocket_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequestAPI {
return;
}

if (!handshake_stream_->CanReadFromStream()) {
ReportFailureWithMessage("Handshake stream is not readable.",
ERR_CONNECTION_CLOSED, absl::nullopt);
return;
}

std::unique_ptr<URLRequest> url_request = std::move(url_request_);
WebSocketHandshakeStreamBase* handshake_stream = handshake_stream_.get();
handshake_stream_.reset();
Expand Down

0 comments on commit 2c58623

Please sign in to comment.