fix(json): make cjson instances inherit APISIX's cjson options

apisix/core/json.lua

@@ -38,8 +38,6 @@ local rapidjson_null
 local rapidjson_encode_opts = { sort_keys = true }
 
 
-cjson.encode_escape_forward_slash(false)
-cjson.decode_array_with_array_mt(true)
 local _M = {
     version = 0.1,
     array_mt = cjson.array_mt,

apisix/patch.lua

@@ -364,7 +364,40 @@ local function luasocket_tcp()
 end
 
 
+-- lua-cjson options are per-instance, and cjson.new() always starts from the
+-- compile-time defaults instead of inheriting the singleton's configuration.
+-- core/json.lua only enables decode_array_with_array_mt on the `cjson.safe`
+-- singleton, so every dependency holding a private instance (lua-resty-session,
+-- lua-resty-healthcheck, lua-resty-worker-events, lua-resty-aws, ...) decodes
+-- arrays without the array metatable, and an empty array is then re-encoded as
+-- an object. Enable the option on both singletons and let every instance created
+-- afterwards inherit it. This only changes the default: a library explicitly
+-- calling decode_array_with_array_mt(false) on its own instance still wins.
+--
+-- encode_escape_forward_slash is set here as well. It looks redundant today,
+-- because in the bundled lua-cjson that setter mutates a shared escape table and
+-- therefore already applies process-wide, including to instances created before
+-- it was called. Upstream has since made it per-instance, so once a runtime
+-- upgrade picks that up, every private instance would silently go back to
+-- escaping `/`. Setting it here pins the current behaviour instead of relying on
+-- that implementation detail.
+local function patch_cjson(cjson)
+    cjson.decode_array_with_array_mt(true)
+    cjson.encode_escape_forward_slash(false)
+
+    local original_new = cjson.new
+    cjson.new = function (...)
+        local instance = original_new(...)
+        patch_cjson(instance)
+        return instance
+    end
+end
+
+
 function _M.patch()
+    patch_cjson(require("cjson"))
+    patch_cjson(require("cjson.safe"))
+
     -- make linter happy
     -- luacheck: ignore
     ngx_socket.tcp = function ()

t/core/json.t

@@ -193,3 +193,51 @@ b.d: 1
 e: text
 a or default: default
 top-level null: nil
+
+
+
+=== TEST 9: cjson instances created by dependencies keep empty arrays as arrays
+--- config
+    location /t {
+        content_by_lua_block {
+            -- a dependency that keeps a private instance, e.g. lua-resty-session
+            local safe_instance = require("cjson.safe").new()
+            ngx.say("cjson.safe instance: ",
+                    safe_instance.encode(safe_instance.decode('{"arr":[]}')))
+
+            local instance = require("cjson").new()
+            ngx.say("cjson instance: ", instance.encode(instance.decode('{"arr":[]}')))
+
+            -- the plain singleton is a separate config from cjson.safe
+            local cjson = require("cjson")
+            ngx.say("cjson singleton: ", cjson.encode(cjson.decode('{"arr":[]}')))
+        }
+    }
+--- response_body
+cjson.safe instance: {"arr":[]}
+cjson instance: {"arr":[]}
+cjson singleton: {"arr":[]}
+
+
+
+=== TEST 10: cjson instances created by dependencies do not escape forward slashes
+--- config
+    location /t {
+        content_by_lua_block {
+            -- today the bundled lua-cjson keeps a shared escape table, so this
+            -- already holds; the patch pins it for when the option becomes
+            -- per-instance upstream
+            local safe_instance = require("cjson.safe").new()
+            ngx.say("cjson.safe instance: ", safe_instance.encode({uri = "/a/b"}))
+
+            local instance = require("cjson").new()
+            ngx.say("cjson instance: ", instance.encode({uri = "/a/b"}))
+
+            local cjson = require("cjson")
+            ngx.say("cjson singleton: ", cjson.encode({uri = "/a/b"}))
+        }
+    }
+--- response_body
+cjson.safe instance: {"uri":"/a/b"}
+cjson instance: {"uri":"/a/b"}
+cjson singleton: {"uri":"/a/b"}

t/plugin/openid-connect-userinfo-array.t

@@ -0,0 +1,140 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+        $block->set_value("no_error_log", "[error]");
+    }
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set up a route guarded by openid-connect with set_userinfo_header
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "openid-connect": {
+                                "client_id": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH",
+                                "client_secret": "60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa",
+                                "discovery": "http://127.0.0.1:1980/.well-known/openid-configuration",
+                                "redirect_uri": "http://127.0.0.1:1984/callback",
+                                "ssl_verify": false,
+                                "use_pkce": false,
+                                "set_userinfo_header": true,
+                                "renew_access_token_on_expiry": false,
+                                "session": {
+                                    "secret": "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK"
+                                }
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1980": 1
+                            },
+                            "type": "roundrobin"
+                        },
+                        "uri": "/uri"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: an empty userinfo claim reaches the upstream as an array
+--- config
+    location /t {
+        content_by_lua_block {
+            local http = require("resty.http")
+            local session = require("resty.session")
+            local core = require("apisix.core")
+
+            -- Forge the session openid-connect writes after a successful login,
+            -- so the plugin takes the "already authenticated" path and restores
+            -- the userinfo from the session, exactly as it does on every request
+            -- after the callback. No IdP is contacted: with a non-expired access
+            -- token and renew_access_token_on_expiry disabled, openidc never
+            -- reaches for the discovery document.
+            local s = session.new({ secret = "jwcE5v3pM9VhqLxmxFOH9uZaLo8u7KQK" })
+            s:set("authenticated", true)
+            s:set("last_authenticated", ngx.time())
+            s:set("id_token", { sub = "a UID" })
+            s:set("access_token", "fake-access-token")
+            s:set("access_token_expiration", ngx.time() + 3600)
+            s:set("user", core.json.decode(
+                '{"sub":"a UID","name":"Testuser One","roles":[]}'))
+
+            local ok, err = s:save()
+            if not ok then
+                ngx.say("failed to save session: ", err)
+                return
+            end
+
+            local cookie = ngx.header["Set-Cookie"]
+            if type(cookie) == "table" then
+                cookie = cookie[1]
+            end
+            ngx.header["Set-Cookie"] = nil
+
+            local httpc = http.new()
+            local res, err = httpc:request_uri("http://127.0.0.1:1984/uri", {
+                headers = { Cookie = cookie },
+            })
+            if not res then
+                ngx.say("request failed: ", err)
+                return
+            end
+
+            -- the upstream echoes back the request headers it received
+            local encoded = res.body:match("x%-userinfo: ([%w+/=]+)")
+            if not encoded then
+                ngx.say("no X-Userinfo reached the upstream")
+                return
+            end
+
+            local userinfo = ngx.decode_base64(encoded)
+            ngx.say(core.json.encode(core.json.decode(userinfo).roles))
+        }
+    }
+--- response_body
+[]