Move headers stream from 0 to 60 in v99
This CL also fixes a few tests that were incorrectly using the headers stream.
gfe-relnote: change header stream number, protected by v99 flag
PiperOrigin-RevId: 253691890
Change-Id: I1351cad387871efe39fb4387eac546e9a24efb7c
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 6bd587f..01213d9 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -751,6 +751,19 @@
.length());
}
+TEST_P(EndToEndTestWithTls, SimpleRequestResponseWithIetfDraftSupport) {
+ if (GetParam().negotiated_version.transport_version != QUIC_VERSION_99 ||
+ GetParam().negotiated_version.handshake_protocol != PROTOCOL_TLS1_3) {
+ ASSERT_TRUE(Initialize());
+ return;
+ }
+ QuicVersionInitializeSupportForIetfDraft(1);
+ ASSERT_TRUE(Initialize());
+
+ EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+ EXPECT_EQ("200", client_->response_headers()->find(":status")->second);
+}
+
TEST_P(EndToEndTest, SimpleRequestResponseWithLargeReject) {
chlo_multiplier_ = 1;
ASSERT_TRUE(Initialize());
@@ -1785,6 +1798,7 @@
// TODO(nharper): Needs to get turned back to EndToEndTestWithTls
// when we figure out why the test doesn't work on chrome.
TEST_P(EndToEndTest, MaxStreamsUberTest) {
+ SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 0);
// Connect with lower fake packet loss than we'd like to test. Until
// b/10126687 is fixed, losing handshake packets is pretty brutal.
SetPacketLossPercentage(1);
@@ -2891,6 +2905,7 @@
const size_t kNumMaxStreams = 10;
EndToEndTestServerPush() : EndToEndTest() {
+ SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 0);
client_config_.SetMaxIncomingBidirectionalStreamsToSend(kNumMaxStreams);
server_config_.SetMaxIncomingBidirectionalStreamsToSend(kNumMaxStreams);
client_config_.SetMaxIncomingUnidirectionalStreamsToSend(kNumMaxStreams);
@@ -3389,7 +3404,7 @@
TEST_P(EndToEndTest,
SendStatelessResetIfServerConnectionClosedLocallyAfterHandshake) {
// Prevent the connection from expiring in the time wait list.
- FLAGS_quic_time_wait_list_seconds = 10000;
+ SetQuicFlag(FLAGS_quic_time_wait_list_seconds, 10000);
connect_to_server_on_initialize_ = false;
ASSERT_TRUE(Initialize());
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc
index a41235f..68d32de 100644
--- a/quic/core/http/quic_headers_stream_test.cc
+++ b/quic/core/http/quic_headers_stream_test.cc
@@ -144,14 +144,20 @@
struct TestParams {
TestParams(const ParsedQuicVersion& version, Perspective perspective)
: version(version), perspective(perspective) {
- QUIC_LOG(INFO) << "TestParams: version: "
- << ParsedQuicVersionToString(version)
- << ", perspective: " << perspective;
+ QUIC_LOG(INFO) << "TestParams: " << *this;
}
TestParams(const TestParams& other)
: version(other.version), perspective(other.perspective) {}
+ friend std::ostream& operator<<(std::ostream& os, const TestParams& tp) {
+ os << "{ version: " << ParsedQuicVersionToString(tp.version)
+ << ", perspective: "
+ << (tp.perspective == Perspective::IS_CLIENT ? "client" : "server")
+ << "}";
+ return os;
+ }
+
ParsedQuicVersion version;
Perspective perspective;
};
@@ -390,6 +396,9 @@
}
TEST_P(QuicHeadersStreamTest, WritePushPromises) {
+ if (GetParam().version.DoesNotHaveHeadersStream()) {
+ return;
+ }
for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_;
stream_id += next_stream_id_) {
QuicStreamId promised_stream_id = NextPromisedStreamId();
@@ -461,6 +470,9 @@
}
TEST_P(QuicHeadersStreamTest, ProcessPushPromise) {
+ if (GetParam().version.DoesNotHaveHeadersStream()) {
+ return;
+ }
if (perspective() == Perspective::IS_SERVER) {
return;
}
@@ -470,6 +482,7 @@
SpdyPushPromiseIR push_promise(stream_id, promised_stream_id,
headers_.Clone());
SpdySerializedFrame frame(framer_->SerializeFrame(push_promise));
+ bool connection_closed = false;
if (perspective() == Perspective::IS_SERVER) {
EXPECT_CALL(*connection_,
CloseConnection(QUIC_INVALID_HEADERS_STREAM_DATA,
@@ -477,6 +490,8 @@
.WillRepeatedly(InvokeWithoutArgs(
this, &QuicHeadersStreamTest::TearDownLocalConnectionState));
} else {
+ ON_CALL(*connection_, CloseConnection(_, _, _))
+ .WillByDefault(testing::Assign(&connection_closed, true));
EXPECT_CALL(session_, OnPromiseHeaderList(stream_id, promised_stream_id,
frame.size(), _))
.WillOnce(
@@ -487,12 +502,18 @@
headers_stream_->OnStreamFrame(stream_frame_);
if (perspective() == Perspective::IS_CLIENT) {
stream_frame_.offset += frame.size();
+ // CheckHeaders crashes if the connection is closed so this ensures we
+ // fail the test instead of crashing.
+ ASSERT_FALSE(connection_closed);
CheckHeaders();
}
}
}
TEST_P(QuicHeadersStreamTest, ProcessPriorityFrame) {
+ if (GetParam().version.DoesNotHaveHeadersStream()) {
+ return;
+ }
QuicStreamId parent_stream_id = 0;
for (SpdyPriority priority = 0; priority < 7; ++priority) {
for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_;
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index 9aff364..bfde618 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -241,7 +241,10 @@
EXPECT_CALL(*connection_, SendControlFrame(_)).Times(AnyNumber());
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(AnyNumber());
- const uint32_t kServerMaxIncomingStreams = 1;
+ uint32_t kServerMaxIncomingStreams = 1;
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ kServerMaxIncomingStreams = 0;
+ }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -257,15 +260,22 @@
EXPECT_FALSE(stream);
if (GetParam().transport_version == QUIC_VERSION_99) {
- // Ensure that we have/have had 3 open streams, crypto, header, and the
- // 1 test stream. Primary purpose of this is to fail when crypto
- // no longer uses a normal stream. Some above constants will then need
- // to be changed.
- EXPECT_EQ(QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
- ->outgoing_static_stream_count() +
- 1,
- QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
- ->outgoing_stream_count());
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ EXPECT_EQ(1u,
+ QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
+ ->outgoing_stream_count());
+
+ } else {
+ // Ensure that we have/have had 3 open streams, crypto, header, and the
+ // 1 test stream. Primary purpose of this is to fail when crypto
+ // no longer uses a normal stream. Some above constants will then need
+ // to be changed.
+ EXPECT_EQ(QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
+ ->outgoing_static_stream_count() +
+ 1,
+ QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
+ ->outgoing_stream_count());
+ }
}
}
@@ -279,7 +289,10 @@
EXPECT_CALL(*connection_, SendControlFrame(_)).Times(AnyNumber());
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(AnyNumber());
- const uint32_t kServerMaxIncomingStreams = 1;
+ uint32_t kServerMaxIncomingStreams = 1;
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ kServerMaxIncomingStreams = 0;
+ }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -312,7 +325,11 @@
if (GetParam().transport_version == QUIC_VERSION_99) {
// Ensure that we have/have had three open streams: two test streams and the
// header stream.
- EXPECT_EQ(3u,
+ QuicStreamCount expected_stream_count = 3;
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ expected_stream_count = 2;
+ }
+ EXPECT_EQ(expected_stream_count,
QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
->outgoing_stream_count());
}
@@ -329,7 +346,10 @@
// the server sends trailing headers (trailers). Receipt of the trailers by
// the client should result in all outstanding stream state being tidied up
// (including flow control, and number of available outgoing streams).
- const uint32_t kServerMaxIncomingStreams = 1;
+ uint32_t kServerMaxIncomingStreams = 1;
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ kServerMaxIncomingStreams = 0;
+ }
CompleteCryptoHandshake(kServerMaxIncomingStreams);
QuicSpdyClientStream* stream = session_->CreateOutgoingBidirectionalStream();
@@ -392,7 +412,11 @@
if (GetParam().transport_version == QUIC_VERSION_99) {
// Ensure that we have/have had three open streams: two test streams and the
// header stream.
- EXPECT_EQ(3u,
+ QuicStreamCount expected_stream_count = 3;
+ if (VersionLacksHeadersStream(GetParam().transport_version)) {
+ expected_stream_count = 2;
+ }
+ EXPECT_EQ(expected_stream_count,
QuicSessionPeer::v99_bidirectional_stream_id_manager(&*session_)
->outgoing_stream_count());
}
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index b9006ca..5004557 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -351,13 +351,15 @@
void QuicSpdySession::Initialize() {
QuicSession::Initialize();
- if (perspective() == Perspective::IS_SERVER) {
- set_largest_peer_created_stream_id(
- QuicUtils::GetHeadersStreamId(connection()->transport_version()));
- } else {
- QuicStreamId headers_stream_id = GetNextOutgoingBidirectionalStreamId();
- DCHECK_EQ(headers_stream_id,
- QuicUtils::GetHeadersStreamId(connection()->transport_version()));
+ if (!connection()->version().DoesNotHaveHeadersStream()) {
+ if (perspective() == Perspective::IS_SERVER) {
+ set_largest_peer_created_stream_id(
+ QuicUtils::GetHeadersStreamId(connection()->transport_version()));
+ } else {
+ QuicStreamId headers_stream_id = GetNextOutgoingBidirectionalStreamId();
+ DCHECK_EQ(headers_stream_id, QuicUtils::GetHeadersStreamId(
+ connection()->transport_version()));
+ }
}
if (VersionUsesQpack(connection()->transport_version())) {
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index fc1ae0a..ce4d5a9 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -483,9 +483,12 @@
// stream ID, the next ID should fail. Since the actual limit
// is not the number of open streams, we allocate the max and the max+2.
// Get the max allowed stream ID, this should succeed.
+ QuicStreamCount headers_stream_offset =
+ VersionLacksHeadersStream(QUIC_VERSION_99) ? 1 : 0;
QuicStreamId stream_id = StreamCountToId(
QuicSessionPeer::v99_streamid_manager(&session_)
- ->actual_max_allowed_incoming_bidirectional_streams(),
+ ->actual_max_allowed_incoming_bidirectional_streams() -
+ headers_stream_offset,
Perspective::IS_CLIENT, // Client initates stream, allocs stream id.
/*bidirectional=*/true);
EXPECT_NE(nullptr, session_.GetOrCreateDynamicStream(stream_id));
@@ -500,7 +503,7 @@
stream_id = StreamCountToId(
QuicSessionPeer::v99_streamid_manager(&session_)
->actual_max_allowed_incoming_bidirectional_streams() +
- 1,
+ 1 - headers_stream_offset,
Perspective::IS_CLIENT,
/*bidirectional=*/true);
EXPECT_EQ(nullptr, session_.GetOrCreateDynamicStream(stream_id));
@@ -1588,10 +1591,12 @@
.Times(1);
} else {
// On version 99 opening such a stream results in a connection close.
- EXPECT_CALL(
- *connection_,
- CloseConnection(QUIC_INVALID_STREAM_ID,
- "Stream id 24 would exceed stream count limit 6", _));
+ EXPECT_CALL(*connection_,
+ CloseConnection(
+ QUIC_INVALID_STREAM_ID,
+ testing::MatchesRegex(
+ "Stream id [0-9]+ would exceed stream count limit 6"),
+ _));
}
// Create one more data streams to exceed limit of open stream.
QuicStreamFrame data1(kFinalStreamId, false, 0, QuicStringPiece("HT"));
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 9ca1df2..66100bb 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -260,9 +260,12 @@
VersionUsesQpack(GetParam().transport_version);
if (version_uses_qpack) {
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_HEADERS_STREAM_DATA_DECOMPRESS_FAILURE,
- "Too large headers received on stream 4", _));
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HEADERS_STREAM_DATA_DECOMPRESS_FAILURE,
+ testing::MatchesRegex(
+ "Too large headers received on stream [0-9]+"),
+ _));
} else {
EXPECT_CALL(*session_,
SendRstStream(stream_->id(), QUIC_HEADERS_TOO_LARGE, 0));
@@ -1817,11 +1820,13 @@
std::string stream_frame_payload = QuicStrCat(headers, data);
QuicStreamFrame frame(stream_->id(), false, 0, stream_frame_payload);
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_DECOMPRESSION_FAILURE,
- "Error decompressing header block on stream 4: "
- "Incomplete header block.",
- _))
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_DECOMPRESSION_FAILURE,
+ testing::MatchesRegex(
+ "Error decompressing header block on stream [0-9]+: "
+ "Incomplete header block."),
+ _))
.WillOnce(
(Invoke([this](QuicErrorCode error, const std::string& error_details,
ConnectionCloseBehavior connection_close_behavior) {
