From f71538a51c90d6ac330d31a3127890fd094e2570 Mon Sep 17 00:00:00 2001 From: Hisham Muhammad Date: Tue, 7 Nov 2023 19:35:10 -0300 Subject: [PATCH] Revert "[wip] make TEST 10 pass by avoiding FFI returns. TEST 9 still fails..." This reverts commit efe1c314164ea69ff9dd9f66153a769c1468ef68. --- 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, 24 insertions(+), 42 deletions(-) diff --git a/lib/resty/wasmx/proxy_wasm.lua b/lib/resty/wasmx/proxy_wasm.lua index 32888ec29..aa2e77d28 100644 --- a/lib/resty/wasmx/proxy_wasm.lua +++ b/lib/resty/wasmx/proxy_wasm.lua @@ -49,14 +49,12 @@ 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); - 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_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); 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); ]] @@ -250,11 +248,8 @@ function _M.set_property(key, value) data = value, len = value and #value or 0 }) - local rcp = ffi_new("ngx_int_t[1]", { 0 }) - C.ngx_http_wasm_ffi_set_property(r, ckey, cvalue, rcp) - - local rc = rcp[0] + local rc = C.ngx_http_wasm_ffi_set_property(r, ckey, cvalue) if rc == FFI_ERROR then return nil, "unknown error" end @@ -279,11 +274,8 @@ 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 = tonumber(rcp[0]) + local rc = C.ngx_http_wasm_ffi_get_property(r, ckey, cvalue) if rc == FFI_ERROR then return nil, "unknown error", ERROR end @@ -292,10 +284,6 @@ 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 f04dc8ff3..f6bdf0b74 100644 --- a/src/common/lua/ngx_wasm_lua_ffi.c +++ b/src/common/lua/ngx_wasm_lua_ffi.c @@ -196,34 +196,31 @@ get_pwctx(ngx_http_request_t *r) } -void +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 *rc) + ngx_str_t *key, ngx_str_t *value) { ngx_http_wasm_req_ctx_t *rctx; ngx_proxy_wasm_ctx_t *pwctx; if (ngx_http_wasm_rctx(r, &rctx) != NGX_OK) { - *rc = NGX_ERROR; - return; + return NGX_ERROR; } pwctx = ngx_proxy_wasm_ctx(NULL, 0, NGX_PROXY_WASM_ISOLATION_STREAM, &ngx_http_proxy_wasm, rctx); if (pwctx == NULL) { - *rc = NGX_ERROR; - return; + return NGX_ERROR; } - *rc = ngx_proxy_wasm_properties_set(pwctx, key, value); - return; + return ngx_proxy_wasm_properties_set(pwctx, key, value); } -void +ngx_int_t ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, - ngx_str_t *key, ngx_str_t *value, ngx_int_t *rc) + ngx_str_t *key, ngx_str_t *value) { ngx_http_wasm_req_ctx_t *rctx; ngx_proxy_wasm_ctx_t *pwctx; @@ -233,20 +230,17 @@ 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 */ - *rc = NGX_ERROR; - return; + return NGX_ERROR; } pwctx = ngx_proxy_wasm_ctx(NULL, 0, NGX_PROXY_WASM_ISOLATION_STREAM, &ngx_http_proxy_wasm, rctx); if (pwctx == NULL) { - *rc = NGX_ERROR; - return; + return NGX_ERROR; } - *rc = ngx_proxy_wasm_properties_get(pwctx, key, value); - return; + return ngx_proxy_wasm_properties_get(pwctx, key, value); } @@ -325,7 +319,6 @@ 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"; @@ -339,6 +332,7 @@ 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 26b5d97f4..889306263 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); -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_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); 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 fb053dce0..980e021bb 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,7 +398,6 @@ ok === TEST 9: set_property_getter() - getter returning nil produces property not found ---- ONLY --- wasm_modules: hostcalls --- http_config init_worker_by_lua_block { @@ -446,6 +445,7 @@ 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"