From e2705a06b18f752a9b7760132cf09b35619876b3 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Mon, 2 Oct 2023 22:19:28 -0700 Subject: [PATCH] fix(proxy-wasm) execute the whole filter chain on local responses Co-authored-by: Caio Ramos Casimiro --- src/common/proxy_wasm/ngx_proxy_wasm.h | 12 ++++++ src/common/proxy_wasm/ngx_proxy_wasm_host.c | 1 + .../hfuncs/102-proxy_send_local_response.t | 38 ++++++++++++++++++- t/05-others/010-connection_drop.t | 37 +++++++++--------- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/common/proxy_wasm/ngx_proxy_wasm.h b/src/common/proxy_wasm/ngx_proxy_wasm.h index 11fad5072..63a37a67f 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm.h +++ b/src/common/proxy_wasm/ngx_proxy_wasm.h @@ -433,6 +433,18 @@ ngx_proxy_wasm_ctx_set_next_action(ngx_proxy_wasm_ctx_t *pwctx, } +static ngx_inline void +ngx_proxy_wasm_ctx_reset_chain(ngx_proxy_wasm_ctx_t *pwctx) +{ + ngx_proxy_wasm_log_error(NGX_LOG_DEBUG, pwctx->log, 0, + "resetting filter chain: pwctx->exec_index " + "%ld to 0 (pwctx: %p)", + pwctx->exec_index, pwctx); + + pwctx->exec_index = 0; +} + + static ngx_inline ngx_proxy_wasm_exec_t * ngx_proxy_wasm_instance2pwexec(ngx_wavm_instance_t *instance) { diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_host.c b/src/common/proxy_wasm/ngx_proxy_wasm_host.c index 826cf48ab..8ad072e7e 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm_host.c +++ b/src/common/proxy_wasm/ngx_proxy_wasm_host.c @@ -929,6 +929,7 @@ ngx_proxy_wasm_hfuncs_send_local_response(ngx_wavm_instance_t *instance, switch (rc) { case NGX_OK: + ngx_proxy_wasm_ctx_reset_chain(pwexec->parent); ngx_proxy_wasm_ctx_set_next_action(pwexec->parent, NGX_PROXY_WASM_ACTION_DONE); break; diff --git a/t/03-proxy_wasm/hfuncs/102-proxy_send_local_response.t b/t/03-proxy_wasm/hfuncs/102-proxy_send_local_response.t index e9f90253e..a45668565 100644 --- a/t/03-proxy_wasm/hfuncs/102-proxy_send_local_response.t +++ b/t/03-proxy_wasm/hfuncs/102-proxy_send_local_response.t @@ -420,6 +420,42 @@ Hello world === TEST 18: proxy_wasm - send_local_response() in chained filters +should run all response phases of chained filters +--- wasm_modules: hostcalls +--- config + location /t { + proxy_wasm hostcalls; + proxy_wasm hostcalls 'test=/t/send_local_response/body'; + proxy_wasm hostcalls; + } +--- response_body +Hello world +--- grep_error_log eval: qr/\[hostcalls\] .*/ +--- grep_error_log_out eval +qr/.*? on_request_headers, \d+ headers.* +.*? testing in "RequestHeaders".* +.*? on_request_headers, \d+ headers.* +.*? testing in "RequestHeaders".* +.*? on_response_headers, \d+ headers.* +.*? on_response_headers, \d+ headers.* +.*? on_response_headers, \d+ headers.* +.*? on_response_body, \d+ bytes, eof: true.* +.*? on_response_body, \d+ bytes, eof: true.* +.*? on_response_body, \d+ bytes, eof: true.* +.*? on_done.* +.*? on_log.* +.*? on_done.* +.*? on_log.* +.*? on_done.* +.*? on_log.*/ +--- no_error_log +[error] +[crit] +[alert] + + + +=== TEST 19: proxy_wasm - send_local_response() invoked twice in chained filters should interrupt the current phase, preventing "response already stashed" should still run all response phases --- wasm_modules: hostcalls @@ -449,7 +485,7 @@ qr/.*? on_request_headers, \d+ headers.* -=== TEST 19: proxy_wasm - send_local_response() in chained filters as a subrequest +=== TEST 20: proxy_wasm - send_local_response() in chained filters as a subrequest should interrupt the current phase, preventing "response already stashed" should still run all response phases should not have a log phase (subrequest) diff --git a/t/05-others/010-connection_drop.t b/t/05-others/010-connection_drop.t index e5a5259d5..846bd119b 100644 --- a/t/05-others/010-connection_drop.t +++ b/t/05-others/010-connection_drop.t @@ -126,31 +126,30 @@ Filters running on the same request will have their response entrypoints invoked --- wasm_modules: hostcalls on_phases --- config location /t { - proxy_wasm on_phases; + proxy_wasm hostcalls; proxy_wasm hostcalls 'test=/t/send_local_response/body'; - proxy_wasm on_phases; + proxy_wasm hostcalls; } --- shutdown --- ignore_response --- grep_error_log eval: qr/\[info\] .*? on_(response|log|done).*/ --- grep_error_log_out eval -qr/.*\["hostcalls" #1\].* on_response_headers.* -.*\["on_phases" #2\].* on_response_headers.* -.*\["on_phases" #1\].* on_response_body.* -.*\["hostcalls" #1\].* on_response_body.* -.*\["on_phases" #2\].* on_response_body.* -.*\["on_phases" #1\].* on_done.* -.*\["on_phases" #1\].* on_log.* -.*\["hostcalls" #1\].* on_done.* -.*\["hostcalls" #1\].* on_log.* -.*\["on_phases" #2\].* on_done.* -.*\["on_phases" #2\].* on_log.*/ ---- no_error_log eval -[ - qr/.*\["on_phases" #1\].* on_response_headers.*/, - "[error]", - "[crit]", -] +qr/\A.*?\["hostcalls" #1\].*? on_response_headers.* +.*?\["hostcalls" #2\].*? on_response_headers.* +.*?\["hostcalls" #3\].*? on_response_headers.* +.*?\["hostcalls" #1\].*? on_response_body.* +.*?\["hostcalls" #2\].*? on_response_body.* +.*?\["hostcalls" #3\].*? on_response_body.* +.*?\["hostcalls" #1\].*? on_done.* +.*?\["hostcalls" #1\].*? on_log.* +.*?\["hostcalls" #2\].*? on_done.* +.*?\["hostcalls" #2\].*? on_log.* +.*?\["hostcalls" #3\].*? on_done.* +.*?\["hostcalls" #3\].*? on_log.*\Z/ +--- no_error_log +[error] +[crit] +[emerg]