diff --git a/changelog/unreleased/kong/fix-admin-api-for-empty-tags.yml b/changelog/unreleased/kong/fix-admin-api-for-empty-tags.yml new file mode 100644 index 000000000000..9d96d0738467 --- /dev/null +++ b/changelog/unreleased/kong/fix-admin-api-for-empty-tags.yml @@ -0,0 +1,3 @@ +message: Fix for querying admin API entities with empty tags +type: bugfix +scope: Admin API diff --git a/kong/api/endpoints.lua b/kong/api/endpoints.lua index 3129fe6ddbe7..1a0b41bbd3ca 100644 --- a/kong/api/endpoints.lua +++ b/kong/api/endpoints.lua @@ -137,7 +137,7 @@ local function handle_error(err_t) end -local function extract_options(args, schema, context) +local function extract_options(db, args, schema, context) local options = { nulls = true, pagination = { @@ -156,6 +156,11 @@ local function extract_options(args, schema, context) end if schema.fields.tags and args.tags ~= nil and context == "page" then + if args.tags == null or #args.tags == 0 then + local error_message = "cannot be null" + return nil, error_message, db[schema.name].errors:invalid_options({tags = error_message}) + end + local tags = args.tags if type(tags) == "table" then tags = tags[1] @@ -207,7 +212,11 @@ local function query_entity(context, self, db, schema, method) args = self.args.uri end - local opts = extract_options(args, schema, context) + local opts, err, err_t = extract_options(db, args, schema, context) + if err then + return nil, err, err_t + end + local schema_name = schema.name local dao = db[schema_name] diff --git a/kong/api/routes/consumers.lua b/kong/api/routes/consumers.lua index 470281f60989..fc1c595e22f5 100644 --- a/kong/api/routes/consumers.lua +++ b/kong/api/routes/consumers.lua @@ -20,7 +20,11 @@ return { -- Search by custom_id: /consumers?custom_id=xxx if custom_id then - local opts = endpoints.extract_options(args, db.consumers.schema, "select") + local opts, _, err_t = endpoints.extract_options(db, args, db.consumers.schema, "select") + if err_t then + return endpoints.handle_error(err_t) + end + local consumer, _, err_t = db.consumers:select_by_custom_id(custom_id, opts) if err_t then return endpoints.handle_error(err_t) diff --git a/kong/api/routes/upstreams.lua b/kong/api/routes/upstreams.lua index 614bef1721dc..df97aa230fe4 100644 --- a/kong/api/routes/upstreams.lua +++ b/kong/api/routes/upstreams.lua @@ -25,7 +25,12 @@ local function set_target_health(self, db, is_healthy) target, _, err_t = endpoints.select_entity(self, db, db.targets.schema) else - local opts = endpoints.extract_options(self.args.uri, db.targets.schema, "select") + local opts + opts, _, err_t = endpoints.extract_options(db, self.args.uri, db.targets.schema, "select") + if err_t then + return endpoints.handle_error(err_t) + end + local upstream_pk = db.upstreams.schema:extract_pk_values(upstream) local filter = { target = unescape_uri(self.params.targets) } target, _, err_t = db.targets:select_by_upstream_filter(upstream_pk, filter, opts) @@ -94,7 +99,11 @@ local function target_endpoint(self, db, callback) target, _, err_t = endpoints.select_entity(self, db, db.targets.schema) else - local opts = endpoints.extract_options(self.args.uri, db.targets.schema, "select") + local opts + opts, _, err_t = endpoints.extract_options(db, self.args.uri, db.targets.schema, "select") + if err_t then + return endpoints.handle_error(err_t) + end local upstream_pk = db.upstreams.schema:extract_pk_values(upstream) local filter = { target = unescape_uri(self.params.targets) } target, _, err_t = db.targets:select_by_upstream_filter(upstream_pk, filter, opts) diff --git a/spec/02-integration/04-admin_api/03-consumers_routes_spec.lua b/spec/02-integration/04-admin_api/03-consumers_routes_spec.lua index 06fbb8968899..fc7c87b48e05 100644 --- a/spec/02-integration/04-admin_api/03-consumers_routes_spec.lua +++ b/spec/02-integration/04-admin_api/03-consumers_routes_spec.lua @@ -282,6 +282,16 @@ describe("Admin API (#" .. strategy .. "): ", function() local body = assert.response(res).has.status(200) assert.match('"data":%[%]', body) end) + it("returns bad request for empty tags", function() + local res = assert(client:send { + method = "GET", + path = "/consumers", + query = { tags = ngx.null} + }) + res = assert.res_status(400, res) + local json = cjson.decode(res) + assert.same("invalid option (tags: cannot be null)", json.message) + end) end) it("returns 405 on invalid method", function() local methods = {"DELETE", "PATCH"} diff --git a/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua b/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua index 69f7bb52ea74..8012e5d4d849 100644 --- a/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua +++ b/spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua @@ -637,6 +637,16 @@ describe("Admin API: #" .. strategy, function() }) assert.res_status(200, res) end) + it("returns bad request for empty tags", function() + local res = assert(client:send { + method = "GET", + path = "/upstreams", + query = { tags = ngx.null} + }) + res = assert.res_status(400, res) + local json = cjson.decode(res) + assert.same("invalid option (tags: cannot be null)", json.message) + end) describe("empty results", function() lazy_setup(function() diff --git a/spec/02-integration/04-admin_api/14-tags_spec.lua b/spec/02-integration/04-admin_api/14-tags_spec.lua index 37586104683d..99bb91d9adca 100644 --- a/spec/02-integration/04-admin_api/14-tags_spec.lua +++ b/spec/02-integration/04-admin_api/14-tags_spec.lua @@ -73,6 +73,26 @@ describe("Admin API - tags", function() end end) + it("filter by empty tag", function() + local res = assert(client:send { + method = "GET", + path = "/consumers?tags=" + }) + local body = assert.res_status(400, res) + local json = cjson.decode(body) + assert.same("invalid option (tags: cannot be null)", json.message) + end) + + it("filter by empty string tag", function() + local res = assert(client:send { + method = "GET", + path = "/consumers?tags=''" + }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equals(0, #json.data) + end) + it("filter by multiple tags with AND", function() local res = assert(client:send { method = "GET",