Skip to content
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.

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

http1 encode trailers in chunk encoding #8667

Merged
merged 69 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
08ae97c
http1 encode trailers in chunk encoding
Chuongv Oct 18, 2019
5de8533
Add integration test for trailers http1<--->Envoy<--->http2
Chuongv Oct 24, 2019
9c550cb
Use tcp client for integration test for trailers
Chuongv Oct 24, 2019
f0d53cd
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Oct 25, 2019
f9c9f48
Update tests for trailers http1
Chuongv Oct 25, 2019
e002b4b
Undo changes in codec_impl
Chuongv Oct 25, 2019
10a9089
Implementation of trailers support for http1
Chuongv Oct 28, 2019
de29acb
Trailer code clean up
Chuongv Oct 28, 2019
96913f9
Remove unnecessary check for trailers
Chuongv Oct 28, 2019
77ec2ac
Keep the trailer processing on MessageComplete
Chuongv Oct 28, 2019
8b03163
Remove trailer logic in onHeaderComplete
Chuongv Oct 28, 2019
6955a1a
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Oct 28, 2019
92421a8
Accomodate trailers in grpc reverse bridge filter
Chuongv Oct 31, 2019
e7cb695
Reuse encodeFormattedHeaders in encodeTrailers
Chuongv Oct 31, 2019
592e315
Fix the formatting
Chuongv Oct 31, 2019
9e2a48a
Refactor and clean up redundant parser fields
Chuongv Oct 31, 2019
cd56fd8
Add empty space to match the creation of trailers
Chuongv Oct 31, 2019
e9c27e4
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Oct 31, 2019
c70e7b0
Update tests to account for trailers in H1
Chuongv Oct 31, 2019
557e027
Fix formatting
Chuongv Oct 31, 2019
614ff9b
Refactor trailers based on feedback
Chuongv Nov 1, 2019
4205198
Move trailer tests to appropriate test file
Chuongv Nov 1, 2019
b63375d
Fix typo
Chuongv Nov 3, 2019
92d4efc
Add flag to disable support of trailers
Chuongv Nov 8, 2019
dae9233
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 8, 2019
07a0e79
Update docs to include information about encoding h1 trailers
Chuongv Nov 8, 2019
e124271
Update encode trailer docs to include ref link
Chuongv Nov 8, 2019
962afd2
Add next free field to pass formatter
Chuongv Nov 8, 2019
fce0ddb
Correct the label in the version history docs
Chuongv Nov 8, 2019
9f990a5
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 8, 2019
de672b8
Correct the label in the version history docs
Chuongv Nov 11, 2019
b41c492
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 11, 2019
ee426ea
Set the enable_trailers so it defaults to false
Chuongv Nov 13, 2019
1296eff
Switch flag to enable_trailers_ for unit test
Chuongv Nov 13, 2019
ce552b8
Fix spelling
Chuongv Nov 13, 2019
1b87726
Refactor and clean up integration tests
Chuongv Nov 13, 2019
fd5c7de
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 13, 2019
4ed8b97
Move decoding trailers to be guarded by configuration flag
Chuongv Nov 16, 2019
2f18c1c
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 16, 2019
dfbd03e
Don't check for trailers in H2/H1 tests
Chuongv Nov 16, 2019
76a0a6d
Clear out trailers in grpc-web
Chuongv Nov 21, 2019
3041acb
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 21, 2019
e59101d
Address comments and move tests
Chuongv Nov 24, 2019
c784ce4
Use bitfield for enable_trailers
Chuongv Nov 25, 2019
f28af8f
Add more unit and integration tests and address PR feedback
Chuongv Nov 26, 2019
86fed8a
Allow configuration of incomplete streams for Autonomous Upstream
Chuongv Nov 26, 2019
cee19aa
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Nov 26, 2019
32e0c7a
Use bitfields for processing_trailers
Chuongv Nov 26, 2019
4388cf4
Update todo for clearing trailers
Chuongv Nov 26, 2019
8b13bdf
Add some helpful comments on codec_impl.h
Chuongv Nov 27, 2019
4aadf64
Merge branch 'AutonomousStreamIncompleteStream' into http1-trailers-c…
Chuongv Nov 27, 2019
14ac61b
Allow incomplete streams in PipelineTrailers test
Chuongv Nov 27, 2019
41135aa
Move enable_trailers_ for optimal packing
Chuongv Dec 4, 2019
678222d
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 4, 2019
dd6844a
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 4, 2019
b90f535
Address PR comments
Chuongv Dec 6, 2019
fbdf83d
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 6, 2019
9e8be74
Docs not liking the identations
Chuongv Dec 6, 2019
c179840
Fix the identation in the api docs for v3alpha
Chuongv Dec 6, 2019
915fb06
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 9, 2019
ce5cb82
Use set trailers
Chuongv Dec 10, 2019
b406f71
Update docs with attention for enable_trailers
Chuongv Dec 10, 2019
5447d69
Use the new trailers.clear() method
Chuongv Dec 10, 2019
be14d7e
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 10, 2019
0c717f8
Address PR comments
Chuongv Dec 12, 2019
a347b6f
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 13, 2019
fb321d8
Add tests for reverse bridge filter with trailers
Chuongv Dec 13, 2019
d1c66e8
Move trailers/headers to constant strings. Use Consistent Http1 naming
Chuongv Dec 16, 2019
15b6715
Merge remote-tracking branch 'envoy/master' into http1-trailers-chunk…
Chuongv Dec 16, 2019
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
4 changes: 4 additions & 0 deletions api/envoy/api/v2/core/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ message HttpProtocolOptions {
google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];
}

// [#next-free-field: 6]
message Http1ProtocolOptions {
message HeaderKeyFormat {
message ProperCaseWords {
Expand Down Expand Up @@ -83,6 +84,9 @@ message Http1ProtocolOptions {
// Describes how the keys for response headers should be formatted. By default, all header keys
// are lower cased.
HeaderKeyFormat header_key_format = 4;

// Enables encoding of trailers for HTTP1.
bool enable_trailers = 5;
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
}

// [#next-free-field: 13]
Expand Down
4 changes: 4 additions & 0 deletions api/envoy/api/v3alpha/core/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ message HttpProtocolOptions {
google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];
}

// [#next-free-field: 6]
message Http1ProtocolOptions {
message HeaderKeyFormat {
message ProperCaseWords {
Expand Down Expand Up @@ -83,6 +84,9 @@ message Http1ProtocolOptions {
// Describes how the keys for response headers should be formatted. By default, all header keys
// are lower cased.
HeaderKeyFormat header_key_format = 4;

// Enables encoding of trailers for HTTP1.
bool enable_trailers = 5;
}

// [#next-free-field: 13]
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Version history
* tcp_proxy: added :ref:`ClusterWeight.metadata_match<envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.WeightedCluster.ClusterWeight.metadata_match>`
* tcp_proxy: added :ref:`hash_policy<envoy_api_field_config.filter.network.tcp_proxy.v2.TcpProxy.hash_policy>`
* tls: remove TLS 1.0 and 1.1 from client defaults
* http: add support for encoding http1 trailers, to enable use :ref:`enable_trailers <envoy_api_field_core.Http1ProtocolOptions.enable_trailers>`.
* router: exposed DOWNSTREAM_REMOTE_ADDRESS as custom HTTP request/response headers.

1.12.0 (October 31, 2019)
Expand Down
2 changes: 2 additions & 0 deletions include/envoy/http/codec.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ struct Http1Settings {
bool accept_http_10_{false};
// Set a default host if no Host: header is present for HTTP/1.0 requests.`
std::string default_host_for_http_10_;
// Encode trailers in Http1
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
bool enable_trailers_{false};

enum class HeaderKeyFormat {
// By default no formatting is performed, presenting all headers in lowercase (as Envoy
Expand Down
83 changes: 65 additions & 18 deletions source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ HeaderKeyFormatterPtr formatter(const Http::Http1Settings& settings) {
} // namespace

const std::string StreamEncoderImpl::CRLF = "\r\n";
const std::string StreamEncoderImpl::LAST_CHUNK = "0\r\n\r\n";
// Last chunk as defined here https://tools.ietf.org/html/rfc7230#section-4.1
const std::string StreamEncoderImpl::LAST_CHUNK = "0\r\n";

StreamEncoderImpl::StreamEncoderImpl(ConnectionImpl& connection,
HeaderKeyFormatter* header_key_formatter)
Expand Down Expand Up @@ -187,7 +188,31 @@ void StreamEncoderImpl::encodeData(Buffer::Instance& data, bool end_stream) {
}
}

void StreamEncoderImpl::encodeTrailers(const HeaderMap&) { endEncode(); }
void StreamEncoderImpl::encodeTrailers(const HeaderMap& trailers) {
if (!connection_.enable_trailers_) {
return endEncode();
}
// Trailers only matter if it is a chunk transfer encoding
// https://tools.ietf.org/html/rfc7230#section-4.4
if (chunk_encoding_) {
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
// Finalize the body
connection_.buffer().add(LAST_CHUNK);

trailers.iterate(
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
[](const HeaderEntry& header, void* context) -> HeaderMap::Iterate {
static_cast<StreamEncoderImpl*>(context)->encodeFormattedHeader(
header.key().getStringView(), header.value().getStringView());
return HeaderMap::Iterate::Continue;
},
this);

connection_.flushOutput();
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
connection_.buffer().add(CRLF);
}

connection_.flushOutput();
connection_.onEncodeComplete();
}

void StreamEncoderImpl::encodeMetadata(const MetadataMapVector&) {
connection_.stats().metadata_not_supported_error_.inc();
Expand All @@ -196,6 +221,7 @@ void StreamEncoderImpl::encodeMetadata(const MetadataMapVector&) {
void StreamEncoderImpl::endEncode() {
if (chunk_encoding_) {
connection_.buffer().add(LAST_CHUNK);
connection_.buffer().add(CRLF);
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
}

connection_.flushOutput();
Expand Down Expand Up @@ -353,8 +379,9 @@ const ToLowerTable& ConnectionImpl::toLowerTable() {
ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& stats,
http_parser_type type, uint32_t max_headers_kb,
const uint32_t max_headers_count,
HeaderKeyFormatterPtr&& header_key_formatter)
: connection_(connection), stats_{ALL_HTTP1_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http1."))},
HeaderKeyFormatterPtr&& header_key_formatter, bool enable_trailers)
: enable_trailers_(enable_trailers),
connection_(connection), stats_{ALL_HTTP1_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http1."))},
header_key_formatter_(std::move(header_key_formatter)),
output_buffer_([&]() -> void { this->onBelowLowWatermark(); },
[&]() -> void { this->onAboveHighWatermark(); }),
Expand All @@ -369,11 +396,13 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& st
void ConnectionImpl::completeLastHeader() {
ENVOY_CONN_LOG(trace, "completed header: key={} value={}", connection_,
current_header_field_.getStringView(), current_header_value_.getStringView());

if (!current_header_field_.empty()) {
toLowerTable().toLowerCase(current_header_field_.buffer(), current_header_field_.size());
current_header_map_->addViaMove(std::move(current_header_field_),
std::move(current_header_value_));
}

// Check if the number of headers exceeds the limit.
if (current_header_map_->size() > max_headers_count_) {
error_code_ = Http::Code::RequestHeaderFieldsTooLarge;
Expand Down Expand Up @@ -447,11 +476,13 @@ size_t ConnectionImpl::dispatchSlice(const char* slice, size_t len) {
}

void ConnectionImpl::onHeaderField(const char* data, size_t length) {
ENVOY_CONN_LOG(trace, "onHeaderField: data={}", connection_, absl::string_view(data, length));
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
// We previously already finished up the headers, these headers are
// now trailers
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
if (header_parsing_state_ == HeaderParsingState::Done) {
// Ignore trailers.
return;
processing_trailers_ = true;
header_parsing_state_ = HeaderParsingState::Field;
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
}

if (header_parsing_state_ == HeaderParsingState::Value) {
completeLastHeader();
}
Expand All @@ -460,12 +491,11 @@ void ConnectionImpl::onHeaderField(const char* data, size_t length) {
}

void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
if (header_parsing_state_ == HeaderParsingState::Done) {
// Ignore trailers.
return;
}

const absl::string_view header_value = absl::string_view(data, length);
ENVOY_CONN_LOG(trace, "onHeaderValue: data={}", connection_, header_value);
if (!current_header_map_) {
current_header_map_ = std::make_unique<HeaderMapImpl>();
}

if (strict_header_validation_) {
if (!Http::HeaderUtility::headerIsValid(header_value)) {
Expand Down Expand Up @@ -498,8 +528,9 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
}

int ConnectionImpl::onHeadersCompleteBase() {
ENVOY_CONN_LOG(trace, "headers complete", connection_);
ENVOY_CONN_LOG(trace, "onHeadersCompleteBase complete", connection_);
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
completeLastHeader();

// Validate that the completed HeaderMap's cached byte size exists and is correct.
// This assert iterates over the HeaderMap.
ASSERT(current_header_map_->byteSize().has_value() &&
Expand Down Expand Up @@ -536,6 +567,7 @@ int ConnectionImpl::onHeadersCompleteBase() {

int rc = onHeadersComplete(std::move(current_header_map_));
current_header_map_.reset();

header_parsing_state_ = HeaderParsingState::Done;

// Returning 2 informs http_parser to not expect a body or further data on this connection.
Expand All @@ -544,6 +576,7 @@ int ConnectionImpl::onHeadersCompleteBase() {

void ConnectionImpl::onMessageCompleteBase() {
ENVOY_CONN_LOG(trace, "message complete", connection_);

if (handling_upgrade_) {
// If this is an upgrade request, swallow the onMessageComplete. The
// upgrade payload will be treated as stream body.
Expand All @@ -552,7 +585,14 @@ void ConnectionImpl::onMessageCompleteBase() {
http_parser_pause(&parser_, 1);
return;
}
onMessageComplete();

// If true, this indicates we were processing trailers and must
// move the last header into current_header_map_
if (header_parsing_state_ == HeaderParsingState::Value) {
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
completeLastHeader();
}

onMessageComplete(std::move(current_header_map_));
}

void ConnectionImpl::onMessageBeginBase() {
Expand All @@ -578,7 +618,7 @@ ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Stat
Http1Settings settings, uint32_t max_request_headers_kb,
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
const uint32_t max_request_headers_count)
: ConnectionImpl(connection, stats, HTTP_REQUEST, max_request_headers_kb,
max_request_headers_count, formatter(settings)),
max_request_headers_count, formatter(settings), settings.enable_trailers_),
callbacks_(callbacks), codec_settings_(settings) {}

void ServerConnectionImpl::onEncodeComplete() {
Expand Down Expand Up @@ -635,6 +675,8 @@ void ServerConnectionImpl::handlePath(HeaderMapImpl& headers, unsigned int metho
}

int ServerConnectionImpl::onHeadersComplete(HeaderMapImplPtr&& headers) {
ENVOY_CONN_LOG(trace, "Server: onHeadersComplete size={}", connection_, headers->size());

// Handle the case where response happens prior to request complete. It's up to upper layer code
// to disconnect the connection but we shouldn't fire any more events since it doesn't make
// sense.
Expand Down Expand Up @@ -699,7 +741,7 @@ void ServerConnectionImpl::onBody(const char* data, size_t length) {
}
}

void ServerConnectionImpl::onMessageComplete() {
void ServerConnectionImpl::onMessageComplete(HeaderMapImplPtr&& trailers) {
if (active_request_) {
Buffer::OwnedImpl buffer;
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
active_request_->remote_complete_ = true;
Expand All @@ -708,6 +750,8 @@ void ServerConnectionImpl::onMessageComplete() {
active_request_->request_decoder_->decodeHeaders(std::move(deferred_end_stream_headers_),
true);
deferred_end_stream_headers_.reset();
} else if (processing_trailers_) {
active_request_->request_decoder_->decodeTrailers(std::move(trailers));
} else {
active_request_->request_decoder_->decodeData(buffer, true);
}
Expand Down Expand Up @@ -754,7 +798,7 @@ ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Stat
ConnectionCallbacks&, const Http1Settings& settings,
const uint32_t max_response_headers_count)
: ConnectionImpl(connection, stats, HTTP_RESPONSE, MAX_RESPONSE_HEADERS_KB,
max_response_headers_count, formatter(settings)) {}
max_response_headers_count, formatter(settings), settings.enable_trailers_) {}

bool ClientConnectionImpl::cannotHaveBody() {
if ((!pending_responses_.empty() && pending_responses_.front().head_request_) ||
Expand Down Expand Up @@ -787,6 +831,7 @@ void ClientConnectionImpl::onEncodeHeaders(const HeaderMap& headers) {
}

int ClientConnectionImpl::onHeadersComplete(HeaderMapImplPtr&& headers) {
ENVOY_CONN_LOG(trace, "Client: onHeadersComplete size={}", connection_, headers->size());
headers->setStatus(parser_.status_code);

// Handle the case where the client is closing a kept alive connection (by sending a 408
Expand Down Expand Up @@ -821,7 +866,7 @@ void ClientConnectionImpl::onBody(const char* data, size_t length) {
}
}

void ClientConnectionImpl::onMessageComplete() {
void ClientConnectionImpl::onMessageComplete(HeaderMapImplPtr&& trailers) {
ENVOY_CONN_LOG(trace, "message complete", connection_);
if (ignore_message_complete_for_100_continue_) {
ignore_message_complete_for_100_continue_ = false;
Expand All @@ -847,6 +892,8 @@ void ClientConnectionImpl::onMessageComplete() {
if (deferred_end_stream_headers_) {
response.decoder_->decodeHeaders(std::move(deferred_end_stream_headers_), true);
deferred_end_stream_headers_.reset();
} else if (processing_trailers_) {
response.decoder_->decodeTrailers(std::move(trailers));
} else {
Buffer::OwnedImpl buffer;
response.decoder_->decodeData(buffer, true);
Expand Down
11 changes: 7 additions & 4 deletions source/common/http/http1/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,12 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log

CodecStats& stats() { return stats_; }

const bool enable_trailers_;
Copy link
Member

Choose a reason for hiding this comment

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

Can you reformat all the bools in this class as a bit field? It would be nice to not waste the extra RAM for each connection.

Copy link
Contributor Author

@Chuongv Chuongv Nov 25, 2019

Choose a reason for hiding this comment

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

Just to be sure, you are talking about
enable_trailers_

protected:
handling_upgrade_
private:
connection_header_sanitization_
strict_header_validation_
reset_stream_called_

Which would reduce it from 5 bytes into one 4 byte bitfield. Am I missing anything else? Is it worth it to put all those to save one byte?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

The number of bytes the existing fields take is dependent on ordering and alignment. It may very well be significantly larger than 5 bytes. The main issue here is we are trying to be very stringent on memory usage for things like connections. IMO this is a trivial change so I see no reason not to do it as part of this change while you wait for reviews from @alyssawilk and @PiotrSikora

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change is being done in this PR #9124

I'll make sure to align the enable_trailers once that PR is merged in.


protected:
ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, http_parser_type type,
uint32_t max_headers_kb, const uint32_t max_headers_count,
HeaderKeyFormatterPtr&& header_key_formatter);
HeaderKeyFormatterPtr&& header_key_formatter, bool enable_trailers);

bool resetStreamCalled() { return reset_stream_called_; }

Expand All @@ -204,6 +206,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
http_parser parser_;
HeaderMapPtr deferred_end_stream_headers_;
Http::Code error_code_{Http::Code::BadRequest};
bool processing_trailers_{};
bool handling_upgrade_{};
const HeaderKeyFormatterPtr header_key_formatter_;

Expand Down Expand Up @@ -269,7 +272,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
* Called when the request/response is complete.
*/
void onMessageCompleteBase();
virtual void onMessageComplete() PURE;
virtual void onMessageComplete(HeaderMapImplPtr&& trailers) PURE;

/**
* @see onResetStreamBase().
Expand Down Expand Up @@ -353,7 +356,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
void onUrl(const char* data, size_t length) override;
int onHeadersComplete(HeaderMapImplPtr&& headers) override;
void onBody(const char* data, size_t length) override;
void onMessageComplete() override;
void onMessageComplete(HeaderMapImplPtr&& trailers) override;
void onResetStream(StreamResetReason reason) override;
void sendProtocolError() override;
void onAboveHighWatermark() override;
Expand Down Expand Up @@ -393,7 +396,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
void onUrl(const char*, size_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
int onHeadersComplete(HeaderMapImplPtr&& headers) override;
void onBody(const char* data, size_t length) override;
void onMessageComplete() override;
void onMessageComplete(HeaderMapImplPtr&& trailers) override;
void onResetStream(StreamResetReason reason) override;
void sendProtocolError() override {}
void onAboveHighWatermark() override;
Expand Down
1 change: 1 addition & 0 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ Utility::parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOptions& co
ret.allow_absolute_url_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, allow_absolute_url, true);
ret.accept_http_10_ = config.accept_http_10();
ret.default_host_for_http_10_ = config.default_host_for_http_10();
ret.enable_trailers_ = config.enable_trailers();

if (config.header_key_format().has_proper_case_words()) {
ret.header_key_format_ = Http1Settings::HeaderKeyFormat::ProperCase;
Expand Down
41 changes: 27 additions & 14 deletions source/extensions/filters/http/grpc_http1_reverse_bridge/filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,21 +172,8 @@ Http::FilterDataStatus Filter::encodeData(Buffer::Instance& buffer, bool end_str
trailers.setGrpcStatus(grpc_status_);

if (withhold_grpc_frames_) {
// Compute the size of the payload and construct the length prefix.
//
// We do this even if the upstream failed: If the response returned non-200,
// we'll respond with a grpc-status with an error, so clients will know that the request
// was unsuccessful. Since we're guaranteed at this point to have a valid response
// (unless upstream lied in content-type) we attempt to return a well-formed gRPC
// response body.
const auto length = buffer.length() + buffer_.length();

std::array<uint8_t, Grpc::GRPC_FRAME_HEADER_SIZE> frame;
Grpc::Encoder().newFrame(Grpc::GRPC_FH_DEFAULT, length, frame);

buffer.prepend(buffer_);
Buffer::OwnedImpl frame_buffer(frame.data(), frame.size());
buffer.prepend(frame_buffer);
buildGrpcFrameHeader(buffer);
}

return Http::FilterDataStatus::Continue;
Expand All @@ -203,6 +190,32 @@ Http::FilterDataStatus Filter::encodeData(Buffer::Instance& buffer, bool end_str
}
}

Http::FilterTrailersStatus Filter::encodeTrailers(Http::HeaderMap& trailers) {
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
trailers.insertGrpcStatus().value(grpc_status_);

if (withhold_grpc_frames_) {
Chuongv marked this conversation as resolved.
Show resolved Hide resolved
buildGrpcFrameHeader(buffer_);
encoder_callbacks_->addEncodedData(buffer_, false);
}

return Http::FilterTrailersStatus::Continue;
}

void Filter::buildGrpcFrameHeader(Buffer::Instance& buffer) {
// Compute the size of the payload and construct the length prefix.
//
// We do this even if the upstream failed: If the response returned non-200,
// we'll respond with a grpc-status with an error, so clients will know that the request
// was unsuccessful. Since we're guaranteed at this point to have a valid response
// (unless upstream lied in content-type) we attempt to return a well-formed gRPC
// response body.
const auto length = buffer.length();
std::array<uint8_t, Grpc::GRPC_FRAME_HEADER_SIZE> frame;
Grpc::Encoder().newFrame(Grpc::GRPC_FH_DEFAULT, length, frame);
Buffer::OwnedImpl frame_buffer(frame.data(), frame.size());
buffer.prepend(frame_buffer);
}

} // namespace GrpcHttp1ReverseBridge
} // namespace HttpFilters
} // namespace Extensions
Expand Down
Loading