diff --git a/mobile/examples/swift/hello_world/DemoFilter.swift b/mobile/examples/swift/hello_world/DemoFilter.swift index fad0b0d2a016..b34e1428a7a8 100644 --- a/mobile/examples/swift/hello_world/DemoFilter.swift +++ b/mobile/examples/swift/hello_world/DemoFilter.swift @@ -3,7 +3,7 @@ import Foundation struct DemoFilter: ResponseFilter { // TODO(goaway): Update once dynamic registration is in place. - let name = "PlatformStub" + static let name = "PlatformStub" func onResponseHeaders(_ headers: ResponseHeaders, endStream: Bool) -> FilterHeaderStatus diff --git a/mobile/examples/swift/hello_world/ViewController.swift b/mobile/examples/swift/hello_world/ViewController.swift index b79876f14ce9..3b60def8d4bd 100644 --- a/mobile/examples/swift/hello_world/ViewController.swift +++ b/mobile/examples/swift/hello_world/ViewController.swift @@ -15,7 +15,7 @@ final class ViewController: UITableViewController { super.viewDidLoad() do { NSLog("starting Envoy...") - self.client = try StreamClientBuilder().addFilter(DemoFilter()).build() + self.client = try StreamClientBuilder().addFilter(DemoFilter.self).build() } catch let error { NSLog("starting Envoy failed: \(error)") } diff --git a/mobile/library/common/extensions/filters/http/platform_bridge/BUILD b/mobile/library/common/extensions/filters/http/platform_bridge/BUILD index ff45273656ea..93892c6802ce 100644 --- a/mobile/library/common/extensions/filters/http/platform_bridge/BUILD +++ b/mobile/library/common/extensions/filters/http/platform_bridge/BUILD @@ -25,6 +25,7 @@ envoy_cc_library( "//library/common/http:header_utility_lib", "//library/common/types:c_types_lib", "@envoy//include/envoy/http:filter_interface", + "@envoy//source/common/common:minimal_logger_lib", "@envoy//source/extensions/filters/http/common:pass_through_filter_lib", ], ) diff --git a/mobile/library/common/extensions/filters/http/platform_bridge/c_types.h b/mobile/library/common/extensions/filters/http/platform_bridge/c_types.h index ca8896a51ca6..e029caa31c54 100644 --- a/mobile/library/common/extensions/filters/http/platform_bridge/c_types.h +++ b/mobile/library/common/extensions/filters/http/platform_bridge/c_types.h @@ -57,23 +57,35 @@ typedef struct { extern "C" { // function pointers #endif +/** + * Function signature for filter factory. Implementations must return a instance_context + * capable of dispatching envoy_http_filter calls (below) to a platform filter instance. + */ +typedef const void* (*envoy_filter_init_f)(const void* context); + /** * Function signature for on-headers filter invocations. */ typedef envoy_filter_headers_status (*envoy_filter_on_headers_f)(envoy_headers headers, - bool end_stream, void* context); + bool end_stream, + const void* context); /** * Function signature for on-data filter invocations. */ typedef envoy_filter_data_status (*envoy_filter_on_data_f)(envoy_data data, bool end_stream, - void* context); + const void* context); /** * Function signature for on-trailers filter invocations. */ typedef envoy_filter_trailers_status (*envoy_filter_on_trailers_f)(envoy_headers trailers, - void* context); + const void* context); + +/** + * Function signature to release a filter instance once the filter chain is finished with it. + */ +typedef void (*envoy_filter_release_f)(const void* context); #ifdef __cplusplus } // function pointers @@ -84,11 +96,14 @@ typedef envoy_filter_trailers_status (*envoy_filter_on_trailers_f)(envoy_headers * PlatformBridgeFilter. */ typedef struct { + envoy_filter_init_f init_filter; envoy_filter_on_headers_f on_request_headers; envoy_filter_on_data_f on_request_data; envoy_filter_on_trailers_f on_request_trailers; envoy_filter_on_headers_f on_response_headers; envoy_filter_on_data_f on_response_data; envoy_filter_on_trailers_f on_response_trailers; - void* context; + envoy_filter_release_f release_filter; + const void* static_context; + const void* instance_context; } envoy_http_filter; diff --git a/mobile/library/common/extensions/filters/http/platform_bridge/filter.cc b/mobile/library/common/extensions/filters/http/platform_bridge/filter.cc index 46336b731a9f..1c9ff3556444 100644 --- a/mobile/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/mobile/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -17,22 +17,54 @@ namespace PlatformBridge { PlatformBridgeFilterConfig::PlatformBridgeFilterConfig( const envoymobile::extensions::filters::http::platform_bridge::PlatformBridge& proto_config) - : platform_filter_(static_cast( + : filter_name_(proto_config.platform_filter_name()), + platform_filter_(static_cast( Api::External::retrieveApi(proto_config.platform_filter_name()))) {} PlatformBridgeFilter::PlatformBridgeFilter(PlatformBridgeFilterConfigSharedPtr config) - : platform_filter_(config->platform_filter()) {} + : filter_name_(config->filter_name()), platform_filter_(*config->platform_filter()) { + // The initialization above sets platform_filter_ to a copy of the struct stored on the config. + // In the typical case, this will represent a filter implementation that needs to be intantiated. + // static_context will contain the necessary platform-specific mechanism to produce a filter + // instance. instance_context will initially be null, but after initialization, set to the + // context needed for actual filter invocations. + + // If init_filter is missing, zero out the rest of the struct for safety. + if (platform_filter_.init_filter == nullptr) { + ENVOY_LOG(debug, "platform bridge filter: missing initializer for {}", filter_name_); + platform_filter_ = {}; + return; + } + + // Set the instance_context to the result of the initialization call. Cleanup will ultimately + // occur during in the onDestroy() invocation below. + platform_filter_.instance_context = platform_filter_.init_filter(platform_filter_.static_context); + ASSERT(platform_filter_.instance_context, + fmt::format("init_filter unsuccessful for {}", filter_name_)); +} + +void PlatformBridgeFilter::onDestroy() { + // Allow nullptr as no-op only if nothing was initialized. + if (platform_filter_.release_filter == nullptr) { + ASSERT(!platform_filter_.instance_context, + fmt::format("release_filter required for {}", filter_name_)); + return; + } + + platform_filter_.release_filter(platform_filter_.instance_context); + platform_filter_.instance_context = nullptr; +} Http::FilterHeadersStatus PlatformBridgeFilter::onHeaders(Http::HeaderMap& headers, bool end_stream, envoy_filter_on_headers_f on_headers) { - // Allow nullptr to act as (optimized) no-op. + // Allow nullptr to act as no-op. if (on_headers == nullptr) { return Http::FilterHeadersStatus::Continue; } envoy_headers in_headers = Http::Utility::toBridgeHeaders(headers); envoy_filter_headers_status result = - on_headers(in_headers, end_stream, platform_filter_->context); + 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 @@ -50,22 +82,19 @@ Http::FilterHeadersStatus PlatformBridgeFilter::onHeaders(Http::HeaderMap& heade Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool end_stream, envoy_filter_on_data_f on_data) { - // Allow nullptr to act as (optimized) no-op. + // Allow nullptr to act as no-op. if (on_data == nullptr) { return Http::FilterDataStatus::Continue; } envoy_data in_data = Buffer::Utility::toBridgeData(data); - envoy_filter_data_status result = on_data(in_data, end_stream, platform_filter_->context); + envoy_filter_data_status result = on_data(in_data, end_stream, platform_filter_.instance_context); Http::FilterDataStatus status = static_cast(result.status); - // Current platform implementations expose immutable data, thus any modification necessitates a - // full copy. If the returned buffer is identical, we assume no modification was made and elide - // the copy here. See also https://github.com/lyft/envoy-mobile/issues/949 for potential future - // optimization. - if (in_data.bytes != result.data.bytes) { - data.drain(data.length()); - data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); - } + // 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. + data.drain(data.length()); + data.addBufferFragment(*Buffer::BridgeFragment::createBridgeFragment(result.data)); return status; } @@ -73,12 +102,12 @@ Http::FilterDataStatus PlatformBridgeFilter::onData(Buffer::Instance& data, bool 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); + return onHeaders(headers, end_stream, platform_filter_.on_request_headers); } Http::FilterDataStatus PlatformBridgeFilter::decodeData(Buffer::Instance& data, bool end_stream) { // Delegate to shared implementation for request and response path. - return onData(data, end_stream, platform_filter_->on_request_data); + return onData(data, end_stream, platform_filter_.on_request_data); } Http::FilterTrailersStatus @@ -98,12 +127,12 @@ PlatformBridgeFilter::encode100ContinueHeaders(Http::ResponseHeaderMap& /*header 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); + 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. - return onData(data, end_stream, platform_filter_->on_response_data); + return onData(data, end_stream, platform_filter_.on_response_data); } Http::FilterTrailersStatus diff --git a/mobile/library/common/extensions/filters/http/platform_bridge/filter.h b/mobile/library/common/extensions/filters/http/platform_bridge/filter.h index b9c93c3ee159..20e1bcd6f147 100644 --- a/mobile/library/common/extensions/filters/http/platform_bridge/filter.h +++ b/mobile/library/common/extensions/filters/http/platform_bridge/filter.h @@ -2,6 +2,8 @@ #include "envoy/http/filter.h" +#include "common/common/logger.h" + #include "extensions/filters/http/common/pass_through_filter.h" #include "library/common/extensions/filters/http/platform_bridge/c_types.h" @@ -17,9 +19,11 @@ class PlatformBridgeFilterConfig { PlatformBridgeFilterConfig( const envoymobile::extensions::filters::http::platform_bridge::PlatformBridge& proto_config); + const std::string& filter_name() { return filter_name_; } const envoy_http_filter* platform_filter() const { return platform_filter_; } private: + const std::string filter_name_; const envoy_http_filter* platform_filter_; }; @@ -28,10 +32,14 @@ typedef std::shared_ptr PlatformBridgeFilterConfigSh /** * Harness to bridge Envoy filter invocations up to the platform layer. */ -class PlatformBridgeFilter final : public Http::PassThroughFilter { +class PlatformBridgeFilter final : public Http::PassThroughFilter, + Logger::Loggable { public: PlatformBridgeFilter(PlatformBridgeFilterConfigSharedPtr config); + // StreamFilterBase + void onDestroy() override; + // StreamDecoderFilter Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool end_stream) override; @@ -52,7 +60,8 @@ class PlatformBridgeFilter final : public Http::PassThroughFilter { envoy_filter_on_headers_f on_headers); Http::FilterDataStatus onData(Buffer::Instance& data, bool end_stream, envoy_filter_on_data_f on_data); - const envoy_http_filter* platform_filter_; + const std::string filter_name_; + envoy_http_filter platform_filter_; }; } // namespace PlatformBridge diff --git a/mobile/library/objective-c/BUILD b/mobile/library/objective-c/BUILD index ae05429668cf..115722837a78 100644 --- a/mobile/library/objective-c/BUILD +++ b/mobile/library/objective-c/BUILD @@ -13,6 +13,7 @@ objc_library( "EnvoyEngineImpl.m", "EnvoyHTTPCallbacks.m", "EnvoyHTTPFilter.m", + "EnvoyHTTPFilterFactory.m", "EnvoyHTTPStreamImpl.m", "EnvoyNetworkMonitor.m", ], diff --git a/mobile/library/objective-c/EnvoyConfiguration.m b/mobile/library/objective-c/EnvoyConfiguration.m index 06228e3307eb..42d30fffc862 100644 --- a/mobile/library/objective-c/EnvoyConfiguration.m +++ b/mobile/library/objective-c/EnvoyConfiguration.m @@ -9,7 +9,7 @@ - (instancetype)initWithStatsDomain:(NSString *)statsDomain dnsRefreshSeconds:(UInt32)dnsRefreshSeconds dnsFailureRefreshSecondsBase:(UInt32)dnsFailureRefreshSecondsBase dnsFailureRefreshSecondsMax:(UInt32)dnsFailureRefreshSecondsMax - filterChain:(NSArray *)httpFilters + filterChain:(NSArray *)httpFilterFactories statsFlushSeconds:(UInt32)statsFlushSeconds appVersion:(NSString *)appVersion appId:(NSString *)appId @@ -24,7 +24,7 @@ - (instancetype)initWithStatsDomain:(NSString *)statsDomain self.dnsRefreshSeconds = dnsRefreshSeconds; self.dnsFailureRefreshSecondsBase = dnsFailureRefreshSecondsBase; self.dnsFailureRefreshSecondsMax = dnsFailureRefreshSecondsMax; - self.httpFilters = httpFilters; + self.httpFilterFactories = httpFilterFactories; self.statsFlushSeconds = statsFlushSeconds; self.appVersion = appVersion; self.appId = appId; diff --git a/mobile/library/objective-c/EnvoyEngine.h b/mobile/library/objective-c/EnvoyEngine.h index 3a8e0aa076b5..a3602339fc2b 100644 --- a/mobile/library/objective-c/EnvoyEngine.h +++ b/mobile/library/objective-c/EnvoyEngine.h @@ -68,14 +68,22 @@ extern const int kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; @interface EnvoyHTTPFilter : NSObject -@property (nonatomic, strong) NSString *name; - @property (nonatomic, strong) NSArray * (^onRequestHeaders)(EnvoyHeaders *headers, BOOL endStream); @property (nonatomic, strong) NSArray * (^onResponseHeaders)(EnvoyHeaders *headers, BOOL endStream); @end +#pragma mark - EnvoyHTTPFilterFactory + +@interface EnvoyHTTPFilterFactory : NSObject + +@property (nonatomic, strong) NSString *filterName; + +@property (nonatomic, strong) EnvoyHTTPFilter * (^create)(); + +@end + #pragma mark - EnvoyHTTPStream @protocol EnvoyHTTPStream @@ -143,7 +151,7 @@ extern const int kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; @property (nonatomic, assign) UInt32 dnsRefreshSeconds; @property (nonatomic, assign) UInt32 dnsFailureRefreshSecondsBase; @property (nonatomic, assign) UInt32 dnsFailureRefreshSecondsMax; -@property (nonatomic, strong) NSArray *httpFilters; +@property (nonatomic, strong) NSArray *httpFilterFactories; @property (nonatomic, assign) UInt32 statsFlushSeconds; @property (nonatomic, strong) NSString *appVersion; @property (nonatomic, strong) NSString *appId; @@ -157,7 +165,7 @@ extern const int kEnvoyFilterHeadersStatusStopAllIterationAndBuffer; dnsRefreshSeconds:(UInt32)dnsRefreshSeconds dnsFailureRefreshSecondsBase:(UInt32)dnsFailureRefreshSecondsBase dnsFailureRefreshSecondsMax:(UInt32)dnsFailureRefreshSecondsMax - filterChain:(NSArray *)httpFilters + filterChain:(NSArray *)httpFilterFactories statsFlushSeconds:(UInt32)statsFlushSeconds appVersion:(NSString *)appVersion appId:(NSString *)appId diff --git a/mobile/library/objective-c/EnvoyEngineImpl.m b/mobile/library/objective-c/EnvoyEngineImpl.m index 219c30ffa89f..0026859d73ee 100644 --- a/mobile/library/objective-c/EnvoyEngineImpl.m +++ b/mobile/library/objective-c/EnvoyEngineImpl.m @@ -11,10 +11,6 @@ static void ios_on_exit() { NSLog(@"[Envoy] library is exiting"); } -typedef struct { - __unsafe_unretained EnvoyHTTPFilter *filter; -} ios_http_filter_context; - // TODO(goaway): The mapping code below contains a great deal of duplication from // EnvoyHTTPStreamImpl.m, however retain/release semantics are slightly different and need to be // reconciled before this can be factored into a generic set of utility functions. @@ -66,11 +62,17 @@ static envoy_headers toNativeHeaders(EnvoyHeaders *headers) { return (envoy_headers){length, header_array}; } +static const void *ios_http_filter_init(const void *context) { + EnvoyHTTPFilterFactory *filterFactory = (__bridge EnvoyHTTPFilterFactory *)context; + EnvoyHTTPFilter *filter = filterFactory.create(); + return CFBridgingRetain(filter); +} + static envoy_filter_headers_status -ios_http_filter_on_request_headers(envoy_headers headers, bool end_stream, void *context) { +ios_http_filter_on_request_headers(envoy_headers headers, bool end_stream, const void *context) { // TODO(goaway): optimize unmodified case - ios_http_filter_context *c = (ios_http_filter_context *)context; - if (c->filter.onRequestHeaders == nil) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onRequestHeaders == nil) { return (envoy_filter_headers_status){/*status*/ kEnvoyFilterHeadersStatusContinue, /*headers*/ headers}; } @@ -78,16 +80,16 @@ static envoy_headers toNativeHeaders(EnvoyHeaders *headers) { EnvoyHeaders *platformHeaders = to_ios_headers(headers); release_envoy_headers(headers); // TODO(goaway): consider better solution for compound return - NSArray *result = c->filter.onRequestHeaders(platformHeaders, end_stream); + NSArray *result = filter.onRequestHeaders(platformHeaders, end_stream); return (envoy_filter_headers_status){/*status*/ [result[0] intValue], /*headers*/ toNativeHeaders(result[1])}; } static envoy_filter_headers_status -ios_http_filter_on_response_headers(envoy_headers headers, bool end_stream, void *context) { +ios_http_filter_on_response_headers(envoy_headers headers, bool end_stream, const void *context) { // TODO(goaway): optimize unmodified case - ios_http_filter_context *c = (ios_http_filter_context *)context; - if (c->filter.onResponseHeaders == nil) { + EnvoyHTTPFilter *filter = (__bridge EnvoyHTTPFilter *)context; + if (filter.onResponseHeaders == nil) { return (envoy_filter_headers_status){/*status*/ kEnvoyFilterHeadersStatusContinue, /*headers*/ headers}; } @@ -95,11 +97,16 @@ static envoy_headers toNativeHeaders(EnvoyHeaders *headers) { EnvoyHeaders *platformHeaders = to_ios_headers(headers); release_envoy_headers(headers); // TODO(goaway): consider better solution for compound return - NSArray *result = c->filter.onResponseHeaders(platformHeaders, end_stream); + NSArray *result = filter.onResponseHeaders(platformHeaders, end_stream); return (envoy_filter_headers_status){/*status*/ [result[0] intValue], /*headers*/ toNativeHeaders(result[1])}; } +static void ios_http_filter_release(const void *context) { + CFRelease(context); + return; +} + @implementation EnvoyEngineImpl { envoy_engine_t _engineHandle; } @@ -119,19 +126,20 @@ - (void)dealloc { [[NSNotificationCenter defaultCenter] removeObserver:self]; } -- (int)registerFilter:(EnvoyHTTPFilter *)filter { +- (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 - ios_http_filter_context *context = safe_malloc(sizeof(ios_http_filter_context)); - CFBridgingRetain(filter); - context->filter = filter; 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 = NULL; api->on_response_headers = ios_http_filter_on_response_headers; api->on_response_data = NULL; - api->context = context; - register_platform_api(filter.name.UTF8String, api); + 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; } @@ -142,8 +150,8 @@ - (int)runWithConfig:(EnvoyConfiguration *)config logLevel:(NSString *)logLevel return kEnvoyFailure; } - for (EnvoyHTTPFilter *filter in config.httpFilters) { - [self registerFilter:filter]; + for (EnvoyHTTPFilterFactory *filterFactory in config.httpFilterFactories) { + [self registerFilterFactory:filterFactory]; } return [self runWithConfigYAML:resolvedYAML logLevel:logLevel]; diff --git a/mobile/library/objective-c/EnvoyHTTPFilterFactory.m b/mobile/library/objective-c/EnvoyHTTPFilterFactory.m new file mode 100644 index 000000000000..7326595de383 --- /dev/null +++ b/mobile/library/objective-c/EnvoyHTTPFilterFactory.m @@ -0,0 +1,4 @@ +#import "library/objective-c/EnvoyEngine.h" + +@implementation EnvoyHTTPFilterFactory +@end diff --git a/mobile/library/swift/src/StreamClientBuilder.swift b/mobile/library/swift/src/StreamClientBuilder.swift index af826597287f..677246735b7b 100644 --- a/mobile/library/swift/src/StreamClientBuilder.swift +++ b/mobile/library/swift/src/StreamClientBuilder.swift @@ -21,7 +21,7 @@ public final class StreamClientBuilder: NSObject { private var statsFlushSeconds: UInt32 = 60 private var appVersion: String = "unspecified" private var appId: String = "unspecified" - private var filterChain: [EnvoyHTTPFilter] = [] + private var filterChain: [EnvoyHTTPFilterFactory] = [] private var virtualClusters: String = "[]" // MARK: - Public @@ -117,9 +117,8 @@ public final class StreamClientBuilder: NSObject { /// /// - returns: This builder. @discardableResult - public func addFilter(_ filter: Filter) -> StreamClientBuilder { - // TODO(goaway): Update types here for per-stream instances. - self.filterChain.append(EnvoyHTTPFilter(filter: filter)) + public func addFilter(_ filterType: Filter.Type) -> StreamClientBuilder { + self.filterChain.append(EnvoyHTTPFilterFactory(filterType: filterType)) return self } diff --git a/mobile/library/swift/src/filters/Filter.swift b/mobile/library/swift/src/filters/Filter.swift index f6b32d62829c..3a49b299bd63 100644 --- a/mobile/library/swift/src/filters/Filter.swift +++ b/mobile/library/swift/src/filters/Filter.swift @@ -4,14 +4,27 @@ import Foundation /// Interface representing a filter. See `RequestFilter` and `ResponseFilter` for more details. public protocol Filter { /// A unique name for a filter implementation. Needed for extension registration. - var name: String { get } + static var name: String { get } + + /// Required initializer for internal creation. + init() +} + +extension EnvoyHTTPFilterFactory { + convenience init(filterType: Filter.Type) { + self.init() + + self.filterName = filterType.name + self.create = { + return EnvoyHTTPFilter(filter: filterType.init()) + } + } } extension EnvoyHTTPFilter { /// Initialize an EnvoyHTTPFilter using the instance methods of a concrete Filter implementation. convenience init(filter: Filter) { self.init() - self.name = filter.name if let requestFilter = filter as? RequestFilter { self.onRequestHeaders = { envoyHeaders, endStream in