diff --git a/lib/resty/wasmx/proxy_wasm.lua b/lib/resty/wasmx/proxy_wasm.lua index 2cd60ad3d..662d793bb 100644 --- a/lib/resty/wasmx/proxy_wasm.lua +++ b/lib/resty/wasmx/proxy_wasm.lua @@ -88,9 +88,15 @@ do -- so we prepare these wrappers, which in turn call a Lua function. local function store_c_value(r, ckey, cvalue, lvalue, is_const) - local value = tostring(lvalue) - cvalue.data = value - cvalue.len = #value + if lvalue ~= nil then + local value = tostring(lvalue) + cvalue.data = value + cvalue.len = #value + + else + cvalue.data = nil + cvalue.len = 0 + end return C.ngx_http_wasm_ffi_set_host_property(r, ckey, cvalue, is_const and 1 or 0, @@ -100,7 +106,10 @@ do c_prop_setter = ffi.cast("ngx_wasm_host_prop_fn_t", function(r, ckey, cvalue) local lkey = ffi_str(ckey.data, ckey.len) - local lval = ffi_str(cvalue.data, cvalue.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) @@ -115,10 +124,6 @@ do return FFI_ERROR end - if lvalue == nil then - return FFI_OK - end - return store_c_value(r, ckey, cvalue, lvalue, is_const) end) @@ -143,7 +148,12 @@ do return FFI_DECLINED end - return store_c_value(r, ckey, cvalue, lvalue, is_const) + local rc = store_c_value(r, ckey, cvalue, lvalue, is_const) + if rc == FFI_OK and lvalue == nil then + return FFI_DECLINED + end + + return rc end) end @@ -310,7 +320,7 @@ function _M.set_property(key, value) error("key must be a string", 2) end - if type(value) ~= "string" then + if value ~= nil and type(value) ~= "string" then error("value must be a string", 2) end @@ -320,7 +330,7 @@ function _M.set_property(key, value) end local ckey = ffi_new("ngx_str_t", { data = key, len = #key }) - local cvalue = ffi_new("ngx_str_t", { data = value, len = #value }) + local cvalue = ffi_new("ngx_str_t", { data = value, len = value and #value or 0 }) local rc = C.ngx_http_wasm_ffi_set_property(r, ckey, cvalue) if rc == FFI_ERROR then diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_properties.c b/src/common/proxy_wasm/ngx_proxy_wasm_properties.c index bd7610643..2fb9ad50e 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm_properties.c +++ b/src/common/proxy_wasm/ngx_proxy_wasm_properties.c @@ -54,6 +54,7 @@ typedef struct { ngx_str_node_t sn; ngx_str_t value; unsigned is_const:1; + unsigned negative_cache:1; } host_props_node_t; @@ -860,7 +861,7 @@ ngx_proxy_wasm_properties_set_ngx(ngx_proxy_wasm_ctx_t *pwctx, static ngx_int_t ngx_proxy_wasm_properties_get_host(ngx_proxy_wasm_ctx_t *pwctx, - ngx_str_t *path, ngx_str_t *value) + ngx_str_t *path, ngx_str_t *value, unsigned *negative_cache) { host_props_node_t *hpn; uint32_t hash; @@ -883,6 +884,10 @@ ngx_proxy_wasm_properties_get_host(ngx_proxy_wasm_ctx_t *pwctx, value->data = hpn->value.data; value->len = hpn->value.len; + if (hpn->negative_cache) { + *negative_cache = 1; + } + return NGX_OK; } @@ -896,6 +901,7 @@ ngx_proxy_wasm_properties_set_host(ngx_proxy_wasm_ctx_t *pwctx, { host_props_node_t *hpn; uint32_t hash; + unsigned char *new_data; unsigned new_entry = 1; #ifdef NGX_WASM_HTTP ngx_http_wasm_req_ctx_t *rctx = pwctx->data; @@ -912,24 +918,19 @@ ngx_proxy_wasm_properties_set_host(ngx_proxy_wasm_ctx_t *pwctx, hpn = (host_props_node_t *) ngx_str_rbtree_lookup(&pwctx->host_props_tree, path, hash); - if (value->data == NULL) { - if (hpn) { - if (hpn->is_const) { - return NGX_DECLINED; - } - - ngx_rbtree_delete(&pwctx->host_props_tree, &hpn->sn.node); - } - - return NGX_OK; - } - if (hpn) { if (hpn->is_const) { return NGX_DECLINED; } ngx_pfree(pwctx->pool, hpn->value.data); + + if (value->data == NULL && !is_const) { + ngx_rbtree_delete(&pwctx->host_props_tree, &hpn->sn.node); + + return NGX_OK; + } + new_entry = 0; } else { @@ -944,12 +945,20 @@ ngx_proxy_wasm_properties_set_host(ngx_proxy_wasm_ctx_t *pwctx, ngx_memcpy(hpn->sn.str.data, path->data, path->len); } + if (value->data == NULL) { + new_data = NULL; + hpn->negative_cache = 1; + + } else { + new_data = ngx_pstrdup(pwctx->pool, value); + if (new_data == NULL) { + return NGX_ERROR; + } + } + hpn->is_const = is_const; hpn->value.len = value->len; - hpn->value.data = ngx_pstrdup(pwctx->pool, value); - if (hpn->value.data == NULL) { - return NGX_ERROR; - } + hpn->value.data = new_data; if (new_entry) { ngx_rbtree_insert(&pwctx->host_props_tree, &hpn->sn.node); @@ -985,6 +994,7 @@ ngx_proxy_wasm_properties_get(ngx_proxy_wasm_ctx_t *pwctx, ngx_uint_t key; pwm2ngx_mapping_t *m; ngx_int_t rc; + unsigned negative_cache = 0; p.data = replace_nulls_by_dots(path, dotted_path_buf); key = ngx_hash_key(p.data, p.len); @@ -1014,8 +1024,13 @@ ngx_proxy_wasm_properties_get(ngx_proxy_wasm_ctx_t *pwctx, /* host variable */ /* even if there is a getter, try reading const value first */ - rc = ngx_proxy_wasm_properties_get_host(pwctx, &p, value); + rc = ngx_proxy_wasm_properties_get_host(pwctx, &p, value, + &negative_cache); if (rc == NGX_OK) { + if (negative_cache) { + return NGX_DECLINED; + } + return NGX_OK; } 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 02507acac..e6c8f764f 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 @@ -474,6 +474,18 @@ ok elseif key == "wasmx.const_property" then return true, tostring(counter), true + + elseif key == "wasmx.nil_dyn_property" then + return true, nil + + elseif key == "wasmx.nil_const_property" then + return true, nil, true + + elseif key == "wasmx.empty_dyn_property" then + return true, "" + + elseif key == "wasmx.empty_const_property" then + return true, "", true end end @@ -488,7 +500,7 @@ ok local function log_property(k) local v = proxy_wasm.get_property(k) - ngx.log(ngx.INFO, k, ": ", v) + ngx.log(ngx.INFO, k, ": ", type(v), " \"", v, "\"") end -- four get_property operations, @@ -499,6 +511,16 @@ ok log_property("wasmx.dyn_property") -- returns 2 log_property("wasmx.const_property") -- returns 1 log_property("wasmx.dyn_property") -- returns 3 + + log_property("wasmx.nil_const_property") -- returns nil + log_property("wasmx.nil_dyn_property") -- returns nil + log_property("wasmx.nil_const_property") -- returns nil + log_property("wasmx.nil_dyn_property") -- returns nil + + log_property("wasmx.empty_const_property") -- returns "" + log_property("wasmx.empty_dyn_property") -- returns "" + log_property("wasmx.empty_const_property") -- returns "" + log_property("wasmx.empty_dyn_property") -- returns "" } } --- response_body @@ -506,12 +528,26 @@ ok --- grep_error_log eval: qr/.*wasmx.*property.*/ --- grep_error_log_out eval qr/^[^\n]*?\[info\] [^\n]*? getting wasmx.const_property, counter: 1[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.const_property: 1[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.const_property: string "1"[^\n]*? [^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property, counter: 2[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: 2[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.const_property: 1[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: string "2"[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.const_property: string "1"[^\n]*? [^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property, counter: 3[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: 3[^\n]*?$/ +[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: string "3"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.nil_const_property, counter: 4[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_const_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.nil_dyn_property, counter: 5[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_dyn_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_const_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.nil_dyn_property, counter: 6[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_dyn_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.empty_const_property, counter: 7[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_const_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.empty_dyn_property, counter: 8[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_dyn_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_const_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.empty_dyn_property, counter: 9[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_dyn_property: string ""[^\n]*?$/ --- no_error_log [error] [crit] 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 ab93da50c..471385d3d 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 @@ -467,17 +467,15 @@ qr/^[^\n]*?\[error\] [^\n]*? error setting property: first wasmx property custom local function setter(key, value) counter = counter + 1 - local new_value = value:upper() .. " " .. tostring(counter) - tbl[key] = new_value + if value and #value > 0 then + value = value:upper() .. " " .. tostring(counter) + end - ngx.log(ngx.INFO, "setting ", key, " to ", new_value) + tbl[key] = value - if key == "wasmx.dyn_property" then - return true, new_value + ngx.log(ngx.INFO, "setting ", key, " to ", value, ", counter: " .. counter) - elseif key == "wasmx.const_property" then - return true, new_value, true - end + return true, value, (key:match("const") and true or false) end assert(proxy_wasm.set_property_setter(setter)) @@ -485,11 +483,15 @@ qr/^[^\n]*?\[error\] [^\n]*? error setting property: first wasmx property custom local function getter(key) counter = counter + 1 - ngx.log(ngx.INFO, "getting ", key, " via getter") + ngx.log(ngx.INFO, "getting ", key, " via getter, counter: ", counter) + local value = tbl[key] - if value then - return true, value .. " " .. tostring(counter) + + if value and #value > 0 then + value = value .. " " .. tostring(counter) end + + return true, value end assert(proxy_wasm.set_property_getter(getter)) @@ -503,7 +505,7 @@ qr/^[^\n]*?\[error\] [^\n]*? error setting property: first wasmx property custom local function log_property(k) local v = proxy_wasm.get_property(k) - ngx.log(ngx.INFO, k, ": ", v) + ngx.log(ngx.INFO, k, ": ", type(v), " \"", v, "\"") end -- two set_property operations, each will hit the setter. @@ -516,20 +518,50 @@ qr/^[^\n]*?\[error\] [^\n]*? error setting property: first wasmx property custom log_property("wasmx.dyn_property") -- returns "HI 2 3" log_property("wasmx.const_property") -- returns "HELLO 1" log_property("wasmx.dyn_property") -- returns "HI 2 4" + + proxy_wasm.set_property("wasmx.nil_const_property", nil) -- sets nil + log_property("wasmx.nil_const_property") -- returns nil + proxy_wasm.set_property("wasmx.nil_dyn_property", nil) -- sets nil + log_property("wasmx.nil_dyn_property") -- returns nil + log_property("wasmx.nil_const_property") -- returns nil + log_property("wasmx.nil_dyn_property") -- returns nil + + proxy_wasm.set_property("wasmx.empty_const_property", "") -- sets "" + log_property("wasmx.empty_const_property") -- returns "" + proxy_wasm.set_property("wasmx.empty_dyn_property", "") -- sets "" + log_property("wasmx.empty_dyn_property") -- returns "" + log_property("wasmx.empty_const_property") -- returns "" + log_property("wasmx.empty_dyn_property") -- returns "" } } --- response_body ok --- grep_error_log eval: qr/.*wasmx.*property.*/ --- grep_error_log_out eval -qr/^[^\n]*?\[info\] [^\n]*? setting wasmx.const_property to HELLO 1[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.const_property: HELLO 1[^\n]*? -[^\n]*?\[info\] [^\n]*? setting wasmx.dyn_property to HI 2[^\n]*? -[^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property via getter[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: HI 2 3[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.const_property: HELLO 1[^\n]*? -[^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property via getter[^\n]*? -[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: HI 2 4[^\n]*?$/ +qr/^[^\n]*?\[info\] [^\n]*? setting wasmx.const_property to HELLO 1, counter: 1[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.const_property: string "HELLO 1"[^\n]*? +[^\n]*?\[info\] [^\n]*? setting wasmx.dyn_property to HI 2, counter: 2[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property via getter, counter: 3[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: string "HI 2 3"[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.const_property: string "HELLO 1"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.dyn_property via getter, counter: 4[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.dyn_property: string "HI 2 4"[^\n]*? +[^\n]*?\[info\] [^\n]*? setting wasmx.nil_const_property to nil, counter: 5[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_const_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? setting wasmx.nil_dyn_property to nil, counter: 6[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.nil_dyn_property via getter, counter: 7[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_dyn_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_const_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.nil_dyn_property via getter, counter: 8[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.nil_dyn_property: nil "nil"[^\n]*? +[^\n]*?\[info\] [^\n]*? setting wasmx.empty_const_property to , counter: 9[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_const_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? setting wasmx.empty_dyn_property to , counter: 10[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.empty_dyn_property via getter, counter: 11[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_dyn_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_const_property: string ""[^\n]*? +[^\n]*?\[info\] [^\n]*? getting wasmx.empty_dyn_property via getter, counter: 12[^\n]*? +[^\n]*?\[info\] [^\n]*? wasmx.empty_dyn_property: string ""[^\n]*?$/ --- no_error_log [error] [crit]