From 5567409ef4a9e6b400bcefa9d26b04690d2379ad Mon Sep 17 00:00:00 2001 From: gargnupur Date: Thu, 14 Nov 2019 11:43:44 -0800 Subject: [PATCH 1/5] Use connection's filter state object when available as compared to request's filter state object Signed-off-by: gargnupur Fixed based on feedback Signed-off-by: gargnupur Fixed formatting Signed-off-by: gargnupur --- source/extensions/common/wasm/context.cc | 35 +++++++++++++++++++++--- source/extensions/common/wasm/context.h | 7 +++++ source/extensions/common/wasm/wasm.cc | 1 - 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index b6b917970f..e48acf15f2 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -343,8 +343,11 @@ WasmResult serializeValue(Filters::Common::Expr::CelValue value, std::string* re // An expression wrapper for the WASM state class WasmStateWrapper : public google::api::expr::runtime::CelMap { public: - WasmStateWrapper(const StreamInfo::FilterState& filter_state) : filter_state_(filter_state) {} - absl::optional + WasmStateWrapper(const StreamInfo::FilterState& filter_state, + const StreamInfo::FilterState* connection_filter_state) + : filter_state_(filter_state), connection_filter_state_(connection_filter_state) {} + WasmStateWrapper(const StreamInfo::FilterState& filter_state) + : filter_state_(filter_state), connection_filter_state_(nullptr) {}absl::optional operator[](google::api::expr::runtime::CelValue key) const override { if (!key.IsString()) { return {}; @@ -353,9 +356,18 @@ class WasmStateWrapper : public google::api::expr::runtime::CelMap { try { const WasmState& result = filter_state_.getDataReadOnly(value); return google::api::expr::runtime::CelValue::CreateBytes(&result.value()); + } catch (const EnvoyException& e) { + // If doesn't exist in request filter state, try looking up in connection filter state. + try { + if (connection_filter_state_) { + const WasmState& result = connection_filter_state_->getDataReadOnly(value); + return google::api::expr::runtime::CelValue::CreateBytes(&result.value()); + } } catch (const EnvoyException& e) { return {}; } + return {}; + } } int size() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } bool empty() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } @@ -365,6 +377,7 @@ class WasmStateWrapper : public google::api::expr::runtime::CelMap { private: const StreamInfo::FilterState& filter_state_; + const StreamInfo::FilterState* connection_filter_state_; }; #define PROPERTY_TOKENS(_f) \ @@ -427,8 +440,13 @@ WasmResult Context::getProperty(absl::string_view path, std::string* result) { value = CelValue::CreateMessage(&info->dynamicMetadata(), &arena); break; case PropertyToken::FILTER_STATE: - value = CelValue::CreateMap( - Protobuf::Arena::Create(&arena, info->filterState())); + if (getConnection()) { + value = CelValue::CreateMap(Protobuf::Arena::Create( + &arena, info->filterState(), &getConnection()->streamInfo().filterState())); + } else { + value = CelValue::CreateMap( + Protobuf::Arena::Create(&arena, info->filterState())); + } break; case PropertyToken::REQUEST: value = CelValue::CreateMap(Protobuf::Arena::Create( @@ -974,6 +992,15 @@ StreamInfo::StreamInfo* Context::getRequestStreamInfo() const { return nullptr; } +const Network::Connection* Context::getConnection() const { + if (encoder_callbacks_) { + return encoder_callbacks_->connection(); + } else if (decoder_callbacks_) { + return decoder_callbacks_->connection(); + } + return nullptr; +} + WasmResult Context::setProperty(absl::string_view key, absl::string_view serialized_value) { auto* stream_info = getRequestStreamInfo(); if (!stream_info) { diff --git a/source/extensions/common/wasm/context.h b/source/extensions/common/wasm/context.h index 4a84ac9823..050909b91e 100644 --- a/source/extensions/common/wasm/context.h +++ b/source/extensions/common/wasm/context.h @@ -85,6 +85,13 @@ class Context : public Logger::Loggable, const StreamInfo::StreamInfo* getConstRequestStreamInfo() const; StreamInfo::StreamInfo* getRequestStreamInfo() const; + // Retrieves the connection object associated with the request (a.k.a active stream). + // It selects a value based on the following order: encoder callback, decoder + // callback. As long as any one of the callbacks is invoked, the value should be + // available. + const Network::Connection* getConnection() const; + + // // VM level downcalls into the WASM code on Context(id == 0). // diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index c01c3d46e0..aa48d6c053 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -51,7 +51,6 @@ namespace Common { namespace Wasm { namespace { - std::atomic active_wasm_; std::string base64Sha256(absl::string_view data) { From 829ccdc6735bccf75d79b41827863e311ded3a5a Mon Sep 17 00:00:00 2001 From: gargnupur Date: Wed, 20 Nov 2019 13:21:17 -0800 Subject: [PATCH 2/5] Fixed formatting Signed-off-by: gargnupur --- source/extensions/common/wasm/context.cc | 33 ++++++++++++------------ source/extensions/common/wasm/context.h | 9 +++---- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index e48acf15f2..ffa64a1b7c 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -344,10 +344,11 @@ WasmResult serializeValue(Filters::Common::Expr::CelValue value, std::string* re class WasmStateWrapper : public google::api::expr::runtime::CelMap { public: WasmStateWrapper(const StreamInfo::FilterState& filter_state, - const StreamInfo::FilterState* connection_filter_state) - : filter_state_(filter_state), connection_filter_state_(connection_filter_state) {} + const StreamInfo::FilterState* connection_filter_state) + : filter_state_(filter_state), connection_filter_state_(connection_filter_state) {} WasmStateWrapper(const StreamInfo::FilterState& filter_state) - : filter_state_(filter_state), connection_filter_state_(nullptr) {}absl::optional + : filter_state_(filter_state), connection_filter_state_(nullptr) {} + absl::optional operator[](google::api::expr::runtime::CelValue key) const override { if (!key.IsString()) { return {}; @@ -358,14 +359,14 @@ class WasmStateWrapper : public google::api::expr::runtime::CelMap { return google::api::expr::runtime::CelValue::CreateBytes(&result.value()); } catch (const EnvoyException& e) { // If doesn't exist in request filter state, try looking up in connection filter state. - try { - if (connection_filter_state_) { - const WasmState& result = connection_filter_state_->getDataReadOnly(value); - return google::api::expr::runtime::CelValue::CreateBytes(&result.value()); + try { + if (connection_filter_state_) { + const WasmState& result = connection_filter_state_->getDataReadOnly(value); + return google::api::expr::runtime::CelValue::CreateBytes(&result.value()); + } + } catch (const EnvoyException& e) { + return {}; } - } catch (const EnvoyException& e) { - return {}; - } return {}; } } @@ -441,12 +442,12 @@ WasmResult Context::getProperty(absl::string_view path, std::string* result) { break; case PropertyToken::FILTER_STATE: if (getConnection()) { - value = CelValue::CreateMap(Protobuf::Arena::Create( - &arena, info->filterState(), &getConnection()->streamInfo().filterState())); - } else { - value = CelValue::CreateMap( - Protobuf::Arena::Create(&arena, info->filterState())); - } + value = CelValue::CreateMap(Protobuf::Arena::Create( + &arena, info->filterState(), &getConnection()->streamInfo().filterState())); + } else { + value = CelValue::CreateMap( + Protobuf::Arena::Create(&arena, info->filterState())); + } break; case PropertyToken::REQUEST: value = CelValue::CreateMap(Protobuf::Arena::Create( diff --git a/source/extensions/common/wasm/context.h b/source/extensions/common/wasm/context.h index 050909b91e..60237e8f37 100644 --- a/source/extensions/common/wasm/context.h +++ b/source/extensions/common/wasm/context.h @@ -86,11 +86,10 @@ class Context : public Logger::Loggable, StreamInfo::StreamInfo* getRequestStreamInfo() const; // Retrieves the connection object associated with the request (a.k.a active stream). - // It selects a value based on the following order: encoder callback, decoder - // callback. As long as any one of the callbacks is invoked, the value should be - // available. - const Network::Connection* getConnection() const; - + // It selects a value based on the following order: encoder callback, decoder + // callback. As long as any one of the callbacks is invoked, the value should be + // available. + const Network::Connection* getConnection() const; // // VM level downcalls into the WASM code on Context(id == 0). From d460624e7e25c9de0357aca9f613d19d2068c522 Mon Sep 17 00:00:00 2001 From: gargnupur Date: Wed, 20 Nov 2019 13:25:08 -0800 Subject: [PATCH 3/5] Fixed formatting Signed-off-by: gargnupur --- source/extensions/common/wasm/wasm.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/common/wasm/wasm.cc b/source/extensions/common/wasm/wasm.cc index aa48d6c053..c01c3d46e0 100644 --- a/source/extensions/common/wasm/wasm.cc +++ b/source/extensions/common/wasm/wasm.cc @@ -51,6 +51,7 @@ namespace Common { namespace Wasm { namespace { + std::atomic active_wasm_; std::string base64Sha256(absl::string_view data) { From 9edb18d139157982798b68252c46cdd85e4bd2e0 Mon Sep 17 00:00:00 2001 From: gargnupur Date: Tue, 3 Dec 2019 10:40:10 -0800 Subject: [PATCH 4/5] Fixed based on feedback Signed-off-by: gargnupur --- source/extensions/common/wasm/context.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index ffa64a1b7c..e548f03adf 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -441,9 +441,10 @@ WasmResult Context::getProperty(absl::string_view path, std::string* result) { value = CelValue::CreateMessage(&info->dynamicMetadata(), &arena); break; case PropertyToken::FILTER_STATE: - if (getConnection()) { + const Envoy::Network::Connection* connection = getConnection(); + if (connection) { value = CelValue::CreateMap(Protobuf::Arena::Create( - &arena, info->filterState(), &getConnection()->streamInfo().filterState())); + &arena, info->filterState(), &connection->streamInfo().filterState())); } else { value = CelValue::CreateMap( Protobuf::Arena::Create(&arena, info->filterState())); From a2d38ae365d529a1f41d19e84c8eebf1481dcdd8 Mon Sep 17 00:00:00 2001 From: gargnupur Date: Tue, 3 Dec 2019 13:28:28 -0800 Subject: [PATCH 5/5] Fixed err Signed-off-by: gargnupur --- source/extensions/common/wasm/context.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index e548f03adf..6547d3d9a7 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -440,7 +440,7 @@ WasmResult Context::getProperty(absl::string_view path, std::string* result) { case PropertyToken::METADATA: value = CelValue::CreateMessage(&info->dynamicMetadata(), &arena); break; - case PropertyToken::FILTER_STATE: + case PropertyToken::FILTER_STATE: { const Envoy::Network::Connection* connection = getConnection(); if (connection) { value = CelValue::CreateMap(Protobuf::Arena::Create( @@ -450,6 +450,7 @@ WasmResult Context::getProperty(absl::string_view path, std::string* result) { Protobuf::Arena::Create(&arena, info->filterState())); } break; + } case PropertyToken::REQUEST: value = CelValue::CreateMap(Protobuf::Arena::Create( &arena, request_headers, *info));