Skip to content

Commit 4d478c6

Browse files
hachi029hachi029
authored
fix: plugins repeats the rewrite phase when consumer configs with a plugin (#12048)
1 parent 6e17320 commit 4d478c6

File tree

2 files changed

+131
-30
lines changed

2 files changed

+131
-30
lines changed

‎apisix/plugin.lua‎

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -494,22 +494,35 @@ function _M.filter(ctx, conf, plugins, route_conf, phase)
494494
goto continue
495495
end
496496

497-
if not check_disable(plugin_conf) then
498-
if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then
499-
local plugin_conf_in_route = route_plugin_conf[name]
500-
local disable_in_route = check_disable(plugin_conf_in_route)
501-
if plugin_conf_in_route and not disable_in_route then
502-
goto continue
503-
end
504-
end
497+
if check_disable(plugin_conf) then
498+
goto continue
499+
end
505500

506-
if plugin_conf._meta and plugin_conf._meta.priority then
507-
custom_sort = true
501+
if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then
502+
local plugin_conf_in_route = route_plugin_conf[name]
503+
local disable_in_route = check_disable(plugin_conf_in_route)
504+
if plugin_conf_in_route and not disable_in_route then
505+
goto continue
508506
end
509-
core.table.insert(plugins, plugin_obj)
510-
core.table.insert(plugins, plugin_conf)
511507
end
512508

509+
-- in the rewrite phase, the plugin executes in the following order:
510+
-- 1. execute the rewrite phase of the plugins on route(including the auth plugins)
511+
-- 2. merge plugins from consumer and route
512+
-- 3. execute the rewrite phase of the plugins on consumer(phase: rewrite_in_consumer)
513+
-- in this case, we need to skip the plugins that was already executed(step 1)
514+
if phase == "rewrite_in_consumer"
515+
and (not plugin_conf._from_consumer or plugin_obj.type == "auth") then
516+
plugin_conf._skip_rewrite_in_consumer = true
517+
end
518+
519+
if plugin_conf._meta and plugin_conf._meta.priority then
520+
custom_sort = true
521+
end
522+
523+
core.table.insert(plugins, plugin_obj)
524+
core.table.insert(plugins, plugin_conf)
525+
513526
::continue::
514527
end
515528

@@ -523,15 +536,6 @@ function _M.filter(ctx, conf, plugins, route_conf, phase)
523536
local plugin_obj = plugins[i]
524537
local plugin_conf = plugins[i + 1]
525538

526-
-- in the rewrite phase, the plugin executes in the following order:
527-
-- 1. execute the rewrite phase of the plugins on route(including the auth plugins)
528-
-- 2. merge plugins from consumer and route
529-
-- 3. execute the rewrite phase of the plugins on consumer(phase: rewrite_in_consumer)
530-
-- in this case, we need to skip the plugins that was already executed(step 1)
531-
if phase == "rewrite_in_consumer" and not plugin_conf._from_consumer then
532-
plugin_conf._skip_rewrite_in_consumer = true
533-
end
534-
535539
tmp_plugin_objs[plugin_conf] = plugin_obj
536540
core.table.insert(tmp_plugin_confs, plugin_conf)
537541

@@ -1179,20 +1183,13 @@ function _M.run_plugin(phase, plugins, api_ctx)
11791183
and phase ~= "delayed_body_filter"
11801184
then
11811185
for i = 1, #plugins, 2 do
1182-
local phase_func
1183-
if phase == "rewrite_in_consumer" then
1184-
if plugins[i].type == "auth" then
1185-
plugins[i + 1]._skip_rewrite_in_consumer = true
1186-
end
1187-
phase_func = plugins[i]["rewrite"]
1188-
else
1189-
phase_func = plugins[i][phase]
1190-
end
11911186

11921187
if phase == "rewrite_in_consumer" and plugins[i + 1]._skip_rewrite_in_consumer then
11931188
goto CONTINUE
11941189
end
11951190

1191+
local phase_func = phase == "rewrite_in_consumer" and plugins[i]["rewrite"]
1192+
or plugins[i][phase]
11961193
if phase_func then
11971194
local conf = plugins[i + 1]
11981195
if not meta_filter(api_ctx, plugins[i]["name"], conf)then

‎t/node/plugin1.t‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one or more
3+
# contributor license agreements. See the NOTICE file distributed with
4+
# this work for additional information regarding copyright ownership.
5+
# The ASF licenses this file to You under the Apache License, Version 2.0
6+
# (the "License"); you may not use this file except in compliance with
7+
# the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
#
17+
use strict;
18+
use warnings FATAL => 'all';
19+
use t::APISIX 'no_plan';
20+
21+
no_long_string();
22+
no_root_location();
23+
no_shuffle();
24+
25+
add_block_preprocessor(sub {
26+
my ($block) = @_;
27+
28+
if (!$block->request) {
29+
$block->set_value("request", "GET /apisix/admin/routes");
30+
}
31+
32+
if (!$block->no_error_log && !$block->error_log) {
33+
$block->set_value("no_error_log", "[error]\n[alert]");
34+
}
35+
});
36+
run_tests;
37+
__DATA__
38+
39+
=== TEST 1: set up configuration
40+
--- config
41+
location /t {
42+
content_by_lua_block {
43+
local t = require("lib.test_admin").test
44+
local code, body = t('/apisix/admin/consumers/jack',
45+
ngx.HTTP_PUT,
46+
[[{
47+
"username": "jack",
48+
"plugins": {
49+
"key-auth": {
50+
"key": "auth-jack"
51+
}
52+
}
53+
}]]
54+
)
55+
if code >= 300 then
56+
ngx.say(body)
57+
return
58+
end
59+
local code, body = t('/apisix/admin/routes/1',
60+
ngx.HTTP_PUT,
61+
[[{
62+
"plugins": {
63+
"key-auth": {},
64+
"proxy-rewrite": {
65+
"headers": {
66+
"add": {
67+
"xtest": "123"
68+
}
69+
}
70+
},
71+
"serverless-post-function": {
72+
"functions": [
73+
"return function(conf, ctx) \n ngx.say(ngx.req.get_headers().xtest); \n end"
74+
]
75+
}
76+
},
77+
"upstream": {
78+
"nodes": {
79+
"127.0.0.1:1980": 1
80+
},
81+
"type": "roundrobin"
82+
},
83+
"uri": "/hello"
84+
}]]
85+
)
86+
ngx.say(body)
87+
}
88+
}
89+
--- request
90+
GET /t
91+
--- timeout: 15
92+
--- response_body
93+
passed
94+
95+
96+
97+
=== TEST 2: the proxy-rewrite runs at 'rewrite' phase and should get executed only once, hence the response body is expected '123' not '123123'
98+
--- request
99+
GET /hello
100+
--- more_headers
101+
apikey: auth-jack
102+
--- timeout: 15
103+
--- response_body
104+
123

0 commit comments

Comments
 (0)