Skip to content

quiche: add 100 continue support #14246

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 5 commits into from
Jan 27, 2021
Merged

quiche: add 100 continue support #14246

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
00c5734
add 100 continue
danzh1989 Dec 1, 2020
5bafa4a
add tests
danzh1989 Dec 2, 2020
85465ff
debug logging
danzh1989 Dec 3, 2020
48597cd
pass non-100 headers
danzh1989 Jan 25, 2021
b7c9a2d
added e2e test for 101 response code
danzh1989 Jan 26, 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
1 change: 1 addition & 0 deletions source/extensions/quic_listeners/quiche/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ envoy_cc_library(
":quic_filter_manager_connection_lib",
"//source/common/buffer:buffer_lib",
"//source/common/common:assert_lib",
"//source/common/http:codes_lib",
"//source/common/http:header_map_lib",
"//source/common/http:header_utility_lib",
"//source/extensions/quic_listeners/quiche/platform:quic_platform_mem_slice_storage_impl_lib",
Expand Down
41 changes: 33 additions & 8 deletions source/extensions/quic_listeners/quiche/envoy_quic_client_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
#include "extensions/quic_listeners/quiche/envoy_quic_client_session.h"

#include "common/buffer/buffer_impl.h"
#include "common/http/codes.h"
#include "common/http/header_map_impl.h"
#include "common/http/header_utility.h"
#include "common/http/utility.h"
#include "common/common/assert.h"

namespace Envoy {
Expand Down Expand Up @@ -132,25 +134,42 @@ void EnvoyQuicClientStream::switchStreamBlockState(bool should_block) {

void EnvoyQuicClientStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
const quic::QuicHeaderList& header_list) {
if (rst_sent()) {
if (read_side_closed()) {
Comment thread
alyssawilk marked this conversation as resolved.
return;
}
quic::QuicSpdyStream::OnInitialHeadersComplete(fin, frame_len, header_list);
ASSERT(headers_decompressed() && !header_list.empty());

response_decoder_->decodeHeaders(
quicHeadersToEnvoyHeaders<Http::ResponseHeaderMapImpl>(header_list),
/*end_stream=*/fin);
ENVOY_STREAM_LOG(debug, "Received headers: {}.", *this, header_list.DebugString());
if (fin) {
end_stream_decoded_ = true;
}
std::unique_ptr<Http::ResponseHeaderMapImpl> headers =
quicHeadersToEnvoyHeaders<Http::ResponseHeaderMapImpl>(header_list);
const uint64_t status = Http::Utility::getResponseStatus(*headers);
if (Http::CodeUtility::is1xx(status)) {

@alyssawilk alyssawilk Dec 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So what happens with 1xx headers which are not 100-continue?

For non-QUIC we send them up via decodeHeaders.

@danzh2010 danzh2010 Dec 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, I didn't know that. Currently we drop these headers on the floor. Will fix it.
Question please: QUIC doens't support 101 response code: https://tools.ietf.org/html/draft-ietf-quic-http-32#section-4.3. What shall we do in that case?

@alyssawilk alyssawilk Dec 10, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh fair point. maybe treat it as invalid and fast-fail the response then?

@danzh2010 danzh2010 Jan 25, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Re-wrote 1xx header handling logic here. PTAL!

// These are Informational 1xx headers, not the actual response headers.
set_headers_decompressed(false);
if (status == 100 && !decoded_100_continue_) {
// This is 100 Continue, only decode it once to support Expect:100-Continue header.
decoded_100_continue_ = true;
response_decoder_->decode100ContinueHeaders(std::move(headers));
}
} else {
response_decoder_->decodeHeaders(std::move(headers),
/*end_stream=*/fin);
}

ConsumeHeaderList();
}

void EnvoyQuicClientStream::OnBodyAvailable() {
ASSERT(FinishedReadingHeaders());
ASSERT(read_disable_counter_ == 0);
ASSERT(!in_decode_data_callstack_);
if (read_side_closed()) {
return;
}
in_decode_data_callstack_ = true;

Buffer::InstancePtr buffer = std::make_unique<Buffer::OwnedImpl>();
Expand Down Expand Up @@ -178,13 +197,13 @@ void EnvoyQuicClientStream::OnBodyAvailable() {
// already delivered it or decodeTrailers will be called.
bool skip_decoding = (buffer->length() == 0 && !fin_read_and_no_trailers) || end_stream_decoded_;
if (!skip_decoding) {
response_decoder_->decodeData(*buffer, fin_read_and_no_trailers);
if (fin_read_and_no_trailers) {
end_stream_decoded_ = true;
}
response_decoder_->decodeData(*buffer, fin_read_and_no_trailers);
}

if (!sequencer()->IsClosed()) {
if (!sequencer()->IsClosed() || read_side_closed()) {
in_decode_data_callstack_ = false;
if (read_disable_counter_ > 0) {
// If readDisable() was ever called during decodeData() and it meant to disable
Expand All @@ -204,6 +223,10 @@ void EnvoyQuicClientStream::OnBodyAvailable() {

void EnvoyQuicClientStream::OnTrailingHeadersComplete(bool fin, size_t frame_len,
const quic::QuicHeaderList& header_list) {
if (read_side_closed()) {
return;
}
ENVOY_STREAM_LOG(debug, "Received trailers: {}.", *this, header_list.DebugString());
quic::QuicSpdyStream::OnTrailingHeadersComplete(fin, frame_len, header_list);
ASSERT(trailers_decompressed());
if (session()->connection()->connected() && !rst_sent()) {
Expand All @@ -215,9 +238,9 @@ void EnvoyQuicClientStream::maybeDecodeTrailers() {
if (sequencer()->IsClosed() && !FinishedReadingTrailers()) {
ASSERT(!received_trailers().empty());
// Only decode trailers after finishing decoding body.
end_stream_decoded_ = true;
response_decoder_->decodeTrailers(
spdyHeaderBlockToEnvoyHeaders<Http::ResponseTrailerMapImpl>(received_trailers()));
end_stream_decoded_ = true;
MarkTrailersConsumed();
}
}
Expand All @@ -236,7 +259,9 @@ void EnvoyQuicClientStream::Reset(quic::QuicRstStreamErrorCode error) {
void EnvoyQuicClientStream::OnConnectionClosed(quic::QuicErrorCode error,
quic::ConnectionCloseSource source) {
quic::QuicSpdyClientStream::OnConnectionClosed(error, source);
runResetCallbacks(quicErrorCodeToEnvoyResetReason(error));
if (!end_stream_decoded_) {
runResetCallbacks(quicErrorCodeToEnvoyResetReason(error));
}
}

void EnvoyQuicClientStream::OnClose() {
Expand Down
2 changes: 2 additions & 0 deletions source/extensions/quic_listeners/quiche/envoy_quic_client_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream,
void maybeDecodeTrailers();

Http::ResponseDecoder* response_decoder_{nullptr};

bool decoded_100_continue_{false};
Comment thread
alyssawilk marked this conversation as resolved.
};

} // namespace Quic
Expand Down
28 changes: 20 additions & 8 deletions source/extensions/quic_listeners/quiche/envoy_quic_server_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,25 +150,28 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
const quic::QuicHeaderList& header_list) {
// TODO(danzh) Fix in QUICHE. If the stream has been reset in the call stack,
// OnInitialHeadersComplete() shouldn't be called.
if (rst_sent()) {
if (read_side_closed()) {
return;
}
quic::QuicSpdyServerStreamBase::OnInitialHeadersComplete(fin, frame_len, header_list);
ASSERT(headers_decompressed() && !header_list.empty());

request_decoder_->decodeHeaders(
quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(header_list),
/*end_stream=*/fin);
ENVOY_STREAM_LOG(debug, "Received headers: {}.", *this, header_list.DebugString());
if (fin) {
end_stream_decoded_ = true;
}
request_decoder_->decodeHeaders(
quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(header_list),
/*end_stream=*/fin);
ConsumeHeaderList();
}

void EnvoyQuicServerStream::OnBodyAvailable() {
ASSERT(FinishedReadingHeaders());
ASSERT(read_disable_counter_ == 0);
ASSERT(!in_decode_data_callstack_);
if (read_side_closed()) {
return;
}
in_decode_data_callstack_ = true;

Buffer::InstancePtr buffer = std::make_unique<Buffer::OwnedImpl>();
Expand All @@ -195,13 +198,13 @@ void EnvoyQuicServerStream::OnBodyAvailable() {
// already delivered it or decodeTrailers will be called.
bool skip_decoding = (buffer->length() == 0 && !fin_read_and_no_trailers) || end_stream_decoded_;
if (!skip_decoding) {
request_decoder_->decodeData(*buffer, fin_read_and_no_trailers);
if (fin_read_and_no_trailers) {
end_stream_decoded_ = true;
}
request_decoder_->decodeData(*buffer, fin_read_and_no_trailers);
}

if (!sequencer()->IsClosed()) {
if (!sequencer()->IsClosed() || read_side_closed()) {
in_decode_data_callstack_ = false;
if (read_disable_counter_ > 0) {
// If readDisable() was ever called during decodeData() and it meant to disable
Expand All @@ -221,20 +224,29 @@ void EnvoyQuicServerStream::OnBodyAvailable() {

void EnvoyQuicServerStream::OnTrailingHeadersComplete(bool fin, size_t frame_len,
const quic::QuicHeaderList& header_list) {
if (read_side_closed()) {
return;
}
ENVOY_STREAM_LOG(debug, "Received trailers: {}.", *this, received_trailers().DebugString());
quic::QuicSpdyServerStreamBase::OnTrailingHeadersComplete(fin, frame_len, header_list);
ASSERT(trailers_decompressed());
if (session()->connection()->connected() && !rst_sent()) {
maybeDecodeTrailers();
}
}

void EnvoyQuicServerStream::OnHeadersTooLarge() {
ENVOY_STREAM_LOG(debug, "Headers too large.", *this);
quic::QuicSpdyServerStreamBase::OnHeadersTooLarge();
}

void EnvoyQuicServerStream::maybeDecodeTrailers() {
if (sequencer()->IsClosed() && !FinishedReadingTrailers()) {
ASSERT(!received_trailers().empty());
// Only decode trailers after finishing decoding body.
end_stream_decoded_ = true;
request_decoder_->decodeTrailers(
spdyHeaderBlockToEnvoyHeaders<Http::RequestTrailerMapImpl>(received_trailers()));
end_stream_decoded_ = true;
MarkTrailersConsumed();
}
}
Expand Down
1 change: 1 addition & 0 deletions source/extensions/quic_listeners/quiche/envoy_quic_server_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase,
const quic::QuicHeaderList& header_list) override;
void OnTrailingHeadersComplete(bool fin, size_t frame_len,
const quic::QuicHeaderList& header_list) override;
void OnHeadersTooLarge() override;

private:
QuicFilterManagerConnectionImpl* filterManagerConnection();
Expand Down
Loading