Skip to content

quiche: client session supports creating bidi stream #17543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alyssawilk merged 8 commits into from
Aug 10, 2021
Merged

quiche: client session supports creating bidi stream #17543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c275f94
create unidirectional read stream
danzh1989 Jul 29, 2021
531cfbc
Merge branch 'master' into pendingstream
danzh1989 Jul 30, 2021
38774bb
format
danzh1989 Jul 30, 2021
dcf9fef
Merge branch 'master' into pendingstream
danzh1989 Jul 30, 2021
4392432
not create server initiated stream
danzh1989 Jul 30, 2021
fa24b94
return nullptr for pending stream
danzh1989 Aug 5, 2021
25ffac5
Merge branch 'master' into pendingstream
danzh1989 Aug 9, 2021
4b84ac3
close connection for server push stream
danzh1989 Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/common/quic/envoy_quic_client_session.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ std::unique_ptr<quic::QuicSpdyClientStream> EnvoyQuicClientSession::CreateClient
}

quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::QuicStreamId /*id*/) {
// Disallow server initiated stream.
NOT_REACHED_GCOVR_EXCL_LINE;
// Envoy doesn't support server initiated stream.
return nullptr;
}

quic::QuicSpdyStream*
EnvoyQuicClientSession::CreateIncomingStream(quic::PendingStream* /*pending*/) {
// Disallow server initiated stream.
// Envoy doesn't support server push.
NOT_REACHED_GCOVR_EXCL_LINE;
}

Expand Down
10 changes: 0 additions & 10 deletions source/common/quic/envoy_quic_client_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,6 @@ EnvoyQuicClientStream::EnvoyQuicClientStream(
"Send buffer limit should be larger than 8KB.");
}

EnvoyQuicClientStream::EnvoyQuicClientStream(
quic::PendingStream* pending, quic::QuicSpdyClientSession* client_session,
quic::StreamType type, Http::Http3::CodecStats& stats,
const envoy::config::core::v3::Http3ProtocolOptions& http3_options)
: quic::QuicSpdyClientStream(pending, client_session, type),
EnvoyQuicStream(
static_cast<uint32_t>(GetReceiveWindow().value()), *filterManagerConnection(),
[this]() { runLowWatermarkCallbacks(); }, [this]() { runHighWatermarkCallbacks(); },
stats, http3_options) {}

Copy link
Contributor

@antoniovicente antoniovicente Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem unrelated. Was this code unused?

Copy link
Contributor Author

@danzh2010 danzh2010 Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are only called by server push stream.

Http::Status EnvoyQuicClientStream::encodeHeaders(const Http::RequestHeaderMap& headers,
bool end_stream) {
// Required headers must be present. This can only happen by some erroneous processing after the
Expand Down
3 changes: 0 additions & 3 deletions source/common/quic/envoy_quic_client_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream,
EnvoyQuicClientStream(quic::QuicStreamId id, quic::QuicSpdyClientSession* client_session,
quic::StreamType type, Http::Http3::CodecStats& stats,
const envoy::config::core::v3::Http3ProtocolOptions& http3_options);
EnvoyQuicClientStream(quic::PendingStream* pending, quic::QuicSpdyClientSession* client_session,
quic::StreamType type, Http::Http3::CodecStats& stats,
const envoy::config::core::v3::Http3ProtocolOptions& http3_options);

void setResponseDecoder(Http::ResponseDecoder& decoder) { response_decoder_ = &decoder; }

Expand Down
16 changes: 0 additions & 16 deletions source/common/quic/envoy_quic_server_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,6 @@ EnvoyQuicServerStream::EnvoyQuicServerStream(
"Send buffer limit should be larger than 8KB.");
}

EnvoyQuicServerStream::EnvoyQuicServerStream(
quic::PendingStream* pending, quic::QuicSpdySession* session, quic::StreamType type,
Http::Http3::CodecStats& stats,
const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action)
: quic::QuicSpdyServerStreamBase(pending, session, type),
EnvoyQuicStream(
// This should be larger than 8k to fully utilize congestion control
// window. And no larger than the max stream flow control window for
// the stream to buffer all the data.
static_cast<uint32_t>(GetReceiveWindow().value()), *filterManagerConnection(),
[this]() { runLowWatermarkCallbacks(); }, [this]() { runHighWatermarkCallbacks(); },
stats, http3_options),
headers_with_underscores_action_(headers_with_underscores_action) {}

void EnvoyQuicServerStream::encode100ContinueHeaders(const Http::ResponseHeaderMap& headers) {
ASSERT(headers.Status()->value() == "100");
encodeHeaders(headers, false);
Expand Down
6 changes: 0 additions & 6 deletions source/common/quic/envoy_quic_server_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase,
envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action);

EnvoyQuicServerStream(quic::PendingStream* pending, quic::QuicSpdySession* session,
quic::StreamType type, Http::Http3::CodecStats& stats,
const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action);

void setRequestDecoder(Http::RequestDecoder& decoder) { request_decoder_ = &decoder; }

// Http::StreamEncoder
Expand Down
1 change: 1 addition & 0 deletions source/common/quic/platform/quiche_flags_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ absl::flat_hash_map<absl::string_view, Flag*> makeFlagMap() {
#undef QUIC_FLAG
// Disable IETF draft 29 implementation. Envoy only supports RFC-v1.
FLAGS_quic_reloadable_flag_quic_disable_version_draft_29->setValue(true);
FLAGS_quic_reloadable_flag_quic_decline_server_push_stream->setValue(true);

#define QUIC_PROTOCOL_FLAG(type, flag, ...) flags.emplace(FLAGS_##flag->name(), FLAGS_##flag);
#include "quiche/quic/core/quic_protocol_flags_list.h"
Expand Down
1 change: 1 addition & 0 deletions test/common/quic/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ envoy_cc_test(
"//test/mocks/stats:stats_mocks",
"//test/test_common:logging_lib",
"//test/test_common:simulated_time_system_lib",
"@com_googlesource_quiche//:quic_test_tools_session_peer_lib",
],
)

Expand Down
24 changes: 22 additions & 2 deletions test/common/quic/envoy_quic_client_session_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "quiche/quic/core/crypto/null_encrypter.h"
#include "quiche/quic/test_tools/crypto_test_utils.h"
#include "quiche/quic/test_tools/quic_session_peer.h"
#include "quiche/quic/test_tools/quic_test_utils.h"

#if defined(__GNUC__)
Expand Down Expand Up @@ -69,8 +70,10 @@ class EnvoyQuicClientSessionTest : public testing::Test {
EnvoyQuicClientSessionTest()
: api_(Api::createApiForTest(time_system_)),
dispatcher_(api_->allocateDispatcher("test_thread")), connection_helper_(*dispatcher_),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()),
quic_version_({quic::CurrentSupportedHttp3Versions()[0]}),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_([]() {
SetQuicReloadableFlag(quic_decline_server_push_stream, true);
return quic::CurrentSupportedHttp3Versions();
}()),
peer_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(),
12345)),
self_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(),
Expand Down Expand Up @@ -324,5 +327,22 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) {
EXPECT_EQ(1U, TestUtility::findCounter(store_, "http3.quic_version_rfc_v1")->value());
}

TEST_F(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) {
quic::QuicStreamId stream_id = 1u;
quic::QuicStreamFrame stream_frame(stream_id, false, 0, "aaa");
envoy_quic_session_.OnStreamFrame(stream_frame);
EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id));
// IETF stream 3 is server initiated uni-directional stream.
stream_id = 3u;
auto payload = std::make_unique<char[]>(8);
quic::QuicDataWriter payload_writer(8, payload.get());
EXPECT_TRUE(payload_writer.WriteVarInt62(1ul));
EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose));
EXPECT_CALL(*quic_connection_, SendConnectionClosePacket(quic::QUIC_HTTP_RECEIVE_SERVER_PUSH, _,
"Received server push stream"));
quic::QuicStreamFrame stream_frame2(stream_id, false, 0, absl::string_view(payload.get(), 1));
envoy_quic_session_.OnStreamFrame(stream_frame2);
}

} // namespace Quic
} // namespace Envoy
23 changes: 21 additions & 2 deletions test/common/quic/envoy_quic_server_session_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,10 @@ class EnvoyQuicServerSessionTest : public testing::Test {
EnvoyQuicServerSessionTest()
: api_(Api::createApiForTest(time_system_)),
dispatcher_(api_->allocateDispatcher("test_thread")), connection_helper_(*dispatcher_),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()),
quic_version_({quic::CurrentSupportedHttp3Versions()[0]}),
alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_({[]() {
SetQuicReloadableFlag(quic_decline_server_push_stream, true);
return quic::CurrentSupportedHttp3Versions()[0];
}()}),
quic_stat_names_(listener_config_.listenerScope().symbolTable()),
quic_connection_(new MockEnvoyQuicServerConnection(
connection_helper_, alarm_factory_, writer_, quic_version_, *listener_config_.socket_)),
Expand Down Expand Up @@ -1023,5 +1025,22 @@ TEST_F(EnvoyQuicServerSessionTest, SendBufferWatermark) {
EXPECT_TRUE(stream2->write_side_closed());
}

TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) {
installReadFilter();
Http::MockRequestDecoder request_decoder;
Http::MockStreamCallbacks stream_callbacks;
// IETF stream 2 is client initiated uni-directional stream.
quic::QuicStreamId stream_id = 2u;
auto payload = std::make_unique<char[]>(8);
quic::QuicDataWriter payload_writer(8, payload.get());
EXPECT_TRUE(payload_writer.WriteVarInt62(1ul));
EXPECT_CALL(http_connection_callbacks_, newStream(_, false)).Times(0u);
EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose));
EXPECT_CALL(*quic_connection_, SendConnectionClosePacket(quic::QUIC_HTTP_RECEIVE_SERVER_PUSH, _,
"Received server push stream"));
quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1));
envoy_quic_session_.OnStreamFrame(stream_frame);
}

} // namespace Quic
} // namespace Envoy