Skip to content

Commit

Permalink
Intermediate commit adding support for IETF QUIC MAX_STREAM_ID
Browse files Browse the repository at this point in the history
This commit is an intermediate commit on the path to adding support
for IETF QUIC's STREAM_ID_BLOCKED frame and MAX_STREAM_ID frame.
This is an intermediate/safety commit while working on a test issue.

R=rch@chromium.org

Change-Id: Ie2c24114cb62b640beb31252ee4c216f08b59533
Reviewed-on: https://chromium-review.googlesource.com/c/1344197
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Frank Kastenholz <fkastenholz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611810}
  • Loading branch information
Frank Kastenholz authored and Commit Bot committed Nov 28, 2018
1 parent 644e4b1 commit 878763b
Show file tree
Hide file tree
Showing 38 changed files with 2,055 additions and 120 deletions.
3 changes: 3 additions & 0 deletions components/domain_reliability/quic_error_mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@ const struct QuicErrorMapping {
"quic.try.to.write.data.on.read.unidirectional.stream"},
{quic::QUIC_INVALID_RETIRE_CONNECTION_ID_DATA,
"quic.invalid.retire.connection.id.data"},
{quic::QUIC_STREAM_ID_BLOCKED_ERROR,
"quic.stream.id.in.stream_id_blocked.frame"},
{quic::QUIC_MAX_STREAM_ID_ERROR, "quic.stream.id.in.max_stream_id.frame"},

// No error. Used as bound while iterating.
{quic::QUIC_LAST_ERROR, "quic.last_error"}};
Expand Down
5 changes: 5 additions & 0 deletions net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,8 @@ component("net") {
"third_party/quic/core/quic_stream.cc",
"third_party/quic/core/quic_stream.h",
"third_party/quic/core/quic_stream_frame_data_producer.h",
"third_party/quic/core/quic_stream_id_manager.cc",
"third_party/quic/core/quic_stream_id_manager.h",
"third_party/quic/core/quic_stream_send_buffer.cc",
"third_party/quic/core/quic_stream_send_buffer.h",
"third_party/quic/core/quic_stream_sequencer.cc",
Expand Down Expand Up @@ -3188,6 +3190,8 @@ source_set("quic_test_tools") {
"third_party/quic/test_tools/quic_session_peer.h",
"third_party/quic/test_tools/quic_spdy_session_peer.cc",
"third_party/quic/test_tools/quic_spdy_session_peer.h",
"third_party/quic/test_tools/quic_stream_id_manager_peer.cc",
"third_party/quic/test_tools/quic_stream_id_manager_peer.h",
"third_party/quic/test_tools/quic_stream_peer.cc",
"third_party/quic/test_tools/quic_stream_peer.h",
"third_party/quic/test_tools/quic_stream_send_buffer_peer.cc",
Expand Down Expand Up @@ -5186,6 +5190,7 @@ test("net_unittests") {
"third_party/quic/core/quic_session_test.cc",
"third_party/quic/core/quic_simple_buffer_allocator_test.cc",
"third_party/quic/core/quic_socket_address_coder_test.cc",
"third_party/quic/core/quic_stream_id_manager_test.cc",
"third_party/quic/core/quic_stream_send_buffer_test.cc",
"third_party/quic/core/quic_stream_sequencer_buffer_test.cc",
"third_party/quic/core/quic_stream_sequencer_test.cc",
Expand Down
6 changes: 3 additions & 3 deletions net/quic/quic_chromium_client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1336,9 +1336,9 @@ void QuicChromiumClientSession::SendRstStream(
}

void QuicChromiumClientSession::OnCanCreateNewOutgoingStream() {
if (GetNumOpenOutgoingStreams() < max_open_outgoing_streams() &&
!stream_requests_.empty() && crypto_stream_->encryption_established() &&
!goaway_received() && !going_away_ && connection()->connected()) {
if (CanOpenNextOutgoingStream() && !stream_requests_.empty() &&
crypto_stream_->encryption_established() && !goaway_received() &&
!going_away_ && connection()->connected()) {
StreamRequest* request = stream_requests_.front();
// TODO(ckrasic) - analyze data and then add logic to mark QUIC
// broken if wait times are excessive.
Expand Down
79 changes: 58 additions & 21 deletions net/quic/quic_chromium_client_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,26 @@ TEST_P(QuicChromiumClientSessionTest, CancelStreamRequestBeforeRelease) {

TEST_P(QuicChromiumClientSessionTest, AsyncStreamRequest) {
MockQuicData quic_data;

quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeInitialSettingsPacket(1, nullptr));
quic_data.AddWrite(SYNCHRONOUS, client_maker_.MakeRstPacket(
2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
if (version_ == quic::QUIC_VERSION_99) {
// The open stream limit is set to 50 by
// MockCryptoClientStream::SetConfigNegotiated() so when the 51st stream is
// requested, a STREAM_ID_BLOCKED will be sent.
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeStreamIdBlockedPacket(3, true, 102));
// After the STREAM_ID_BLOCKED is sent, receive a MAX_STREAM_ID to increase
// the limit.
quic_data.AddRead(ASYNC,
server_maker_.MakeMaxStreamIdPacket(1, true, 102 + 2));
}
quic_data.AddRead(ASYNC, ERR_IO_PENDING);
quic_data.AddRead(ASYNC, OK); // EOF
quic_data.AddSocketDataToFactory(&socket_factory_);

Initialize();
CompleteCryptoHandshake();

Expand All @@ -529,6 +540,10 @@ TEST_P(QuicChromiumClientSessionTest, AsyncStreamRequest) {
GetNthClientInitiatedStreamId(0),
quic::QUIC_STREAM_CANCELLED, 0);
session_->OnRstStream(rst);

// Pump the message loop to read the max stream id packet.
base::RunLoop().RunUntilIdle();

ASSERT_TRUE(callback.have_result());
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_TRUE(handle->ReleaseStream() != nullptr);
Expand Down Expand Up @@ -595,6 +610,13 @@ TEST_P(QuicChromiumClientSessionTest, CancelPendingStreamRequest) {
quic_data.AddWrite(SYNCHRONOUS, client_maker_.MakeRstPacket(
2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
if (version_ == quic::QUIC_VERSION_99) {
// The open stream limit is set to 50 by
// MockCryptoClientStream::SetConfigNegotiated() so when the 51st stream is
// requested, a STREAM_ID_BLOCKED will be sent.
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeStreamIdBlockedPacket(3, true, 102));
}
quic_data.AddRead(ASYNC, ERR_IO_PENDING);
quic_data.AddRead(ASYNC, OK); // EOF
quic_data.AddSocketDataToFactory(&socket_factory_);
Expand Down Expand Up @@ -737,16 +759,23 @@ TEST_P(QuicChromiumClientSessionTest, ConnectionCloseWithPendingStreamRequest) {
}

TEST_P(QuicChromiumClientSessionTest, MaxNumStreams) {
MockRead reads[] = {MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0)};
std::unique_ptr<quic::QuicEncryptedPacket> settings_packet(
client_maker_.MakeInitialSettingsPacket(1, nullptr));
std::unique_ptr<quic::QuicEncryptedPacket> client_rst(
client_maker_.MakeRstPacket(2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
MockWrite writes[] = {
MockWrite(ASYNC, settings_packet->data(), settings_packet->length(), 1),
MockWrite(ASYNC, client_rst->data(), client_rst->length(), 2)};
socket_data_.reset(new SequencedSocketData(reads, writes));
MockQuicData quic_data;
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeInitialSettingsPacket(1, nullptr));
quic_data.AddWrite(SYNCHRONOUS, client_maker_.MakeRstPacket(
2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
if (version_ == quic::QUIC_VERSION_99) {
// stream id blocked, stream id blocked on is #102, which is
// the 51st stream id...
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeStreamIdBlockedPacket(3, true, 102));
quic_data.AddRead(ASYNC,
server_maker_.MakeMaxStreamIdPacket(1, true, 102 + 2));
}
quic_data.AddRead(ASYNC, ERR_IO_PENDING);
quic_data.AddRead(ASYNC, OK); // EOF
quic_data.AddSocketDataToFactory(&socket_factory_);

Initialize();
CompleteCryptoHandshake();
Expand Down Expand Up @@ -774,6 +803,7 @@ TEST_P(QuicChromiumClientSessionTest, MaxNumStreams) {
quic::QUIC_STREAM_NO_ERROR, 0);
session_->OnRstStream(rst1);
EXPECT_EQ(kMaxOpenStreams - 1, session_->GetNumOpenOutgoingStreams());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(
QuicChromiumClientSessionPeer::CreateOutgoingStream(session_.get()));
}
Expand Down Expand Up @@ -1045,16 +1075,21 @@ TEST_P(QuicChromiumClientSessionTest, CancelPushAfterReceivingResponse) {
}

TEST_P(QuicChromiumClientSessionTest, MaxNumStreamsViaRequest) {
MockRead reads[] = {MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0)};
std::unique_ptr<quic::QuicEncryptedPacket> settings_packet(
client_maker_.MakeInitialSettingsPacket(1, nullptr));
std::unique_ptr<quic::QuicEncryptedPacket> client_rst(
client_maker_.MakeRstPacket(2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
MockWrite writes[] = {
MockWrite(ASYNC, settings_packet->data(), settings_packet->length(), 1),
MockWrite(ASYNC, client_rst->data(), client_rst->length(), 2)};
socket_data_.reset(new SequencedSocketData(reads, writes));
MockQuicData quic_data;
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeInitialSettingsPacket(1, nullptr));
quic_data.AddWrite(SYNCHRONOUS, client_maker_.MakeRstPacket(
2, true, GetNthClientInitiatedStreamId(0),
quic::QUIC_RST_ACKNOWLEDGEMENT));
if (version_ == quic::QUIC_VERSION_99) {
quic_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeStreamIdBlockedPacket(3, true, 102));
quic_data.AddRead(ASYNC,
server_maker_.MakeMaxStreamIdPacket(1, true, 102 + 2));
}
quic_data.AddRead(ASYNC, ERR_IO_PENDING);
quic_data.AddRead(ASYNC, OK); // EOF
quic_data.AddSocketDataToFactory(&socket_factory_);

Initialize();
CompleteCryptoHandshake();
Expand Down Expand Up @@ -1082,6 +1117,8 @@ TEST_P(QuicChromiumClientSessionTest, MaxNumStreamsViaRequest) {
quic::QuicRstStreamFrame rst1(quic::kInvalidControlFrameId, stream_id,
quic::QUIC_STREAM_NO_ERROR, 0);
session_->OnRstStream(rst1);
// Pump data, bringing in the max-stream-id
base::RunLoop().RunUntilIdle();
ASSERT_TRUE(callback.have_result());
EXPECT_THAT(callback.WaitForResult(), IsOk());
EXPECT_TRUE(handle->ReleaseStream() != nullptr);
Expand Down
44 changes: 44 additions & 0 deletions net/quic/quic_connection_logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,28 @@ void QuicConnectionLogger::OnFrameAddedToPacket(const quic::QuicFrame& frame) {
break;
case quic::MTU_DISCOVERY_FRAME:
break;
case quic::APPLICATION_CLOSE_FRAME:
break;
case quic::NEW_CONNECTION_ID_FRAME:
break;
case quic::MAX_STREAM_ID_FRAME:
break;
case quic::STREAM_ID_BLOCKED_FRAME:
break;
case quic::PATH_RESPONSE_FRAME:
break;
case quic::PATH_CHALLENGE_FRAME:
break;
case quic::STOP_SENDING_FRAME:
break;
case quic::MESSAGE_FRAME:
break;
case quic::CRYPTO_FRAME:
break;
case quic::NEW_TOKEN_FRAME:
break;
case quic::RETIRE_CONNECTION_ID_FRAME:
break;
default:
DCHECK(false) << "Illegal frame type: " << frame.type;
}
Expand Down Expand Up @@ -455,6 +477,28 @@ void QuicConnectionLogger::OnFrameAddedToPacket(const quic::QuicFrame& frame) {
// MtuDiscoveryFrame is PingFrame on wire, it does not have any payload.
net_log_.AddEvent(NetLogEventType::QUIC_SESSION_MTU_DISCOVERY_FRAME_SENT);
break;
case quic::APPLICATION_CLOSE_FRAME:
break;
case quic::NEW_CONNECTION_ID_FRAME:
break;
case quic::MAX_STREAM_ID_FRAME:
break;
case quic::STREAM_ID_BLOCKED_FRAME:
break;
case quic::PATH_RESPONSE_FRAME:
break;
case quic::PATH_CHALLENGE_FRAME:
break;
case quic::STOP_SENDING_FRAME:
break;
case quic::MESSAGE_FRAME:
break;
case quic::CRYPTO_FRAME:
break;
case quic::NEW_TOKEN_FRAME:
break;
case quic::RETIRE_CONNECTION_ID_FRAME:
break;
default:
DCHECK(false) << "Illegal frame type: " << frame.type;
}
Expand Down
7 changes: 7 additions & 0 deletions net/quic/quic_stream_factory_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1672,6 +1672,12 @@ TEST_P(QuicStreamFactoryTest, MaxOpenStream) {
socket_data.AddRead(ASYNC,
server_maker_.MakeRstPacket(1, false, stream_id,
quic::QUIC_STREAM_CANCELLED));
if (version_ == quic::QUIC_VERSION_99) {
socket_data.AddWrite(SYNCHRONOUS,
client_maker_.MakeStreamIdBlockedPacket(3, true, 102));
socket_data.AddRead(ASYNC,
server_maker_.MakeMaxStreamIdPacket(4, true, 102 + 2));
}
socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
socket_data.AddSocketDataToFactory(socket_factory_.get());

Expand Down Expand Up @@ -1719,6 +1725,7 @@ TEST_P(QuicStreamFactoryTest, MaxOpenStream) {
streams.front()->Close(false);
// Trigger exchange of RSTs that in turn allow progress for the last
// stream.
base::RunLoop().RunUntilIdle();
EXPECT_THAT(callback_.WaitForResult(), IsOk());

EXPECT_TRUE(socket_data.AllReadDataConsumed());
Expand Down
40 changes: 40 additions & 0 deletions net/quic/quic_test_packet_maker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,46 @@ std::unique_ptr<quic::QuicReceivedPacket> QuicTestPacketMaker::MakeRstPacket(
return MakePacket(header, quic::QuicFrame(&rst));
}

std::unique_ptr<quic::QuicReceivedPacket>
QuicTestPacketMaker::MakeStreamIdBlockedPacket(quic::QuicPacketNumber num,
bool include_version,
quic::QuicStreamId stream_id) {
quic::QuicPacketHeader header;
header.destination_connection_id = connection_id_;
header.destination_connection_id_length = GetDestinationConnectionIdLength();
header.source_connection_id = connection_id_;
header.source_connection_id_length = GetSourceConnectionIdLength();
header.reset_flag = false;
header.version_flag = ShouldIncludeVersion(include_version);
header.long_packet_type = long_header_type_;
header.packet_number_length = GetPacketNumberLength();
header.packet_number = num;

quic::QuicStreamIdBlockedFrame frame(1, stream_id);
DVLOG(1) << "Adding frame: " << quic::QuicFrame(frame);
return MakePacket(header, quic::QuicFrame(frame));
}

std::unique_ptr<quic::QuicReceivedPacket>
QuicTestPacketMaker::MakeMaxStreamIdPacket(quic::QuicPacketNumber num,
bool include_version,
quic::QuicStreamId stream_id) {
quic::QuicPacketHeader header;
header.destination_connection_id = connection_id_;
header.destination_connection_id_length = GetDestinationConnectionIdLength();
header.source_connection_id = connection_id_;
header.source_connection_id_length = GetSourceConnectionIdLength();
header.reset_flag = false;
header.version_flag = ShouldIncludeVersion(include_version);
header.long_packet_type = long_header_type_;
header.packet_number_length = GetPacketNumberLength();
header.packet_number = num;

quic::QuicMaxStreamIdFrame frame(1, stream_id);
DVLOG(1) << "Adding frame: " << quic::QuicFrame(frame);
return MakePacket(header, quic::QuicFrame(frame));
}

std::unique_ptr<quic::QuicReceivedPacket>
QuicTestPacketMaker::MakeRstAndRequestHeadersPacket(
quic::QuicPacketNumber num,
Expand Down
11 changes: 11 additions & 0 deletions net/quic/quic_test_packet_maker.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ class QuicTestPacketMaker {
quic::QuicPacketNumber largest_received,
quic::QuicPacketNumber smallest_received,
quic::QuicPacketNumber least_unacked);

std::unique_ptr<quic::QuicReceivedPacket> MakeStreamIdBlockedPacket(
quic::QuicPacketNumber num,
bool include_version,
quic::QuicStreamId stream_id);

std::unique_ptr<quic::QuicReceivedPacket> MakeMaxStreamIdPacket(
quic::QuicPacketNumber num,
bool include_version,
quic::QuicStreamId stream_id);

std::unique_ptr<quic::QuicReceivedPacket> MakeRstPacket(
quic::QuicPacketNumber num,
bool include_version,
Expand Down
9 changes: 7 additions & 2 deletions net/socket/socket_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,13 @@ SequencedSocketData::SequencedSocketData(base::span<const MockRead> reads,
++next_sequence_number;
continue;
}
CHECK(false) << "Sequence number not found where expected: "
<< next_sequence_number;
if (next_write != writes.end()) {
CHECK(false) << "Sequence number " << next_write->sequence_number
<< " not found where expected: " << next_sequence_number;
} else {
CHECK(false) << "Too few writes, next expected sequence number: "
<< next_sequence_number;
}
return;
}

Expand Down
6 changes: 6 additions & 0 deletions net/third_party/quic/core/frames/quic_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ QuicFrame CopyRetransmittableControlFrame(const QuicFrame& frame) {
case STOP_SENDING_FRAME:
copy = QuicFrame(new QuicStopSendingFrame(*frame.stop_sending_frame));
break;
case STREAM_ID_BLOCKED_FRAME:
copy = QuicFrame(QuicStreamIdBlockedFrame(frame.stream_id_blocked_frame));
break;
case MAX_STREAM_ID_FRAME:
copy = QuicFrame(QuicMaxStreamIdFrame(frame.max_stream_id_frame));
break;
default:
QUIC_BUG << "Try to copy a non-retransmittable control frame: " << frame;
copy = QuicFrame(QuicPingFrame(kInvalidControlFrameId));
Expand Down
17 changes: 13 additions & 4 deletions net/third_party/quic/core/http/end_to_end_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "net/third_party/quic/test_tools/quic_server_peer.h"
#include "net/third_party/quic/test_tools/quic_session_peer.h"
#include "net/third_party/quic/test_tools/quic_spdy_session_peer.h"
#include "net/third_party/quic/test_tools/quic_stream_id_manager_peer.h"
#include "net/third_party/quic/test_tools/quic_stream_peer.h"
#include "net/third_party/quic/test_tools/quic_stream_sequencer_peer.h"
#include "net/third_party/quic/test_tools/quic_test_client.h"
Expand Down Expand Up @@ -1413,6 +1414,8 @@ TEST_P(EndToEndTestWithTls, MaxIncomingDynamicStreamsLimitRespected) {
kServerMaxIncomingDynamicStreams);
ASSERT_TRUE(Initialize());
EXPECT_TRUE(client_->client()->WaitForCryptoHandshakeConfirmed());
QuicConnection* client_connection =
client_->client()->client_session()->connection();

// Make the client misbehave after negotiation.
const int kServerMaxStreams = kMaxStreamsMinimumIncrement + 1;
Expand All @@ -1432,10 +1435,16 @@ TEST_P(EndToEndTestWithTls, MaxIncomingDynamicStreamsLimitRespected) {
client_->SendMessage(headers, "", /*fin=*/false);
}
client_->WaitForResponse();

EXPECT_TRUE(client_->connected());
EXPECT_EQ(QUIC_REFUSED_STREAM, client_->stream_error());
EXPECT_EQ(QUIC_NO_ERROR, client_->connection_error());
if (client_connection->transport_version() != QUIC_VERSION_99) {
EXPECT_TRUE(client_->connected());
EXPECT_EQ(QUIC_REFUSED_STREAM, client_->stream_error());
EXPECT_EQ(QUIC_NO_ERROR, client_->connection_error());
} else {
// Version 99 disconnects the connection if we exceed the stream limit.
EXPECT_FALSE(client_->connected());
EXPECT_EQ(QUIC_STREAM_CONNECTION_ERROR, client_->stream_error());
EXPECT_EQ(QUIC_INVALID_STREAM_ID, client_->connection_error());
}
}

TEST_P(EndToEndTest, SetIndependentMaxIncomingDynamicStreamsLimits) {
Expand Down
Loading

0 comments on commit 878763b

Please sign in to comment.