Send correct STOP_SENDING/RESET_STREAM frames for different stream types. Upon receiving RESET_STREAM for a write-only stream, the connection will be closed with error. gfe-relnote: protected by disabled v99 flag. PiperOrigin-RevId: 279850700 Change-Id: I9f0e6215a388dac226d430458a340e8934432662
diff --git a/quic/core/http/quic_spdy_server_stream_base_test.cc b/quic/core/http/quic_spdy_server_stream_base_test.cc index 1888e03..1a8eef2 100644 --- a/quic/core/http/quic_spdy_server_stream_base_test.cc +++ b/quic/core/http/quic_spdy_server_stream_base_test.cc
@@ -31,6 +31,7 @@ : session_(new MockQuicConnection(&helper_, &alarm_factory_, Perspective::IS_SERVER)) { + session_.Initialize(); stream_ = new TestQuicSpdyServerStream(GetNthClientInitiatedBidirectionalStreamId( session_.transport_version(), 0),
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 205f7ad..3ec2401 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -9,6 +9,7 @@ #include <utility> #include "net/third_party/quiche/src/quic/core/quic_connection.h" +#include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_flow_controller.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" @@ -263,6 +264,8 @@ return; } + stream->OnStopSending(frame.application_error_code); + stream->set_stream_error( static_cast<QuicRstStreamErrorCode>(frame.application_error_code)); SendRstStreamInner( @@ -302,6 +305,16 @@ return; } + if (VersionHasIetfQuicFrames(transport_version()) && + QuicUtils::GetStreamType(stream_id, perspective(), + IsIncomingStream(stream_id)) == + WRITE_UNIDIRECTIONAL) { + connection()->CloseConnection( + QUIC_INVALID_STREAM_ID, "Received RESET_STREAM for a write-only stream", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } + if (visitor_) { visitor_->OnRstStreamReceived(frame); } @@ -690,21 +703,23 @@ bool close_write_side_only) { if (connection()->connected()) { // Only send if still connected. - if (close_write_side_only) { - DCHECK(VersionHasIetfQuicFrames(transport_version())); - // Send a RST_STREAM frame. - control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written); - } else { + if (VersionHasIetfQuicFrames(transport_version())) { // Send a RST_STREAM frame plus, if version 99, an IETF // QUIC STOP_SENDING frame. Both sre sent to emulate // the two-way close that Google QUIC's RST_STREAM does. - if (VersionHasIetfQuicFrames(transport_version())) { - QuicConnection::ScopedPacketFlusher flusher(connection()); - control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written); - control_frame_manager_.WriteOrBufferStopSending(error, id); - } else { + QuicConnection::ScopedPacketFlusher flusher(connection()); + if (QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) != + READ_UNIDIRECTIONAL) { control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written); } + if (!close_write_side_only && + QuicUtils::GetStreamType(id, perspective(), IsIncomingStream(id)) != + WRITE_UNIDIRECTIONAL) { + control_frame_manager_.WriteOrBufferStopSending(error, id); + } + } else { + DCHECK(!close_write_side_only); + control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written); } connection_->OnStreamReset(id, error); }
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index d5d8321..4ee1230 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -389,24 +389,33 @@ } void CloseStream(QuicStreamId id) { - if (VersionHasIetfQuicFrames(session_.transport_version()) && - QuicUtils::GetStreamType(id, session_.perspective(), - session_.IsIncomingStream(id)) == - READ_UNIDIRECTIONAL) { - // Verify reset is not sent for READ_UNIDIRECTIONAL streams. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); - EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(0); - } else { - // Verify reset IS sent for BIDIRECTIONAL streams. - if (VersionHasIetfQuicFrames(session_.transport_version())) { - // Once for the RST_STREAM, Once for the STOP_SENDING + if (VersionHasIetfQuicFrames(transport_version())) { + if (QuicUtils::GetStreamType(id, session_.perspective(), + session_.IsIncomingStream(id)) == + READ_UNIDIRECTIONAL) { + // Verify reset is not sent for READ_UNIDIRECTIONAL streams. + EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(0); + } else if (QuicUtils::GetStreamType(id, session_.perspective(), + session_.IsIncomingStream(id)) == + WRITE_UNIDIRECTIONAL) { + // Verify RESET_STREAM but not STOP_SENDING is sent for write-only + // stream. + EXPECT_CALL(*connection_, SendControlFrame(_)) + .Times(1) + .WillOnce(Invoke(&ClearControlFrame)); + EXPECT_CALL(*connection_, OnStreamReset(id, _)); + } else { + // Verify RESET_STREAM and STOP_SENDING are sent for BIDIRECTIONAL + // streams. EXPECT_CALL(*connection_, SendControlFrame(_)) .Times(2) .WillRepeatedly(Invoke(&ClearControlFrame)); - } else { - EXPECT_CALL(*connection_, SendControlFrame(_)) - .WillOnce(Invoke(&ClearControlFrame)); + EXPECT_CALL(*connection_, OnStreamReset(id, _)); } + } else { + EXPECT_CALL(*connection_, SendControlFrame(_)) + .WillOnce(Invoke(&ClearControlFrame)); EXPECT_CALL(*connection_, OnStreamReset(id, _)); } session_.CloseStream(id); @@ -2753,6 +2762,41 @@ session_.OnStreamFrame(frame1); } +TEST_P(QuicSessionTestServer, ResetForIETFStreamTypes) { + if (!VersionHasIetfQuicFrames(transport_version())) { + return; + } + + QuicStreamId read_only = GetNthClientInitiatedUnidirectionalId(0); + EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + EXPECT_CALL(*connection_, OnStreamReset(read_only, _)); + session_.SendRstStreamInner(read_only, QUIC_STREAM_CANCELLED, 0, + /*close_write_side_only = */ true); + + EXPECT_CALL(*connection_, SendControlFrame(_)) + .Times(1) + .WillOnce(Invoke(&ClearControlFrame)); + EXPECT_CALL(*connection_, OnStreamReset(read_only, _)); + session_.SendRstStreamInner(read_only, QUIC_STREAM_CANCELLED, 0, + /*close_write_side_only = */ false); + + QuicStreamId write_only = GetNthServerInitiatedUnidirectionalId(0); + EXPECT_CALL(*connection_, SendControlFrame(_)) + .Times(1) + .WillOnce(Invoke(&ClearControlFrame)); + EXPECT_CALL(*connection_, OnStreamReset(write_only, _)); + session_.SendRstStreamInner(write_only, QUIC_STREAM_CANCELLED, 0, + /*close_write_side_only = */ false); + + QuicStreamId bidirectional = GetNthClientInitiatedBidirectionalId(0); + EXPECT_CALL(*connection_, SendControlFrame(_)) + .Times(2) + .WillRepeatedly(Invoke(&ClearControlFrame)); + EXPECT_CALL(*connection_, OnStreamReset(bidirectional, _)); + session_.SendRstStreamInner(bidirectional, QUIC_STREAM_CANCELLED, 0, + /*close_write_side_only = */ false); +} + // A client test class that can be used when the automatic configuration is not // desired. class QuicSessionTestClientUnconfigured : public QuicSessionTestBase {
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h index de0f619..c936bc7 100644 --- a/quic/core/quic_stream.h +++ b/quic/core/quic_stream.h
@@ -336,6 +336,9 @@ // this method or not. void SendStopSending(uint16_t code); + // Handle received StopSending frame. + virtual void OnStopSending(uint16_t /*code*/) {} + // Close the write side of the socket. Further writes will fail. // Can be called by the subclass or internally. // Does not send a FIN. May cause the stream to be closed.
diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc index bb1cf19..4a12c09 100644 --- a/quic/tools/quic_simple_server_session_test.cc +++ b/quic/tools/quic_simple_server_session_test.cc
@@ -15,6 +15,7 @@ #include "net/third_party/quiche/src/quic/core/quic_connection.h" #include "net/third_party/quiche/src/quic/core/quic_crypto_server_stream.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_containers.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" @@ -810,6 +811,12 @@ // prevent a promised resource to be send out. TEST_P(QuicSimpleServerSessionServerPushTest, ResetPromisedStreamToCancelServerPush) { + if (VersionHasIetfQuicFrames(transport_version())) { + // This test is resetting a stream that is not opened yet. IETF QUIC has no + // way to handle this. Some similar tests can be added once CANCEL_PUSH is + // supported. + return; + } MaybeConsumeHeadersStreamData(); session_->SetMaxAllowedPushId(kMaxQuicStreamId); @@ -860,8 +867,6 @@ EXPECT_CALL(*connection_, SendStreamData(stream_not_reset, 1, offset, NO_FIN)); offset++; - } - if (VersionUsesHttp3(connection_->transport_version())) { EXPECT_CALL(*connection_, SendStreamData(stream_not_reset, kHeadersFrameHeaderLength, offset, NO_FIN)); @@ -870,8 +875,6 @@ SendStreamData(stream_not_reset, kHeadersFramePayloadLength, offset, NO_FIN)); offset += kHeadersFramePayloadLength; - } - if (VersionUsesHttp3(connection_->transport_version())) { EXPECT_CALL(*connection_, SendStreamData(stream_not_reset, data_frame_header_length, offset, NO_FIN)); @@ -921,9 +924,9 @@ // Resetting an open stream will close the stream and give space for extra // stream to be opened. QuicStreamId stream_got_reset = GetNthServerInitiatedUnidirectionalId(3); - EXPECT_CALL(owner_, OnRstStreamReceived(_)).Times(1); EXPECT_CALL(*connection_, SendControlFrame(_)); if (!VersionHasIetfQuicFrames(transport_version())) { + EXPECT_CALL(owner_, OnRstStreamReceived(_)).Times(1); // For version 99, this is covered in InjectStopSending() EXPECT_CALL(*connection_, OnStreamReset(stream_got_reset, QUIC_RST_ACKNOWLEDGEMENT)); @@ -933,8 +936,6 @@ EXPECT_CALL(*connection_, SendStreamData(stream_to_open, 1, offset, NO_FIN)); offset++; - } - if (VersionUsesHttp3(connection_->transport_version())) { EXPECT_CALL(*connection_, SendStreamData(stream_to_open, kHeadersFrameHeaderLength, offset, NO_FIN)); @@ -943,8 +944,6 @@ SendStreamData(stream_to_open, kHeadersFramePayloadLength, offset, NO_FIN)); offset += kHeadersFramePayloadLength; - } - if (VersionUsesHttp3(connection_->transport_version())) { EXPECT_CALL(*connection_, SendStreamData(stream_to_open, data_frame_header_length, offset, NO_FIN)); @@ -965,8 +964,9 @@ // available as it closes/etc them. session_->OnMaxStreamsFrame( QuicMaxStreamsFrame(0, num_resources + 3, /*unidirectional=*/true)); + } else { + session_->OnRstStream(rst); } - session_->OnRstStream(rst); // Create and inject a STOP_SENDING frame. In GOOGLE QUIC, receiving a // RST_STREAM frame causes a two-way close. For IETF QUIC, RST_STREAM causes // a one-way close.