From cb6d7e24462db63c799622cc4489dc26591f7336 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 20 Apr 2021 15:34:45 +0900 Subject: [PATCH 1/7] Add POSTCHECK on Http/Tcp callbacks. Signed-off-by: Takeshi Yoneda --- src/context.cc | 128 +++++++++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 51 deletions(-) diff --git a/src/context.cc b/src/context.cc index 6825f1336..159488d60 100644 --- a/src/context.cc +++ b/src/context.cc @@ -25,7 +25,7 @@ #include "src/shared_data.h" #include "src/shared_queue.h" -#define CHECK_FAIL(_call, _stream_type, _return_open, _return_closed) \ +#define PRECHECK_FAIL(_call, _stream_type, _return_open, _return_closed) \ if (isFailed()) { \ if (plugin_->fail_open_) { \ return _return_open; \ @@ -39,7 +39,7 @@ } \ } -#define CHECK_FAIL2(_call1, _call2, _stream_type, _return_open, _return_closed) \ +#define PRECHECK_FAIL2(_call1, _call2, _stream_type, _return_open, _return_closed) \ if (isFailed()) { \ if (plugin_->fail_open_) { \ return _return_open; \ @@ -53,12 +53,15 @@ } \ } -#define CHECK_HTTP(_call, _return_open, _return_closed) \ - CHECK_FAIL(_call, WasmStreamType::Request, _return_open, _return_closed) -#define CHECK_HTTP2(_call1, _call2, _return_open, _return_closed) \ - CHECK_FAIL2(_call1, _call2, WasmStreamType::Request, _return_open, _return_closed) -#define CHECK_NET(_call, _return_open, _return_closed) \ - CHECK_FAIL(_call, WasmStreamType::Downstream, _return_open, _return_closed) +#define POSTCHECK_FAIL(_stream_type, _return_open, _return_closed) \ + if (isFailed()) { \ + if (plugin_->fail_open_) { \ + return _return_open; \ + } else { \ + failStream(_stream_type); \ + return _return_closed; \ + } \ + } namespace proxy_wasm { @@ -263,29 +266,33 @@ void ContextBase::onForeignFunction(uint32_t foreign_function_id, uint32_t data_ } FilterStatus ContextBase::onNetworkNewConnection() { - CHECK_NET(on_new_connection_, FilterStatus::Continue, FilterStatus::StopIteration); + PRECHECK_FAIL(on_new_connection_, WasmStreamType::Downstream, FilterStatus::Continue, + FilterStatus::StopIteration); DeferAfterCallActions actions(this); - if (wasm_->on_new_connection_(this, id_).u64_ == 0) { - return FilterStatus::Continue; - } - return FilterStatus::StopIteration; + const auto call_result = wasm_->on_new_connection_(this, id_).u64_; + POSTCHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onDownstreamData(uint32_t data_length, bool end_of_stream) { - CHECK_NET(on_downstream_data_, FilterStatus::Continue, FilterStatus::StopIteration); + PRECHECK_FAIL(on_downstream_data_, WasmStreamType::Downstream, FilterStatus::Continue, + FilterStatus::StopIteration); DeferAfterCallActions actions(this); - auto result = wasm_->on_downstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)); + auto call_result = wasm_->on_downstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)); // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. - return result.u64_ == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + POSTCHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + return call_result.u64_ == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onUpstreamData(uint32_t data_length, bool end_of_stream) { - CHECK_NET(on_upstream_data_, FilterStatus::Continue, FilterStatus::StopIteration); + PRECHECK_FAIL(on_upstream_data_, WasmStreamType::Upstream, FilterStatus::Continue, + FilterStatus::StopIteration); DeferAfterCallActions actions(this); auto result = wasm_->on_upstream_data_(this, id_, static_cast(data_length), static_cast(end_of_stream)); // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. + POSTCHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); return result.u64_ == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } @@ -307,72 +314,91 @@ void ContextBase::onUpstreamConnectionClose(CloseType close_type) { template static uint32_t headerSize(const P &p) { return p ? p->size() : 0; } FilterHeadersStatus ContextBase::onRequestHeaders(uint32_t headers, bool end_of_stream) { - CHECK_HTTP2(on_request_headers_abi_01_, on_request_headers_abi_02_, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); + PRECHECK_FAIL2(on_request_headers_abi_01_, on_request_headers_abi_02_, WasmStreamType::Request, + FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); DeferAfterCallActions actions(this); - return convertVmCallResultToFilterHeadersStatus( - wasm_->on_request_headers_abi_01_ - ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_request_headers_abi_02_(this, id_, headers, - static_cast(end_of_stream)) - .u64_); + const auto call_result = wasm_->on_request_headers_abi_01_ + ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ + : wasm_ + ->on_request_headers_abi_02_( + this, id_, headers, static_cast(end_of_stream)) + .u64_; + POSTCHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); + return convertVmCallResultToFilterHeadersStatus(call_result); } FilterDataStatus ContextBase::onRequestBody(uint32_t data_length, bool end_of_stream) { - CHECK_HTTP(on_request_body_, FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); + PRECHECK_FAIL(on_request_body_, WasmStreamType::Request, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); DeferAfterCallActions actions(this); - return convertVmCallResultToFilterDataStatus( - wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)).u64_); + const auto call_result = + wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)).u64_; + POSTCHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); + return convertVmCallResultToFilterDataStatus(call_result); } FilterTrailersStatus ContextBase::onRequestTrailers(uint32_t trailers) { - CHECK_HTTP(on_request_trailers_, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + PRECHECK_FAIL(on_request_trailers_, WasmStreamType::Request, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); DeferAfterCallActions actions(this); - return convertVmCallResultToFilterTrailersStatus( - wasm_->on_request_trailers_(this, id_, trailers).u64_); + const auto call_result = wasm_->on_request_trailers_(this, id_, trailers).u64_; + POSTCHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); + return convertVmCallResultToFilterTrailersStatus(call_result); } FilterMetadataStatus ContextBase::onRequestMetadata(uint32_t elements) { - CHECK_HTTP(on_request_metadata_, FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); + PRECHECK_FAIL(on_request_metadata_, WasmStreamType::Request, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); DeferAfterCallActions actions(this); return convertVmCallResultToFilterMetadataStatus( wasm_->on_request_metadata_(this, id_, elements).u64_); } FilterHeadersStatus ContextBase::onResponseHeaders(uint32_t headers, bool end_of_stream) { - CHECK_HTTP2(on_response_headers_abi_01_, on_response_headers_abi_02_, - FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); + PRECHECK_FAIL2(on_response_headers_abi_01_, on_response_headers_abi_02_, WasmStreamType::Response, + FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); DeferAfterCallActions actions(this); - return convertVmCallResultToFilterHeadersStatus( - wasm_->on_response_headers_abi_01_ - ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_response_headers_abi_02_(this, id_, headers, - static_cast(end_of_stream)) - .u64_); + const auto call_result = wasm_->on_response_headers_abi_01_ + ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ + : wasm_ + ->on_response_headers_abi_02_( + this, id_, headers, static_cast(end_of_stream)) + .u64_; + POSTCHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); + return convertVmCallResultToFilterHeadersStatus(call_result); } FilterDataStatus ContextBase::onResponseBody(uint32_t body_length, bool end_of_stream) { - CHECK_HTTP(on_response_body_, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + PRECHECK_FAIL(on_response_body_, WasmStreamType::Response, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); DeferAfterCallActions actions(this); - return convertVmCallResultToFilterDataStatus( - wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)).u64_); + const auto call_result = + wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)).u64_; + POSTCHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); + return convertVmCallResultToFilterDataStatus(call_result); } FilterTrailersStatus ContextBase::onResponseTrailers(uint32_t trailers) { - CHECK_HTTP(on_response_trailers_, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + PRECHECK_FAIL(on_response_trailers_, WasmStreamType::Response, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); DeferAfterCallActions actions(this); + POSTCHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); return convertVmCallResultToFilterTrailersStatus( wasm_->on_response_trailers_(this, id_, trailers).u64_); } FilterMetadataStatus ContextBase::onResponseMetadata(uint32_t elements) { - CHECK_HTTP(on_response_metadata_, FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); + PRECHECK_FAIL(on_response_metadata_, WasmStreamType::Response, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); DeferAfterCallActions actions(this); + POSTCHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); return convertVmCallResultToFilterMetadataStatus( wasm_->on_response_metadata_(this, id_, elements).u64_); } From a2cdba42400ad23eb5771ee61a9945921798fbfa Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 20 Apr 2021 16:21:42 +0900 Subject: [PATCH 2/7] Fix check. Signed-off-by: Takeshi Yoneda --- src/context.cc | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/context.cc b/src/context.cc index 159488d60..1e0e5614a 100644 --- a/src/context.cc +++ b/src/context.cc @@ -278,22 +278,26 @@ FilterStatus ContextBase::onDownstreamData(uint32_t data_length, bool end_of_str PRECHECK_FAIL(on_downstream_data_, WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); DeferAfterCallActions actions(this); - auto call_result = wasm_->on_downstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)); + auto call_result = wasm_ + ->on_downstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)) + .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. POSTCHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); - return call_result.u64_ == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onUpstreamData(uint32_t data_length, bool end_of_stream) { PRECHECK_FAIL(on_upstream_data_, WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); DeferAfterCallActions actions(this); - auto result = wasm_->on_upstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)); + auto call_result = wasm_ + ->on_upstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)) + .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. POSTCHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); - return result.u64_ == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } void ContextBase::onDownstreamConnectionClose(CloseType close_type) { @@ -387,20 +391,20 @@ FilterTrailersStatus ContextBase::onResponseTrailers(uint32_t trailers) { PRECHECK_FAIL(on_response_trailers_, WasmStreamType::Response, FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); DeferAfterCallActions actions(this); + const auto call_result = wasm_->on_response_trailers_(this, id_, trailers).u64_; POSTCHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); - return convertVmCallResultToFilterTrailersStatus( - wasm_->on_response_trailers_(this, id_, trailers).u64_); + return convertVmCallResultToFilterTrailersStatus(call_result); } FilterMetadataStatus ContextBase::onResponseMetadata(uint32_t elements) { PRECHECK_FAIL(on_response_metadata_, WasmStreamType::Response, FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); DeferAfterCallActions actions(this); + const auto call_result = wasm_->on_response_metadata_(this, id_, elements).u64_; POSTCHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); - return convertVmCallResultToFilterMetadataStatus( - wasm_->on_response_metadata_(this, id_, elements).u64_); + return convertVmCallResultToFilterMetadataStatus(call_result); } void ContextBase::onHttpCallResponse(uint32_t token, uint32_t headers, uint32_t body_size, From 64f2abd728b79b50e2a77b5957e777e902cfc406 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Wed, 21 Apr 2021 11:47:39 +0900 Subject: [PATCH 3/7] Remove POSTCHECK macro and use one CHECK_FAIL. Signed-off-by: Takeshi Yoneda --- src/context.cc | 144 +++++++++++++++++++++++++------------------------ 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/src/context.cc b/src/context.cc index 1e0e5614a..f45079d60 100644 --- a/src/context.cc +++ b/src/context.cc @@ -25,35 +25,7 @@ #include "src/shared_data.h" #include "src/shared_queue.h" -#define PRECHECK_FAIL(_call, _stream_type, _return_open, _return_closed) \ - if (isFailed()) { \ - if (plugin_->fail_open_) { \ - return _return_open; \ - } else { \ - failStream(_stream_type); \ - return _return_closed; \ - } \ - } else { \ - if (!wasm_->_call) { \ - return _return_open; \ - } \ - } - -#define PRECHECK_FAIL2(_call1, _call2, _stream_type, _return_open, _return_closed) \ - if (isFailed()) { \ - if (plugin_->fail_open_) { \ - return _return_open; \ - } else { \ - failStream(_stream_type); \ - return _return_closed; \ - } \ - } else { \ - if (!wasm_->_call1 && !wasm_->_call2) { \ - return _return_open; \ - } \ - } - -#define POSTCHECK_FAIL(_stream_type, _return_open, _return_closed) \ +#define CHECK_FAIL(_stream_type, _return_open, _return_closed) \ if (isFailed()) { \ if (plugin_->fail_open_) { \ return _return_open; \ @@ -266,37 +238,43 @@ void ContextBase::onForeignFunction(uint32_t foreign_function_id, uint32_t data_ } FilterStatus ContextBase::onNetworkNewConnection() { - PRECHECK_FAIL(on_new_connection_, WasmStreamType::Downstream, FilterStatus::Continue, - FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + if (!wasm_->on_new_connection_) { + return FilterStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_new_connection_(this, id_).u64_; - POSTCHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onDownstreamData(uint32_t data_length, bool end_of_stream) { - PRECHECK_FAIL(on_downstream_data_, WasmStreamType::Downstream, FilterStatus::Continue, - FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + if (!wasm_->on_downstream_data_) { + return FilterStatus::Continue; + } DeferAfterCallActions actions(this); auto call_result = wasm_ ->on_downstream_data_(this, id_, static_cast(data_length), static_cast(end_of_stream)) .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. - POSTCHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onUpstreamData(uint32_t data_length, bool end_of_stream) { - PRECHECK_FAIL(on_upstream_data_, WasmStreamType::Upstream, FilterStatus::Continue, - FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); + if (!wasm_->on_upstream_data_) { + return FilterStatus::Continue; + } DeferAfterCallActions actions(this); auto call_result = wasm_ ->on_upstream_data_(this, id_, static_cast(data_length), static_cast(end_of_stream)) .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. - POSTCHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } @@ -318,8 +296,11 @@ void ContextBase::onUpstreamConnectionClose(CloseType close_type) { template static uint32_t headerSize(const P &p) { return p ? p->size() : 0; } FilterHeadersStatus ContextBase::onRequestHeaders(uint32_t headers, bool end_of_stream) { - PRECHECK_FAIL2(on_request_headers_abi_01_, on_request_headers_abi_02_, WasmStreamType::Request, - FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); + if (!wasm_->on_request_headers_abi_01_ && !wasm_->on_request_headers_abi_02_) { + return FilterHeadersStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_request_headers_abi_01_ ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ @@ -327,43 +308,57 @@ FilterHeadersStatus ContextBase::onRequestHeaders(uint32_t headers, bool end_of_ ->on_request_headers_abi_02_( this, id_, headers, static_cast(end_of_stream)) .u64_; - POSTCHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); return convertVmCallResultToFilterHeadersStatus(call_result); } FilterDataStatus ContextBase::onRequestBody(uint32_t data_length, bool end_of_stream) { - PRECHECK_FAIL(on_request_body_, WasmStreamType::Request, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); + if (!wasm_->on_request_body_) { + return FilterDataStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)).u64_; - POSTCHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); return convertVmCallResultToFilterDataStatus(call_result); } FilterTrailersStatus ContextBase::onRequestTrailers(uint32_t trailers) { - PRECHECK_FAIL(on_request_trailers_, WasmStreamType::Request, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); + if (!wasm_->on_request_trailers_) { + return FilterTrailersStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_request_trailers_(this, id_, trailers).u64_; - POSTCHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); return convertVmCallResultToFilterTrailersStatus(call_result); } FilterMetadataStatus ContextBase::onRequestMetadata(uint32_t elements) { - PRECHECK_FAIL(on_request_metadata_, WasmStreamType::Request, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); + CHECK_FAIL(WasmStreamType::Request, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); + if (!wasm_->on_request_metadata_) { + return FilterMetadataStatus::Continue; + } DeferAfterCallActions actions(this); - return convertVmCallResultToFilterMetadataStatus( - wasm_->on_request_metadata_(this, id_, elements).u64_); + const auto call_result = wasm_->on_request_metadata_(this, id_, elements).u64_; + CHECK_FAIL(WasmStreamType::Request, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); + return convertVmCallResultToFilterMetadataStatus(call_result); } FilterHeadersStatus ContextBase::onResponseHeaders(uint32_t headers, bool end_of_stream) { - PRECHECK_FAIL2(on_response_headers_abi_01_, on_response_headers_abi_02_, WasmStreamType::Response, - FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); + if (!wasm_->on_response_headers_abi_01_ && !wasm_->on_response_headers_abi_02_) { + return FilterHeadersStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_response_headers_abi_01_ ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ @@ -371,39 +366,48 @@ FilterHeadersStatus ContextBase::onResponseHeaders(uint32_t headers, bool end_of ->on_response_headers_abi_02_( this, id_, headers, static_cast(end_of_stream)) .u64_; - POSTCHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, + FilterHeadersStatus::StopAllIterationAndWatermark); return convertVmCallResultToFilterHeadersStatus(call_result); } FilterDataStatus ContextBase::onResponseBody(uint32_t body_length, bool end_of_stream) { - PRECHECK_FAIL(on_response_body_, WasmStreamType::Response, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); + if (!wasm_->on_response_body_) { + return FilterDataStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)).u64_; - POSTCHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, + FilterDataStatus::StopIterationNoBuffer); return convertVmCallResultToFilterDataStatus(call_result); } FilterTrailersStatus ContextBase::onResponseTrailers(uint32_t trailers) { - PRECHECK_FAIL(on_response_trailers_, WasmStreamType::Response, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); + if (!wasm_->on_response_trailers_) { + return FilterTrailersStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_response_trailers_(this, id_, trailers).u64_; - POSTCHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, + FilterTrailersStatus::StopIteration); return convertVmCallResultToFilterTrailersStatus(call_result); } FilterMetadataStatus ContextBase::onResponseMetadata(uint32_t elements) { - PRECHECK_FAIL(on_response_metadata_, WasmStreamType::Response, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); + CHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); + if (!wasm_->on_response_metadata_) { + return FilterMetadataStatus::Continue; + } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_response_metadata_(this, id_, elements).u64_; - POSTCHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); + CHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, + FilterMetadataStatus::Continue); return convertVmCallResultToFilterMetadataStatus(call_result); } From ae18fbc40a2a442b8808d71d02d234fd63146d6e Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Wed, 21 Apr 2021 12:13:14 +0900 Subject: [PATCH 4/7] Close both Up/Downstreams in OnNewConnection failure Signed-off-by: Takeshi Yoneda --- src/context.cc | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/context.cc b/src/context.cc index f45079d60..ce2ec9854 100644 --- a/src/context.cc +++ b/src/context.cc @@ -35,6 +35,17 @@ } \ } +#define CHECK_FAIL2(_stream_type, _stream_type2, _return_open, _return_closed) \ + if (isFailed()) { \ + if (plugin_->fail_open_) { \ + return _return_open; \ + } else { \ + failStream(_stream_type); \ + failStream(_stream_type2); \ + return _return_closed; \ + } \ + } + namespace proxy_wasm { DeferAfterCallActions::~DeferAfterCallActions() { @@ -238,13 +249,15 @@ void ContextBase::onForeignFunction(uint32_t foreign_function_id, uint32_t data_ } FilterStatus ContextBase::onNetworkNewConnection() { - CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL2(WasmStreamType::Downstream, WasmStreamType::Upstream, FilterStatus::Continue, + FilterStatus::StopIteration); if (!wasm_->on_new_connection_) { return FilterStatus::Continue; } DeferAfterCallActions actions(this); const auto call_result = wasm_->on_new_connection_(this, id_).u64_; - CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL2(WasmStreamType::Downstream, WasmStreamType::Upstream, FilterStatus::Continue, + FilterStatus::StopIteration); return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } From 10fab1f84673559cbe39337c48a884f7d1ed3e29 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 27 Apr 2021 10:55:06 +0900 Subject: [PATCH 5/7] review: close both direction. Signed-off-by: Takeshi Yoneda --- src/context.cc | 155 +++++++++++++++++++++---------------------------- 1 file changed, 66 insertions(+), 89 deletions(-) diff --git a/src/context.cc b/src/context.cc index ce2ec9854..819dcb6ce 100644 --- a/src/context.cc +++ b/src/context.cc @@ -25,17 +25,7 @@ #include "src/shared_data.h" #include "src/shared_queue.h" -#define CHECK_FAIL(_stream_type, _return_open, _return_closed) \ - if (isFailed()) { \ - if (plugin_->fail_open_) { \ - return _return_open; \ - } else { \ - failStream(_stream_type); \ - return _return_closed; \ - } \ - } - -#define CHECK_FAIL2(_stream_type, _stream_type2, _return_open, _return_closed) \ +#define CHECK_FAIL(_stream_type, _stream_type2, _return_open, _return_closed) \ if (isFailed()) { \ if (plugin_->fail_open_) { \ return _return_open; \ @@ -46,6 +36,11 @@ } \ } +#define CHECK_FAIL_HTTP(_return_open, _return_closed) \ + CHECK_FAIL(WasmStreamType::Request, WasmStreamType::Response, _return_open, _return_closed) +#define CHECK_FAIL_NET(_return_open, _return_closed) \ + CHECK_FAIL(WasmStreamType::Downstream, WasmStreamType::Upstream, _return_open, _return_closed) + namespace proxy_wasm { DeferAfterCallActions::~DeferAfterCallActions() { @@ -249,46 +244,44 @@ void ContextBase::onForeignFunction(uint32_t foreign_function_id, uint32_t data_ } FilterStatus ContextBase::onNetworkNewConnection() { - CHECK_FAIL2(WasmStreamType::Downstream, WasmStreamType::Upstream, FilterStatus::Continue, - FilterStatus::StopIteration); + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); if (!wasm_->on_new_connection_) { return FilterStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_new_connection_(this, id_).u64_; - CHECK_FAIL2(WasmStreamType::Downstream, WasmStreamType::Upstream, FilterStatus::Continue, - FilterStatus::StopIteration); - return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + const auto result = wasm_->on_new_connection_(this, id_).u64_; + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); + return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onDownstreamData(uint32_t data_length, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); if (!wasm_->on_downstream_data_) { return FilterStatus::Continue; } DeferAfterCallActions actions(this); - auto call_result = wasm_ - ->on_downstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)) - .u64_; + auto result = wasm_ + ->on_downstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)) + .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. - CHECK_FAIL(WasmStreamType::Downstream, FilterStatus::Continue, FilterStatus::StopIteration); - return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); + return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } FilterStatus ContextBase::onUpstreamData(uint32_t data_length, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); if (!wasm_->on_upstream_data_) { return FilterStatus::Continue; } DeferAfterCallActions actions(this); - auto call_result = wasm_ - ->on_upstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)) - .u64_; + auto result = wasm_ + ->on_upstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)) + .u64_; // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. - CHECK_FAIL(WasmStreamType::Upstream, FilterStatus::Continue, FilterStatus::StopIteration); - return call_result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; + CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); + return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } void ContextBase::onDownstreamConnectionClose(CloseType close_type) { @@ -309,119 +302,103 @@ void ContextBase::onUpstreamConnectionClose(CloseType close_type) { template static uint32_t headerSize(const P &p) { return p ? p->size() : 0; } FilterHeadersStatus ContextBase::onRequestHeaders(uint32_t headers, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); if (!wasm_->on_request_headers_abi_01_ && !wasm_->on_request_headers_abi_02_) { return FilterHeadersStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_request_headers_abi_01_ - ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_request_headers_abi_02_( - this, id_, headers, static_cast(end_of_stream)) - .u64_; - CHECK_FAIL(WasmStreamType::Request, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); - return convertVmCallResultToFilterHeadersStatus(call_result); + const auto result = wasm_->on_request_headers_abi_01_ + ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ + : wasm_ + ->on_request_headers_abi_02_(this, id_, headers, + static_cast(end_of_stream)) + .u64_; + CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); + return convertVmCallResultToFilterHeadersStatus(result); } FilterDataStatus ContextBase::onRequestBody(uint32_t data_length, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); if (!wasm_->on_request_body_) { return FilterDataStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = + const auto result = wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)).u64_; - CHECK_FAIL(WasmStreamType::Request, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); - return convertVmCallResultToFilterDataStatus(call_result); + CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); + return convertVmCallResultToFilterDataStatus(result); } FilterTrailersStatus ContextBase::onRequestTrailers(uint32_t trailers) { - CHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); if (!wasm_->on_request_trailers_) { return FilterTrailersStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_request_trailers_(this, id_, trailers).u64_; - CHECK_FAIL(WasmStreamType::Request, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); - return convertVmCallResultToFilterTrailersStatus(call_result); + const auto result = wasm_->on_request_trailers_(this, id_, trailers).u64_; + CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); + return convertVmCallResultToFilterTrailersStatus(result); } FilterMetadataStatus ContextBase::onRequestMetadata(uint32_t elements) { - CHECK_FAIL(WasmStreamType::Request, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); + CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); if (!wasm_->on_request_metadata_) { return FilterMetadataStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_request_metadata_(this, id_, elements).u64_; - CHECK_FAIL(WasmStreamType::Request, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); - return convertVmCallResultToFilterMetadataStatus(call_result); + const auto result = wasm_->on_request_metadata_(this, id_, elements).u64_; + CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); + return convertVmCallResultToFilterMetadataStatus(result); } FilterHeadersStatus ContextBase::onResponseHeaders(uint32_t headers, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); + CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); if (!wasm_->on_response_headers_abi_01_ && !wasm_->on_response_headers_abi_02_) { return FilterHeadersStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_response_headers_abi_01_ - ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_response_headers_abi_02_( - this, id_, headers, static_cast(end_of_stream)) - .u64_; - CHECK_FAIL(WasmStreamType::Response, FilterHeadersStatus::Continue, - FilterHeadersStatus::StopAllIterationAndWatermark); - return convertVmCallResultToFilterHeadersStatus(call_result); + const auto result = wasm_->on_response_headers_abi_01_ + ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ + : wasm_ + ->on_response_headers_abi_02_(this, id_, headers, + static_cast(end_of_stream)) + .u64_; + CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); + return convertVmCallResultToFilterHeadersStatus(result); } FilterDataStatus ContextBase::onResponseBody(uint32_t body_length, bool end_of_stream) { - CHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); + CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); if (!wasm_->on_response_body_) { return FilterDataStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = + const auto result = wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)).u64_; - CHECK_FAIL(WasmStreamType::Response, FilterDataStatus::Continue, - FilterDataStatus::StopIterationNoBuffer); - return convertVmCallResultToFilterDataStatus(call_result); + CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); + return convertVmCallResultToFilterDataStatus(result); } FilterTrailersStatus ContextBase::onResponseTrailers(uint32_t trailers) { - CHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); + CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); if (!wasm_->on_response_trailers_) { return FilterTrailersStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_response_trailers_(this, id_, trailers).u64_; - CHECK_FAIL(WasmStreamType::Response, FilterTrailersStatus::Continue, - FilterTrailersStatus::StopIteration); - return convertVmCallResultToFilterTrailersStatus(call_result); + const auto result = wasm_->on_response_trailers_(this, id_, trailers).u64_; + CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); + return convertVmCallResultToFilterTrailersStatus(result); } FilterMetadataStatus ContextBase::onResponseMetadata(uint32_t elements) { - CHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); + CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); if (!wasm_->on_response_metadata_) { return FilterMetadataStatus::Continue; } DeferAfterCallActions actions(this); - const auto call_result = wasm_->on_response_metadata_(this, id_, elements).u64_; - CHECK_FAIL(WasmStreamType::Response, FilterMetadataStatus::Continue, - FilterMetadataStatus::Continue); - return convertVmCallResultToFilterMetadataStatus(call_result); + const auto result = wasm_->on_response_metadata_(this, id_, elements).u64_; + CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); + return convertVmCallResultToFilterMetadataStatus(result); } void ContextBase::onHttpCallResponse(uint32_t token, uint32_t headers, uint32_t body_size, From e6d0d3d31ff9829ebe23e88262a4cb5f680d83c2 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 27 Apr 2021 11:46:23 +0900 Subject: [PATCH 6/7] review: store a raw Word in result var. Signed-off-by: Takeshi Yoneda --- src/context.cc | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/context.cc b/src/context.cc index 819dcb6ce..7a90bdfd6 100644 --- a/src/context.cc +++ b/src/context.cc @@ -249,7 +249,7 @@ FilterStatus ContextBase::onNetworkNewConnection() { return FilterStatus::Continue; } DeferAfterCallActions actions(this); - const auto result = wasm_->on_new_connection_(this, id_).u64_; + const auto result = wasm_->on_new_connection_(this, id_); CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; } @@ -260,10 +260,8 @@ FilterStatus ContextBase::onDownstreamData(uint32_t data_length, bool end_of_str return FilterStatus::Continue; } DeferAfterCallActions actions(this); - auto result = wasm_ - ->on_downstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)) - .u64_; + auto result = wasm_->on_downstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)); // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; @@ -275,10 +273,8 @@ FilterStatus ContextBase::onUpstreamData(uint32_t data_length, bool end_of_strea return FilterStatus::Continue; } DeferAfterCallActions actions(this); - auto result = wasm_ - ->on_upstream_data_(this, id_, static_cast(data_length), - static_cast(end_of_stream)) - .u64_; + auto result = wasm_->on_upstream_data_(this, id_, static_cast(data_length), + static_cast(end_of_stream)); // TODO(PiotrSikora): pull Proxy-WASM's FilterStatus values. CHECK_FAIL_NET(FilterStatus::Continue, FilterStatus::StopIteration); return result == 0 ? FilterStatus::Continue : FilterStatus::StopIteration; @@ -308,11 +304,9 @@ FilterHeadersStatus ContextBase::onRequestHeaders(uint32_t headers, bool end_of_ } DeferAfterCallActions actions(this); const auto result = wasm_->on_request_headers_abi_01_ - ? wasm_->on_request_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_request_headers_abi_02_(this, id_, headers, - static_cast(end_of_stream)) - .u64_; + ? wasm_->on_request_headers_abi_01_(this, id_, headers) + : wasm_->on_request_headers_abi_02_(this, id_, headers, + static_cast(end_of_stream)); CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); return convertVmCallResultToFilterHeadersStatus(result); } @@ -324,7 +318,7 @@ FilterDataStatus ContextBase::onRequestBody(uint32_t data_length, bool end_of_st } DeferAfterCallActions actions(this); const auto result = - wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)).u64_; + wasm_->on_request_body_(this, id_, data_length, static_cast(end_of_stream)); CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); return convertVmCallResultToFilterDataStatus(result); } @@ -335,7 +329,7 @@ FilterTrailersStatus ContextBase::onRequestTrailers(uint32_t trailers) { return FilterTrailersStatus::Continue; } DeferAfterCallActions actions(this); - const auto result = wasm_->on_request_trailers_(this, id_, trailers).u64_; + const auto result = wasm_->on_request_trailers_(this, id_, trailers); CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); return convertVmCallResultToFilterTrailersStatus(result); } @@ -346,7 +340,7 @@ FilterMetadataStatus ContextBase::onRequestMetadata(uint32_t elements) { return FilterMetadataStatus::Continue; } DeferAfterCallActions actions(this); - const auto result = wasm_->on_request_metadata_(this, id_, elements).u64_; + const auto result = wasm_->on_request_metadata_(this, id_, elements); CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); return convertVmCallResultToFilterMetadataStatus(result); } @@ -358,11 +352,9 @@ FilterHeadersStatus ContextBase::onResponseHeaders(uint32_t headers, bool end_of } DeferAfterCallActions actions(this); const auto result = wasm_->on_response_headers_abi_01_ - ? wasm_->on_response_headers_abi_01_(this, id_, headers).u64_ - : wasm_ - ->on_response_headers_abi_02_(this, id_, headers, - static_cast(end_of_stream)) - .u64_; + ? wasm_->on_response_headers_abi_01_(this, id_, headers) + : wasm_->on_response_headers_abi_02_( + this, id_, headers, static_cast(end_of_stream)); CHECK_FAIL_HTTP(FilterHeadersStatus::Continue, FilterHeadersStatus::StopAllIterationAndWatermark); return convertVmCallResultToFilterHeadersStatus(result); } @@ -374,7 +366,7 @@ FilterDataStatus ContextBase::onResponseBody(uint32_t body_length, bool end_of_s } DeferAfterCallActions actions(this); const auto result = - wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)).u64_; + wasm_->on_response_body_(this, id_, body_length, static_cast(end_of_stream)); CHECK_FAIL_HTTP(FilterDataStatus::Continue, FilterDataStatus::StopIterationNoBuffer); return convertVmCallResultToFilterDataStatus(result); } @@ -385,7 +377,7 @@ FilterTrailersStatus ContextBase::onResponseTrailers(uint32_t trailers) { return FilterTrailersStatus::Continue; } DeferAfterCallActions actions(this); - const auto result = wasm_->on_response_trailers_(this, id_, trailers).u64_; + const auto result = wasm_->on_response_trailers_(this, id_, trailers); CHECK_FAIL_HTTP(FilterTrailersStatus::Continue, FilterTrailersStatus::StopIteration); return convertVmCallResultToFilterTrailersStatus(result); } @@ -396,7 +388,7 @@ FilterMetadataStatus ContextBase::onResponseMetadata(uint32_t elements) { return FilterMetadataStatus::Continue; } DeferAfterCallActions actions(this); - const auto result = wasm_->on_response_metadata_(this, id_, elements).u64_; + const auto result = wasm_->on_response_metadata_(this, id_, elements); CHECK_FAIL_HTTP(FilterMetadataStatus::Continue, FilterMetadataStatus::Continue); return convertVmCallResultToFilterMetadataStatus(result); } From fe4365cc0ccb9211e2501283dcd123e1ced64919 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 27 Apr 2021 12:27:30 +0900 Subject: [PATCH 7/7] review: add guard for multiple calls on failStream. Signed-off-by: Takeshi Yoneda --- include/proxy-wasm/context.h | 1 + src/context.cc | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/proxy-wasm/context.h b/include/proxy-wasm/context.h index bf770a7bf..c373cf324 100644 --- a/include/proxy-wasm/context.h +++ b/include/proxy-wasm/context.h @@ -389,6 +389,7 @@ class ContextBase : public RootInterface, std::shared_ptr temp_plugin_; // Remove once ABI v0.1.0 is gone. bool in_vm_context_created_ = false; bool destroyed_ = false; + bool stream_failed_ = false; // Set true after failStream is called in case of VM failure. private: // helper functions diff --git a/src/context.cc b/src/context.cc index 7a90bdfd6..a13f79000 100644 --- a/src/context.cc +++ b/src/context.cc @@ -29,11 +29,12 @@ if (isFailed()) { \ if (plugin_->fail_open_) { \ return _return_open; \ - } else { \ + } else if (!stream_failed_) { \ failStream(_stream_type); \ failStream(_stream_type2); \ - return _return_closed; \ + stream_failed_ = true; \ } \ + return _return_closed; \ } #define CHECK_FAIL_HTTP(_return_open, _return_closed) \