Copilot commented on code in PR #13886:
URL: https://github.com/apache/apisix/pull/13886#discussion_r3891237893
##########
apisix/admin/init.lua:
##########
@@ -554,12 +563,16 @@ function _M.init_worker()
events = require("apisix.events")
events:register(reload_plugins, reload_event, "PUT")
- if plugins_conf_ver_dict and not is_yaml_config_provider then
+ if plugins_conf_ver_dict then
Review Comment:
A delivered control-API reload now reloads plugins twice in standalone mode.
`control/router.lua:202-210` handles `control-api-plugin-reload` by calling
`plugin.load()`, but it cannot update this module's
`applied_plugins_conf_version`; this timer still observes the incremented
shared version and calls `reload_plugins()` again. That repeats every plugin's
destroy/init lifecycle and timers. Share the applied-version acknowledgement
with the control event handler, or consolidate both reload paths.
##########
apisix/cli/file.lua:
##########
@@ -266,7 +266,9 @@ function _M.read_yaml_conf(apisix_home)
if not is_empty_file then
local user_conf = yaml.load(user_conf_yaml)
- if not user_conf then
+ -- lyaml returns a scalar for a document such as `foo`, which would
blow
+ -- up in resolve_conf_var's pairs() below
+ if type(user_conf) ~= "table" then
Review Comment:
The new scalar-document guard is not exercised by the added CLI tests; they
only cover nested YAML-null deployment sections. Add a `config.yaml` containing
a scalar such as `foo` and assert that `make init` reports `invalid config.yaml
file` without a Lua traceback.
##########
apisix/admin/config_validate.lua:
##########
@@ -125,6 +146,15 @@ function _M.validate_configuration(req_body,
collect_all_errors)
local is_valid = true
local validation_results = {}
+ local shape_ok, shape_err = core.schema.check(config_schema, req_body)
Review Comment:
`lyaml` represents a YAML `null`/`~` document as the table sentinel
`yaml.null`. The generic object schema can therefore accept it as an empty
object, after which the standalone updater advances every resource version with
no items and clears the current configuration. Reject this sentinel before
schema validation, and add a YAML-null regression case for both endpoints.
##########
apisix/admin/standalone.lua:
##########
@@ -100,15 +103,19 @@ local function update(ctx)
-- parse the request body
local data
if core.string.has_prefix(content_type, "application/yaml") then
- data = yaml.load(req_body, { all = false })
- if not data or type(data) ~= "table" then
+ -- yaml.load raises on a malformed document, it does not return an
error
+ local ok, result = pcall(yaml.load, req_body, { all = false })
+ if not ok or type(result) ~= "table" then
Review Comment:
The standalone tests only send malformed JSON, so the newly protected
`yaml.load` exception path remains untested. Add an `application/yaml` request
with malformed YAML and assert a 400 response rather than a worker error.
--
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]
