Skip to content

http_filter: add addEncodedTrailers and addDecodedTrailers #3980

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
mattklein123 merged 20 commits into from
Aug 14, 2018
Merged

http_filter: add addEncodedTrailers and addDecodedTrailers #3980

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
34bd3d1
http_filter: add addEncodedTrailers
Jul 28, 2018
688bc4d
mock out new method
Jul 28, 2018
e0b74f3
add addDecodedTrailers and use move semantics
Jul 31, 2018
acfa3d8
only allow trailers added in decodeData(true)
Aug 2, 2018
baee8aa
throw exceptions on bad access + add comments
Aug 3, 2018
42443b0
return by reference, throw exception on invalid usage
Aug 7, 2018
52c5780
expect exception in encode/decodeTrailers + add @throws comment
Aug 7, 2018
8495e41
asserts instead of exceptions
Aug 7, 2018
36103a1
add integration test for addEncodedTrailers
Aug 8, 2018
f6ca600
fix addDecodedTrailers
Aug 8, 2018
483b48b
only check for upstream trailers when upstream supports trailers
Aug 8, 2018
5412bb8
only execute callback for filters after filter that added trailers
Aug 9, 2018
cd25726
also keep track of filter for decode
Aug 9, 2018
c3a900b
test adding trailers in integration_test.cc
Aug 9, 2018
90c6233
fixup comments
Aug 9, 2018
3667d6a
formating
Aug 9, 2018
7637fa3
remove debug line
Aug 10, 2018
f2b6210
add @return comments
Aug 10, 2018
815a07b
update comments and other feedback
Aug 13, 2018
2321031
add const qualifier and update comment
Aug 14, 2018
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
10 changes: 10 additions & 0 deletions include/envoy/http/filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks {
*/
virtual void addDecodedData(Buffer::Instance& data, bool streaming_filter) PURE;

/**
* Adds decoded trailers. This will overwrite any existing trailers should any already exist.
*/

@alyssawilk alyssawilk Aug 8, 2018

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.

Sorry, I should have called this out before, but please doc the return as @return here and below

virtual void addDecodedTrailers(HeaderMapPtr&& trailers) PURE;

@lizan lizan Aug 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

still -1 to moving a pointer here, and prefer the first revision: HeaderMap& addDecodedTrailers() for reasons:

  • moving a pointer doesn't prevent filters from keeping reference to HeaderMap itself too long, so that still can happen. Returning a reference is already clear that Envoy core (ActiveStream) owns the HeaderMap.
  • the filter will now have to know implementations of HeaderMap (i.e. HeaderMapImpl) and instantiate, I know there is already a couple places doing this, mostly to deal with AsyncHttpClient, perhaps we shouldn't add another place requiring that?

@mattklein123 thoughts?

@mattklein123 mattklein123 Aug 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

moving a pointer doesn't prevent filters from keeping reference to HeaderMap itself too long, so that still can happen. Returning a reference is already clear that Envoy core (ActiveStream) owns the HeaderMap.

IMO move semantics make it clear that the pointer is now owned by the callee. We do this in a variety of places.

the filter will now have to know implementations of HeaderMap (i.e. HeaderMapImpl) and instantiate, I know there is already a couple places doing this, mostly to deal with AsyncHttpClient, perhaps we shouldn't add another place requiring that?

I think this is a better argument. Assuming with the new semantics we have discussed it's just as easy to return a reference to either the existing trailers or a newly created trailers, it sounds reasonable to me to switch it back to what we had before.

@snowp snowp Aug 2, 2018 •
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.

The one thing that's unclear to me is what would HeaderPtr& addEncodedTrailers() return if called before encodeTrailers(..., true)? would we just throw/assert?

@mattklein123 mattklein123 Aug 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I would throw some type of bad argument exception that will cause Envoy to crash and can be checked in tests. (I would probably not derive from EnvoyException and pick a std exception for this).

@alyssawilk alyssawilk Aug 2, 2018

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.

Is this out of date? With the code as it stands now, I think we can only call under decodeData with end_stream = true, and subsequent filters will never encounter that state (end stream will be false for future invocations) so should not be able to overwrite trailers.

I do think we should document when this is safe to call but if you prefer a TODO while the design is being hashed out that's fine :-)


/**
* Create a locally generated response using the provided response_code and body_text parameters.
* If the request was a gRPC request the local reply will be encoded as a gRPC response with a 200
Expand Down Expand Up @@ -395,6 +400,11 @@ class StreamEncoderFilterCallbacks : public virtual StreamFilterCallbacks {
*/
virtual void addEncodedData(Buffer::Instance& data, bool streaming_filter) PURE;

/**
* Adds encoded trailers. This will overwrite any existing trailers should any already exist.
*/
virtual void addEncodedTrailers(HeaderMapPtr&& trailers) PURE;

/**
* Called when an encoder filter goes over its high watermark.
*/
Expand Down
1 change: 1 addition & 0 deletions source/common/http/async_client_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
Tracing::Span& activeSpan() override { return active_span_; }
const Tracing::Config& tracingConfig() override { return tracing_config_; }
void continueDecoding() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void addDecodedTrailers(HeaderMapPtr&&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void addDecodedData(Buffer::Instance&, bool) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
const Buffer::Instance* decodingBuffer() override { return buffered_body_.get(); }
void sendLocalReply(Code code, const std::string& body,
Expand Down
60 changes: 56 additions & 4 deletions source/common/http/conn_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,15 +775,36 @@ void ConnectionManagerImpl::ActiveStream::decodeData(ActiveStreamDecoderFilter*

for (; entry != decoder_filters_.end(); entry++) {
ASSERT(!(state_.filter_call_state_ & FilterCallState::DecodeData));

bool end_stream_no_trailers = end_stream && !request_trailers_;

@alyssawilk alyssawilk Aug 2, 2018

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.

Mind adding a comment that we check the state of end stream inside the loop in case a filter adds trailers?

if (end_stream_no_trailers) {
state_.filter_call_state_ |= FilterCallState::LastDataFrame;
}

@alyssawilk alyssawilk Aug 2, 2018

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.

I'd argue changing HCM state probably bumps this up to a medium-risk change.

@mattklein123 mattklein123 Aug 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@snowp snowp Aug 3, 2018

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.

What does HCM stand for in this context?

@mattklein123 mattklein123 Aug 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Http Connection Manager (HCM)

state_.filter_call_state_ |= FilterCallState::DecodeData;
FilterDataStatus status = (*entry)->handle_->decodeData(data, end_stream);
FilterDataStatus status = (*entry)->handle_->decodeData(data, end_stream_no_trailers);
state_.filter_call_state_ &= ~FilterCallState::DecodeData;
if (end_stream_no_trailers) {
state_.filter_call_state_ &= ~FilterCallState::LastDataFrame;
}
ENVOY_STREAM_LOG(trace, "decode data called: filter={} status={}", *this,
static_cast<const void*>((*entry).get()), static_cast<uint64_t>(status));
if (!(*entry)->commonHandleAfterDataCallback(status, data, state_.decoder_filters_streaming_)) {
return;
}
}

if (end_stream && request_trailers_) {

@alyssawilk alyssawilk Aug 2, 2018

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.

again let's comment this is explicitly for the case that the remote side didn't send trailers but a filter added them.

decodeTrailers(filter, *request_trailers_);

@mattklein123 mattklein123 Aug 8, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't we need to actually only send trailers to the filter that set the trailer and filters after it? I think we need to actually track which filter added the trailers... (See how this is handled in the headers/data case).

}
}

void ConnectionManagerImpl::ActiveStream::addDecodedTrailers(HeaderMapPtr&& trailers) {
if (state_.filter_call_state_ & FilterCallState::LastDataFrame) {
request_trailers_ = std::move(trailers);

@alyssawilk alyssawilk Aug 2, 2018

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.

If I'm correct above that we can only set request_trailers_once now, can we also have an invariant ASSERT that they're empty when this is called?

} else {
// do nothing
// todo: should this throw/assert?

@alyssawilk alyssawilk Aug 2, 2018

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.

I think it should :-)

I also think we should have integration tests for both the happy path and over-enthusiastic filters. I think we can start on those now, because even if the APIs change a bit the end to end tests should stay the same

@mattklein123 mattklein123 Aug 3, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 same as my comment elsewhere. We should be doing this in a bunch of places (see some other TODOs from me) so we might as well start doing it now w/ tests.

}
}

void ConnectionManagerImpl::ActiveStream::addDecodedData(ActiveStreamDecoderFilter& filter,
Expand Down Expand Up @@ -1058,6 +1079,16 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte
}
}

void ConnectionManagerImpl::ActiveStream::addEncodedTrailers(HeaderMapPtr&& trailers) {
if (state_.filter_call_state_ & FilterCallState::LastDataFrame) {
response_trailers_ = std::move(trailers);
state_.local_complete_ = false;

@snowp snowp Aug 2, 2018

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.

this was necessary because state_.local_complete_ gets set to true at the start of encodeData, so we have to undo that so that the ASSERT in commonEncodePrefix doesn't fire at the start of encodeTrailers

@alyssawilk alyssawilk Aug 2, 2018

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.

If it's worth a comment in the PR it's worth a comment in the code :-)

} else {
// do nothing
// todo: should this throw/assert?
}
}

void ConnectionManagerImpl::ActiveStream::addEncodedData(ActiveStreamEncoderFilter& filter,
Buffer::Instance& data, bool streaming) {
if (state_.filter_call_state_ == 0 ||
Expand Down Expand Up @@ -1085,9 +1116,17 @@ void ConnectionManagerImpl::ActiveStream::encodeData(ActiveStreamEncoderFilter*
std::list<ActiveStreamEncoderFilterPtr>::iterator entry = commonEncodePrefix(filter, end_stream);
for (; entry != encoder_filters_.end(); entry++) {
ASSERT(!(state_.filter_call_state_ & FilterCallState::EncodeData));

bool end_stream_no_trailers = end_stream && !response_trailers_;
state_.filter_call_state_ |= FilterCallState::EncodeData;
FilterDataStatus status = (*entry)->handle_->encodeData(data, end_stream);
if (end_stream_no_trailers) {
state_.filter_call_state_ |= FilterCallState::LastDataFrame;
}
FilterDataStatus status = (*entry)->handle_->encodeData(data, end_stream_no_trailers);
state_.filter_call_state_ &= ~FilterCallState::EncodeData;
if (end_stream_no_trailers) {
state_.filter_call_state_ &= ~FilterCallState::LastDataFrame;
}
ENVOY_STREAM_LOG(trace, "encode data called: filter={} status={}", *this,
static_cast<const void*>((*entry).get()), static_cast<uint64_t>(status));
if (!(*entry)->commonHandleAfterDataCallback(status, data, state_.encoder_filters_streaming_)) {
Expand All @@ -1099,8 +1138,13 @@ void ConnectionManagerImpl::ActiveStream::encodeData(ActiveStreamEncoderFilter*
end_stream);

request_info_.addBytesSent(data.length());
response_encoder_->encodeData(data, end_stream);
maybeEndEncode(end_stream);
if (end_stream && response_trailers_) {

@mattklein123 mattklein123 Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm a little confused about the semantics here. When is a filter expected to add trailers for the first time? Do we want to support setting trailers during the final data frame where end_stream was otherwise true? I don't think we want to support that and we probably do? Also where is the parallel to this code on the decoding side?

@snowp snowp Aug 1, 2018 •
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.

What happens right now (since you can override trailers whenever)

  1. Trailers are set during header/data when upstream has no trailers: trailers get written out at the end of [en|de]codeData(_, true)
  2. Trailers are set during onHeader/onData when upstream HAS trailers: the original trailers override the trailers set previously (because in that case the upstream trailers are moved into response_trailers_ AFTER we've gone through the header/data callbacks). Trailers are written out at the end of [en|de]codeTrailers(_)
  3. Trailers are set during onTrailers: the trailers override those provided by upstream. Trailers are written out at the end of [en|de]codeTrailers(_)

3 is definitely confusing, so I'd be happy to prevent that. The question to me that remains is whether we want trailers to be written out at the end of [en|de]codeData(_, true) or if we want setting the trailers to cause [en|de]codeTrailers to be executed. I think based on your question about the decoding side that you were thinking more along the lines of the latter, because iirc that's what would allow the router to include it in the upstream request?

What steered me away from that originally was that was that it doesn't seem like addEncodedData will cause the onEncodedData callbacks to run if called in onHeaders(_, true).

So after thinking about it some, how about this:
At the end of [en|de]codeData(_, true) we trigger [en|de]codeTrailers if the trailers ptr is non-null. This should cover both encoding and decoding and we'd end up writing out the trailers to the encoder like I'm already doing. If addTrailers is called during the [en|de]codeTrailers(_) cb we can fail with an ASSERT.

This does mean we get both a [en|de]codeData(_, true) and [en|de]codeTrailers which might be confusing, but it would allow a filter to inject trailers when it can see that there aren't already any there (because it sees [en|de]codeData(_, true)).

@mattklein123 mattklein123 Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So after thinking about it some, how about this:
At the end of [en|de]codeData(, true) we trigger [en|de]codeTrailers if the trailers ptr is non-null. This should cover both encoding and decoding and we'd end up writing out the trailers to the encoder like I'm already doing. If addTrailers is called during the [en|de]codeTrailers() cb we can fail with an ASSERT.

I think this is more on the right track.

This does mean we get both a [en|de]codeData(, true) and [en|de]codeTrailers which might be confusing, but it would allow a filter to inject trailers when it can see that there aren't already any there (because it sees [en|de]codeData(, true)).

I don't think this is going to work as it will completely confuse filters that do things when end_stream is true. I think this is the behavior you want:

  1. Do what you said previously about detecting if there are trailers set during a decodeData call.
  2. If trailers get set by a decode data call, make sure that subsequent filter decodeData() calls have end_stream set to false (basically key that off of whether trailers is non-null).
  3. Then dispatch trailers.

I don't think this should be too hard to implement and should cleanly cover both encode/decode. Does that make sense?

For the error checking side of things, IMO I would probably just block trailers being added in any context other than decodeData(..., true). Does it make sense anywhere else? The otehr thing to consider is what if someone calls this when a filter has been paused and then calls continue? Will it work correctly?

@snowp snowp Aug 2, 2018

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.

Yeah that makes sense for the most part. One question:

I would probably just block trailers being added in any context other than decodeData(..., true).

Do you mean decodeData(..., false)? Allowing setting the trailers in decodeData(..., true) seems to contradict point 2

@mattklein123 mattklein123 Aug 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No I meant what I said, that I think the only valid context to add trailers should be within a *Data(..., true) call or outside of a direct call in a different callback before calling continue*(). What I mean by (2) is that an intermediate filter can add trailers in the context of data call with end_stream true, but subsequent filters should then see a data call with end_stream false, followed by a trailers call. I believe this captures the intent of what your filter needs to do and IMO is pretty clear. Does that make more sense?

@snowp snowp Aug 2, 2018

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.

Ah, that makes perfect sense. Thanks for clarifying.

response_encoder_->encodeData(data, false);
encodeTrailers(filter, *response_trailers_);

@mattklein123 mattklein123 Aug 8, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same comment here about keeping track of which filter added the trailers?

As an aside, I'm a little confused as to why the logic in encodeHeaders() does not mirror decodeHeaders() WRT to handling continue/last filter. (And yes I know I wrote this code!) I can look into this more tomorrow or Friday, but if you feel like taking a look that would be appreciated.

@snowp snowp Aug 10, 2018

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.

I looked into it a little bit: adding the same last filter check to encodHeaders causes a test failure in HttpConnectionManagerImplTest.HitRequestBufferLimitsIntermediateFilter because of a missing expectation for a call to response_encoder.encodeHeaders, and an integration test fails due to

[2018-08-09 23:36:14.552][694871][critical][assert] bazel-out/darwin-dbg/bin/source/common/request_info/_virtual_includes/request_info_lib/common/request_info/request_info_impl.h:83] assert failure: !first_downstream_tx_byte_sent_.

which presumably comes from hitting this code more than once (towards the end of encodeHeaders):

  // Now actually encode via the codec.
  request_info_.onFirstDownstreamTxByteSent(); // <- this bit
  response_encoder_->encodeHeaders(headers,
                                   end_stream && continue_data_entry == encoder_filters_.end());

The test that's failing is IpVersions/Http2IntegrationTest.HittingDecoderFilterLimit/IPv4 - I'm guessing the filter used there normally stops iteration, but by continuing on the last filter we end up running through the last part of encodeHeaders again. Seems like the issue is that somehow encodeTrailers is being called twice, and special casing the last filter causes the second call to hit onFirstDownstreamTxByteSent.

Unclear whether this is why we don't special case the last filter in encodeHeaders, or if it's just a issue that hasn't surfaced because we don't.

@mattklein123 mattklein123 Aug 10, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, thanks for looking. I can take a look tomorrow or this weekend more. I wouldn't worry about it for now but it seemed strange to me that it wasn't symmetrical and I couldn't remember why.

} else {
response_encoder_->encodeData(data, end_stream && !response_trailers_);
maybeEndEncode(end_stream);
}
}

void ConnectionManagerImpl::ActiveStream::encodeTrailers(ActiveStreamEncoderFilter* filter,
Expand Down Expand Up @@ -1380,6 +1424,10 @@ Buffer::WatermarkBufferPtr ConnectionManagerImpl::ActiveStreamDecoderFilter::cre
return buffer;
}

void ConnectionManagerImpl::ActiveStreamDecoderFilter::addDecodedTrailers(HeaderMapPtr&& trailers) {
parent_.addDecodedTrailers(std::move(trailers));
}

void ConnectionManagerImpl::ActiveStreamDecoderFilter::addDecodedData(Buffer::Instance& data,
bool streaming) {
parent_.addDecodedData(*this, data, streaming);
Expand Down Expand Up @@ -1473,6 +1521,10 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::addEncodedData(Buffer::In
return parent_.addEncodedData(*this, data, streaming);
}

void ConnectionManagerImpl::ActiveStreamEncoderFilter::addEncodedTrailers(HeaderMapPtr&& trailers) {
parent_.addEncodedTrailers(std::move(trailers));
}

void ConnectionManagerImpl::ActiveStreamEncoderFilter::
onEncoderFilterAboveWriteBufferHighWatermark() {
ENVOY_STREAM_LOG(debug, "Disabling upstream stream due to filter callbacks.", parent_);
Expand Down
7 changes: 7 additions & 0 deletions source/common/http/conn_manager_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,

// Http::StreamDecoderFilterCallbacks
void addDecodedData(Buffer::Instance& data, bool streaming) override;
void addDecodedTrailers(HeaderMapPtr&& trailers) override;
void continueDecoding() override;
const Buffer::Instance* decodingBuffer() override {
return parent_.buffered_request_data_.get();
Expand Down Expand Up @@ -229,6 +230,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,

// Http::StreamEncoderFilterCallbacks
void addEncodedData(Buffer::Instance& data, bool streaming) override;
void addEncodedTrailers(HeaderMapPtr&& trailers) override;
void onEncoderFilterAboveWriteBufferHighWatermark() override;
void onEncoderFilterBelowWriteBufferLowWatermark() override;
void setEncoderBufferLimit(uint32_t limit) override { parent_.setBufferLimit(limit); }
Expand Down Expand Up @@ -267,11 +269,13 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
commonEncodePrefix(ActiveStreamEncoderFilter* filter, bool end_stream);
const Network::Connection* connection();
void addDecodedData(ActiveStreamDecoderFilter& filter, Buffer::Instance& data, bool streaming);
void addDecodedTrailers(HeaderMapPtr&& trailers);
void decodeHeaders(ActiveStreamDecoderFilter* filter, HeaderMap& headers, bool end_stream);
void decodeData(ActiveStreamDecoderFilter* filter, Buffer::Instance& data, bool end_stream);
void decodeTrailers(ActiveStreamDecoderFilter* filter, HeaderMap& trailers);
void maybeEndDecode(bool end_stream);
void addEncodedData(ActiveStreamEncoderFilter& filter, Buffer::Instance& data, bool streaming);
void addEncodedTrailers(HeaderMapPtr&& trailers);
void sendLocalReply(bool is_grpc_request, Code code, const std::string& body,
std::function<void(HeaderMap& headers)> modify_headers);
void encode100ContinueHeaders(ActiveStreamEncoderFilter* filter, HeaderMap& headers);
Expand Down Expand Up @@ -339,6 +343,9 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
// to verify we do not encode100Continue headers more than once per
// filter.
static constexpr uint32_t Encode100ContinueHeaders = 0x40;
// Used to indicate that we're processing the final [en|de]Code frame,

@mattklein123 mattklein123 Aug 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: [En|De]codeData

// i.e. end_stream = true
static constexpr uint32_t LastDataFrame = 0x80;
};
// clang-format on

Expand Down
Loading