From 02239affbc5cb8a77942a8aef86cb78f6b2a7714 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Fri, 11 Jul 2025 20:32:00 +0000 Subject: [PATCH 1/6] feat: add knob to customise onRequestHeaders StopIteration behavior Signed-off-by: Michael Warres --- include/proxy-wasm/context.h | 7 +++ src/context.cc | 10 ++-- test/BUILD | 15 ++++++ test/stop_iteration_test.cc | 83 ++++++++++++++++++++++++++++++++ test/test_data/BUILD | 5 ++ test/test_data/stop_iteration.cc | 27 +++++++++++ test/utility.h | 4 ++ 7 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 test/stop_iteration_test.cc create mode 100644 test/test_data/stop_iteration.cc diff --git a/include/proxy-wasm/context.h b/include/proxy-wasm/context.h index 12937041f..c624e3803 100644 --- a/include/proxy-wasm/context.h +++ b/include/proxy-wasm/context.h @@ -397,6 +397,13 @@ class ContextBase : public RootInterface, bool destroyed_ = false; bool stream_failed_ = false; // Set true after failStream is called in case of VM failure. + // If true, convertVmCallResultToFilterHeadersStatus() propagates + // FilterHeadersStatus::StopIteration unmodified to callers. If false, it + // translates FilterHeaderStatus::StopIteration to + // FilterHeadersStatus::StopAllIterationAndWatermark, which is the default + // behavior for v0.2.* of the Proxy-Wasm ABI. + bool allow_on_request_headers_stop_iteration_ = false; + private: // helper functions FilterHeadersStatus convertVmCallResultToFilterHeadersStatus(uint64_t result); diff --git a/src/context.cc b/src/context.cc index 5353a52a5..ac10978e0 100644 --- a/src/context.cc +++ b/src/context.cc @@ -493,10 +493,12 @@ FilterHeadersStatus ContextBase::convertVmCallResultToFilterHeadersStatus(uint64 result > static_cast(FilterHeadersStatus::StopAllIterationAndWatermark)) { return FilterHeadersStatus::StopAllIterationAndWatermark; } - if (result == static_cast(FilterHeadersStatus::StopIteration)) { - // Always convert StopIteration (pause processing headers, but continue processing body) - // to StopAllIterationAndWatermark (pause all processing), since the former breaks all - // assumptions about HTTP processing. + if (result == static_cast(FilterHeadersStatus::StopIteration) && + !allow_on_request_headers_stop_iteration_) { + // Default behavior for Proxy-Wasm 0.2.* ABI is to translate StopIteration + // (pause processing headers, but continue processing body) to + // StopAllIterationAndWatermark (pause all processing), as described in + // https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/143. return FilterHeadersStatus::StopAllIterationAndWatermark; } return static_cast(result); diff --git a/test/BUILD b/test/BUILD index 61973ce17..73787e4b0 100644 --- a/test/BUILD +++ b/test/BUILD @@ -132,6 +132,21 @@ cc_test( ], ) +cc_test( + name = "stop_iteration_test", + srcs = ["stop_iteration_test.cc"], + data = [ + "//test/test_data:stop_iteration.wasm", + ], + linkstatic = 1, + deps = [ + ":utility_lib", + "//:lib", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "security_test", srcs = ["security_test.cc"], diff --git a/test/stop_iteration_test.cc b/test/stop_iteration_test.cc new file mode 100644 index 000000000..d531dddfd --- /dev/null +++ b/test/stop_iteration_test.cc @@ -0,0 +1,83 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "gtest/gtest.h" +#include "include/proxy-wasm/wasm.h" +#include "test/utility.h" + +namespace proxy_wasm { + +INSTANTIATE_TEST_SUITE_P(WasmEngines, TestVm, testing::ValuesIn(getWasmEngines()), + [](const testing::TestParamInfo &info) { + return info.param; + }); + +// TestVm is parameterized for each engine and creates a VM on construction. +TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { + // Read the wasm source. + auto source = readTestWasmFile("stop_iteration.wasm"); + ASSERT_FALSE(source.empty()); + + // Create a WasmBase and load the plugin. + auto wasm = std::make_shared(std::move(vm_)); + ASSERT_TRUE(wasm->load(source, /*allow_precompiled=*/false)); + ASSERT_TRUE(wasm->initialize()); + + // Create a plugin. + const auto plugin = std::make_shared( + /*name=*/"test", /*root_id=*/"", /*vm_id=*/"", + /*engine=*/wasm->wasm_vm()->getEngineName(), /*plugin_config=*/"", + /*fail_open=*/false, /*key=*/""); + + // Create root context, call onStart(). + ContextBase *root_context = wasm->start(plugin); + ASSERT_TRUE(root_context != nullptr); + + // On the root context, call onConfigure(). + ASSERT_TRUE(wasm->configure(root_context, plugin)); + + // By default, stream context onRequestHeaders translates + // FilterHeadersStatus::StopIteration to + // FilterHeadersStatus::StopAllIterationAndWatermark. + { + auto wasm_handle = std::make_shared(wasm); + auto plugin_handle = std::make_shared(wasm_handle, plugin); + auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); + stream_context.set_allow_on_request_headers_stop_iteration(true); + stream_context.onCreate(); + EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), + FilterHeadersStatus::StopIteration); + stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + stream_context.onDone(); + stream_context.onDelete(); + } + ASSERT_FALSE(wasm->isFailed()); + + // Create a stream context that propagates FilterHeadersStatus::StopIteration. + { + auto wasm_handle = std::make_shared(wasm); + auto plugin_handle = std::make_shared(wasm_handle, plugin); + auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); + stream_context.set_allow_on_request_headers_stop_iteration(true); + stream_context.onCreate(); + EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), + FilterHeadersStatus::StopIteration); + stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + stream_context.onDone(); + stream_context.onDelete(); + } + ASSERT_FALSE(wasm->isFailed()); +} + +} // namespace proxy_wasm diff --git a/test/test_data/BUILD b/test/test_data/BUILD index bd70b8eb9..e5ecd439e 100644 --- a/test/test_data/BUILD +++ b/test/test_data/BUILD @@ -89,3 +89,8 @@ proxy_wasm_cc_binary( name = "http_logging.wasm", srcs = ["http_logging.cc"], ) + +proxy_wasm_cc_binary( + name = "stop_iteration.wasm", + srcs = ["stop_iteration.cc"], +) diff --git a/test/test_data/stop_iteration.cc b/test/test_data/stop_iteration.cc new file mode 100644 index 000000000..c4c404429 --- /dev/null +++ b/test/test_data/stop_iteration.cc @@ -0,0 +1,27 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "proxy_wasm_intrinsics.h" + +class StopIterationContext : public Context { +public: + explicit StopIterationContext(uint32_t id, RootContext *root) : Context(id, root) {} + + FilterHeadersStatus onRequestHeaders(uint32_t headers, bool end_of_stream) override { + return FilterHeadersStatus::StopIteration; + } +}; + +static RegisterContextFactory register_StaticContext(CONTEXT_FACTORY(StopIterationContext), + ROOT_FACTORY(RootContext)); diff --git a/test/utility.h b/test/utility.h index 27b3b0493..45c1b8c74 100644 --- a/test/utility.h +++ b/test/utility.h @@ -133,6 +133,10 @@ class TestContext : public ContextBase { .count(); } + void set_allow_on_request_headers_stop_iteration(bool allow) { + allow_on_request_headers_stop_iteration_ = allow; + } + private: std::string log_; static std::string global_log_; From 1afe9020c2dcd30e7fdf04aaf598d31fbe1302ba Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Fri, 11 Jul 2025 21:00:38 +0000 Subject: [PATCH 2/6] Fix test copy-paste error It was testing the same case twice. Signed-off-by: Michael Warres --- test/stop_iteration_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/stop_iteration_test.cc b/test/stop_iteration_test.cc index d531dddfd..b59e4d712 100644 --- a/test/stop_iteration_test.cc +++ b/test/stop_iteration_test.cc @@ -54,10 +54,9 @@ TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { auto wasm_handle = std::make_shared(wasm); auto plugin_handle = std::make_shared(wasm_handle, plugin); auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); - stream_context.set_allow_on_request_headers_stop_iteration(true); stream_context.onCreate(); EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), - FilterHeadersStatus::StopIteration); + FilterHeadersStatus::StopAllIterationAndWatermark); stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); stream_context.onDone(); stream_context.onDelete(); From 769b7bd1e501eb45c8ee1da53e85c29a207f5602 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sat, 12 Jul 2025 01:12:43 +0000 Subject: [PATCH 3/6] rename to allow_on_headers_stop_iteration Rename option to allow_on_headers_stop_iteration and test onResponseHeaders behavior as well. Signed-off-by: Michael Warres --- include/proxy-wasm/context.h | 2 +- src/context.cc | 2 +- test/stop_iteration_test.cc | 14 ++++++++------ test/test_data/stop_iteration.cc | 4 ++++ test/utility.h | 4 ++-- 5 files changed, 16 insertions(+), 10 deletions(-) diff --git a/include/proxy-wasm/context.h b/include/proxy-wasm/context.h index c624e3803..7f675525c 100644 --- a/include/proxy-wasm/context.h +++ b/include/proxy-wasm/context.h @@ -402,7 +402,7 @@ class ContextBase : public RootInterface, // translates FilterHeaderStatus::StopIteration to // FilterHeadersStatus::StopAllIterationAndWatermark, which is the default // behavior for v0.2.* of the Proxy-Wasm ABI. - bool allow_on_request_headers_stop_iteration_ = false; + bool allow_on_headers_stop_iteration_ = false; private: // helper functions diff --git a/src/context.cc b/src/context.cc index ac10978e0..d8dbc9a2a 100644 --- a/src/context.cc +++ b/src/context.cc @@ -494,7 +494,7 @@ FilterHeadersStatus ContextBase::convertVmCallResultToFilterHeadersStatus(uint64 return FilterHeadersStatus::StopAllIterationAndWatermark; } if (result == static_cast(FilterHeadersStatus::StopIteration) && - !allow_on_request_headers_stop_iteration_) { + !allow_on_headers_stop_iteration_) { // Default behavior for Proxy-Wasm 0.2.* ABI is to translate StopIteration // (pause processing headers, but continue processing body) to // StopAllIterationAndWatermark (pause all processing), as described in diff --git a/test/stop_iteration_test.cc b/test/stop_iteration_test.cc index b59e4d712..d07aabbc7 100644 --- a/test/stop_iteration_test.cc +++ b/test/stop_iteration_test.cc @@ -24,7 +24,7 @@ INSTANTIATE_TEST_SUITE_P(WasmEngines, TestVm, testing::ValuesIn(getWasmEngines() }); // TestVm is parameterized for each engine and creates a VM on construction. -TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { +TEST_P(TestVm, AllowOnHeadersStopIteration) { // Read the wasm source. auto source = readTestWasmFile("stop_iteration.wasm"); ASSERT_FALSE(source.empty()); @@ -47,8 +47,8 @@ TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { // On the root context, call onConfigure(). ASSERT_TRUE(wasm->configure(root_context, plugin)); - // By default, stream context onRequestHeaders translates - // FilterHeadersStatus::StopIteration to + // By default, stream context onRequestHeaders and onResponseHeaders + // translates FilterHeadersStatus::StopIteration to // FilterHeadersStatus::StopAllIterationAndWatermark. { auto wasm_handle = std::make_shared(wasm); @@ -57,7 +57,8 @@ TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { stream_context.onCreate(); EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), FilterHeadersStatus::StopAllIterationAndWatermark); - stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_EQ(stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false), + FilterHeadersStatus::StopAllIterationAndWatermark); stream_context.onDone(); stream_context.onDelete(); } @@ -68,11 +69,12 @@ TEST_P(TestVm, AllowOnRequestHeadersStopIteration) { auto wasm_handle = std::make_shared(wasm); auto plugin_handle = std::make_shared(wasm_handle, plugin); auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); - stream_context.set_allow_on_request_headers_stop_iteration(true); + stream_context.set_allow_on_headers_stop_iteration(true); stream_context.onCreate(); EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), FilterHeadersStatus::StopIteration); - stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false); + EXPECT_EQ(stream_context.onResponseHeaders(/*headers=*/0, /*end_of_stream=*/false), + FilterHeadersStatus::StopIteration); stream_context.onDone(); stream_context.onDelete(); } diff --git a/test/test_data/stop_iteration.cc b/test/test_data/stop_iteration.cc index c4c404429..55594285f 100644 --- a/test/test_data/stop_iteration.cc +++ b/test/test_data/stop_iteration.cc @@ -21,6 +21,10 @@ class StopIterationContext : public Context { FilterHeadersStatus onRequestHeaders(uint32_t headers, bool end_of_stream) override { return FilterHeadersStatus::StopIteration; } + + FilterHeadersStatus onResponseHeaders(uint32_t headers, bool end_of_stream) override { + return FilterHeadersStatus::StopIteration; + } }; static RegisterContextFactory register_StaticContext(CONTEXT_FACTORY(StopIterationContext), diff --git a/test/utility.h b/test/utility.h index 45c1b8c74..6e98a832d 100644 --- a/test/utility.h +++ b/test/utility.h @@ -133,8 +133,8 @@ class TestContext : public ContextBase { .count(); } - void set_allow_on_request_headers_stop_iteration(bool allow) { - allow_on_request_headers_stop_iteration_ = allow; + void set_allow_on_headers_stop_iteration(bool allow) { + allow_on_headers_stop_iteration_ = allow; } private: From b9815c70017bc0e8121108f208101ec304a7c418 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sat, 12 Jul 2025 16:16:39 +0000 Subject: [PATCH 4/6] no-op comment change to trigger workflow retry Signed-off-by: Michael Warres --- test/stop_iteration_test.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/stop_iteration_test.cc b/test/stop_iteration_test.cc index d07aabbc7..f0faa0526 100644 --- a/test/stop_iteration_test.cc +++ b/test/stop_iteration_test.cc @@ -40,11 +40,9 @@ TEST_P(TestVm, AllowOnHeadersStopIteration) { /*engine=*/wasm->wasm_vm()->getEngineName(), /*plugin_config=*/"", /*fail_open=*/false, /*key=*/""); - // Create root context, call onStart(). + // Create root context, call onStart() and onConfigure() ContextBase *root_context = wasm->start(plugin); ASSERT_TRUE(root_context != nullptr); - - // On the root context, call onConfigure(). ASSERT_TRUE(wasm->configure(root_context, plugin)); // By default, stream context onRequestHeaders and onResponseHeaders From 7e525611d7b2618bb986c3b72314bdc8ebc34e51 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sat, 12 Jul 2025 23:09:43 +0000 Subject: [PATCH 5/6] apply clang-format-12 Signed-off-by: Michael Warres --- test/utility.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/utility.h b/test/utility.h index 6e98a832d..ccd2a59b6 100644 --- a/test/utility.h +++ b/test/utility.h @@ -133,9 +133,7 @@ class TestContext : public ContextBase { .count(); } - void set_allow_on_headers_stop_iteration(bool allow) { - allow_on_headers_stop_iteration_ = allow; - } + void set_allow_on_headers_stop_iteration(bool allow) { allow_on_headers_stop_iteration_ = allow; } private: std::string log_; From 6c906aa459d5c3a323e47aaa0b2c381199f512ff Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Sun, 13 Jul 2025 00:33:18 +0000 Subject: [PATCH 6/6] move test case wasm_handle and plugin_handle to proper scope Previously, the test was creating a new wasm_handle and plugin_handle for each request, which led to the WasmHandleBase destructor calling wasm_base_->startShutdown() mid-test. Apparently V8 tolerates this, but WAMR (correctly) does not. Signed-off-by: Michael Warres --- test/stop_iteration_test.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/stop_iteration_test.cc b/test/stop_iteration_test.cc index f0faa0526..9ff443f24 100644 --- a/test/stop_iteration_test.cc +++ b/test/stop_iteration_test.cc @@ -45,12 +45,13 @@ TEST_P(TestVm, AllowOnHeadersStopIteration) { ASSERT_TRUE(root_context != nullptr); ASSERT_TRUE(wasm->configure(root_context, plugin)); + auto wasm_handle = std::make_shared(wasm); + auto plugin_handle = std::make_shared(wasm_handle, plugin); + // By default, stream context onRequestHeaders and onResponseHeaders // translates FilterHeadersStatus::StopIteration to // FilterHeadersStatus::StopAllIterationAndWatermark. { - auto wasm_handle = std::make_shared(wasm); - auto plugin_handle = std::make_shared(wasm_handle, plugin); auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); stream_context.onCreate(); EXPECT_EQ(stream_context.onRequestHeaders(/*headers=*/0, /*end_of_stream=*/false), @@ -64,8 +65,6 @@ TEST_P(TestVm, AllowOnHeadersStopIteration) { // Create a stream context that propagates FilterHeadersStatus::StopIteration. { - auto wasm_handle = std::make_shared(wasm); - auto plugin_handle = std::make_shared(wasm_handle, plugin); auto stream_context = TestContext(wasm.get(), root_context->id(), plugin_handle); stream_context.set_allow_on_headers_stop_iteration(true); stream_context.onCreate();