Skip to content

Commit 6eee234

Browse files
committed
fix(core): stop shallow_prefix from matching sibling paths
Address review feedback: `has_prefix(parent, shallow_prefix)` also matched siblings such as "self.value.plugins2", and the comment on the deepcopy call in merge_consumer_route still described the old, wrong rationale. Signed-off-by: AlinsRan <alinsran@apache.org>
1 parent a16d026 commit 6eee234

3 files changed

Lines changed: 50 additions & 3 deletions

File tree

apisix/core/table.lua

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ local pairs = pairs
3131
local type = type
3232
local ngx_re = require("ngx.re")
3333
local isarray = require("table.isarray")
34+
local str_byte = string.byte
3435
local str_has_prefix = require("apisix.core.string").has_prefix
36+
local DOT = str_byte(".")
3537

3638

3739
local _M = {
@@ -121,6 +123,18 @@ end
121123

122124
local deepcopy
123125
do
126+
-- the members of `parent` are shallow-copied when `parent` is `prefix`
127+
-- itself or one of its descendants. The match has to stop at a path
128+
-- separator, otherwise a prefix like "self.value.plugins" would also
129+
-- claim a sibling named "self.value.plugins2".
130+
local function under_shallow_prefix(parent, prefix)
131+
if not str_has_prefix(parent, prefix) then
132+
return false
133+
end
134+
135+
return #parent == #prefix or str_byte(parent, #prefix + 1) == DOT
136+
end
137+
124138
local function _deepcopy(orig, copied, parent, opts)
125139
-- If the array-like table contains nil in the middle,
126140
-- the len might be smaller than the expected.
@@ -132,7 +146,8 @@ do
132146
for orig_key, orig_value in pairs(orig) do
133147
local path = parent .. "." .. tostring(orig_key)
134148
if opts and (array_find(opts.shallows, path) or
135-
(opts.shallow_prefix and str_has_prefix(parent, opts.shallow_prefix))) then
149+
(opts.shallow_prefix and
150+
under_shallow_prefix(parent, opts.shallow_prefix))) then
136151
copy[orig_key] = orig_value
137152
else
138153
if type(orig_value) == "table" then

apisix/plugin.lua

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -818,8 +818,12 @@ local function merge_consumer_route(route_conf, consumer_conf, consumer_group_co
818818
return route_conf
819819
end
820820

821-
-- the plugins subtree is fully overwritten below, so there is no need to
822-
-- deep-copy it; shallow-copy that subtree to avoid the wasted work
821+
-- some plugins cache request-time state on their conf object (resolved DNS
822+
-- nodes, probed backend versions, ...). Deep-copying the plugin confs would
823+
-- hand every consumer its own copy and drop that state, so keep them shared
824+
-- by reference, as they already are on the route without consumer auth. The
825+
-- `plugins` container itself is still a fresh table, so the merge below
826+
-- overwrites its keys without touching the original route conf.
823827
local new_route_conf = core.table.deepcopy(route_conf,
824828
{ shallow_prefix = "self.value.plugins" })
825829

t/core/table.t

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,3 +393,31 @@ table copied: {"plugins":{"p1":{"name":"plugin1_new_modified"},"p2":{"name":"plu
393393
table original: {"plugins":{"p1":{"name":"plugin1"},"p2":{"name":"plugin2_modified"}}}
394394
--- no_error_log
395395
[error]
396+
397+
398+
399+
=== TEST 14: shallow_prefix only matches at a path separator
400+
--- config
401+
location /t {
402+
content_by_lua_block {
403+
local core = require("apisix.core")
404+
local deepcopy = core.table.deepcopy
405+
local tab = {
406+
plugins = { p1 = {name = "p1"} },
407+
-- a sibling whose name starts with the prefix
408+
plugins2 = { p2 = {name = "p2"} },
409+
}
410+
411+
local tab_copied = deepcopy(tab, { shallow_prefix = "self.plugins" })
412+
413+
ngx.say("plugins.p1 shared: ", tab_copied.plugins.p1 == tab.plugins.p1)
414+
ngx.say("plugins2.p2 shared: ", tab_copied.plugins2.p2 == tab.plugins2.p2)
415+
}
416+
}
417+
--- request
418+
GET /t
419+
--- response_body
420+
plugins.p1 shared: true
421+
plugins2.p2 shared: false
422+
--- no_error_log
423+
[error]

0 commit comments

Comments
 (0)