Skip to content

Commit

Permalink
compiles and passes current tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@goaway
goaway committed Sep 22, 2020
1 parent 58fa5d3 commit 7f89085
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 76 deletions.
2 changes: 1 addition & 1 deletion library/common/extensions/filters/http/platform_bridge/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ api_proto_package()
envoy_cc_library(
name = "platform_bridge_filter_lib",
srcs = [
"c_types.cc",
"c_type_definitions.h",
"filter.cc",
],
hdrs = [
Expand Down
6 changes: 1 addition & 5 deletions ...s/filters/http/platform_bridge/c_types.cc → ...http/platform_bridge/c_type_definitions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@

#include "envoy/http/filter.h"

const envoy_data envoy_unaltered_data = {0};

const envoy_headers envoy_unaltered_headers = {0};

const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinue =
static_cast<envoy_filter_headers_status_t>(Envoy::Http::FilterHeadersStatus::Continue);
const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopIteration =
Expand Down Expand Up @@ -46,5 +42,5 @@ const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusContinue =
const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIteration =
static_cast<envoy_filter_trailers_status_t>(Envoy::Http::FilterTrailersStatus::StopIteration);
// See comment above.
extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration =
const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration =
kEnvoyFilterTrailersStatusContinue - 1;
171 changes: 114 additions & 57 deletions library/common/extensions/filters/http/platform_bridge/filter.cc
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "library/common/extensions/filters/http/platform_bridge/filter.h"
#include "library/common/extensions/filters/http/platform_bridge/c_type_definitions.h"

#include "envoy/server/filter_config.h"

Expand Down Expand Up @@ -56,6 +57,17 @@ void PlatformBridgeFilter::onDestroy() {
platform_filter_.instance_context = nullptr;
}

void PlatformBridgeFilter::replaceHeaders(Http::HeaderMap& headers, envoy_headers c_headers) {
headers.clear();
for (envoy_header_size_t i = 0; i < c_headers.length; i++) {
headers.addCopy(
Http::LowerCaseString(Http::Utility::convertToString(c_headers.headers[i].key)),
Http::Utility::convertToString(c_headers.headers[i].value));
}
// The C envoy_headers struct can be released now because the headers have been copied.
release_envoy_headers(c_headers);
}

Http::FilterHeadersStatus PlatformBridgeFilter::onHeaders(Http::HeaderMap& headers, bool end_stream,
envoy_filter_on_headers_f on_headers) {
// Allow nullptr to act as no-op.
Expand All @@ -66,31 +78,33 @@ Http::FilterHeadersStatus PlatformBridgeFilter::onHeaders(Http::HeaderMap& heade
envoy_headers in_headers = Http::Utility::toBridgeHeaders(headers);
envoy_filter_headers_status result =
on_headers(in_headers, end_stream, platform_filter_.instance_context);
Http::FilterHeadersStatus status = static_cast<Http::FilterHeadersStatus>(result.status);
// TODO(goaway): Current platform implementations expose immutable headers, thus any modification
// necessitates a full copy. Add 'modified' bit to determine when we can elide the copy. See also
// https://github.com/lyft/envoy-mobile/issues/949 for potential future optimization.
headers.clear();
for (envoy_header_size_t i = 0; i < result.headers.length; i++) {
headers.addCopy(
Http::LowerCaseString(Http::Utility::convertToString(result.headers.headers[i].key)),
Http::Utility::convertToString(result.headers.headers[i].value));

switch (result.status) {
case kEnvoyFilterHeadersStatusContinue:
PlatformBridgeFilter::replaceHeaders(headers, result.headers);
return Http::FilterHeadersStatus::Continue;

case kEnvoyFilterHeadersStatusStopIteration:
iteration_state_ = IterationState::Stopped;
return Http::FilterHeadersStatus::StopIteration;

default:
PANIC("invalid filter state: unsupported status for platform filters");
}
// The C envoy_headers struct can be released now because the headers have been copied.
release_envoy_headers(result.headers);
return status;

NOT_REACHED_GCOVR_EXCL_LINE;
}

Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool end_stream,
Buffer::Instance* internal_buffer,
Http::HeaderMap** pending_headers,
envoy_filter_on_data_f on_data) {
// Allow nullptr to act as no-op.
if (on_data == nullptr) {
return Http::FilterDataStatus::Continue;
}

envoy_data in_data;

if (iteration_state_ == IterationState::Stopped && internal_buffer &&
internal_buffer->length() > 0) {
// Pre-emptively buffer data to present aggregate to platform.
Expand All @@ -101,27 +115,26 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool
}

envoy_filter_data_status result = on_data(in_data, end_stream, platform_filter_.instance_context);
Http::FilterDataStatus status = static_cast<Http::FilterDataStatus>(result.status);
switch (status) {
case Http::FilterDataStatus::Continue:

switch (result.status) {
case kEnvoyFilterDataStatusContinue:
if (iteration_state_ == IterationState::Stopped) {
// When platform filter iteration is Stopped, Resume must be used to start iterating again.
// TODO(goaway): decide on the means to surface/handle errors here. Options include:
// - crashing
// - creating an error response for this stream
// - letting Envoy handle any invalid resulting state via its own guards
PANIC("invalid filter state: filter iteration must be resumed with ResumeIteration");
}
break;
case Http::FilterDataStatus::StopIterationAndBuffer:
data.drain(data.length());
data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data));
return Http::FilterDataStatus::Continue;

case kEnvoyFilterDataStatusStopIterationAndBuffer:
if (iteration_state_ == IterationState::Stopped) {
// Data will already have been buffered (above).
status = Http::FilterDataStatus::StopIterationNoBuffer;
} else {
// Data will be buffered on return.
iteration_state_ = IterationState::Stopped;
return Http::FilterDataStatus::StopIterationNoBuffer;
}
break;
case Http::FilterDataStatus::StopIterationNoBuffer:
// Data will be buffered on return.
iteration_state_ = IterationState::Stopped;
return Http::FilterDataStatus::StopIterationAndBuffer;

case kEnvoyFilterDataStatusStopIterationNoBuffer:
// In this context all previously buffered data can/should be dropped. If no data has been
// buffered, this is a no-op. If data was previously buffered, the most likely case is
// that a filter has decided to handle generating a response itself and no longer needs it.
Expand All @@ -133,24 +146,32 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool
internal_buffer->drain(internal_buffer->length());
}
iteration_state_ = IterationState::Stopped;
break;
default:
PANIC("unsupported status for platform filters");
}
return Http::FilterDataStatus::StopIterationNoBuffer;

// TODO(goaway): Current platform implementations expose immutable data, thus any modification
// necessitates a full copy. Add 'modified' bit to determine when we can elide the copy. See also
// https://github.com/lyft/envoy-mobile/issues/949 for potential future optimization.
if (iteration_state_ == IterationState::Ongoing) {
data.drain(data.length());
data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data));
case kEnvoyFilterDataStatusResumeIteration:
if (iteration_state_ != IterationState::Stopped) {
PANIC("invalid filter state: ResumeIteration may only be used when filter iteration is stopped");
}
if (result.extra_headers) {
PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.extra_headers);
*pending_headers = nullptr;
free(result.extra_headers);
}
internal_buffer->drain(internal_buffer->length());
internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data));
return Http::FilterDataStatus::Continue;

default:
PANIC("invalid filter state: unsupported status for platform filters");
}

return status;
NOT_REACHED_GCOVR_EXCL_LINE;
}

Http::FilterTrailersStatus
PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers,
Buffer::Instance* internal_buffer,
Http::HeaderMap** pending_headers,
envoy_filter_on_trailers_f on_trailers) {
// Allow nullptr to act as no-op.
if (on_trailers == nullptr) {
Expand All @@ -159,19 +180,41 @@ PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers,

envoy_headers in_trailers = Http::Utility::toBridgeHeaders(trailers);
envoy_filter_trailers_status result = on_trailers(in_trailers, platform_filter_.instance_context);
Http::FilterTrailersStatus status = static_cast<Http::FilterTrailersStatus>(result.status);
// TODO(goaway): Current platform implementations expose immutable trailers, thus any modification
// necessitates a full copy. Add 'modified' bit to determine when we can elide the copy. See also
// https://github.com/lyft/envoy-mobile/issues/949 for potential future optimization.
trailers.clear();
for (envoy_header_size_t i = 0; i < result.trailers.length; i++) {
trailers.addCopy(
Http::LowerCaseString(Http::Utility::convertToString(result.trailers.headers[i].key)),
Http::Utility::convertToString(result.trailers.headers[i].value));
}
// The C envoy_trailers struct can be released now because the trailers have been copied.
release_envoy_headers(result.trailers);
return status;

switch (result.status) {
case kEnvoyFilterTrailersStatusContinue:
if (iteration_state_ == IterationState::Stopped) {
PANIC("invalid filter state: ResumeIteration may only be used when filter iteration is stopped");
}
PlatformBridgeFilter::replaceHeaders(trailers, result.trailers);
return Http::FilterTrailersStatus::Continue;

case kEnvoyFilterTrailersStatusStopIteration:
iteration_state_ = IterationState::Stopped;
return Http::FilterTrailersStatus::StopIteration;

case kEnvoyFilterTrailersStatusResumeIteration:
if (iteration_state_ != IterationState::Stopped) {
PANIC("invalid filter state: ResumeIteration may only be used when filter iteration is stopped");
}
if (result.extra_headers) {
PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.extra_headers);
*pending_headers = nullptr;
free(result.extra_headers);
}
if (result.extra_data) {
internal_buffer->drain(internal_buffer->length());
internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(*result.extra_data));
free(result.extra_data);
}
PlatformBridgeFilter::replaceHeaders(trailers, result.trailers);
return Http::FilterTrailersStatus::Continue;

default:
PANIC("invalid filter state: unsupported status for platform filters");
}

NOT_REACHED_GCOVR_EXCL_LINE;
}

Http::FilterHeadersStatus PlatformBridgeFilter::decodeHeaders(Http::RequestHeaderMap& headers,
Expand Down Expand Up @@ -203,7 +246,7 @@ Http::FilterDataStatus PlatformBridgeFilter::decodeData(Buffer::Instance& data,
});
}

return onData(data, end_stream, internal_buffer, platform_filter_.on_request_data);
return onData(data, end_stream, internal_buffer, &pending_request_headers_, platform_filter_.on_request_data);
}

Http::FilterDataStatus PlatformBridgeFilter::encodeData(Buffer::Instance& data, bool end_stream) {
Expand All @@ -215,12 +258,19 @@ Http::FilterDataStatus PlatformBridgeFilter::encodeData(Buffer::Instance& data,
});
}

return onData(data, end_stream, internal_buffer, platform_filter_.on_response_data);
return onData(data, end_stream, internal_buffer, &pending_response_headers_, platform_filter_.on_response_data);
}

Http::FilterTrailersStatus PlatformBridgeFilter::decodeTrailers(Http::RequestTrailerMap& trailers) {
// Delegate to shared implementation for request and response path.
auto status = onTrailers(trailers, platform_filter_.on_request_trailers);
Buffer::Instance* internal_buffer = nullptr;
if (decoder_callbacks_->decodingBuffer()) {
decoder_callbacks_->modifyDecodingBuffer([&internal_buffer](Buffer::Instance& mutable_buffer) {
internal_buffer = &mutable_buffer;
});
}

auto status = onTrailers(trailers, internal_buffer, &pending_request_headers_, platform_filter_.on_request_trailers);
if (status == Http::FilterTrailersStatus::StopIteration) {
pending_request_trailers_ = &trailers;
}
Expand All @@ -230,7 +280,14 @@ Http::FilterTrailersStatus PlatformBridgeFilter::decodeTrailers(Http::RequestTra
Http::FilterTrailersStatus
PlatformBridgeFilter::encodeTrailers(Http::ResponseTrailerMap& trailers) {
// Delegate to shared implementation for request and response path.
auto status = onTrailers(trailers, platform_filter_.on_response_trailers);
Buffer::Instance* internal_buffer = nullptr;
if (encoder_callbacks_->encodingBuffer()) {
encoder_callbacks_->modifyEncodingBuffer([&internal_buffer](Buffer::Instance& mutable_buffer) {
internal_buffer = &mutable_buffer;
});
}

auto status = onTrailers(trailers, internal_buffer, &pending_response_headers_, platform_filter_.on_response_trailers);
if (status == Http::FilterTrailersStatus::StopIteration) {
pending_response_trailers_ = &trailers;
}
Expand Down
15 changes: 10 additions & 5 deletions library/common/extensions/filters/http/platform_bridge/filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,24 @@ class PlatformBridgeFilter final : public Http::PassThroughFilter,
Http::FilterTrailersStatus encodeTrailers(Http::ResponseTrailerMap& trailers) override;

private:
static void replaceHeaders(Http::HeaderMap& headers, envoy_headers c_headers);
Http::FilterHeadersStatus onHeaders(Http::HeaderMap& headers, bool end_stream,
envoy_filter_on_headers_f on_headers);
Http::FilterDataStatus onData(Buffer::Instance& data, bool end_stream,
Buffer::Instance* internal_buffer, envoy_filter_on_data_f on_data);
Buffer::Instance* internal_buffer,
Http::HeaderMap** pending_headers,
envoy_filter_on_data_f on_data);
Http::FilterTrailersStatus onTrailers(Http::HeaderMap& trailers,
Buffer::Instance* internal_buffer,
Http::HeaderMap** pending_headers,
envoy_filter_on_trailers_f on_trailers);
const std::string filter_name_;
IterationState iteration_state_;
envoy_http_filter platform_filter_;
Http::RequestHeaderMap* pending_request_headers_{};
Http::ResponseHeaderMap* pending_response_headers_{};
Http::RequestTrailerMap* pending_request_trailers_{};
Http::ResponseTrialerMap* pending_response_trailers_{};
Http::HeaderMap* pending_request_headers_{};
Http::HeaderMap* pending_response_headers_{};
Http::HeaderMap* pending_request_trailers_{};
Http::HeaderMap* pending_response_trailers_{};
};

} // namespace PlatformBridge
Expand Down
16 changes: 8 additions & 8 deletions test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ TEST_F(PlatformBridgeFilterTest, BasicContinueOnRequestData) {
EXPECT_EQ(to_string(c_data), "request body");
EXPECT_TRUE(end_stream);
invocations->on_request_data_calls++;
return {kEnvoyFilterDataStatusContinue, c_data};
return {kEnvoyFilterDataStatusContinue, c_data, nullptr};
};

setUpFilter(R"EOF(
Expand Down Expand Up @@ -196,7 +196,7 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferOnRequestData) {
EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls++]);
EXPECT_FALSE(end_stream);
c_data.release(c_data.context);
return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata};
return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr};
};

Buffer::OwnedImpl decoding_buffer;
Expand Down Expand Up @@ -250,7 +250,7 @@ TEST_F(PlatformBridgeFilterTest, StopNoBufferOnRequestData) {
EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls++]);
EXPECT_FALSE(end_stream);
c_data.release(c_data.context);
return {kEnvoyFilterDataStatusStopIterationNoBuffer, envoy_nodata};
return {kEnvoyFilterDataStatusStopIterationNoBuffer, envoy_nodata, nullptr};
};

setUpFilter(R"EOF(
Expand Down Expand Up @@ -289,7 +289,7 @@ TEST_F(PlatformBridgeFilterTest, BasicContinueOnRequestTrailers) {
EXPECT_EQ(to_string(c_trailers.headers[0].key), "x-test-trailer");
EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer");
invocations->on_request_trailers_calls++;
return {kEnvoyFilterTrailersStatusContinue, c_trailers};
return {kEnvoyFilterTrailersStatusContinue, c_trailers, nullptr, nullptr};
};

setUpFilter(R"EOF(
Expand Down Expand Up @@ -353,7 +353,7 @@ TEST_F(PlatformBridgeFilterTest, BasicContinueOnResponseData) {
EXPECT_EQ(to_string(c_data), "response body");
EXPECT_TRUE(end_stream);
invocations->on_response_data_calls++;
return {kEnvoyFilterDataStatusContinue, c_data};
return {kEnvoyFilterDataStatusContinue, c_data, nullptr};
};

setUpFilter(R"EOF(
Expand Down Expand Up @@ -384,7 +384,7 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferOnResponseData) {
EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls++]);
EXPECT_FALSE(end_stream);
c_data.release(c_data.context);
return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata};
return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr};
};

Buffer::OwnedImpl encoding_buffer;
Expand Down Expand Up @@ -438,7 +438,7 @@ TEST_F(PlatformBridgeFilterTest, StopNoBufferOnResponseData) {
EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls++]);
EXPECT_FALSE(end_stream);
c_data.release(c_data.context);
return {kEnvoyFilterDataStatusStopIterationNoBuffer, envoy_nodata};
return {kEnvoyFilterDataStatusStopIterationNoBuffer, envoy_nodata, nullptr};
};

setUpFilter(R"EOF(
Expand Down Expand Up @@ -477,7 +477,7 @@ TEST_F(PlatformBridgeFilterTest, BasicContinueOnResponseTrailers) {
EXPECT_EQ(to_string(c_trailers.headers[0].key), "x-test-trailer");
EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer");
invocations->on_response_trailers_calls++;
return {kEnvoyFilterTrailersStatusContinue, c_trailers};
return {kEnvoyFilterTrailersStatusContinue, c_trailers, nullptr, nullptr};
};

setUpFilter(R"EOF(
Expand Down

0 comments on commit 7f89085

Please sign in to comment.