Prevent QUIC streams from closing the connection directly.
Instead the error is passed through a delegate to the QUIC session to handle.
gfe-relnote: no behavior change. not protected.
PiperOrigin-RevId: 294729094
Change-Id: I4f0fc7b4ddbcd34ddf549ae4d053cbcc78535987
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index c66e696..ab7a55b 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -145,10 +145,9 @@
private:
void CloseConnectionOnWrongFrame(quiche::QuicheStringPiece frame_type) {
- stream_->session()->connection()->CloseConnection(
+ stream_->CloseConnectionWithDetails(
QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM,
- quiche::QuicheStrCat(frame_type, " frame received on control stream"),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ quiche::QuicheStrCat(frame_type, " frame received on control stream"));
}
QuicReceiveControlStream* stream_;
@@ -171,9 +170,8 @@
const QuicRstStreamFrame& /*frame*/) {
// TODO(renjietang) Change the error code to H/3 specific
// HTTP_CLOSED_CRITICAL_STREAM.
- session()->connection()->CloseConnection(
- QUIC_INVALID_STREAM_ID, "Attempt to reset receive control stream",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
+ "Attempt to reset receive control stream");
}
void QuicReceiveControlStream::OnDataAvailable() {
@@ -200,9 +198,8 @@
QuicByteCount /* header_length */) {
if (settings_frame_received_) {
// TODO(renjietang): Change error code to HTTP_UNEXPECTED_FRAME.
- session()->connection()->CloseConnection(
- QUIC_INVALID_STREAM_ID, "Settings frames are received twice.",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
+ "Settings frames are received twice.");
return false;
}
@@ -226,10 +223,9 @@
bool QuicReceiveControlStream::OnPriorityUpdateFrameStart(
QuicByteCount /* header_length */) {
if (!settings_frame_received_) {
- session()->connection()->CloseConnection(
+ stream_delegate()->OnStreamError(
QUIC_INVALID_STREAM_ID,
- "PRIORITY_UPDATE frame received before SETTINGS.",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ "PRIORITY_UPDATE frame received before SETTINGS.");
return false;
}
return true;
@@ -255,10 +251,9 @@
int urgency;
if (!quiche::QuicheTextUtils::StringToInt(value, &urgency) || urgency < 0 ||
urgency > 7) {
- session()->connection()->CloseConnection(
+ stream_delegate()->OnStreamError(
QUIC_INVALID_STREAM_ID,
- "Invalid value for PRIORITY_UPDATE urgency parameter.",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ "Invalid value for PRIORITY_UPDATE urgency parameter.");
return false;
}
diff --git a/quic/core/http/quic_send_control_stream.cc b/quic/core/http/quic_send_control_stream.cc
index 79ea11b..0d8616c 100644
--- a/quic/core/http/quic_send_control_stream.cc
+++ b/quic/core/http/quic_send_control_stream.cc
@@ -34,9 +34,8 @@
void QuicSendControlStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
// TODO(renjietang) Change the error code to H/3 specific
// HTTP_CLOSED_CRITICAL_STREAM.
- session()->connection()->CloseConnection(
- QUIC_INVALID_STREAM_ID, "Attempt to reset send control stream",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
+ "Attempt to reset send control stream");
}
void QuicSendControlStream::MaybeSendSettingsFrame() {
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 89748a9..67234a9 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -41,9 +41,8 @@
HttpDecoderVisitor& operator=(const HttpDecoderVisitor&) = delete;
void OnError(HttpDecoder* decoder) override {
- stream_->session()->connection()->CloseConnection(
- decoder->error(), decoder->error_detail(),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_->CloseConnectionWithDetails(decoder->error(),
+ decoder->error_detail());
}
bool OnCancelPushFrame(const CancelPushFrame& /*frame*/) override {
@@ -170,10 +169,9 @@
private:
void CloseConnectionOnWrongFrame(quiche::QuicheStringPiece frame_type) {
- stream_->session()->connection()->CloseConnection(
+ stream_->CloseConnectionWithDetails(
QUIC_HTTP_FRAME_UNEXPECTED_ON_SPDY_STREAM,
- quiche::QuicheStrCat(frame_type, " frame received on data stream"),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ quiche::QuicheStrCat(frame_type, " frame received on data stream"));
}
QuicSpdyStream* stream_;
@@ -649,9 +647,8 @@
const QuicHeaderList& /*header_list */) {
// To be overridden in QuicSpdyClientStream. Not supported on
// server side.
- session()->connection()->CloseConnection(
- QUIC_INVALID_HEADERS_STREAM_DATA, "Promise headers received by server",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
+ "Promise headers received by server");
}
void QuicSpdyStream::OnTrailingHeadersComplete(
@@ -663,18 +660,16 @@
if (!VersionUsesHttp3(transport_version()) && fin_received()) {
QUIC_DLOG(INFO) << ENDPOINT
<< "Received Trailers after FIN, on stream: " << id();
- session()->connection()->CloseConnection(
- QUIC_INVALID_HEADERS_STREAM_DATA, "Trailers after fin",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
+ "Trailers after fin");
return;
}
if (!VersionUsesHttp3(transport_version()) && !fin) {
QUIC_DLOG(INFO) << ENDPOINT
<< "Trailers must have FIN set, on stream: " << id();
- session()->connection()->CloseConnection(
- QUIC_INVALID_HEADERS_STREAM_DATA, "Fin missing from trailers",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
+ "Fin missing from trailers");
return;
}
@@ -685,9 +680,8 @@
&received_trailers_)) {
QUIC_DLOG(ERROR) << ENDPOINT << "Trailers for stream " << id()
<< " are malformed.";
- session()->connection()->CloseConnection(
- QUIC_INVALID_HEADERS_STREAM_DATA, "Trailers are malformed",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
+ "Trailers are malformed");
return;
}
trailers_decompressed_ = true;
@@ -858,9 +852,8 @@
DCHECK(VersionUsesHttp3(transport_version()));
if (!headers_decompressed_ || trailers_decompressed_) {
// TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
- session()->connection()->CloseConnection(
- QUIC_INVALID_HEADERS_STREAM_DATA, "Unexpected DATA frame received.",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
+ "Unexpected DATA frame received.");
return false;
}
@@ -938,10 +931,9 @@
if (trailers_decompressed_) {
// TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
- session()->connection()->CloseConnection(
+ stream_delegate()->OnStreamError(
QUIC_INVALID_HEADERS_STREAM_DATA,
- "HEADERS frame received after trailing HEADERS.",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ "HEADERS frame received after trailing HEADERS.");
return false;
}
diff --git a/quic/core/qpack/qpack_receive_stream.cc b/quic/core/qpack/qpack_receive_stream.cc
index a5df828..f896ca4 100644
--- a/quic/core/qpack/qpack_receive_stream.cc
+++ b/quic/core/qpack/qpack_receive_stream.cc
@@ -16,9 +16,8 @@
void QpackReceiveStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
// TODO(renjietang) Change the error code to H/3 specific
// HTTP_CLOSED_CRITICAL_STREAM.
- session()->connection()->CloseConnection(
- QUIC_INVALID_STREAM_ID, "Attempt to reset Qpack receive stream",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
+ "Attempt to reset Qpack receive stream");
}
void QpackReceiveStream::OnDataAvailable() {
diff --git a/quic/core/qpack/qpack_send_stream.cc b/quic/core/qpack/qpack_send_stream.cc
index a42d0d0..57493e3 100644
--- a/quic/core/qpack/qpack_send_stream.cc
+++ b/quic/core/qpack/qpack_send_stream.cc
@@ -19,9 +19,8 @@
void QpackSendStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
// TODO(renjietang) Change the error code to H/3 specific
// HTTP_CLOSED_CRITICAL_STREAM.
- session()->connection()->CloseConnection(
- QUIC_INVALID_STREAM_ID, "Attempt to reset qpack send stream",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
+ "Attempt to reset qpack send stream");
}
void QpackSendStream::WriteStreamData(quiche::QuicheStringPiece data) {
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index ebd0380..a85ab2d 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -839,6 +839,13 @@
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
}
+void QuicSession::OnStreamError(QuicErrorCode error_code,
+ std::string error_details) {
+ connection_->CloseConnection(
+ error_code, error_details,
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+}
+
void QuicSession::SendMaxStreams(QuicStreamCount stream_count,
bool unidirectional) {
control_frame_manager_.WriteOrBufferMaxStreams(stream_count, unidirectional);
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 138f6c1..390dc73 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -28,6 +28,7 @@
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/quic_write_blocked_list.h"
#include "net/third_party/quiche/src/quic/core/session_notifier_interface.h"
+#include "net/third_party/quiche/src/quic/core/stream_delegate_interface.h"
#include "net/third_party/quiche/src/quic/core/uber_quic_stream_id_manager.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
@@ -50,7 +51,8 @@
public SessionNotifierInterface,
public QuicStreamFrameDataProducer,
public QuicStreamIdManager::DelegateInterface,
- public HandshakerDelegateInterface {
+ public HandshakerDelegateInterface,
+ public StreamDelegateInterface {
public:
// An interface from the session to the entity owning the session.
// This lets the session notify its owner (the Dispatcher) when the connection
@@ -256,6 +258,10 @@
void NeuterUnencryptedData() override;
void NeuterHandshakeData() override;
+ // Implement StreamDelegateInterface.
+ void OnStreamError(QuicErrorCode error_code,
+ std::string error_details) override;
+
// Called by the QuicCryptoStream when a handshake message is sent.
virtual void OnCryptoHandshakeMessageSent(
const CryptoHandshakeMessage& message);
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc
index f2c7c88..9d8407a 100644
--- a/quic/core/quic_stream.cc
+++ b/quic/core/quic_stream.cc
@@ -112,6 +112,7 @@
PendingStream::PendingStream(QuicStreamId id, QuicSession* session)
: id_(id),
session_(session),
+ stream_delegate_(session),
stream_bytes_read_(0),
fin_received_(false),
connection_flow_controller_(session->flow_controller()),
@@ -151,8 +152,7 @@
void PendingStream::CloseConnectionWithDetails(QuicErrorCode error,
const std::string& details) {
- session_->connection()->CloseConnection(
- error, details, ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate_->OnStreamError(error, details);
}
QuicStreamId PendingStream::id() const {
@@ -337,6 +337,7 @@
: sequencer_(std::move(sequencer)),
id_(id),
session_(session),
+ stream_delegate_(session),
precedence_(CalculateDefaultPriority(session)),
stream_bytes_read_(stream_bytes_read),
stream_error_(QUIC_STREAM_NO_ERROR),
@@ -550,8 +551,7 @@
void QuicStream::CloseConnectionWithDetails(QuicErrorCode error,
const std::string& details) {
- session()->connection()->CloseConnection(
- error, details, ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ stream_delegate_->OnStreamError(error, details);
}
const spdy::SpdyStreamPrecedence& QuicStream::precedence() const {
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h
index 0688641..1d05292 100644
--- a/quic/core/quic_stream.h
+++ b/quic/core/quic_stream.h
@@ -28,6 +28,7 @@
#include "net/third_party/quiche/src/quic/core/quic_stream_sequencer.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/session_notifier_interface.h"
+#include "net/third_party/quiche/src/quic/core/stream_delegate_interface.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice_span.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_optional.h"
@@ -91,7 +92,9 @@
QuicStreamId id_;
// Session which owns this.
+ // TODO(b/136274541): Remove session pointer from streams.
QuicSession* session_;
+ StreamDelegateInterface* stream_delegate_;
// Bytes read refers to payload bytes only: they do not include framing,
// encryption overhead etc.
@@ -400,6 +403,8 @@
// this virtual so that subclasses can implement their own logics.
virtual void OnDeadlinePassed();
+ StreamDelegateInterface* stream_delegate() { return stream_delegate_; }
+
bool fin_buffered() const { return fin_buffered_; }
const QuicSession* session() const { return session_; }
@@ -451,7 +456,9 @@
QuicStreamSequencer sequencer_;
QuicStreamId id_;
// Pointer to the owning QuicSession object.
+ // TODO(b/136274541): Remove session pointer from streams.
QuicSession* session_;
+ StreamDelegateInterface* stream_delegate_;
// The precedence of the stream, once parsed.
spdy::SpdyStreamPrecedence precedence_;
// Bytes read refers to payload bytes only: they do not include framing,
diff --git a/quic/core/quic_stream_sequencer.h b/quic/core/quic_stream_sequencer.h
index 68845b8..0733664 100644
--- a/quic/core/quic_stream_sequencer.h
+++ b/quic/core/quic_stream_sequencer.h
@@ -35,7 +35,7 @@
virtual void OnFinRead() = 0;
// Called when bytes have been consumed from the sequencer.
virtual void AddBytesConsumed(QuicByteCount bytes) = 0;
- // TODO(rch): Clean up this interface via OnUnrecoverableError and
+ // TODO(b/136274541): Clean up this interface via OnUnrecoverableError and
// remove PeerAddressOfLatestPacket().
// Called when an error has occurred which should result in the stream
// being reset.
diff --git a/quic/core/stream_delegate_interface.h b/quic/core/stream_delegate_interface.h
new file mode 100644
index 0000000..f6d052a
--- /dev/null
+++ b/quic/core/stream_delegate_interface.h
@@ -0,0 +1,23 @@
+// Copyright (c) 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef QUICHE_QUIC_CORE_STREAM_DELEGATE_INTERFACE_H_
+#define QUICHE_QUIC_CORE_STREAM_DELEGATE_INTERFACE_H_
+
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
+
+namespace quic {
+
+class QUIC_EXPORT_PRIVATE StreamDelegateInterface {
+ public:
+ virtual ~StreamDelegateInterface() {}
+
+ // Called when the stream has encountered errors that it can't handle.
+ virtual void OnStreamError(QuicErrorCode error_code,
+ std::string error_details) = 0;
+};
+
+} // namespace quic
+
+#endif // QUICHE_QUIC_CORE_STREAM_DELEGATE_INTERFACE_H_
