diff --git a/lib/resty/wasmx/proxy_wasm.lua b/lib/resty/wasmx/proxy_wasm.lua index 095607d4e..aa2e77d28 100644 --- a/lib/resty/wasmx/proxy_wasm.lua +++ b/lib/resty/wasmx/proxy_wasm.lua @@ -8,11 +8,9 @@ local C = ffi.C local ngx = ngx local error = error local type = type -local pcall = pcall local ffi_gc = ffi.gc local ffi_new = ffi.new local ffi_str = ffi.string -local tostring = tostring local subsystem = ngx.config.subsystem local get_request = wasmx.get_request local get_err_ptr = wasmx.get_err_ptr @@ -57,15 +55,8 @@ if subsystem == "http" then 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_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, - ngx_wasm_host_prop_fn_t fn); - ngx_int_t ngx_http_wasm_ffi_set_property_getter(ngx_http_request_t *r, - ngx_wasm_host_prop_fn_t fn); + 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); ]] else @@ -297,166 +288,72 @@ function _M.get_property(key) end -do - -- LuaJIT C callbacks are a limited resource: - -- We can't create one of these on each request, - -- so we prepare these wrappers, which in turn call a Lua function. - local lua_prop_setter - local lua_prop_getter - local c_prop_setter - local c_prop_getter - - - local function update_cvalue(cvalue, lvalue) - if lvalue == nil then - cvalue.data = nil - cvalue.len = 0 - return - end - - local value = tostring(lvalue) - cvalue.data = value - cvalue.len = #value +--- +-- Define a setter function for host-managed properties. +-- +-- @param setter The setter function is the handler for updating properties. +-- Its type signature is `function(string, string): boolean, string`, +-- where the inputs are the property key and value and the results are: +-- * `truthy` - success setting the property +-- * `truthy, value` - success, cache property value (useful if hosts +-- sets only a setter but not a getter) +-- * `truthy, value, truthy` - success, constant property value: further +-- requests to the same property during a request are cached by +-- ngx_wasm_module and do not invoke the Lua getter. +-- * `falsy, err` - failure, and an optional error message +-- @return true on success setting the getter, or nil and an error message. +function _M.set_property_setter(setter) + if type(setter) ~= "function" then + error("setter must be a function", 2) end - - local function jit_wrap(fn) - return function(r, ckey, cvalue) - jit.off() - local rc = fn(r, ckey, cvalue) - jit.on() - return rc - end + local r = get_request() + if not r then + error("no request found", 2) end + _M.property_setter = setter - c_prop_setter = ffi.cast("ngx_wasm_host_prop_fn_t", - jit_wrap(function(r, ckey, cvalue) - local lkey = ffi_str(ckey.data, ckey.len) - local lval - if cvalue.data ~= nil then - lval = ffi_str(cvalue.data, cvalue.len) - end - - local pok, ok, lvalue, is_const = pcall(lua_prop_setter, lkey, lval) - if not pok then - ngx.log(ngx.ERR, "error setting property from Lua: ", ok) - return FFI_ERROR - end - - if not ok then - local err = lvalue or "unknown error" - ngx.log(ngx.ERR, "error setting property: ", err) - return FFI_ERROR - end - - update_cvalue(cvalue, lvalue) - - return C.ngx_http_wasm_ffi_set_host_property(r, ckey, cvalue, - is_const and 1 or 0, - 0) - end)) - - - c_prop_getter = ffi.cast("ngx_wasm_host_prop_fn_t", - jit_wrap(function(r, ckey, cvalue) - local lkey = ffi_str(ckey.data, ckey.len) - - local pok, ok, lvalue, is_const = pcall(lua_prop_getter, lkey) - if not pok then - ngx.log(ngx.ERR, "error getting property from Lua: ", ok) - return FFI_ERROR - end - - if not ok then - if lvalue then - ngx.log(ngx.ERR, "error setting property: ", lvalue) - return FFI_ERROR - end - - return FFI_DECLINED - end - - update_cvalue(cvalue, lvalue) - - local rc = C.ngx_http_wasm_ffi_set_host_property(r, ckey, cvalue, - is_const and 1 or 0, - 1) - if rc == FFI_OK and lvalue == nil then - return FFI_DECLINED - end - - return rc - end)) - - - --- - -- Define a setter function for host-managed properties. - -- - -- @param setter The setter function is the handler for updating properties. - -- Its type signature is `function(string, string): boolean, string`, - -- where the inputs are the property key and value and the results are: - -- * `truthy` - success setting the property - -- * `truthy, value` - success, cache property value (useful if hosts - -- sets only a setter but not a getter) - -- * `truthy, value, truthy` - success, constant property value: further - -- requests to the same property during a request are cached by - -- ngx_wasm_module and do not invoke the Lua getter. - -- * `falsy, err` - failure, and an optional error message - -- @return true on success setting the getter, or nil and an error message. - function _M.set_property_setter(setter) - if type(setter) ~= "function" then - error("setter must be a function", 2) - end - - local r = get_request() - if not r then - error("no request found", 2) - end - - lua_prop_setter = setter + local rc = C.ngx_http_wasm_ffi_enable_property_setter(r) + if rc ~= FFI_OK then + return nil, "could not set property setter" + end - local rc = C.ngx_http_wasm_ffi_set_property_setter(r, c_prop_setter) - if rc ~= FFI_OK then - return nil, "could not set property setter" - end + return true +end - return true - end - - - --- - -- Define a getter function for host-managed properties. - -- - -- @param getter The getter function is the handler for resolving properties. - -- Its type signature is `function(string): boolean, string, boolean`, - -- where the input is the property key and the results are: - -- * `truthy, value` - success, property value - -- * `truthy, value, truthy` - success, constant property value: further - -- requests to the same property during a request are cached by - -- ngx_wasm_module and do not invoke the Lua getter. - -- * `falsy` - property not found - -- * `falsy, err` - failure, error message - -- @return true on success setting the getter, or nil and an error message. - function _M.set_property_getter(getter) - if type(getter) ~= "function" then - error("getter must be a function", 2) - end - local r = get_request() - if not r then - error("no request found", 2) - end +--- +-- Define a getter function for host-managed properties. +-- +-- @param getter The getter function is the handler for resolving properties. +-- Its type signature is `function(string): boolean, string, boolean`, +-- where the input is the property key and the results are: +-- * `truthy, value` - success, property value +-- * `truthy, value, truthy` - success, constant property value: further +-- requests to the same property during a request are cached by +-- ngx_wasm_module and do not invoke the Lua getter. +-- * `falsy` - property not found +-- * `falsy, err` - failure, error message +-- @return true on success setting the getter, or nil and an error message. +function _M.set_property_getter(getter) + if type(getter) ~= "function" then + error("getter must be a function", 2) + end - lua_prop_getter = getter + local r = get_request() + if not r then + error("no request found", 2) + end - local rc = C.ngx_http_wasm_ffi_set_property_getter(r, c_prop_getter) - if rc ~= FFI_OK then - return nil, "could not set property getter" - end + _M.property_getter = getter - return true + local rc = C.ngx_http_wasm_ffi_enable_property_getter(r) + if rc ~= FFI_OK then + return nil, "could not set property getter" end + + return true end diff --git a/src/common/lua/ngx_wasm_lua_ffi.c b/src/common/lua/ngx_wasm_lua_ffi.c index aaa5b54af..fac9d1df3 100644 --- a/src/common/lua/ngx_wasm_lua_ffi.c +++ b/src/common/lua/ngx_wasm_lua_ffi.c @@ -3,6 +3,7 @@ #endif #include "ddebug.h" +#include #include @@ -243,45 +244,153 @@ ngx_http_wasm_ffi_get_property(ngx_http_request_t *r, } -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) +static ngx_int_t +property_handler(void *data, ngx_str_t *key, ngx_str_t *value) { - ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(r); + ngx_wasm_ffi_host_property_ctx_t *hpctx = data; + ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(hpctx->r); + lua_State *L = hpctx->L; + ngx_int_t rc; + unsigned is_const; + int pok; + int top; + + top = lua_gettop(L); + + lua_getglobal(L, "package"); + lua_getfield(L, -1, "loaded"); + lua_getfield(L, -1, "resty.wasmx.proxy_wasm"); + if (lua_type(L, -1) != LUA_TTABLE) { + goto fail; + } + + lua_getfield(L, -1, + hpctx->is_getter ? "property_getter" : "property_setter"); + if (lua_type(L, -1) != LUA_TFUNCTION) { + goto fail; + } + + ngx_wasm_assert(key && key->data); + + lua_pushlstring(L, (char *) key->data, key->len); + + ngx_wasm_assert(value); + + if (value->data == NULL) { + lua_pushnil(L); + + } else { + if ((int) value->len == 0) { + lua_pushliteral(L, ""); + + } else { + lua_pushlstring(L, (char *) value->data, value->len); + } + } + + pok = lua_pcall(L, 2, 3, 0); + if (pok != 0) { + ngx_wavm_log_error(NGX_LOG_ERR, pwctx->log, NULL, + "Lua error in property handler: %s", + lua_tostring(L, -1)); + goto fail; + } + + if (lua_toboolean(L, -3) == 0) { + if (lua_isstring(L, -2)) { + ngx_wavm_log_error(NGX_LOG_ERR, pwctx->log, NULL, + "error in property handler: %s", + lua_tostring(L, -2)); + goto fail; + + } else if (!hpctx->is_getter) { + ngx_wavm_log_error(NGX_LOG_ERR, pwctx->log, NULL, + "error in property handler: unknown error"); + goto fail; + + } + + return NGX_DECLINED; + } + + if (lua_isnil(L, -2)) { + value->data = NULL; + value->len = 0; + + } else { + value->data = (u_char *) lua_tolstring(L, -2, &value->len); + } + + is_const = lua_toboolean(L, -1); if (pwctx == NULL) { return NGX_ERROR; } - return ngx_proxy_wasm_properties_set_host(pwctx, key, value, is_const, - retrieve); + rc = ngx_proxy_wasm_properties_set_host(pwctx, key, value, is_const, + hpctx->is_getter); + + lua_settop(L, top); + + if (hpctx->is_getter && rc == NGX_OK && value->data == NULL) { + return NGX_DECLINED; + } + + return rc; + +fail: + + lua_settop(L, top); + return NGX_ERROR; } ngx_int_t -ngx_http_wasm_ffi_set_property_setter(ngx_http_request_t *r, - ngx_wasm_host_prop_fn_t fn) +ngx_http_wasm_ffi_enable_property_setter(ngx_http_request_t *r) { - ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(r); + ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(r); + ngx_wasm_ffi_host_property_ctx_t *hpctx; if (pwctx == NULL) { return NGX_ERROR; } - return ngx_proxy_wasm_properties_set_host_prop_setter(pwctx, fn, r); + hpctx = ngx_palloc(pwctx->pool, sizeof(ngx_wasm_ffi_host_property_ctx_t)); + if (hpctx == NULL) { + return NGX_ERROR; + } + + hpctx->r = r; + hpctx->L = ngx_http_lua_get_lua_vm(r, NULL); + hpctx->is_getter = 0; + + return ngx_proxy_wasm_properties_set_host_prop_setter(pwctx, + property_handler, + hpctx); } ngx_int_t -ngx_http_wasm_ffi_set_property_getter(ngx_http_request_t *r, - ngx_wasm_host_prop_fn_t fn) +ngx_http_wasm_ffi_enable_property_getter(ngx_http_request_t *r) { - ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(r); + ngx_proxy_wasm_ctx_t *pwctx = get_pwctx(r); + ngx_wasm_ffi_host_property_ctx_t *hpctx; if (pwctx == NULL) { return NGX_ERROR; } - return ngx_proxy_wasm_properties_set_host_prop_getter(pwctx, fn, r); + hpctx = ngx_palloc(pwctx->pool, sizeof(ngx_wasm_ffi_host_property_ctx_t)); + if (hpctx == NULL) { + return NGX_ERROR; + } + + hpctx->r = r; + hpctx->L = ngx_http_lua_get_lua_vm(r, NULL); + hpctx->is_getter = 1; + + return ngx_proxy_wasm_properties_set_host_prop_getter(pwctx, + property_handler, + hpctx); } #endif diff --git a/src/common/lua/ngx_wasm_lua_ffi.h b/src/common/lua/ngx_wasm_lua_ffi.h index 1d87e22d1..0dc78299d 100644 --- a/src/common/lua/ngx_wasm_lua_ffi.h +++ b/src/common/lua/ngx_wasm_lua_ffi.h @@ -21,6 +21,13 @@ typedef struct { } ngx_wasm_ffi_filter_t; +typedef struct { + ngx_http_request_t *r; + lua_State *L; + unsigned is_getter; +} ngx_wasm_ffi_host_property_ctx_t; + + ngx_int_t ngx_http_wasm_ffi_plan_new(ngx_wavm_t *vm, ngx_wasm_ffi_filter_t *filters, size_t n_filters, ngx_wasm_ops_plan_t **out, u_char *err, size_t *errlen); 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 e6c8f764f..ac8f8786b 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 @@ -591,7 +591,7 @@ ok --- error_log eval [ qr/\[info\] .*? \[hostcalls\] on_response_body/, - qr/\[error\] .*? error getting property from Lua: rewrite_by_lua\(nginx.conf:[0-9]+\):[0-9]+: crash\!/, + qr/\[error\] .*? Lua error in property handler: rewrite_by_lua\(nginx.conf:[0-9]+\):[0-9]+: crash\!/, ] --- no_error_log [emerg] diff --git a/t/04-openresty/ffi/305-proxy_wasm_set_property_setter.t b/t/04-openresty/ffi/305-proxy_wasm_set_property_setter.t index 471385d3d..04f05af05 100644 --- a/t/04-openresty/ffi/305-proxy_wasm_set_property_setter.t +++ b/t/04-openresty/ffi/305-proxy_wasm_set_property_setter.t @@ -431,10 +431,10 @@ unknown error unknown error --- grep_error_log eval: qr/.*property.*/ --- grep_error_log_out eval -qr/^[^\n]*?\[error\] [^\n]*? error setting property: first wasmx property custom error[^\n]*? -[^\n]*?\[error\] [^\n]*? error setting property: second wasmx property custom error[^\n]*? -[^\n]*?\[error\] [^\n]*? error setting property: unknown error[^\n]*? -[^\n]*?\[error\] [^\n]*? error setting property: unknown error[^\n]*? +qr/^[^\n]*?\[error\] [^\n]*? error in property handler: first wasmx property custom error[^\n]*? +[^\n]*?\[error\] [^\n]*? error in property handler: second wasmx property custom error[^\n]*? +[^\n]*?\[error\] [^\n]*? error in property handler: unknown error[^\n]*? +[^\n]*?\[error\] [^\n]*? error in property handler: unknown error[^\n]*? [^\n]*?\[info\] [^\n]*? property not found: wasmx.fail1_property[^\n]*?$/ --- no_error_log [crit] @@ -610,7 +610,7 @@ ok --- error_log eval [ qr/\[info\] .*? \[hostcalls\] on_response_body/, - qr/\[error\] .*? error setting property from Lua: rewrite_by_lua\(nginx.conf:[0-9]+\):[0-9]+: crash\!/, + qr/\[error\] .*? Lua error in property handler: rewrite_by_lua\(nginx.conf:[0-9]+\):[0-9]+: crash\!/, ] --- no_error_log [emerg]