From 6ff3659e75c914c86d706aa6c84aab1be6ddbb99 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Mon, 21 Sep 2020 22:06:38 -0700 Subject: [PATCH 01/21] filters: add support for resuming iteration with mutations to pending HTTP entities Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/c_types.cc | 19 +++++++++ .../filters/http/platform_bridge/c_types.h | 6 +++ .../filters/http/platform_bridge/filter.cc | 42 +++++++++++++------ .../filters/http/platform_bridge/filter.h | 4 ++ library/common/types/c_types.cc | 2 + library/common/types/c_types.h | 5 ++- .../envoyproxy/envoymobile/filters/Filter.kt | 9 ++-- .../filters/FilterTrailersStatus.kt | 2 +- library/objective-c/EnvoyBridgeUtility.h | 8 ++++ library/objective-c/EnvoyEngineImpl.m | 29 +++++++++---- library/swift/src/filters/Filter.swift | 16 +++---- 11 files changed, 106 insertions(+), 36 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.cc b/library/common/extensions/filters/http/platform_bridge/c_types.cc index adbb213d93..e437f7b147 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.cc +++ b/library/common/extensions/filters/http/platform_bridge/c_types.cc @@ -13,6 +13,19 @@ const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinueAndEndStrea const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopAllIterationAndBuffer = static_cast( Envoy::Http::FilterHeadersStatus::StopAllIterationAndBuffer); +// ResumeIteration is not a status supported by Envoy itself, and only has relevance in Envoy +// Mobile's implementation of platform filters. +// +// Regarding enum values, the C++11 standard (7.2/2) states: +// If the first enumerator has no initializer, the value of the corresponding constant is zero. +// An enumerator-definition without an initializer gives the enumerator the value obtained by +// increasing the value of the previous enumerator by one. +// +// Creating a new return status like this is brittle, but at least somewhat more resilient to +// a new status being added in Envoy, since it won't overlap as long as the new status is added +// rather than prepended. +const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusResumeIteration = + kEnvoyFilterHeadersStatusContinue - 1; const envoy_filter_data_status_t kEnvoyFilterDataStatusContinue = static_cast(Envoy::Http::FilterDataStatus::Continue); @@ -20,8 +33,14 @@ const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationAndBuffer = static_cast(Envoy::Http::FilterDataStatus::StopIterationAndBuffer); const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationNoBuffer = static_cast(Envoy::Http::FilterDataStatus::StopIterationNoBuffer); +// See comment above. +const envoy_filter_data_status_t kEnvoyFilterDataStatusResumeIteration = + kEnvoyFilterDataStatusContinue - 1; const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusContinue = static_cast(Envoy::Http::FilterTrailersStatus::Continue); const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIteration = static_cast(Envoy::Http::FilterTrailersStatus::StopIteration); +// See comment above. +extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration = + kEnvoyFilterTrailersStatusContinue - 1; diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.h b/library/common/extensions/filters/http/platform_bridge/c_types.h index ca5e03d4e4..a381117c64 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.h +++ b/library/common/extensions/filters/http/platform_bridge/c_types.h @@ -12,6 +12,7 @@ extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinue; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopIteration; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinueAndEndStream; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; +extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusResumeIteration; /** * Compound return type for on-headers filter invocations. @@ -28,6 +29,7 @@ typedef int envoy_filter_data_status_t; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusContinue; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationAndBuffer; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationNoBuffer; +extern const envoy_filter_data_status_t kEnvoyFilterDataStatusResumeIteration; /** * Compound return type for on-data filter invocations. @@ -35,6 +37,7 @@ extern const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationNoBuf typedef struct { envoy_filter_data_status_t status; envoy_data data; + envoy_headers extra_headers; } envoy_filter_data_status; /** @@ -43,6 +46,7 @@ typedef struct { typedef int envoy_filter_trailers_status_t; extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusContinue; extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIteration; +extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration; /** * Compound return type for on-trailers filter invocations. @@ -50,6 +54,8 @@ extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIterat typedef struct { envoy_filter_trailers_status_t status; envoy_headers trailers; + envoy_headers extra_headers; + envoy_data extra_data; } envoy_filter_trailers_status; #ifdef __cplusplus diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index f4f4a95ec2..96edc15755 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -177,7 +177,21 @@ PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers, Http::FilterHeadersStatus PlatformBridgeFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool end_stream) { // Delegate to shared implementation for request and response path. - return onHeaders(headers, end_stream, platform_filter_.on_request_headers); + auto status = onHeaders(headers, end_stream, platform_filter_.on_request_headers); + if (status == Http::FilterHeadersStatus::StopIteration) { + pending_request_headers_ = &headers; + } + return status; +} + +Http::FilterHeadersStatus PlatformBridgeFilter::encodeHeaders(Http::ResponseHeaderMap& headers, + bool end_stream) { + // Delegate to shared implementation for request and response path. + auto status = onHeaders(headers, end_stream, platform_filter_.on_response_headers); + if (status == Http::FilterHeadersStatus::StopIteration) { + pending_response_headers_ = &headers; + } + return status; } Http::FilterDataStatus PlatformBridgeFilter::decodeData(Buffer::Instance& data, bool end_stream) { @@ -192,17 +206,6 @@ Http::FilterDataStatus PlatformBridgeFilter::decodeData(Buffer::Instance& data, return onData(data, end_stream, internal_buffer, platform_filter_.on_request_data); } -Http::FilterTrailersStatus PlatformBridgeFilter::decodeTrailers(Http::RequestTrailerMap& trailers) { - // Delegate to shared implementation for request and response path. - return onTrailers(trailers, platform_filter_.on_request_trailers); -} - -Http::FilterHeadersStatus PlatformBridgeFilter::encodeHeaders(Http::ResponseHeaderMap& headers, - bool end_stream) { - // Delegate to shared implementation for request and response path. - return onHeaders(headers, end_stream, platform_filter_.on_response_headers); -} - Http::FilterDataStatus PlatformBridgeFilter::encodeData(Buffer::Instance& data, bool end_stream) { // Delegate to shared implementation for request and response path. Buffer::Instance* internal_buffer = nullptr; @@ -215,10 +218,23 @@ Http::FilterDataStatus PlatformBridgeFilter::encodeData(Buffer::Instance& data, return onData(data, end_stream, internal_buffer, 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); + if (status == Http::FilterTrailersStatus::StopIteration) { + pending_request_trailers_ = &trailers; + } + return status; +} + Http::FilterTrailersStatus PlatformBridgeFilter::encodeTrailers(Http::ResponseTrailerMap& trailers) { // Delegate to shared implementation for request and response path. - return onTrailers(trailers, platform_filter_.on_response_trailers); + auto status = onTrailers(trailers, platform_filter_.on_response_trailers); + if (status == Http::FilterTrailersStatus::StopIteration) { + pending_response_trailers_ = &trailers; + } + return status; } } // namespace PlatformBridge diff --git a/library/common/extensions/filters/http/platform_bridge/filter.h b/library/common/extensions/filters/http/platform_bridge/filter.h index f19f3455cd..c6a0a4910b 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.h +++ b/library/common/extensions/filters/http/platform_bridge/filter.h @@ -64,6 +64,10 @@ class PlatformBridgeFilter final : public Http::PassThroughFilter, 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_{}; }; } // namespace PlatformBridge diff --git a/library/common/types/c_types.cc b/library/common/types/c_types.cc index 254dcafcd9..a3694a07f1 100644 --- a/library/common/types/c_types.cc +++ b/library/common/types/c_types.cc @@ -58,3 +58,5 @@ envoy_data copy_envoy_data(size_t length, const uint8_t* src_bytes) { } const envoy_data envoy_nodata = {0, NULL, envoy_noop_release, NULL}; + +const envoy_headers envoy_noheaders = {0, NULL}; diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index da263b4144..4299fce61d 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -149,7 +149,10 @@ envoy_data copy_envoy_data(size_t length, const uint8_t* src_bytes); // For example when sending a headers-only request. extern const envoy_data envoy_nodata; -/** +// Convenience constant to pass to function calls with no headers. +extern const envoy_headers envoy_noheaders; + +/* * Error struct. */ typedef struct { diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt b/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt index c50acf9567..9f62b8b828 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt @@ -53,7 +53,7 @@ internal class EnvoyHTTPFilterAdapter( is FilterDataStatus.Continue<*> -> arrayOf(result.status, result.data) is FilterDataStatus.StopIterationAndBuffer<*> -> arrayOf(result.status, data) is FilterDataStatus.StopIterationNoBuffer<*> -> arrayOf(result.status, data) - is FilterDataStatus.ResumeIteration<*> -> arrayOf(result.status, result.data) + is FilterDataStatus.ResumeIteration<*> -> arrayOf(result.status, result.headers?.headers, result.data) } } return arrayOf(0, data) @@ -66,7 +66,7 @@ internal class EnvoyHTTPFilterAdapter( is FilterDataStatus.Continue<*> -> arrayOf(result.status, result.data) is FilterDataStatus.StopIterationAndBuffer<*> -> arrayOf(result.status, data) is FilterDataStatus.StopIterationNoBuffer<*> -> arrayOf(result.status, data) - is FilterDataStatus.ResumeIteration<*> -> arrayOf(result.status, result.data) + is FilterDataStatus.ResumeIteration<*> -> arrayOf(result.status, result.headers?.headers, result.data) } } return arrayOf(0, data) @@ -78,7 +78,7 @@ internal class EnvoyHTTPFilterAdapter( return when (result) { is FilterTrailersStatus.Continue<*, *> -> arrayOf(result.status, result.trailers.headers) is FilterTrailersStatus.StopIteration<*, *> -> arrayOf(result.status, trailers) - is FilterTrailersStatus.ResumeIteration<*, *> -> arrayOf(result.status, result.trailers!!.headers) + is FilterTrailersStatus.ResumeIteration<*, *> -> arrayOf(result.status, result.headers?.headers, result.data, result.trailers.headers) } } return arrayOf(0, trailers) @@ -90,7 +90,8 @@ internal class EnvoyHTTPFilterAdapter( return when (result) { is FilterTrailersStatus.Continue<*, *> -> arrayOf(result.status, result.trailers.headers) is FilterTrailersStatus.StopIteration<*, *> -> arrayOf(result.status, trailers) - is FilterTrailersStatus.ResumeIteration<*, *> -> arrayOf(result.status, result.trailers!!.headers) + is FilterTrailersStatus.ResumeIteration<*, *> -> arrayOf(result.status, result.headers?.headers, result.data, result.trailers.headers) + } } return arrayOf(0, trailers) diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/filters/FilterTrailersStatus.kt b/library/kotlin/src/io/envoyproxy/envoymobile/filters/FilterTrailersStatus.kt index 1a68a5a47c..e4a1fe93d7 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/filters/FilterTrailersStatus.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/filters/FilterTrailersStatus.kt @@ -40,6 +40,6 @@ sealed class FilterTrailersStatus( class ResumeIteration( val headers: T?, val data: ByteBuffer?, - val trailers: U? + val trailers: U ) : FilterTrailersStatus(-1) } diff --git a/library/objective-c/EnvoyBridgeUtility.h b/library/objective-c/EnvoyBridgeUtility.h index cc9a3608f1..0c3a51ecd8 100644 --- a/library/objective-c/EnvoyBridgeUtility.h +++ b/library/objective-c/EnvoyBridgeUtility.h @@ -3,6 +3,10 @@ #import "library/common/types/c_types.h" static inline envoy_data toNativeData(NSData *data) { + if (data == nil) { + return envoy_nodata; + } + uint8_t *native_bytes = (uint8_t *)safe_malloc(sizeof(uint8_t) * data.length); memcpy(native_bytes, data.bytes, data.length); envoy_data ret = {data.length, native_bytes, free, native_bytes}; @@ -18,6 +22,10 @@ static inline envoy_data toManagedNativeString(NSString *s) { } static inline envoy_headers toNativeHeaders(EnvoyHeaders *headers) { + if (headers == nil) { + return envoy_noheaders; + } + envoy_header_size_t length = 0; for (NSString *headerKey in headers) { length += [headers[headerKey] count]; diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 5848b5d78a..3f7564d24c 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -51,7 +51,6 @@ static void ios_on_exit(void *context) { } EnvoyHeaders *platformHeaders = to_ios_headers(headers); - // TODO(goaway): consider better solution for compound return NSArray *result = filter.onResponseHeaders(platformHeaders, end_stream); return (envoy_filter_headers_status){/*status*/ [result[0] intValue], /*headers*/ toNativeHeaders(result[1])}; @@ -62,13 +61,15 @@ static envoy_filter_data_status ios_http_filter_on_request_data(envoy_data data, EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onRequestData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, - /*data*/ data}; + /*data*/ data, + /*extra_headers*/ envoy_noheaders; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onRequestData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1])}; + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; } static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, @@ -76,13 +77,15 @@ static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onResponseData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, - /*data*/ data}; + /*data*/ data, + /*extra_headers*/ envoy_noheaders}; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onResponseData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1])}; + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; } static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, @@ -90,13 +93,17 @@ static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_he EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onRequestTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers}; + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_trailers*/ envoy_nodata}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onRequestTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1])}; + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; } static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, @@ -104,13 +111,17 @@ static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_h EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onResponseTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers}; + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_data*/ envoy_nodata}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onResponseTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1])}; + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; } static void ios_http_filter_release(const void *context) { diff --git a/library/swift/src/filters/Filter.swift b/library/swift/src/filters/Filter.swift index 504fb015ed..0affbe5209 100644 --- a/library/swift/src/filters/Filter.swift +++ b/library/swift/src/filters/Filter.swift @@ -39,8 +39,8 @@ extension EnvoyHTTPFilter { return [kEnvoyFilterDataStatusStopIterationAndBuffer, data] case .stopIterationNoBuffer: return [kEnvoyFilterDataStatusStopIterationNoBuffer, data] - case .resumeIteration(_, let data): - return [kEnvoyFilterDataStatusContinue, data] + case .resumeIteration(let headers, let data): + return [kEnvoyFilterDataStatusContinue, headers?.headers, data] } } @@ -51,8 +51,8 @@ extension EnvoyHTTPFilter { return [kEnvoyFilterTrailersStatusContinue, trailers.headers] case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] - case .resumeIteration(_, _, let trailers): - return [kEnvoyFilterTrailersStatusContinue, trailers.headers] + case .resumeIteration(let headers, let data, let trailers): + return [kEnvoyFilterTrailersStatusContinue, headers?.headers, data, trailers.headers] } } } @@ -78,8 +78,8 @@ extension EnvoyHTTPFilter { return [kEnvoyFilterDataStatusStopIterationAndBuffer, data] case .stopIterationNoBuffer: return [kEnvoyFilterDataStatusStopIterationNoBuffer, data] - case .resumeIteration(_, let data): - return [kEnvoyFilterDataStatusContinue, data] + case .resumeIteration(let headers, let data): + return [kEnvoyFilterDataStatusContinue, headers?.headers, data] } } @@ -90,8 +90,8 @@ extension EnvoyHTTPFilter { return [kEnvoyFilterTrailersStatusContinue, trailers.headers] case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] - case .resumeIteration(_, _, let trailers): - return [kEnvoyFilterTrailersStatusContinue, trailers.headers] + case .resumeIteration(let headers, let data, let trailers): + return [kEnvoyFilterTrailersStatusContinue, headers?.headers, data, trailers.headers] } } } From 5e6eef299bccf958c3dc73812de8d56ff7645750 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 22 Sep 2020 22:56:40 +0800 Subject: [PATCH 02/21] compiles and passes tests Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/BUILD | 2 +- .../{c_types.cc => c_type_definitions.h} | 6 +- .../filters/http/platform_bridge/c_types.h | 16 +- .../filters/http/platform_bridge/filter.cc | 179 +++++++---- .../filters/http/platform_bridge/filter.h | 14 +- .../envoyproxy/envoymobile/filters/Filter.kt | 13 +- library/objective-c/EnvoyEngineImpl.m | 287 +++++++++--------- .../platform_bridge_filter_test.cc | 16 +- 8 files changed, 304 insertions(+), 229 deletions(-) rename library/common/extensions/filters/http/platform_bridge/{c_types.cc => c_type_definitions.h} (96%) diff --git a/library/common/extensions/filters/http/platform_bridge/BUILD b/library/common/extensions/filters/http/platform_bridge/BUILD index 93892c6802..f0fac266c3 100644 --- a/library/common/extensions/filters/http/platform_bridge/BUILD +++ b/library/common/extensions/filters/http/platform_bridge/BUILD @@ -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 = [ diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.cc b/library/common/extensions/filters/http/platform_bridge/c_type_definitions.h similarity index 96% rename from library/common/extensions/filters/http/platform_bridge/c_types.cc rename to library/common/extensions/filters/http/platform_bridge/c_type_definitions.h index e437f7b147..25adb2f30e 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.cc +++ b/library/common/extensions/filters/http/platform_bridge/c_type_definitions.h @@ -1,8 +1,8 @@ // NOLINT(namespace-envoy) -#include "library/common/extensions/filters/http/platform_bridge/c_types.h" - #include "envoy/http/filter.h" +#include "library/common/extensions/filters/http/platform_bridge/c_types.h" + const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinue = static_cast(Envoy::Http::FilterHeadersStatus::Continue); const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopIteration = @@ -42,5 +42,5 @@ const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusContinue = const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIteration = static_cast(Envoy::Http::FilterTrailersStatus::StopIteration); // See comment above. -extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration = +const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration = kEnvoyFilterTrailersStatusContinue - 1; diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.h b/library/common/extensions/filters/http/platform_bridge/c_types.h index a381117c64..734db99d6a 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.h +++ b/library/common/extensions/filters/http/platform_bridge/c_types.h @@ -4,6 +4,16 @@ // NOLINT(namespace-envoy) +/** + * Convenience constant indicating no changes to data. + */ +extern const envoy_data envoy_unaltered_data; + +/** + * Convenience constant indicating no changes to headers. + */ +extern const envoy_headers envoy_unaltered_headers; + /** * Return codes for on-headers filter invocations. @see envoy/http/filter.h */ @@ -37,7 +47,7 @@ extern const envoy_filter_data_status_t kEnvoyFilterDataStatusResumeIteration; typedef struct { envoy_filter_data_status_t status; envoy_data data; - envoy_headers extra_headers; + envoy_headers* extra_headers; } envoy_filter_data_status; /** @@ -54,8 +64,8 @@ extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIter typedef struct { envoy_filter_trailers_status_t status; envoy_headers trailers; - envoy_headers extra_headers; - envoy_data extra_data; + envoy_headers* extra_headers; + envoy_data* extra_data; } envoy_filter_trailers_status; #ifdef __cplusplus diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 96edc15755..f34598cd93 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -8,6 +8,7 @@ #include "library/common/api/external.h" #include "library/common/buffer/bridge_fragment.h" #include "library/common/buffer/utility.h" +#include "library/common/extensions/filters/http/platform_bridge/c_type_definitions.h" #include "library/common/http/header_utility.h" namespace Envoy { @@ -56,6 +57,16 @@ 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. @@ -66,23 +77,26 @@ 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(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) { @@ -90,7 +104,6 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool } envoy_data in_data; - if (iteration_state_ == IterationState::Stopped && internal_buffer && internal_buffer->length() > 0) { // Pre-emptively buffer data to present aggregate to platform. @@ -101,27 +114,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(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. @@ -133,24 +145,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, +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) { @@ -159,19 +179,44 @@ 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(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, @@ -203,7 +248,8 @@ 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) { @@ -215,12 +261,21 @@ 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; } @@ -230,7 +285,15 @@ 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; } diff --git a/library/common/extensions/filters/http/platform_bridge/filter.h b/library/common/extensions/filters/http/platform_bridge/filter.h index c6a0a4910b..06bbc69d29 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.h +++ b/library/common/extensions/filters/http/platform_bridge/filter.h @@ -55,19 +55,23 @@ 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 diff --git a/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt b/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt index 9f62b8b828..9d96b432e3 100644 --- a/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt +++ b/library/kotlin/src/io/envoyproxy/envoymobile/filters/Filter.kt @@ -24,7 +24,7 @@ internal class FilterFactory( internal class EnvoyHTTPFilterAdapter( private val filter: Filter ) : EnvoyHTTPFilter { - override fun onRequestHeaders(headers: Map>, endStream: Boolean): Array { + override fun onRequestHeaders(headers: Map>, endStream: Boolean): Array { (filter as? RequestFilter)?.let { requestFilter -> val result = requestFilter.onRequestHeaders(RequestHeaders(headers), endStream) return when (result) { @@ -35,7 +35,7 @@ internal class EnvoyHTTPFilterAdapter( return arrayOf(0, headers) } - override fun onResponseHeaders(headers: Map>, endStream: Boolean): Array { + override fun onResponseHeaders(headers: Map>, endStream: Boolean): Array { (filter as? ResponseFilter)?.let { responseFilter -> val result = responseFilter.onResponseHeaders(ResponseHeaders(headers), endStream) return when (result) { @@ -46,7 +46,7 @@ internal class EnvoyHTTPFilterAdapter( return arrayOf(0, headers) } - override fun onRequestData(data: ByteBuffer, endStream: Boolean): Array { + override fun onRequestData(data: ByteBuffer, endStream: Boolean): Array { (filter as? RequestFilter)?.let { requestFilter -> val result = requestFilter.onRequestData(data, endStream) return when (result) { @@ -59,7 +59,7 @@ internal class EnvoyHTTPFilterAdapter( return arrayOf(0, data) } - override fun onResponseData(data: ByteBuffer, endStream: Boolean): Array { + override fun onResponseData(data: ByteBuffer, endStream: Boolean): Array { (filter as? ResponseFilter)?.let { responseFilter -> val result = responseFilter.onResponseData(data, endStream) return when (result) { @@ -72,7 +72,7 @@ internal class EnvoyHTTPFilterAdapter( return arrayOf(0, data) } - override fun onRequestTrailers(trailers: Map>): Array { + override fun onRequestTrailers(trailers: Map>): Array { (filter as? RequestFilter)?.let { requestFilter -> val result = requestFilter.onRequestTrailers(RequestTrailers(trailers)) return when (result) { @@ -84,14 +84,13 @@ internal class EnvoyHTTPFilterAdapter( return arrayOf(0, trailers) } - override fun onResponseTrailers(trailers: Map>): Array { + override fun onResponseTrailers(trailers: Map>): Array { (filter as? ResponseFilter)?.let { responseFilter -> val result = responseFilter.onResponseTrailers(ResponseTrailers(trailers)) return when (result) { is FilterTrailersStatus.Continue<*, *> -> arrayOf(result.status, result.trailers.headers) is FilterTrailersStatus.StopIteration<*, *> -> arrayOf(result.status, trailers) is FilterTrailersStatus.ResumeIteration<*, *> -> arrayOf(result.status, result.headers?.headers, result.data, result.trailers.headers) - } } return arrayOf(0, trailers) diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 3f7564d24c..04b2e87708 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -60,173 +60,172 @@ static envoy_filter_data_status ios_http_filter_on_request_data(envoy_data data, const void *context) { EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onRequestData == nil) { - return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, - /*data*/ data, - /*extra_headers*/ envoy_noheaders; + return (envoy_filter_data_status) { /*status*/ + kEnvoyFilterDataStatusContinue, + /*data*/ data, + /*extra_headers*/ envoy_noheaders; + } + + NSData *platformData = to_ios_data(data); + NSArray *result = filter.onRequestData(platformData, end_stream); + return (envoy_filter_data_status){/*status*/ [result[0] intValue], + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; } - NSData *platformData = to_ios_data(data); - NSArray *result = filter.onRequestData(platformData, end_stream); - return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; -} + static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onResponseData == nil) { + return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, + /*data*/ data, + /*extra_headers*/ envoy_noheaders}; + } + + NSData *platformData = to_ios_data(data); + NSArray *result = filter.onResponseData(platformData, end_stream); + return (envoy_filter_data_status){/*status*/ [result[0] intValue], + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; + } -static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onResponseData == nil) { - return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, - /*data*/ data, - /*extra_headers*/ envoy_noheaders}; + static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onRequestTrailers == nil) { + return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_trailers*/ envoy_nodata}; + } + + EnvoyHeaders *platformTrailers = to_ios_headers(trailers); + NSArray *result = filter.onRequestTrailers(platformTrailers); + return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; } - NSData *platformData = to_ios_data(data); - NSArray *result = filter.onResponseData(platformData, end_stream); - return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; -} + static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onResponseTrailers == nil) { + return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_data*/ envoy_nodata}; + } + + EnvoyHeaders *platformTrailers = to_ios_headers(trailers); + NSArray *result = filter.onResponseTrailers(platformTrailers); + return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; + } -static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onRequestTrailers == nil) { - return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_trailers*/ envoy_nodata}; - } - - EnvoyHeaders *platformTrailers = to_ios_headers(trailers); - NSArray *result = filter.onRequestTrailers(platformTrailers); - return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; -} + static void ios_http_filter_release(const void *context) { + CFRelease(context); + return; + } -static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onResponseTrailers == nil) { - return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_data*/ envoy_nodata}; - } - - EnvoyHeaders *platformTrailers = to_ios_headers(trailers); - NSArray *result = filter.onResponseTrailers(platformTrailers); - return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; -} + @implementation EnvoyEngineImpl { + envoy_engine_t _engineHandle; + } -static void ios_http_filter_release(const void *context) { - CFRelease(context); - return; -} + -(instancetype)init { + self = [super init]; + if (!self) { + return nil; + } -@implementation EnvoyEngineImpl { - envoy_engine_t _engineHandle; -} + _engineHandle = init_engine(); + [EnvoyNetworkMonitor startReachabilityIfNeeded]; + return self; + } -- (instancetype)init { - self = [super init]; - if (!self) { - return nil; + -(void)dealloc { + [[NSNotificationCenter defaultCenter] removeObserver:self]; } - _engineHandle = init_engine(); - [EnvoyNetworkMonitor startReachabilityIfNeeded]; - return self; -} + -(int)registerFilterFactory : (EnvoyHTTPFilterFactory *)filterFactory { + // TODO(goaway): Everything here leaks, but it's all be tied to the life of the engine. + // This will need to be updated for https://github.com/lyft/envoy-mobile/issues/332 + envoy_http_filter *api = safe_malloc(sizeof(envoy_http_filter)); + api->init_filter = ios_http_filter_init; + api->on_request_headers = ios_http_filter_on_request_headers; + api->on_request_data = ios_http_filter_on_request_data; + api->on_request_trailers = ios_http_filter_on_request_trailers; + api->on_response_headers = ios_http_filter_on_response_headers; + api->on_response_data = ios_http_filter_on_response_data; + api->on_response_trailers = ios_http_filter_on_response_trailers; + api->release_filter = ios_http_filter_release; + api->static_context = CFBridgingRetain(filterFactory); + api->instance_context = NULL; + + register_platform_api(filterFactory.filterName.UTF8String, api); + return kEnvoySuccess; + } -- (void)dealloc { - [[NSNotificationCenter defaultCenter] removeObserver:self]; -} + -(int)runWithConfig : (EnvoyConfiguration *)config logLevel : (NSString *)logLevel onEngineRunning + : (nullable void (^)())onEngineRunning { + NSString *templateYAML = [[NSString alloc] initWithUTF8String:config_template]; + NSString *resolvedYAML = [config resolveTemplate:templateYAML]; + if (resolvedYAML == nil) { + return kEnvoyFailure; + } -- (int)registerFilterFactory:(EnvoyHTTPFilterFactory *)filterFactory { - // TODO(goaway): Everything here leaks, but it's all be tied to the life of the engine. - // This will need to be updated for https://github.com/lyft/envoy-mobile/issues/332 - envoy_http_filter *api = safe_malloc(sizeof(envoy_http_filter)); - api->init_filter = ios_http_filter_init; - api->on_request_headers = ios_http_filter_on_request_headers; - api->on_request_data = ios_http_filter_on_request_data; - api->on_request_trailers = ios_http_filter_on_request_trailers; - api->on_response_headers = ios_http_filter_on_response_headers; - api->on_response_data = ios_http_filter_on_response_data; - api->on_response_trailers = ios_http_filter_on_response_trailers; - api->release_filter = ios_http_filter_release; - api->static_context = CFBridgingRetain(filterFactory); - api->instance_context = NULL; - - register_platform_api(filterFactory.filterName.UTF8String, api); - return kEnvoySuccess; -} + for (EnvoyHTTPFilterFactory *filterFactory in config.httpFilterFactories) { + [self registerFilterFactory:filterFactory]; + } -- (int)runWithConfig:(EnvoyConfiguration *)config - logLevel:(NSString *)logLevel - onEngineRunning:(nullable void (^)())onEngineRunning { - NSString *templateYAML = [[NSString alloc] initWithUTF8String:config_template]; - NSString *resolvedYAML = [config resolveTemplate:templateYAML]; - if (resolvedYAML == nil) { - return kEnvoyFailure; + return [self runWithConfigYAML:resolvedYAML logLevel:logLevel onEngineRunning:onEngineRunning]; } - for (EnvoyHTTPFilterFactory *filterFactory in config.httpFilterFactories) { - [self registerFilterFactory:filterFactory]; + -(int)runWithConfigYAML : (NSString *)configYAML logLevel : (NSString *)logLevel onEngineRunning + : (nullable void (^)())onEngineRunning { + self.onEngineRunning = onEngineRunning; + [self startObservingLifecycleNotifications]; + + // Envoy exceptions will only be caught here when compiled for 64-bit arches. + // https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Articles/Exceptions64Bit.html + @try { + envoy_engine_callbacks native_callbacks = {ios_on_engine_running, ios_on_exit, + (__bridge void *)(self)}; + return (int)run_engine(_engineHandle, native_callbacks, configYAML.UTF8String, + logLevel.UTF8String); + } @catch (NSException *exception) { + NSLog(@"[Envoy] exception caught: %@", exception); + [NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self]; + return kEnvoyFailure; + } } - return [self runWithConfigYAML:resolvedYAML logLevel:logLevel onEngineRunning:onEngineRunning]; -} - -- (int)runWithConfigYAML:(NSString *)configYAML - logLevel:(NSString *)logLevel - onEngineRunning:(nullable void (^)())onEngineRunning { - self.onEngineRunning = onEngineRunning; - [self startObservingLifecycleNotifications]; - - // Envoy exceptions will only be caught here when compiled for 64-bit arches. - // https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Articles/Exceptions64Bit.html - @try { - envoy_engine_callbacks native_callbacks = {ios_on_engine_running, ios_on_exit, - (__bridge void *)(self)}; - return (int)run_engine(_engineHandle, native_callbacks, configYAML.UTF8String, - logLevel.UTF8String); - } @catch (NSException *exception) { - NSLog(@"[Envoy] exception caught: %@", exception); - [NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self]; - return kEnvoyFailure; + -(id)startStreamWithCallbacks : (EnvoyHTTPCallbacks *)callbacks { + return [[EnvoyHTTPStreamImpl alloc] initWithHandle:init_stream(_engineHandle) + callbacks:callbacks]; } -} - -- (id)startStreamWithCallbacks:(EnvoyHTTPCallbacks *)callbacks { - return [[EnvoyHTTPStreamImpl alloc] initWithHandle:init_stream(_engineHandle) - callbacks:callbacks]; -} -- (int)recordCounter:(NSString *)elements count:(NSUInteger)count { - return record_counter(_engineHandle, elements.UTF8String, count); -} + -(int)recordCounter : (NSString *)elements count : (NSUInteger)count { + return record_counter(_engineHandle, elements.UTF8String, count); + } #pragma mark - Private -- (void)startObservingLifecycleNotifications { - // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748 - // gets fixed. - NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter]; - [notificationCenter addObserver:self - selector:@selector(terminateNotification:) - name:UIApplicationWillTerminateNotification - object:nil]; -} + -(void)startObservingLifecycleNotifications { + // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748 + // gets fixed. + NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter]; + [notificationCenter addObserver:self + selector:@selector(terminateNotification:) + name:UIApplicationWillTerminateNotification + object:nil]; + } -- (void)terminateNotification:(NSNotification *)notification { - NSLog(@"[Envoy %ld] terminating engine (%@)", _engineHandle, notification.name); - terminate_engine(_engineHandle); -} + -(void)terminateNotification : (NSNotification *)notification { + NSLog(@"[Envoy %ld] terminating engine (%@)", _engineHandle, notification.name); + terminate_engine(_engineHandle); + } -@end + @end diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 6df17f453a..e68adf7715 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -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( @@ -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; @@ -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( @@ -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( @@ -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( @@ -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; @@ -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( @@ -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( From 5c012fa9351e946c8237603655bd78ef06e5a2a3 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 22 Sep 2020 23:10:01 +0800 Subject: [PATCH 03/21] syntax and format fix Signed-off-by: Mike Schore --- library/objective-c/EnvoyEngineImpl.m | 287 +++++++++++++------------- 1 file changed, 144 insertions(+), 143 deletions(-) diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 04b2e87708..988cf90eca 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -60,172 +60,173 @@ static envoy_filter_data_status ios_http_filter_on_request_data(envoy_data data, const void *context) { EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; if (filter.onRequestData == nil) { - return (envoy_filter_data_status) { /*status*/ - kEnvoyFilterDataStatusContinue, - /*data*/ data, - /*extra_headers*/ envoy_noheaders; - } - - NSData *platformData = to_ios_data(data); - NSArray *result = filter.onRequestData(platformData, end_stream); - return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; + return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, + /*data*/ data, + /*extra_headers*/ envoy_noheaders}; } - static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onResponseData == nil) { - return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, - /*data*/ data, - /*extra_headers*/ envoy_noheaders}; - } - - NSData *platformData = to_ios_data(data); - NSArray *result = filter.onResponseData(platformData, end_stream); - return (envoy_filter_data_status){/*status*/ [result[0] intValue], - /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; - } + NSData *platformData = to_ios_data(data); + NSArray *result = filter.onRequestData(platformData, end_stream); + return (envoy_filter_data_status){/*status*/ [result[0] intValue], + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; +} - static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onRequestTrailers == nil) { - return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_trailers*/ envoy_nodata}; - } - - EnvoyHeaders *platformTrailers = to_ios_headers(trailers); - NSArray *result = filter.onRequestTrailers(platformTrailers); - return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; +static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onResponseData == nil) { + return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, + /*data*/ data, + /*extra_headers*/ envoy_noheaders}; } - static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, - const void *context) { - EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; - if (filter.onResponseTrailers == nil) { - return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, - /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_data*/ envoy_nodata}; - } - - EnvoyHeaders *platformTrailers = to_ios_headers(trailers); - NSArray *result = filter.onResponseTrailers(platformTrailers); - return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], - /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; - } + NSData *platformData = to_ios_data(data); + NSArray *result = filter.onResponseData(platformData, end_stream); + return (envoy_filter_data_status){/*status*/ [result[0] intValue], + /*data*/ toNativeData(result[1]), + /*extra_headers*/ toNativeHeaders(result[2])}; +} - static void ios_http_filter_release(const void *context) { - CFRelease(context); - return; - } +static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onRequestTrailers == nil) { + return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_trailers*/ envoy_nodata}; + } + + EnvoyHeaders *platformTrailers = to_ios_headers(trailers); + NSArray *result = filter.onRequestTrailers(platformTrailers); + return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; +} - @implementation EnvoyEngineImpl { - envoy_engine_t _engineHandle; - } +static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, + const void *context) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onResponseTrailers == nil) { + return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, + /*trailers*/ trailers, + /*extra_headers*/ envoy_noheaders, + /*extra_data*/ envoy_nodata}; + } + + EnvoyHeaders *platformTrailers = to_ios_headers(trailers); + NSArray *result = filter.onResponseTrailers(platformTrailers); + return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], + /*trailers*/ toNativeHeaders(result[1]), + /*extra_headers*/ toNativeHeaders(result[2]), + /*extra_data*/ toNativeData(result[3])}; +} - -(instancetype)init { - self = [super init]; - if (!self) { - return nil; - } +static void ios_http_filter_release(const void *context) { + CFRelease(context); + return; +} - _engineHandle = init_engine(); - [EnvoyNetworkMonitor startReachabilityIfNeeded]; - return self; - } +@implementation EnvoyEngineImpl { + envoy_engine_t _engineHandle; +} - -(void)dealloc { - [[NSNotificationCenter defaultCenter] removeObserver:self]; +- (instancetype)init { + self = [super init]; + if (!self) { + return nil; } - -(int)registerFilterFactory : (EnvoyHTTPFilterFactory *)filterFactory { - // TODO(goaway): Everything here leaks, but it's all be tied to the life of the engine. - // This will need to be updated for https://github.com/lyft/envoy-mobile/issues/332 - envoy_http_filter *api = safe_malloc(sizeof(envoy_http_filter)); - api->init_filter = ios_http_filter_init; - api->on_request_headers = ios_http_filter_on_request_headers; - api->on_request_data = ios_http_filter_on_request_data; - api->on_request_trailers = ios_http_filter_on_request_trailers; - api->on_response_headers = ios_http_filter_on_response_headers; - api->on_response_data = ios_http_filter_on_response_data; - api->on_response_trailers = ios_http_filter_on_response_trailers; - api->release_filter = ios_http_filter_release; - api->static_context = CFBridgingRetain(filterFactory); - api->instance_context = NULL; - - register_platform_api(filterFactory.filterName.UTF8String, api); - return kEnvoySuccess; - } + _engineHandle = init_engine(); + [EnvoyNetworkMonitor startReachabilityIfNeeded]; + return self; +} - -(int)runWithConfig : (EnvoyConfiguration *)config logLevel : (NSString *)logLevel onEngineRunning - : (nullable void (^)())onEngineRunning { - NSString *templateYAML = [[NSString alloc] initWithUTF8String:config_template]; - NSString *resolvedYAML = [config resolveTemplate:templateYAML]; - if (resolvedYAML == nil) { - return kEnvoyFailure; - } +- (void)dealloc { + [[NSNotificationCenter defaultCenter] removeObserver:self]; +} - for (EnvoyHTTPFilterFactory *filterFactory in config.httpFilterFactories) { - [self registerFilterFactory:filterFactory]; - } +- (int)registerFilterFactory:(EnvoyHTTPFilterFactory *)filterFactory { + // TODO(goaway): Everything here leaks, but it's all be tied to the life of the engine. + // This will need to be updated for https://github.com/lyft/envoy-mobile/issues/332 + envoy_http_filter *api = safe_malloc(sizeof(envoy_http_filter)); + api->init_filter = ios_http_filter_init; + api->on_request_headers = ios_http_filter_on_request_headers; + api->on_request_data = ios_http_filter_on_request_data; + api->on_request_trailers = ios_http_filter_on_request_trailers; + api->on_response_headers = ios_http_filter_on_response_headers; + api->on_response_data = ios_http_filter_on_response_data; + api->on_response_trailers = ios_http_filter_on_response_trailers; + api->release_filter = ios_http_filter_release; + api->static_context = CFBridgingRetain(filterFactory); + api->instance_context = NULL; + + register_platform_api(filterFactory.filterName.UTF8String, api); + return kEnvoySuccess; +} - return [self runWithConfigYAML:resolvedYAML logLevel:logLevel onEngineRunning:onEngineRunning]; +- (int)runWithConfig:(EnvoyConfiguration *)config + logLevel:(NSString *)logLevel + onEngineRunning:(nullable void (^)())onEngineRunning { + NSString *templateYAML = [[NSString alloc] initWithUTF8String:config_template]; + NSString *resolvedYAML = [config resolveTemplate:templateYAML]; + if (resolvedYAML == nil) { + return kEnvoyFailure; } - -(int)runWithConfigYAML : (NSString *)configYAML logLevel : (NSString *)logLevel onEngineRunning - : (nullable void (^)())onEngineRunning { - self.onEngineRunning = onEngineRunning; - [self startObservingLifecycleNotifications]; - - // Envoy exceptions will only be caught here when compiled for 64-bit arches. - // https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Articles/Exceptions64Bit.html - @try { - envoy_engine_callbacks native_callbacks = {ios_on_engine_running, ios_on_exit, - (__bridge void *)(self)}; - return (int)run_engine(_engineHandle, native_callbacks, configYAML.UTF8String, - logLevel.UTF8String); - } @catch (NSException *exception) { - NSLog(@"[Envoy] exception caught: %@", exception); - [NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self]; - return kEnvoyFailure; - } + for (EnvoyHTTPFilterFactory *filterFactory in config.httpFilterFactories) { + [self registerFilterFactory:filterFactory]; } - -(id)startStreamWithCallbacks : (EnvoyHTTPCallbacks *)callbacks { - return [[EnvoyHTTPStreamImpl alloc] initWithHandle:init_stream(_engineHandle) - callbacks:callbacks]; - } + return [self runWithConfigYAML:resolvedYAML logLevel:logLevel onEngineRunning:onEngineRunning]; +} - -(int)recordCounter : (NSString *)elements count : (NSUInteger)count { - return record_counter(_engineHandle, elements.UTF8String, count); +- (int)runWithConfigYAML:(NSString *)configYAML + logLevel:(NSString *)logLevel + onEngineRunning:(nullable void (^)())onEngineRunning { + self.onEngineRunning = onEngineRunning; + [self startObservingLifecycleNotifications]; + + // Envoy exceptions will only be caught here when compiled for 64-bit arches. + // https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Articles/Exceptions64Bit.html + @try { + envoy_engine_callbacks native_callbacks = {ios_on_engine_running, ios_on_exit, + (__bridge void *)(self)}; + return (int)run_engine(_engineHandle, native_callbacks, configYAML.UTF8String, + logLevel.UTF8String); + } @catch (NSException *exception) { + NSLog(@"[Envoy] exception caught: %@", exception); + [NSNotificationCenter.defaultCenter postNotificationName:@"EnvoyError" object:self]; + return kEnvoyFailure; } +} + +- (id)startStreamWithCallbacks:(EnvoyHTTPCallbacks *)callbacks { + return [[EnvoyHTTPStreamImpl alloc] initWithHandle:init_stream(_engineHandle) + callbacks:callbacks]; +} + +- (int)recordCounter:(NSString *)elements count:(NSUInteger)count { + return record_counter(_engineHandle, elements.UTF8String, count); +} #pragma mark - Private - -(void)startObservingLifecycleNotifications { - // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748 - // gets fixed. - NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter]; - [notificationCenter addObserver:self - selector:@selector(terminateNotification:) - name:UIApplicationWillTerminateNotification - object:nil]; - } +- (void)startObservingLifecycleNotifications { + // re-enable lifecycle-based stat flushing when https://github.com/lyft/envoy-mobile/issues/748 + // gets fixed. + NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter]; + [notificationCenter addObserver:self + selector:@selector(terminateNotification:) + name:UIApplicationWillTerminateNotification + object:nil]; +} - -(void)terminateNotification : (NSNotification *)notification { - NSLog(@"[Envoy %ld] terminating engine (%@)", _engineHandle, notification.name); - terminate_engine(_engineHandle); - } +- (void)terminateNotification:(NSNotification *)notification { + NSLog(@"[Envoy %ld] terminating engine (%@)", _engineHandle, notification.name); + terminate_engine(_engineHandle); +} - @end +@end From 3d4caa06a7d23e86bc8e26175ad7de53e6e38c1f Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 23 Sep 2020 15:59:16 +0800 Subject: [PATCH 04/21] ios fixes Signed-off-by: Mike Schore --- library/objective-c/EnvoyBridgeUtility.h | 20 ++++++++++++++++++++ library/objective-c/EnvoyEngine.h | 2 ++ library/objective-c/EnvoyEngineImpl.m | 24 ++++++++++++------------ library/swift/src/filters/Filter.swift | 8 ++++---- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/library/objective-c/EnvoyBridgeUtility.h b/library/objective-c/EnvoyBridgeUtility.h index 0c3a51ecd8..56119fa7cf 100644 --- a/library/objective-c/EnvoyBridgeUtility.h +++ b/library/objective-c/EnvoyBridgeUtility.h @@ -13,6 +13,16 @@ static inline envoy_data toNativeData(NSData *data) { return ret; } +static inline envoy_data * toNativeDataPtr(NSData *data) { + if (data == nil) { + return nil; + } + + envoy_data *ret = (envoy_data *)safe_malloc(sizeof(envoy_data)); + *ret = toNativeData(data); + return ret; +} + static inline envoy_data toManagedNativeString(NSString *s) { size_t length = s.length; uint8_t *native_string = (uint8_t *)safe_malloc(sizeof(uint8_t) * length); @@ -45,6 +55,16 @@ static inline envoy_headers toNativeHeaders(EnvoyHeaders *headers) { return ret; } +static inline envoy_headers * toNativeHeadersPtr(EnvoyHeaders *headers) { + if (headers == nil) { + return NULL; + } + + envoy_headers *ret = (envoy_headers *)safe_malloc(sizeof(envoy_headers)); + *ret = toNativeHeaders(headers); + return ret; +} + static inline NSData *to_ios_data(envoy_data data) { // TODO: we are copying from envoy_data to NSData. // https://github.com/lyft/envoy-mobile/issues/398 diff --git a/library/objective-c/EnvoyEngine.h b/library/objective-c/EnvoyEngine.h index 14266ef4cf..a0bf737f55 100644 --- a/library/objective-c/EnvoyEngine.h +++ b/library/objective-c/EnvoyEngine.h @@ -67,10 +67,12 @@ extern const int kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; extern const int kEnvoyFilterDataStatusContinue; extern const int kEnvoyFilterDataStatusStopIterationAndBuffer; extern const int kEnvoyFilterDataStatusStopIterationNoBuffer; +extern const int kEnvoyFilterDataStatusResumeIteration; /// Return codes for on-trailers filter invocations. @see envoy/http/filter.h extern const int kEnvoyFilterTrailersStatusContinue; extern const int kEnvoyFilterTrailersStatusStopIteration; +extern const int kEnvoyFilterTrailersStatusResumeIteration; @interface EnvoyHTTPFilter : NSObject diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index 988cf90eca..d1b5ab4679 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -62,14 +62,14 @@ static envoy_filter_data_status ios_http_filter_on_request_data(envoy_data data, if (filter.onRequestData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, /*data*/ data, - /*extra_headers*/ envoy_noheaders}; + /*extra_headers*/ NULL}; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onRequestData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; + /*extra_headers*/ toNativeHeadersPtr(result[2])}; } static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, @@ -78,14 +78,14 @@ static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data if (filter.onResponseData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, /*data*/ data, - /*extra_headers*/ envoy_noheaders}; + /*extra_headers*/ NULL}; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onResponseData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeaders(result[2])}; + /*extra_headers*/ toNativeHeadersPtr(result[2])}; } static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, @@ -94,16 +94,16 @@ static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_he if (filter.onRequestTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_trailers*/ envoy_nodata}; + /*extra_headers*/ NULL, + /*extra_trailers*/ NULL}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onRequestTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; + /*extra_headers*/ toNativeHeadersPtr(result[2]), + /*extra_data*/ toNativeDataPtr(result[3])}; } static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, @@ -112,16 +112,16 @@ static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_h if (filter.onResponseTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, /*trailers*/ trailers, - /*extra_headers*/ envoy_noheaders, - /*extra_data*/ envoy_nodata}; + /*extra_headers*/ NULL, + /*extra_data*/ NULL}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onResponseTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeaders(result[2]), - /*extra_data*/ toNativeData(result[3])}; + /*extra_headers*/ toNativeHeadersPtr(result[2]), + /*extra_data*/ toNativeDataPtr(result[3])}; } static void ios_http_filter_release(const void *context) { diff --git a/library/swift/src/filters/Filter.swift b/library/swift/src/filters/Filter.swift index 0affbe5209..9e7a22c7ba 100644 --- a/library/swift/src/filters/Filter.swift +++ b/library/swift/src/filters/Filter.swift @@ -40,7 +40,7 @@ extension EnvoyHTTPFilter { case .stopIterationNoBuffer: return [kEnvoyFilterDataStatusStopIterationNoBuffer, data] case .resumeIteration(let headers, let data): - return [kEnvoyFilterDataStatusContinue, headers?.headers, data] + return [kEnvoyFilterDataStatusResumeIteration, headers?.headers as Any, data] } } @@ -52,7 +52,7 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusContinue, headers?.headers, data, trailers.headers] + return [kEnvoyFilterTrailersStatusResumeIteration, headers?.headers as Any, data as Any, trailers.headers] } } } @@ -79,7 +79,7 @@ extension EnvoyHTTPFilter { case .stopIterationNoBuffer: return [kEnvoyFilterDataStatusStopIterationNoBuffer, data] case .resumeIteration(let headers, let data): - return [kEnvoyFilterDataStatusContinue, headers?.headers, data] + return [kEnvoyFilterDataStatusResumeIteration, headers?.headers as Any, data] } } @@ -91,7 +91,7 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusContinue, headers?.headers, data, trailers.headers] + return [kEnvoyFilterTrailersStatusResumeIteration, headers?.headers as Any, data as Any, trailers.headers] } } } From 6b6e1a29f93796029f220e960b29b1d072f3bff4 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 23 Sep 2020 18:11:07 +0800 Subject: [PATCH 05/21] format Signed-off-by: Mike Schore --- library/objective-c/EnvoyBridgeUtility.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/objective-c/EnvoyBridgeUtility.h b/library/objective-c/EnvoyBridgeUtility.h index 56119fa7cf..4b5641969a 100644 --- a/library/objective-c/EnvoyBridgeUtility.h +++ b/library/objective-c/EnvoyBridgeUtility.h @@ -13,7 +13,7 @@ static inline envoy_data toNativeData(NSData *data) { return ret; } -static inline envoy_data * toNativeDataPtr(NSData *data) { +static inline envoy_data *toNativeDataPtr(NSData *data) { if (data == nil) { return nil; } @@ -55,7 +55,7 @@ static inline envoy_headers toNativeHeaders(EnvoyHeaders *headers) { return ret; } -static inline envoy_headers * toNativeHeadersPtr(EnvoyHeaders *headers) { +static inline envoy_headers *toNativeHeadersPtr(EnvoyHeaders *headers) { if (headers == nil) { return NULL; } From aaf2a888ea1368cd6144089b4262d2f027367c0e Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 23 Sep 2020 11:31:30 -0700 Subject: [PATCH 06/21] swiftlint Signed-off-by: Mike Schore --- library/swift/src/filters/Filter.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/library/swift/src/filters/Filter.swift b/library/swift/src/filters/Filter.swift index 9e7a22c7ba..c7cae92cf2 100644 --- a/library/swift/src/filters/Filter.swift +++ b/library/swift/src/filters/Filter.swift @@ -52,7 +52,10 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusResumeIteration, headers?.headers as Any, data as Any, trailers.headers] + return [kEnvoyFilterTrailersStatusResumeIteration, + headers?.headers as Any, + data as Any, + trailers.headers] } } } @@ -91,7 +94,10 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusResumeIteration, headers?.headers as Any, data as Any, trailers.headers] + return [kEnvoyFilterTrailersStatusResumeIteration, + headers?.headers as Any, + data as Any, + trailers.headers] } } } From 6afe9d6803082eda872417a9bdfc6f71c0b6a4a6 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 23 Sep 2020 11:36:01 -0700 Subject: [PATCH 07/21] swiftlint 2 Signed-off-by: Mike Schore --- library/swift/src/filters/Filter.swift | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/library/swift/src/filters/Filter.swift b/library/swift/src/filters/Filter.swift index c7cae92cf2..0508da3516 100644 --- a/library/swift/src/filters/Filter.swift +++ b/library/swift/src/filters/Filter.swift @@ -52,10 +52,12 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusResumeIteration, - headers?.headers as Any, - data as Any, - trailers.headers] + return [ + kEnvoyFilterTrailersStatusResumeIteration, + headers?.headers as Any, + data as Any, + trailers.headers, + ] } } } @@ -94,10 +96,12 @@ extension EnvoyHTTPFilter { case .stopIteration: return [kEnvoyFilterTrailersStatusStopIteration, envoyTrailers] case .resumeIteration(let headers, let data, let trailers): - return [kEnvoyFilterTrailersStatusResumeIteration, - headers?.headers as Any, - data as Any, - trailers.headers] + return [ + kEnvoyFilterTrailersStatusResumeIteration, + headers?.headers as Any, + data as Any, + trailers.headers, + ] } } } From e0914aba75c9cea10c3d93b3bb15fcc3d0c8fc05 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 09:01:28 +0800 Subject: [PATCH 08/21] change some panics to release assertions Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/filter.cc | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index f34598cd93..d30462ed44 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -117,9 +117,8 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool switch (result.status) { case kEnvoyFilterDataStatusContinue: - if (iteration_state_ == IterationState::Stopped) { - PANIC("invalid filter state: filter iteration must be resumed with ResumeIteration"); - } + RELEASE_ASSERT(iteration_state_ != IterationState::Stopped, + "invalid filter state: filter iteration must be resumed with ResumeIteration"); data.drain(data.length()); data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); return Http::FilterDataStatus::Continue; @@ -148,10 +147,9 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool return Http::FilterDataStatus::StopIterationNoBuffer; case kEnvoyFilterDataStatusResumeIteration: - if (iteration_state_ != IterationState::Stopped) { - PANIC("invalid filter state: ResumeIteration may only be used when filter iteration is " - "stopped"); - } + RELEASE_ASSERT(iteration_state_ == IterationState::Stopped, + "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; @@ -182,10 +180,9 @@ PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers, Buffer::Instance* in switch (result.status) { case kEnvoyFilterTrailersStatusContinue: - if (iteration_state_ == IterationState::Stopped) { - PANIC("invalid filter state: ResumeIteration may only be used when filter iteration is " - "stopped"); - } + RELEASE_ASSERT(iteration_state_ != IterationState::Stopped, + "invalid filter state: ResumeIteration may only be used when filter iteration " + "is stopped"); PlatformBridgeFilter::replaceHeaders(trailers, result.trailers); return Http::FilterTrailersStatus::Continue; @@ -194,10 +191,9 @@ PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers, Buffer::Instance* in 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"); - } + RELEASE_ASSERT(iteration_state_ == IterationState::Stopped, + "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; From 36e55fddc1d6a183f636bed7a563e8f3deb33414 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 09:13:01 +0800 Subject: [PATCH 09/21] use NULL Signed-off-by: Mike Schore --- library/objective-c/EnvoyBridgeUtility.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/objective-c/EnvoyBridgeUtility.h b/library/objective-c/EnvoyBridgeUtility.h index 4b5641969a..8702300f44 100644 --- a/library/objective-c/EnvoyBridgeUtility.h +++ b/library/objective-c/EnvoyBridgeUtility.h @@ -15,7 +15,7 @@ static inline envoy_data toNativeData(NSData *data) { static inline envoy_data *toNativeDataPtr(NSData *data) { if (data == nil) { - return nil; + return NULL; } envoy_data *ret = (envoy_data *)safe_malloc(sizeof(envoy_data)); From 09088e73c05229f4f367b934a739e132e80354cf Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 09:21:53 +0800 Subject: [PATCH 10/21] change 'extra' to 'pending' and add comments Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/c_types.h | 6 ++-- .../filters/http/platform_bridge/filter.cc | 30 +++++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.h b/library/common/extensions/filters/http/platform_bridge/c_types.h index 734db99d6a..c46c5a1094 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.h +++ b/library/common/extensions/filters/http/platform_bridge/c_types.h @@ -47,7 +47,7 @@ extern const envoy_filter_data_status_t kEnvoyFilterDataStatusResumeIteration; typedef struct { envoy_filter_data_status_t status; envoy_data data; - envoy_headers* extra_headers; + envoy_headers* pending_headers; } envoy_filter_data_status; /** @@ -64,8 +64,8 @@ extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIter typedef struct { envoy_filter_trailers_status_t status; envoy_headers trailers; - envoy_headers* extra_headers; - envoy_data* extra_data; + envoy_headers* pending_headers; + envoy_data* pending_data; } envoy_filter_trailers_status; #ifdef __cplusplus diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index d30462ed44..060509f85c 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -146,15 +146,21 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool iteration_state_ = IterationState::Stopped; return Http::FilterDataStatus::StopIterationNoBuffer; + // Resume previously-stopped iteration, possibly forwarding headers if iteration was stopped + // during an on*Headers invocation. case kEnvoyFilterDataStatusResumeIteration: RELEASE_ASSERT(iteration_state_ == IterationState::Stopped, "invalid filter state: ResumeIteration may only be used when filter iteration " "is stopped"); - if (result.extra_headers) { - PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.extra_headers); + // Update pending henders before resuming iteration, if needed. + if (result.pending_headers) { + PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.pending_headers); *pending_headers = nullptr; - free(result.extra_headers); + free(result.pending_headers); } + // We've already moved data into the internal buffer and presented it to the platform. Replace + // the internal buffer with any modifications returned by the platform filter prior to + // resumption. internal_buffer->drain(internal_buffer->length()); internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); return Http::FilterDataStatus::Continue; @@ -190,20 +196,26 @@ PlatformBridgeFilter::onTrailers(Http::HeaderMap& trailers, Buffer::Instance* in iteration_state_ = IterationState::Stopped; return Http::FilterTrailersStatus::StopIteration; + // Resume previously-stopped iteration, possibly forwarding headers and data if iteration was + // stopped during an on*Headers or on*Data invocation. case kEnvoyFilterTrailersStatusResumeIteration: RELEASE_ASSERT(iteration_state_ == IterationState::Stopped, "invalid filter state: ResumeIteration may only be used when filter iteration " "is stopped"); - if (result.extra_headers) { - PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.extra_headers); + // Update pending henders before resuming iteration, if needed. + if (result.pending_headers) { + PlatformBridgeFilter::replaceHeaders(**pending_headers, *result.pending_headers); *pending_headers = nullptr; - free(result.extra_headers); + free(result.pending_headers); } - if (result.extra_data) { + // We've already moved data into the internal buffer and presented it to the platform. Replace + // the internal buffer with any modifications returned by the platform filter prior to + // resumption. + if (result.pending_data) { internal_buffer->drain(internal_buffer->length()); internal_buffer->addBufferFragment( - *Buffer::BridgeFragment::createBridgeFragment(*result.extra_data)); - free(result.extra_data); + *Buffer::BridgeFragment::createBridgeFragment(*result.pending_data)); + free(result.pending_data); } PlatformBridgeFilter::replaceHeaders(trailers, result.trailers); return Http::FilterTrailersStatus::Continue; From 6ce7113ba357a217dcee02ef721c7d393e8ddffb Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Mon, 28 Sep 2020 20:14:36 -0700 Subject: [PATCH 11/21] additional inline comments Signed-off-by: Mike Schore --- .../extensions/filters/http/platform_bridge/c_types.h | 6 ++++++ .../extensions/filters/http/platform_bridge/filter.h | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/library/common/extensions/filters/http/platform_bridge/c_types.h b/library/common/extensions/filters/http/platform_bridge/c_types.h index c46c5a1094..dbba792d8d 100644 --- a/library/common/extensions/filters/http/platform_bridge/c_types.h +++ b/library/common/extensions/filters/http/platform_bridge/c_types.h @@ -22,6 +22,8 @@ extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinue; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopIteration; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusContinueAndEndStream; extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; +// Note this return status is unique to platform filters and used only to resume iteration after +// it has been previously stopped. extern const envoy_filter_headers_status_t kEnvoyFilterHeadersStatusResumeIteration; /** @@ -39,6 +41,8 @@ typedef int envoy_filter_data_status_t; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusContinue; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationAndBuffer; extern const envoy_filter_data_status_t kEnvoyFilterDataStatusStopIterationNoBuffer; +// Note this return status is unique to platform filters and used only to resume iteration after +// it has been previously stopped. extern const envoy_filter_data_status_t kEnvoyFilterDataStatusResumeIteration; /** @@ -56,6 +60,8 @@ typedef struct { typedef int envoy_filter_trailers_status_t; extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusContinue; extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusStopIteration; +// Note this return status is unique to platform filters and used only to resume iteration after +// it has been previously stopped. extern const envoy_filter_trailers_status_t kEnvoyFilterTrailersStatusResumeIteration; /** diff --git a/library/common/extensions/filters/http/platform_bridge/filter.h b/library/common/extensions/filters/http/platform_bridge/filter.h index 06bbc69d29..b2acf80aa0 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.h +++ b/library/common/extensions/filters/http/platform_bridge/filter.h @@ -33,6 +33,17 @@ enum class IterationState { Ongoing, Stopped }; /** * Harness to bridge Envoy filter invocations up to the platform layer. + * + * This filter enables filter implementations to be written in high-level platform-specific + * languages and run within the Envoy filter chain. To mirror platform API conventions, the + * semantic structure of platform filters differs slightly from Envoy filters. Platform + * filter invocations (on-headers, on-data, etc.) receive *immutable* entities as parameters + * and are expected to return compound results that include both the filter status, as well + * as any desired modifications to the HTTP entity. Additionally, when platform filters + * stop iteration, they _must_ use a new ResumeIteration status to resume iteration + * at a later point. The Continue status is only valid if iteration is already ongoing. + * + * For more information on implementing platform filters, see the docs. */ class PlatformBridgeFilter final : public Http::PassThroughFilter, Logger::Loggable { From bf4533228d262eed2ec1a49ffcdf9f44f4a35a9a Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 19:35:44 +0800 Subject: [PATCH 12/21] add tests Signed-off-by: Mike Schore --- .../platform_bridge_filter_test.cc | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index e68adf7715..d48bbfc1e6 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -3,6 +3,7 @@ #include "gtest/gtest.h" #include "library/common/api/external.h" +#include "library/common/buffer/utility.h" #include "library/common/extensions/filters/http/platform_bridge/filter.h" #include "library/common/extensions/filters/http/platform_bridge/filter.pb.h" @@ -234,6 +235,67 @@ platform_filter_name: StopAndBufferOnRequestData EXPECT_EQ(invocations.on_request_data_calls, 3); } +TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnRequestData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + + envoy_filter_data_status return_status[2] = { + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, + }; + + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); + EXPECT_FALSE(end_stream); + c_data.release(c_data.context); + + return return_status[invocations->on_request_data_calls++]; + }; + + Buffer::OwnedImpl decoding_buffer; + EXPECT_CALL(decoder_callbacks_, decodingBuffer()) + .Times(2) + .WillRepeatedly(Return(&decoding_buffer)); + EXPECT_CALL(decoder_callbacks_, modifyDecodingBuffer(_)) + .Times(2) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(decoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopAndBufferThenResumeOnRequestData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->decodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + decoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_request_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::Continue, + filter_->decodeData(second_chunk, false)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_request_data_calls, 2); + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(decoding_buffer.toString(), "C"); +} + TEST_F(PlatformBridgeFilterTest, StopNoBufferOnRequestData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; @@ -422,6 +484,67 @@ platform_filter_name: StopAndBufferOnResponseData EXPECT_EQ(invocations.on_response_data_calls, 3); } +TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnResponseData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + + envoy_filter_data_status return_status[2] = { + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, + }; + + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); + EXPECT_FALSE(end_stream); + c_data.release(c_data.context); + + return return_status[invocations->on_response_data_calls++]; + }; + + Buffer::OwnedImpl encoding_buffer; + EXPECT_CALL(encoder_callbacks_, encodingBuffer()) + .Times(2) + .WillRepeatedly(Return(&encoding_buffer)); + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer(_)) + .Times(2) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(encoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopAndBufferThenResumeOnResponseData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->encodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + encoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_response_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::Continue, + filter_->encodeData(second_chunk, false)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_response_data_calls, 2); + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(encoding_buffer.toString(), "C"); +} + TEST_F(PlatformBridgeFilterTest, StopNoBufferOnResponseData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; From 954ee8104a4de93a4f97fe102a8b67fa080aeb9f Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 21:02:50 +0800 Subject: [PATCH 13/21] add tests Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/filter.cc | 9 +- .../platform_bridge_filter_test.cc | 122 ++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 060509f85c..a010510c3b 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -161,8 +161,13 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool // We've already moved data into the internal buffer and presented it to the platform. Replace // the internal buffer with any modifications returned by the platform filter prior to // resumption. - internal_buffer->drain(internal_buffer->length()); - internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); + if (internal_buffer) { + internal_buffer->drain(internal_buffer->length()); + internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); + } else { + data.drain(data.length()); + data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); + } return Http::FilterDataStatus::Continue; default: diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index d48bbfc1e6..49f63048e9 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -20,6 +20,28 @@ std::string to_string(envoy_data data) { return std::string(reinterpret_cast(data.bytes), data.length); } +envoy_data make_envoy_data(const std::string& s) { + return copy_envoy_data(s.size(), reinterpret_cast(s.c_str())); +} + +envoy_headers make_envoy_headers(std::vector> pairs) { + envoy_header* headers = + static_cast(safe_malloc(sizeof(envoy_header) * pairs.size())); + envoy_headers new_headers; + new_headers.length = 0; + new_headers.headers = headers; + + for (const auto& pair : pairs) { + envoy_data key = make_envoy_data(pair.first); + envoy_data value = make_envoy_data(pair.second); + + new_headers.headers[new_headers.length] = {key, value}; + new_headers.length++; + } + + return new_headers; +} + class PlatformBridgeFilterTest : public testing::Test { public: void setUpFilter(std::string&& yaml, envoy_http_filter* platform_filter) { @@ -151,6 +173,56 @@ platform_filter_name: BasicContinueOnRequestHeaders EXPECT_EQ(invocations.on_request_headers_calls, 1); } +TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenResumeOnData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_request_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":authority"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_request_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(to_string(c_data), "request body"); + EXPECT_TRUE(end_stream); + invocations->on_request_data_calls++; + envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "12"}}); + return {kEnvoyFilterDataStatusResumeIteration, c_data, modified_headers}; + }; + + setUpFilter(R"EOF( +platform_filter_name: StopOnRequestHeadersThenResumeOnData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestRequestHeaderMapImpl request_headers{{":authority", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(invocations.on_request_headers_calls, 1); + + Buffer::OwnedImpl request_data = Buffer::OwnedImpl("request body"); + + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(request_data, true)); + EXPECT_EQ(invocations.on_request_data_calls, 1); + + EXPECT_TRUE(request_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(request_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), "12"); +} + TEST_F(PlatformBridgeFilterTest, BasicContinueOnRequestData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; @@ -400,6 +472,56 @@ platform_filter_name: BasicContinueOnResponseHeaders EXPECT_EQ(invocations.on_response_headers_calls, 1); } +TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenResumeOnData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_response_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(to_string(c_data), "response body"); + EXPECT_TRUE(end_stream); + invocations->on_response_data_calls++; + envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "13"}}); + return {kEnvoyFilterDataStatusResumeIteration, c_data, modified_headers}; + }; + + setUpFilter(R"EOF( +platform_filter_name: StopOnResponseHeadersThenResumeOnData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(invocations.on_response_headers_calls, 1); + + Buffer::OwnedImpl response_data = Buffer::OwnedImpl("response body"); + + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_data, true)); + EXPECT_EQ(invocations.on_response_data_calls, 1); + + EXPECT_TRUE(response_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(response_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), "13"); +} + TEST_F(PlatformBridgeFilterTest, BasicContinueOnResponseData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; From b6afb5238aea94945f42a477ba841d78ca3f52a5 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 29 Sep 2020 21:07:03 +0800 Subject: [PATCH 14/21] format Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/filter.cc | 3 +- .../platform_bridge_filter_test.cc | 38 ++++++++++--------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index a010510c3b..3367cfa558 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -163,7 +163,8 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool // resumption. if (internal_buffer) { internal_buffer->drain(internal_buffer->length()); - internal_buffer->addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); + internal_buffer->addBufferFragment( + *Buffer::BridgeFragment::createBridgeFragment(result.data)); } else { data.drain(data.length()); data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 49f63048e9..6afd3c0642 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -198,7 +198,8 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenResumeOnData) { EXPECT_EQ(to_string(c_data), "request body"); EXPECT_TRUE(end_stream); invocations->on_request_data_calls++; - envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "12"}}); return {kEnvoyFilterDataStatusResumeIteration, c_data, modified_headers}; }; @@ -211,7 +212,8 @@ platform_filter_name: StopOnRequestHeadersThenResumeOnData Http::TestRequestHeaderMapImpl request_headers{{":authority", "test.code"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); EXPECT_EQ(invocations.on_request_headers_calls, 1); Buffer::OwnedImpl request_data = Buffer::OwnedImpl("request body"); @@ -220,7 +222,8 @@ platform_filter_name: StopOnRequestHeadersThenResumeOnData EXPECT_EQ(invocations.on_request_data_calls, 1); EXPECT_TRUE(request_headers.get(Http::LowerCaseString("content-length"))); - EXPECT_EQ(request_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), "12"); + EXPECT_EQ(request_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "12"); } TEST_F(PlatformBridgeFilterTest, BasicContinueOnRequestData) { @@ -325,8 +328,8 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnRequestData) { envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, }; EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); @@ -360,8 +363,7 @@ platform_filter_name: StopAndBufferThenResumeOnRequestData EXPECT_EQ(invocations.on_request_data_calls, 1); Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); - EXPECT_EQ(Http::FilterDataStatus::Continue, - filter_->decodeData(second_chunk, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(second_chunk, false)); // Manual update not required, because once iteration is stopped, data is added directly. EXPECT_EQ(invocations.on_request_data_calls, 2); // Buffer has been updated with value from ResumeIteration. @@ -482,7 +484,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenResumeOnData) { return context; }; platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, - const void* context) -> envoy_filter_headers_status { + const void* context) -> envoy_filter_headers_status { filter_invocations* invocations = static_cast(const_cast(context)); EXPECT_EQ(c_headers.length, 1); EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); @@ -492,12 +494,13 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenResumeOnData) { return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, - const void* context) -> envoy_filter_data_status { + const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); EXPECT_EQ(to_string(c_data), "response body"); EXPECT_TRUE(end_stream); invocations->on_response_data_calls++; - envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "13"}}); return {kEnvoyFilterDataStatusResumeIteration, c_data, modified_headers}; }; @@ -510,7 +513,8 @@ platform_filter_name: StopOnResponseHeadersThenResumeOnData Http::TestResponseHeaderMapImpl response_headers{{":status", "test.code"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); EXPECT_EQ(invocations.on_response_headers_calls, 1); Buffer::OwnedImpl response_data = Buffer::OwnedImpl("response body"); @@ -519,7 +523,8 @@ platform_filter_name: StopOnResponseHeadersThenResumeOnData EXPECT_EQ(invocations.on_response_data_calls, 1); EXPECT_TRUE(response_headers.get(Http::LowerCaseString("content-length"))); - EXPECT_EQ(response_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), "13"); + EXPECT_EQ(response_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "13"); } TEST_F(PlatformBridgeFilterTest, BasicContinueOnResponseData) { @@ -616,7 +621,7 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnResponseData) { return context; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, - const void* context) -> envoy_filter_data_status { + const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); std::string expected_data[2] = {"A", "AB"}; @@ -624,8 +629,8 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnResponseData) { envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, }; EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); @@ -659,8 +664,7 @@ platform_filter_name: StopAndBufferThenResumeOnResponseData EXPECT_EQ(invocations.on_response_data_calls, 1); Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); - EXPECT_EQ(Http::FilterDataStatus::Continue, - filter_->encodeData(second_chunk, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(second_chunk, false)); // Manual update not required, because once iteration is stopped, data is added directly. EXPECT_EQ(invocations.on_response_data_calls, 2); // Buffer has been updated with value from ResumeIteration. From 570121daaa2f582499c0082806e3415d62f78d34 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 02:35:31 +0800 Subject: [PATCH 15/21] update variable comment Signed-off-by: Mike Schore --- library/objective-c/EnvoyEngineImpl.m | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/library/objective-c/EnvoyEngineImpl.m b/library/objective-c/EnvoyEngineImpl.m index d1b5ab4679..6f4ba57b74 100644 --- a/library/objective-c/EnvoyEngineImpl.m +++ b/library/objective-c/EnvoyEngineImpl.m @@ -62,14 +62,14 @@ static envoy_filter_data_status ios_http_filter_on_request_data(envoy_data data, if (filter.onRequestData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, /*data*/ data, - /*extra_headers*/ NULL}; + /*pending_headers*/ NULL}; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onRequestData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeadersPtr(result[2])}; + /*pending_headers*/ toNativeHeadersPtr(result[2])}; } static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data, bool end_stream, @@ -78,14 +78,14 @@ static envoy_filter_data_status ios_http_filter_on_response_data(envoy_data data if (filter.onResponseData == nil) { return (envoy_filter_data_status){/*status*/ kEnvoyFilterDataStatusContinue, /*data*/ data, - /*extra_headers*/ NULL}; + /*pending_headers*/ NULL}; } NSData *platformData = to_ios_data(data); NSArray *result = filter.onResponseData(platformData, end_stream); return (envoy_filter_data_status){/*status*/ [result[0] intValue], /*data*/ toNativeData(result[1]), - /*extra_headers*/ toNativeHeadersPtr(result[2])}; + /*pending_headers*/ toNativeHeadersPtr(result[2])}; } static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_headers trailers, @@ -94,16 +94,16 @@ static envoy_filter_trailers_status ios_http_filter_on_request_trailers(envoy_he if (filter.onRequestTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, /*trailers*/ trailers, - /*extra_headers*/ NULL, - /*extra_trailers*/ NULL}; + /*pending_headers*/ NULL, + /*pending_trailers*/ NULL}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onRequestTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeadersPtr(result[2]), - /*extra_data*/ toNativeDataPtr(result[3])}; + /*pending_headers*/ toNativeHeadersPtr(result[2]), + /*pending_data*/ toNativeDataPtr(result[3])}; } static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_headers trailers, @@ -112,16 +112,16 @@ static envoy_filter_trailers_status ios_http_filter_on_response_trailers(envoy_h if (filter.onResponseTrailers == nil) { return (envoy_filter_trailers_status){/*status*/ kEnvoyFilterTrailersStatusContinue, /*trailers*/ trailers, - /*extra_headers*/ NULL, - /*extra_data*/ NULL}; + /*pending_headers*/ NULL, + /*pending_data*/ NULL}; } EnvoyHeaders *platformTrailers = to_ios_headers(trailers); NSArray *result = filter.onResponseTrailers(platformTrailers); return (envoy_filter_trailers_status){/*status*/ [result[0] intValue], /*trailers*/ toNativeHeaders(result[1]), - /*extra_headers*/ toNativeHeadersPtr(result[2]), - /*extra_data*/ toNativeDataPtr(result[3])}; + /*pending_headers*/ toNativeHeadersPtr(result[2]), + /*pending_data*/ toNativeDataPtr(result[3])}; } static void ios_http_filter_release(const void *context) { From 98d8cee416a5c72cc3814692ca74bc8c44e25d95 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 03:14:06 +0800 Subject: [PATCH 16/21] add tests Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/filter.cc | 10 +- .../platform_bridge_filter_test.cc | 170 ++++++++++++++++++ 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 3367cfa558..ccb0d8467e 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -104,8 +104,10 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool } envoy_data in_data; - if (iteration_state_ == IterationState::Stopped && internal_buffer && - internal_buffer->length() > 0) { + bool already_buffering = iteration_state_ == IterationState::Stopped && internal_buffer && + internal_buffer->length() > 0; + + if (already_buffering) { // Pre-emptively buffer data to present aggregate to platform. internal_buffer->move(data); in_data = Buffer::Utility::copyToBridgeData(*internal_buffer); @@ -124,8 +126,8 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool return Http::FilterDataStatus::Continue; case kEnvoyFilterDataStatusStopIterationAndBuffer: - if (iteration_state_ == IterationState::Stopped) { - // Data will already have been buffered (above). + if (already_buffering) { + // Data will already have been added to the internal buffer (above). return Http::FilterDataStatus::StopIterationNoBuffer; } // Data will be buffered on return. diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 6afd3c0642..1a335586bf 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -370,6 +370,91 @@ platform_filter_name: StopAndBufferThenResumeOnRequestData EXPECT_EQ(decoding_buffer.toString(), "C"); } +TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_request_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":authority"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_request_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); + + envoy_filter_data_status return_status[2] = { + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, modified_headers}, + }; + + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); + EXPECT_EQ(end_stream, invocations->on_request_data_calls == 1); // true on second call + c_data.release(c_data.context); + + return return_status[invocations->on_request_data_calls++]; + }; + + Buffer::OwnedImpl decoding_buffer; + EXPECT_CALL(decoder_callbacks_, decodingBuffer()) + .Times(2) + .WillRepeatedly(Return(&decoding_buffer)); + EXPECT_CALL(decoder_callbacks_, modifyDecodingBuffer(_)) + .Times(2) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(decoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopOnRequestHeadersThenBufferThenResumeOnData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestRequestHeaderMapImpl request_headers{{":authority", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(invocations.on_request_headers_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->decodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + decoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_request_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(second_chunk, true)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_request_data_calls, 2); + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(decoding_buffer.toString(), "C"); + + // Pending headers have been updated with value from ResumeIteration. + EXPECT_TRUE(request_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(request_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "1"); +} + TEST_F(PlatformBridgeFilterTest, StopNoBufferOnRequestData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; @@ -671,6 +756,91 @@ platform_filter_name: StopAndBufferThenResumeOnResponseData EXPECT_EQ(encoding_buffer.toString(), "C"); } +TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnData) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_response_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "1"}}); + + envoy_filter_data_status return_status[2] = { + {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, + {kEnvoyFilterDataStatusResumeIteration, final_data, modified_headers}, + }; + + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); + EXPECT_EQ(end_stream, invocations->on_response_data_calls == 1); // true on second call + c_data.release(c_data.context); + + return return_status[invocations->on_response_data_calls++]; + }; + + Buffer::OwnedImpl encoding_buffer; + EXPECT_CALL(encoder_callbacks_, encodingBuffer()) + .Times(2) + .WillRepeatedly(Return(&encoding_buffer)); + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer(_)) + .Times(2) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(encoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopOnResponseHeadersThenBufferThenResumeOnData +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(invocations.on_response_headers_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->encodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + encoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_response_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(second_chunk, true)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_response_data_calls, 2); + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(encoding_buffer.toString(), "C"); + + // Pending headers have been updated with value from ResumeIteration. + EXPECT_TRUE(response_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(response_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "1"); +} + TEST_F(PlatformBridgeFilterTest, StopNoBufferOnResponseData) { envoy_http_filter platform_filter; filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; From 17778271ccbcf1a2ffa2dfe1b1ddd11ede522a62 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 05:41:12 +0800 Subject: [PATCH 17/21] add tests Signed-off-by: Mike Schore --- .../platform_bridge_filter_test.cc | 194 ++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 1a335586bf..3a6d6f1961 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -525,6 +525,103 @@ platform_filter_name: BasicContinueOnRequestTrailers EXPECT_EQ(invocations.on_request_trailers_calls, 1); } +TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnTrailers) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_request_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":authority"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_request_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); + EXPECT_FALSE(end_stream); + c_data.release(c_data.context); + invocations->on_request_data_calls++; + return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}; + }; + platform_filter.on_request_trailers = [](envoy_headers c_trailers, + const void* context) -> envoy_filter_trailers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_trailers.length, 1); + EXPECT_EQ(to_string(c_trailers.headers[0].key), "x-test-trailer"); + EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer"); + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data* modified_data = + static_cast(safe_malloc(sizeof(envoy_data))); + *modified_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); + + invocations->on_request_trailers_calls++; + return {kEnvoyFilterTrailersStatusResumeIteration, c_trailers, modified_headers, modified_data}; + }; + + Buffer::OwnedImpl decoding_buffer; + EXPECT_CALL(decoder_callbacks_, decodingBuffer()) + .Times(3) + .WillRepeatedly(Return(&decoding_buffer)); + EXPECT_CALL(decoder_callbacks_, modifyDecodingBuffer(_)) + .Times(3) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(decoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopOnRequestHeadersThenBufferThenResumeOnTrailers +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestRequestHeaderMapImpl request_headers{{":authority", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers, false)); + EXPECT_EQ(invocations.on_request_headers_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->decodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + decoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_request_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_->decodeData(second_chunk, false)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_request_data_calls, 2); + EXPECT_EQ(decoding_buffer.toString(), "AB"); + + Http::TestRequestTrailerMapImpl request_trailers{{"x-test-trailer", "test trailer"}}; + + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers)); + EXPECT_EQ(invocations.on_request_trailers_calls, 1); + + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(decoding_buffer.toString(), "C"); + + // Pending headers have been updated with value from ResumeIteration. + EXPECT_TRUE(request_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(request_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "1"); +} + // DIVIDE TEST_F(PlatformBridgeFilterTest, BasicContinueOnResponseHeaders) { @@ -911,6 +1008,103 @@ platform_filter_name: BasicContinueOnResponseTrailers EXPECT_EQ(invocations.on_response_trailers_calls, 1); } +TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnTrailers) { + envoy_http_filter platform_filter; + filter_invocations invocations = {0, 0, 0, 0, 0, 0, 0, 0}; + platform_filter.static_context = &invocations; + platform_filter.init_filter = [](const void* context) -> const void* { + filter_invocations* invocations = static_cast(const_cast(context)); + invocations->init_filter_calls++; + return context; + }; + platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, + const void* context) -> envoy_filter_headers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_headers.length, 1); + EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); + EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); + EXPECT_FALSE(end_stream); + invocations->on_response_headers_calls++; + return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; + }; + platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, + const void* context) -> envoy_filter_data_status { + filter_invocations* invocations = static_cast(const_cast(context)); + std::string expected_data[2] = {"A", "AB"}; + EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); + EXPECT_FALSE(end_stream); + c_data.release(c_data.context); + invocations->on_response_data_calls++; + return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}; + }; + platform_filter.on_response_trailers = [](envoy_headers c_trailers, + const void* context) -> envoy_filter_trailers_status { + filter_invocations* invocations = static_cast(const_cast(context)); + EXPECT_EQ(c_trailers.length, 1); + EXPECT_EQ(to_string(c_trailers.headers[0].key), "x-test-trailer"); + EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer"); + + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data* modified_data = + static_cast(safe_malloc(sizeof(envoy_data))); + *modified_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "1"}}); + + invocations->on_response_trailers_calls++; + return {kEnvoyFilterTrailersStatusResumeIteration, c_trailers, modified_headers, modified_data}; + }; + + Buffer::OwnedImpl encoding_buffer; + EXPECT_CALL(encoder_callbacks_, encodingBuffer()) + .Times(3) + .WillRepeatedly(Return(&encoding_buffer)); + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer(_)) + .Times(3) + .WillRepeatedly(Invoke([&](std::function callback) -> void { + callback(encoding_buffer); + })); + + setUpFilter(R"EOF( +platform_filter_name: StopOnResponseHeadersThenBufferThenResumeOnTrailers +)EOF", + &platform_filter); + EXPECT_EQ(invocations.init_filter_calls, 1); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "test.code"}}; + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); + EXPECT_EQ(invocations.on_response_headers_calls, 1); + + Buffer::OwnedImpl first_chunk = Buffer::OwnedImpl("A"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, + filter_->encodeData(first_chunk, false)); + // Since the return code can't be handled in a unit test, manually update the buffer here. + encoding_buffer.move(first_chunk); + EXPECT_EQ(invocations.on_response_data_calls, 1); + + Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_->encodeData(second_chunk, false)); + // Manual update not required, because once iteration is stopped, data is added directly. + EXPECT_EQ(invocations.on_response_data_calls, 2); + EXPECT_EQ(encoding_buffer.toString(), "AB"); + + Http::TestResponseTrailerMapImpl response_trailers{{"x-test-trailer", "test trailer"}}; + + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(response_trailers)); + EXPECT_EQ(invocations.on_response_trailers_calls, 1); + + // Buffer has been updated with value from ResumeIteration. + EXPECT_EQ(encoding_buffer.toString(), "C"); + + // Pending headers have been updated with value from ResumeIteration. + EXPECT_TRUE(response_headers.get(Http::LowerCaseString("content-length"))); + EXPECT_EQ(response_headers.get(Http::LowerCaseString("content-length"))->value().getStringView(), + "1"); +} + } // namespace } // namespace PlatformBridge } // namespace HttpFilters From 154df833d7b3cd2bad8ed357baf810c2c05b41c6 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 05:41:38 +0800 Subject: [PATCH 18/21] format Signed-off-by: Mike Schore --- .../filters/http/platform_bridge/filter.cc | 2 +- .../platform_bridge_filter_test.cc | 24 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index ccb0d8467e..dc81cbe27c 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -105,7 +105,7 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool envoy_data in_data; bool already_buffering = iteration_state_ == IterationState::Stopped && internal_buffer && - internal_buffer->length() > 0; + internal_buffer->length() > 0; if (already_buffering) { // Pre-emptively buffer data to present aggregate to platform. diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 3a6d6f1961..a49c26df56 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -391,7 +391,6 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnData) }; platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, const void* context) -> envoy_filter_data_status { - filter_invocations* invocations = static_cast(const_cast(context)); std::string expected_data[2] = {"A", "AB"}; @@ -562,8 +561,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnTrail EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer"); Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data* modified_data = - static_cast(safe_malloc(sizeof(envoy_data))); + envoy_data* modified_data = static_cast(safe_malloc(sizeof(envoy_data))); *modified_data = Buffer::Utility::toBridgeData(final_buffer); envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); @@ -603,7 +601,8 @@ platform_filter_name: StopOnRequestHeadersThenBufferThenResumeOnTrailers EXPECT_EQ(invocations.on_request_data_calls, 1); Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); - EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_->decodeData(second_chunk, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, + filter_->decodeData(second_chunk, false)); // Manual update not required, because once iteration is stopped, data is added directly. EXPECT_EQ(invocations.on_request_data_calls, 2); EXPECT_EQ(decoding_buffer.toString(), "AB"); @@ -863,7 +862,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnData return context; }; platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, - const void* context) -> envoy_filter_headers_status { + const void* context) -> envoy_filter_headers_status { filter_invocations* invocations = static_cast(const_cast(context)); EXPECT_EQ(c_headers.length, 1); EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); @@ -873,8 +872,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnData return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, - const void* context) -> envoy_filter_data_status { - + const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); std::string expected_data[2] = {"A", "AB"}; @@ -1018,7 +1016,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnTrai return context; }; platform_filter.on_response_headers = [](envoy_headers c_headers, bool end_stream, - const void* context) -> envoy_filter_headers_status { + const void* context) -> envoy_filter_headers_status { filter_invocations* invocations = static_cast(const_cast(context)); EXPECT_EQ(c_headers.length, 1); EXPECT_EQ(to_string(c_headers.headers[0].key), ":status"); @@ -1028,7 +1026,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnTrai return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, - const void* context) -> envoy_filter_data_status { + const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); std::string expected_data[2] = {"A", "AB"}; EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); @@ -1038,15 +1036,14 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnTrai return {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}; }; platform_filter.on_response_trailers = [](envoy_headers c_trailers, - const void* context) -> envoy_filter_trailers_status { + const void* context) -> envoy_filter_trailers_status { filter_invocations* invocations = static_cast(const_cast(context)); EXPECT_EQ(c_trailers.length, 1); EXPECT_EQ(to_string(c_trailers.headers[0].key), "x-test-trailer"); EXPECT_EQ(to_string(c_trailers.headers[0].value), "test trailer"); Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data* modified_data = - static_cast(safe_malloc(sizeof(envoy_data))); + envoy_data* modified_data = static_cast(safe_malloc(sizeof(envoy_data))); *modified_data = Buffer::Utility::toBridgeData(final_buffer); envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); @@ -1086,7 +1083,8 @@ platform_filter_name: StopOnResponseHeadersThenBufferThenResumeOnTrailers EXPECT_EQ(invocations.on_response_data_calls, 1); Buffer::OwnedImpl second_chunk = Buffer::OwnedImpl("B"); - EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_->encodeData(second_chunk, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, + filter_->encodeData(second_chunk, false)); // Manual update not required, because once iteration is stopped, data is added directly. EXPECT_EQ(invocations.on_response_data_calls, 2); EXPECT_EQ(encoding_buffer.toString(), "AB"); From ed82982298617c7a3d5671e2065546033193b919 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 08:57:30 +0800 Subject: [PATCH 19/21] fix asan failures in tests Signed-off-by: Mike Schore --- .../http/platform_bridge/platform_bridge_filter_test.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index a49c26df56..fab95bd0e5 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -190,6 +190,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenResumeOnData) { EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_request_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, @@ -387,6 +388,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnData) EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_request_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, @@ -541,6 +543,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnTrail EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_request_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, @@ -672,6 +675,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenResumeOnData) { EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_response_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, @@ -869,6 +873,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnData EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_response_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, @@ -1023,6 +1028,7 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnTrai EXPECT_EQ(to_string(c_headers.headers[0].value), "test.code"); EXPECT_FALSE(end_stream); invocations->on_response_headers_calls++; + release_envoy_headers(c_headers); return {kEnvoyFilterHeadersStatusStopIteration, envoy_noheaders}; }; platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, From 606505ced339eb24fe6edd8c3fa60d6c67e791b6 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 19:39:46 +0800 Subject: [PATCH 20/21] fix tests for asan Signed-off-by: Mike Schore --- .../platform_bridge_filter_test.cc | 148 +++++++++++------- 1 file changed, 90 insertions(+), 58 deletions(-) diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index fab95bd0e5..25f05f7b78 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -323,21 +323,29 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnRequestData) { platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); - std::string expected_data[2] = {"A", "AB"}; - - Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_filter_data_status return_status; + + if (invocations->on_request_data_calls == 0) { + EXPECT_EQ(to_string(c_data), "A"); + EXPECT_FALSE(end_stream); + + return_status.status = kEnvoyFilterDataStatusStopIterationAndBuffer; + return_status.data = envoy_nodata; + return_status.pending_headers = nullptr; + } else { + EXPECT_EQ(to_string(c_data), "AB"); + EXPECT_FALSE(end_stream); + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + + return_status.status = kEnvoyFilterDataStatusResumeIteration; + return_status.data = final_data; + return_status.pending_headers = nullptr; + } - envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, - }; - - EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); - EXPECT_FALSE(end_stream); + invocations->on_request_data_calls++; c_data.release(c_data.context); - - return return_status[invocations->on_request_data_calls++]; + return return_status; }; Buffer::OwnedImpl decoding_buffer; @@ -394,24 +402,32 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnData) platform_filter.on_request_data = [](envoy_data c_data, bool end_stream, const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); - std::string expected_data[2] = {"A", "AB"}; + envoy_filter_data_status return_status; + + if (invocations->on_request_data_calls == 0) { + EXPECT_EQ(to_string(c_data), "A"); + EXPECT_FALSE(end_stream); + + return_status.status = kEnvoyFilterDataStatusStopIterationAndBuffer; + return_status.data = envoy_nodata; + return_status.pending_headers = nullptr; + } else { + EXPECT_EQ(to_string(c_data), "AB"); + EXPECT_TRUE(end_stream); + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); + + return_status.status = kEnvoyFilterDataStatusResumeIteration; + return_status.data = final_data; + return_status.pending_headers = modified_headers; + } - Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); - envoy_headers* modified_headers = - static_cast(safe_malloc(sizeof(envoy_headers))); - *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); - - envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, modified_headers}, - }; - - EXPECT_EQ(to_string(c_data), expected_data[invocations->on_request_data_calls]); - EXPECT_EQ(end_stream, invocations->on_request_data_calls == 1); // true on second call + invocations->on_request_data_calls++; c_data.release(c_data.context); - - return return_status[invocations->on_request_data_calls++]; + return return_status; }; Buffer::OwnedImpl decoding_buffer; @@ -808,21 +824,29 @@ TEST_F(PlatformBridgeFilterTest, StopAndBufferThenResumeOnResponseData) { platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); - std::string expected_data[2] = {"A", "AB"}; - - Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_filter_data_status return_status; + + if (invocations->on_response_data_calls == 0) { + EXPECT_EQ(to_string(c_data), "A"); + EXPECT_FALSE(end_stream); + + return_status.status = kEnvoyFilterDataStatusStopIterationAndBuffer; + return_status.data = envoy_nodata; + return_status.pending_headers = nullptr; + } else { + EXPECT_EQ(to_string(c_data), "AB"); + EXPECT_FALSE(end_stream); + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + + return_status.status = kEnvoyFilterDataStatusResumeIteration; + return_status.data = final_data; + return_status.pending_headers = nullptr; + } - envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, nullptr}, - }; - - EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); - EXPECT_FALSE(end_stream); + invocations->on_response_data_calls++; c_data.release(c_data.context); - - return return_status[invocations->on_response_data_calls++]; + return return_status; }; Buffer::OwnedImpl encoding_buffer; @@ -879,24 +903,32 @@ TEST_F(PlatformBridgeFilterTest, StopOnResponseHeadersThenBufferThenResumeOnData platform_filter.on_response_data = [](envoy_data c_data, bool end_stream, const void* context) -> envoy_filter_data_status { filter_invocations* invocations = static_cast(const_cast(context)); - std::string expected_data[2] = {"A", "AB"}; + envoy_filter_data_status return_status; + + if (invocations->on_response_data_calls == 0) { + EXPECT_EQ(to_string(c_data), "A"); + EXPECT_FALSE(end_stream); + + return_status.status = kEnvoyFilterDataStatusStopIterationAndBuffer; + return_status.data = envoy_nodata; + return_status.pending_headers = nullptr; + } else { + EXPECT_EQ(to_string(c_data), "AB"); + EXPECT_TRUE(end_stream); + Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); + envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); + envoy_headers* modified_headers = + static_cast(safe_malloc(sizeof(envoy_headers))); + *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "1"}}); + + return_status.status = kEnvoyFilterDataStatusResumeIteration; + return_status.data = final_data; + return_status.pending_headers = modified_headers; + } - Buffer::OwnedImpl final_buffer = Buffer::OwnedImpl("C"); - envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); - envoy_headers* modified_headers = - static_cast(safe_malloc(sizeof(envoy_headers))); - *modified_headers = make_envoy_headers({{":status", "test.code"}, {"content-length", "1"}}); - - envoy_filter_data_status return_status[2] = { - {kEnvoyFilterDataStatusStopIterationAndBuffer, envoy_nodata, nullptr}, - {kEnvoyFilterDataStatusResumeIteration, final_data, modified_headers}, - }; - - EXPECT_EQ(to_string(c_data), expected_data[invocations->on_response_data_calls]); - EXPECT_EQ(end_stream, invocations->on_response_data_calls == 1); // true on second call + invocations->on_response_data_calls++; c_data.release(c_data.context); - - return return_status[invocations->on_response_data_calls++]; + return return_status; }; Buffer::OwnedImpl encoding_buffer; From c9a8028180d08c2df1d7dad0fa7c07a39f835ae4 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Wed, 30 Sep 2020 20:18:31 +0800 Subject: [PATCH 21/21] format Signed-off-by: Mike Schore --- .../http/platform_bridge/platform_bridge_filter_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc index 25f05f7b78..6a72ba15ec 100644 --- a/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc +++ b/test/common/extensions/filters/http/platform_bridge/platform_bridge_filter_test.cc @@ -418,7 +418,8 @@ TEST_F(PlatformBridgeFilterTest, StopOnRequestHeadersThenBufferThenResumeOnData) envoy_data final_data = Buffer::Utility::toBridgeData(final_buffer); envoy_headers* modified_headers = static_cast(safe_malloc(sizeof(envoy_headers))); - *modified_headers = make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); + *modified_headers = + make_envoy_headers({{":authority", "test.code"}, {"content-length", "1"}}); return_status.status = kEnvoyFilterDataStatusResumeIteration; return_status.data = final_data;