Skip to content

Commit

Permalink
fix(proxy-wasm) resume the filter chain on dispatch response
Browse files Browse the repository at this point in the history
The dispatch could be during `on_request_body`, and when the response is
received, subsequent filters' `on_request_body` steps should also be
invoked.

This is a rudimentary fix for the time being, but now that the
proxy-wasm runloop is fully re-entrant, the resume model could also be
refactored into something cleaner and reusable for other C resume
handlers.

Co-Authored-By: Caio Ramos Casimiro <caiorcasimiro@gmail.com>
  • Loading branch information
thibaultcha and casimiro committed Oct 6, 2023
1 parent 2c94197 commit c0ace96
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 7 deletions.
4 changes: 4 additions & 0 deletions src/http/ngx_http_wasm_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,10 @@ ngx_http_wasm_wev_handler(ngx_http_request_t *r)
}
#endif

dd("entered_content_phase: %d, resp_content_chosen: %d, fake: %d",
rctx->entered_content_phase, rctx->resp_content_chosen,
rctx->fake_request);

if (rctx->entered_content_phase || rctx->resp_content_chosen) {
rc = ngx_http_wasm_content(rctx);
dd("wev content rc: %ld", rc);
Expand Down
9 changes: 8 additions & 1 deletion src/http/proxy_wasm/ngx_http_proxy_wasm_dispatch.c
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ ngx_http_proxy_wasm_dispatch_resume_handler(ngx_wasm_socket_tcp_t *sock)
ngx_proxy_wasm_exec_t *pwexec = call->pwexec;
ngx_proxy_wasm_filter_t *filter = pwexec->filter;
ngx_proxy_wasm_err_e ecode = NGX_PROXY_WASM_ERR_NONE;
ngx_proxy_wasm_step_e step;

dd("enter");

Expand Down Expand Up @@ -821,6 +822,9 @@ ngx_http_proxy_wasm_dispatch_resume_handler(ngx_wasm_socket_tcp_t *sock)
*/
pwexec->call = call;

/* save step */
step = pwexec->parent->step;

ecode = ngx_proxy_wasm_run_step(pwexec, pwexec->ictx,
NGX_PROXY_WASM_STEP_DISPATCH_RESPONSE);
if (ecode != NGX_PROXY_WASM_ERR_NONE) {
Expand All @@ -830,7 +834,10 @@ ngx_http_proxy_wasm_dispatch_resume_handler(ngx_wasm_socket_tcp_t *sock)
if (pwexec->call == call) {
/* no further call from the callback */
pwexec->call = NULL;
rc = NGX_OK;

/* resume current step if unfinished */
rc = ngx_proxy_wasm_resume(pwexec->parent, pwexec->parent->phase,
step);

} else {
/* another call was setup during the callback */
Expand Down
5 changes: 1 addition & 4 deletions src/wasm/ngx_wasm_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,17 +252,14 @@ ngx_wasm_ops_resume(ngx_wasm_op_ctx_t *ctx, ngx_uint_t phaseidx)
ops = ctx->ops;
plan = ctx->plan;

dd("enter (phaseidx: %ld, phase: \"%.*s\")",
phaseidx, (int) phase->name.len, phase->name.data);

phase = ngx_wasm_phase_lookup(ops->subsystem, phaseidx);
if (phase == NULL) {
ngx_wasm_log_error(NGX_LOG_WASM_NYI, ctx->log, 0,
"ops resume: no phase for index '%ld'", phaseidx);
goto done;
}

dd("phaseidx: %ld, phase: %.*s",
dd("enter (phaseidx: %ld, phase: \"%.*s\")",
phaseidx, (int) phase->name.len, phase->name.data);

/* check last phase */
Expand Down
4 changes: 2 additions & 2 deletions t/03-proxy_wasm/hfuncs/102-proxy_send_local_response.t
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ Hello world
=== TEST 18: proxy_wasm - send_local_response() executes all chained filters response steps
should run all response steps of all chained filters
=== TEST 18: proxy_wasm - send_local_response() on_request_headers, filter chain execution sanity
should execute all filters request and response steps
--- wasm_modules: hostcalls
--- config
location /t {
Expand Down
90 changes: 90 additions & 0 deletions t/03-proxy_wasm/hfuncs/133-proxy_dispatch_http_edge_cases.t
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,93 @@ on_root_http_call_response \(id: 9, status: 200, headers: 5, body_bytes: 12, tra
[error]
[crit]
[emerg]
=== TEST 5: proxy_wasm - dispatch_http_call() on_request_headers, filter chain execution sanity
should execute all filters request and response steps
--- load_nginx_modules: ngx_http_echo_module
--- wasm_modules: hostcalls
--- config
location /dispatched {
return 200 "Hello back";
}
location /t {
proxy_wasm hostcalls 'on=request_headers \
test=/t/dispatch_http_call \
host=127.0.0.1:$TEST_NGINX_SERVER_PORT \
path=/dispatched';
proxy_wasm hostcalls;
echo ok;
}
--- request
GET /t
Hello world
--- response_body
ok
--- grep_error_log eval: qr/\[info\] .*? on_(http_call_response|request|response|log).*/
--- grep_error_log_out eval
qr/\A.*? on_request_headers.*
.*? on_http_call_response \(id: 0, status: 200, headers: 5, body_bytes: 10, trailers: 0, op: \).*
.*? on_request_headers.*
.*? on_request_body.*
.*? on_request_body.*
.*? on_response_headers.*
.*? on_response_headers.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_log.*
.*? on_log/
--- no_error_log
[error]
[crit]
[emerg]
=== TEST 6: proxy_wasm - dispatch_http_call() on_request_body, filter chain execution sanity
should execute all filters request and response steps
--- load_nginx_modules: ngx_http_echo_module
--- wasm_modules: hostcalls
--- config
location /dispatched {
return 200 "Hello back";
}
location /t {
proxy_wasm hostcalls 'on=request_body \
test=/t/dispatch_http_call \
host=127.0.0.1:$TEST_NGINX_SERVER_PORT \
path=/dispatched';
proxy_wasm hostcalls;
echo ok;
}
--- request
GET /t
Hello world
--- response_body
ok
--- grep_error_log eval: qr/\[info\] .*? on_(http_call_response|request|response|log).*/
--- grep_error_log_out eval
qr/\A.*? on_request_headers.*
.*? on_request_headers.*
.*? on_request_body.*
.*? on_http_call_response \(id: 0, status: 200, headers: 5, body_bytes: 10, trailers: 0, op: \).*
.*? on_request_body.*
.*? on_response_headers.*
.*? on_response_headers.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_response_body.*
.*? on_log.*
.*? on_log/
--- no_error_log
[error]
[crit]
[emerg]

0 comments on commit c0ace96

Please sign in to comment.