Copilot commented on code in PR #13617:
URL: https://github.com/apache/apisix/pull/13617#discussion_r3489239228
##########
apisix/plugin.lua:
##########
@@ -1329,6 +1339,9 @@ function _M.run_plugin(phase, plugins, api_ctx)
run_meta_pre_function(conf, api_ctx, plugins[i]["name"])
plugin_run = true
api_ctx._plugin_name = plugins[i]["name"]
+ if api_ctx._skip_plugins and
api_ctx._skip_plugins[api_ctx._plugin_name] then
+ goto CONTINUE
+ end
Review Comment:
The skip logic uses `goto CONTINUE` after setting `api_ctx._plugin_name` and
after running the plugin meta `pre_function`, but it never clears
`_plugin_name` and still sets `plugin_run = true`. This can leak the previous
plugin name into subsequent iterations/phases and can unexpectedly execute meta
pre-functions for a plugin that is intentionally skipped.
##########
apisix/plugin.lua:
##########
@@ -1367,12 +1380,17 @@ function _M.run_plugin(phase, plugins, api_ctx)
plugin_run = true
run_meta_pre_function(conf, api_ctx, plugins[i]["name"])
api_ctx._plugin_name = plugins[i]["name"]
+ if api_ctx._skip_plugins and
api_ctx._skip_plugins[api_ctx._plugin_name] then
+ goto CONTINUE
+ end
Review Comment:
Same issue in the log/header/body filter plugin loop:
`run_meta_pre_function` and `api_ctx._plugin_name` are set before checking
`_skip_plugins`, and a `goto CONTINUE` skips cleanup (`api_ctx._plugin_name =
nil`) and still marks `plugin_run = true`. Skipping should happen before meta
hooks and `_plugin_name` assignment to avoid leaking state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
