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.
