From 2a8d403136a96a568684d595d2836b4b5c99415b Mon Sep 17 00:00:00 2001
From: Hisham Muhammad <hisham@gobolinux.org>
Date: Mon, 2 Oct 2023 18:53:47 -0300
Subject: [PATCH] fix(proxy-wasm) do not accidentally reset filter list from
 FFI call

When calling `proxy_wasm.set_property` from Lua, we want a `pwctx`
to be able to store a property. But we need to do that before
`proxy_wasm.start`, so `pwctx->ready` didn't have a chance to get
initialized yet.

Passing `NULL, 0` to `filter_ids` and `nfilters` when obtaining the `pwctx`
from the `set_property` FFI call caused the `pwctx` to get initialized with an
empty filter list, defeating the configuration previously set up by
`proxy_wasm.attach`.

In this fix, we make the `NULL` value of `filter_ids` to mean
"please don't do the `pwctx->ready` initialization". The other
use of `ngx_proxy_wasm_ctx` in `ngx_wasm_ops.c` always passes a
non-NULL value (even when the filter list is empty), because the
`elts` pointer is allocated on `ngx_array_init`.

This issue caused an integration failure in Kong Gateway. This PR
includes a regression test for our test suite, which reproduces the
sequence observed in the Gateway; the test fails without the fix
added to `ngx_proxy_wasm.c` and passes with the fix.
---
 src/common/proxy_wasm/ngx_proxy_wasm.c        |  6 ++-
 .../ffi/302-proxy_wasm_set_property_host.t    | 43 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/common/proxy_wasm/ngx_proxy_wasm.c b/src/common/proxy_wasm/ngx_proxy_wasm.c
index 1f60eb9a4..c5394daff 100644
--- a/src/common/proxy_wasm/ngx_proxy_wasm.c
+++ b/src/common/proxy_wasm/ngx_proxy_wasm.c
@@ -275,7 +275,7 @@ ngx_proxy_wasm_ctx(ngx_uint_t *filter_ids, size_t nfilters,
         return NULL;
     }
 
-    if (!pwctx->ready) {
+    if (!pwctx->ready && filter_ids) {
         pwctx->nfilters = nfilters;
         pwctx->isolation = isolation;
 
@@ -399,7 +399,9 @@ ngx_proxy_wasm_ctx_destroy(ngx_proxy_wasm_ctx_t *pwctx)
         ngx_proxy_wasm_release_instance(ictx, 0);
     }
 
-    ngx_proxy_wasm_store_destroy(&pwctx->store);
+    if (pwctx->ready) {
+        ngx_proxy_wasm_store_destroy(&pwctx->store);
+    }
 
 #if 0
     if (pwctx->authority.data) {
diff --git a/t/04-openresty/ffi/302-proxy_wasm_set_property_host.t b/t/04-openresty/ffi/302-proxy_wasm_set_property_host.t
index fe2ff8fb2..bb729b398 100644
--- a/t/04-openresty/ffi/302-proxy_wasm_set_property_host.t
+++ b/t/04-openresty/ffi/302-proxy_wasm_set_property_host.t
@@ -262,3 +262,46 @@ ok
 ]
 --- no_error_log
 [error]
+
+
+
+=== TEST 7: set_property() - regression test: setting properties at startup doesn't reset filter list
+--- wasm_modules: hostcalls
+--- http_config
+    init_worker_by_lua_block {
+        local proxy_wasm = require "resty.wasmx.proxy_wasm"
+        local filters = {
+            { name = "hostcalls", config = "on=log " ..
+                                           "test=/t/log/property " ..
+                                           "name=wasmx.startup_property" },
+        }
+
+        _G.c_plan = assert(proxy_wasm.new(filters))
+
+        assert(proxy_wasm.load(_G.c_plan))
+    }
+--- config
+    location /t {
+        proxy_wasm_request_headers_in_access on;
+
+        access_by_lua_block {
+            local proxy_wasm = require "resty.wasmx.proxy_wasm"
+
+            assert(proxy_wasm.attach(_G.c_plan))
+
+            assert(proxy_wasm.set_property("wasmx.startup_property", "foo"))
+
+            assert(proxy_wasm.start())
+
+            ngx.say("ok")
+        }
+    }
+--- response_body
+ok
+--- error_log eval
+[
+    qr/\[info\] .*? \[hostcalls\] on_log/,
+    qr/\[info\] .*?hostcalls.*? wasmx.startup_property: foo/,
+]
+--- no_error_log
+[error]