Skip to content

Commit

Permalink
Revert "[wip] make TEST 10 pass by avoiding FFI returns. TEST 9 still…
Browse files Browse the repository at this point in the history
… fails..."

This reverts commit efe1c31.
  • Loading branch information
hishamhm committed Nov 7, 2023
1 parent f431dcf commit f71538a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 42 deletions.
28 changes: 8 additions & 20 deletions lib/resty/wasmx/proxy_wasm.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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);
]]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
28 changes: 11 additions & 17 deletions src/common/lua/ngx_wasm_lua_ffi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}


Expand Down Expand Up @@ -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";
Expand All @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions src/common/lua/ngx_wasm_lua_ffi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion t/04-openresty/ffi/304-proxy_wasm_set_property_getter.t
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit f71538a

Please sign in to comment.