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_