From efe1c314164ea69ff9dd9f66153a769c1468ef68 Mon Sep 17 00:00:00 2001 From: Hisham Muhammad Date: Tue, 7 Nov 2023 18:59:26 -0300 Subject: [PATCH] [wip] make TEST 10 pass by avoiding FFI returns. TEST 9 still fails... --- lib/resty/wasmx/proxy_wasm.lua | 28 +++++++++++++------ src/common/lua/ngx_wasm_lua_ffi.c | 28 +++++++++++-------- src/common/lua/ngx_wasm_lua_ffi.h | 8 +++--- .../ffi/304-proxy_wasm_set_property_getter.t | 2 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/lib/resty/wasmx/proxy_wasm.lua b/lib/resty/wasmx/proxy_wasm.lua index aa2e77d28..32888ec29 100644 --- a/lib/resty/wasmx/proxy_wasm.lua +++ b/lib/resty/wasmx/proxy_wasm.lua @@ -49,12 +49,14 @@ if subsystem == "http" then ngx_wasm_plan_t *plan, unsigned int isolation); ngx_int_t ngx_http_wasm_ffi_start(ngx_http_request_t *r); - ngx_int_t ngx_http_wasm_ffi_set_property(ngx_http_request_t *r, - ngx_str_t *key, - ngx_str_t *value); - ngx_int_t ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, - ngx_str_t *key, - ngx_str_t *out); + void ngx_http_wasm_ffi_set_property(ngx_http_request_t *r, + ngx_str_t *key, + ngx_str_t *value, + ngx_int_t *rc); + void ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, + ngx_str_t *key, + ngx_str_t *out, + ngx_int_t *rc); ngx_int_t ngx_http_wasm_ffi_enable_property_setter(ngx_http_request_t *r); ngx_int_t ngx_http_wasm_ffi_enable_property_getter(ngx_http_request_t *r); ]] @@ -248,8 +250,11 @@ function _M.set_property(key, value) data = value, len = value and #value or 0 }) + local rcp = ffi_new("ngx_int_t[1]", { 0 }) - local rc = C.ngx_http_wasm_ffi_set_property(r, ckey, cvalue) + C.ngx_http_wasm_ffi_set_property(r, ckey, cvalue, rcp) + + local rc = rcp[0] if rc == FFI_ERROR then return nil, "unknown error" end @@ -274,8 +279,11 @@ function _M.get_property(key) local ckey = ffi_new("ngx_str_t", { data = key, len = #key }) local cvalue = ffi_new("ngx_str_t", { data = nil, len = 0 }) + local rcp = ffi_new("ngx_int_t[1]", { 0 }) + + C.ngx_http_wasm_ffi_get_property(r, ckey, cvalue, rcp) - local rc = C.ngx_http_wasm_ffi_get_property(r, ckey, cvalue) + local rc = tonumber(rcp[0]) if rc == FFI_ERROR then return nil, "unknown error", ERROR end @@ -284,6 +292,10 @@ function _M.get_property(key) return nil, "property \"" .. key .. "\" not found", NOT_FOUND end + if cvalue.data == nil then + return nil + end + return ffi_str(cvalue.data, cvalue.len) end diff --git a/src/common/lua/ngx_wasm_lua_ffi.c b/src/common/lua/ngx_wasm_lua_ffi.c index f6bdf0b74..f04dc8ff3 100644 --- a/src/common/lua/ngx_wasm_lua_ffi.c +++ b/src/common/lua/ngx_wasm_lua_ffi.c @@ -196,31 +196,34 @@ get_pwctx(ngx_http_request_t *r) } -ngx_int_t +void ngx_http_wasm_ffi_set_property(ngx_http_request_t *r, - ngx_str_t *key, ngx_str_t *value) + ngx_str_t *key, ngx_str_t *value, ngx_int_t *rc) { ngx_http_wasm_req_ctx_t *rctx; ngx_proxy_wasm_ctx_t *pwctx; if (ngx_http_wasm_rctx(r, &rctx) != NGX_OK) { - return NGX_ERROR; + *rc = NGX_ERROR; + return; } pwctx = ngx_proxy_wasm_ctx(NULL, 0, NGX_PROXY_WASM_ISOLATION_STREAM, &ngx_http_proxy_wasm, rctx); if (pwctx == NULL) { - return NGX_ERROR; + *rc = NGX_ERROR; + return; } - return ngx_proxy_wasm_properties_set(pwctx, key, value); + *rc = ngx_proxy_wasm_properties_set(pwctx, key, value); + return; } -ngx_int_t +void ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, - ngx_str_t *key, ngx_str_t *value) + ngx_str_t *key, ngx_str_t *value, ngx_int_t *rc) { ngx_http_wasm_req_ctx_t *rctx; ngx_proxy_wasm_ctx_t *pwctx; @@ -230,17 +233,20 @@ ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, * TODO: return code signifying "no plan attached to request" and co. * return associated constant as err/code from Lua lib */ - return NGX_ERROR; + *rc = NGX_ERROR; + return; } pwctx = ngx_proxy_wasm_ctx(NULL, 0, NGX_PROXY_WASM_ISOLATION_STREAM, &ngx_http_proxy_wasm, rctx); if (pwctx == NULL) { - return NGX_ERROR; + *rc = NGX_ERROR; + return; } - return ngx_proxy_wasm_properties_get(pwctx, key, value); + *rc = ngx_proxy_wasm_properties_get(pwctx, key, value); + return; } @@ -319,6 +325,7 @@ static const char *HOST_PROPERTY_SCRIPT = "" "local ok, new_value, is_const = fn(key, value) \n" "local new_value_len = new_value and #new_value or 0 \n" " \n" + " \n" "ffi.C.ngx_wasm_lua_ffi_host_prop_handler(hprctx, ok and 1 or 0, \n" " new_value, new_value_len, \n" " is_const and 1 or 0) \n"; @@ -332,7 +339,6 @@ property_handler(void *data, ngx_str_t *key, ngx_str_t *value) lua_State *co; ngx_int_t rc = NGX_OK; ngx_wasm_lua_ctx_t *lctx; - int top; // FIXME if this function is changed to return NGX_AGAIN, // hprctx needs to be dynamically allocated: diff --git a/src/common/lua/ngx_wasm_lua_ffi.h b/src/common/lua/ngx_wasm_lua_ffi.h index 889306263..26b5d97f4 100644 --- a/src/common/lua/ngx_wasm_lua_ffi.h +++ b/src/common/lua/ngx_wasm_lua_ffi.h @@ -45,10 +45,10 @@ ngx_int_t ngx_http_wasm_ffi_plan_load(ngx_wasm_ops_plan_t *plan); ngx_int_t ngx_http_wasm_ffi_plan_attach(ngx_http_request_t *r, ngx_wasm_ops_plan_t *plan, ngx_uint_t isolation); ngx_int_t ngx_http_wasm_ffi_start(ngx_http_request_t *r); -ngx_int_t ngx_http_wasm_ffi_set_property(ngx_http_request_t *r, - ngx_str_t *key, ngx_str_t *value); -ngx_int_t ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, - ngx_str_t *key, ngx_str_t *value); +void ngx_http_wasm_ffi_set_property(ngx_http_request_t *r, + ngx_str_t *key, ngx_str_t *value, ngx_int_t *rc); +void ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, + ngx_str_t *key, ngx_str_t *value, ngx_int_t *rc); ngx_int_t ngx_http_wasm_ffi_set_host_property(ngx_http_request_t *r, ngx_str_t *key, ngx_str_t *value, unsigned is_const, unsigned retrieve); ngx_int_t ngx_http_wasm_ffi_set_property_setter(ngx_http_request_t *r, diff --git a/t/04-openresty/ffi/304-proxy_wasm_set_property_getter.t b/t/04-openresty/ffi/304-proxy_wasm_set_property_getter.t index 980e021bb..fb053dce0 100644 --- a/t/04-openresty/ffi/304-proxy_wasm_set_property_getter.t +++ b/t/04-openresty/ffi/304-proxy_wasm_set_property_getter.t @@ -398,6 +398,7 @@ ok === TEST 9: set_property_getter() - getter returning nil produces property not found +--- ONLY --- wasm_modules: hostcalls --- http_config init_worker_by_lua_block { @@ -445,7 +446,6 @@ ok === TEST 10: set_property_getter() - getter returning true on 3rd value caches result --- wasm_modules: hostcalls ---- ONLY --- http_config init_worker_by_lua_block { local proxy_wasm = require "resty.wasmx.proxy_wasm"