diff --git a/kong/api/endpoints.lua b/kong/api/endpoints.lua index ce9096a37c5e..f975f2f07de6 100644 --- a/kong/api/endpoints.lua +++ b/kong/api/endpoints.lua @@ -40,9 +40,9 @@ local ERRORS_HTTP_CODES = { local TAGS_AND_REGEX local TAGS_OR_REGEX do - -- printable ASCII (0x21-0x7E except ','(0x2C) and '/'(0x2F), + -- printable ASCII (0x20-0x7E except ','(0x2C) and '/'(0x2F), -- plus non-ASCII utf8 (0x80-0xF4) - local tag_bytes = [[\x21-\x2B\x2D\x2E\x30-\x7E\x80-\xF4]] + local tag_bytes = [[\x20-\x2B\x2D\x2E\x30-\x7E\x80-\xF4]] TAGS_AND_REGEX = "^([" .. tag_bytes .. "]+(?:,|$))+$" TAGS_OR_REGEX = "^([" .. tag_bytes .. "]+(?:/|$))+$" @@ -222,7 +222,11 @@ local function query_entity(context, self, db, schema, method) return dao[context](dao, size, args.offset, opts) end - return dao[method](dao, self.params[schema.name], size, args.offset, opts) + local key = self.params[schema.name] + if schema.name == "tags" then + key = unescape_uri(key) + end + return dao[method](dao, key, size, args.offset, opts) end local key = self.params[schema.name] @@ -302,7 +306,7 @@ local function get_collection_endpoint(schema, foreign_schema, foreign_field_nam local args = self.args.uri if args.tags then - next_page_tags = "&tags=" .. (type(args.tags) == "table" and args.tags[1] or args.tags) + next_page_tags = "&tags=" .. escape_uri(type(args.tags) == "table" and args.tags[1] or args.tags) end local data, _, err_t, offset = page_collection(self, db, schema, method) diff --git a/kong/db/dao/init.lua b/kong/db/dao/init.lua index b23c4486ea08..d167ff4f43cc 100644 --- a/kong/db/dao/init.lua +++ b/kong/db/dao/init.lua @@ -214,7 +214,7 @@ local function validate_options_value(self, options) end elseif #options.tags > 5 then errors.tags = "cannot query more than 5 tags" - elseif not match(concat(options.tags), "^[\033-\043\045\046\048-\126\128-\244]+$") then + elseif not match(concat(options.tags), "^[ \033-\043\045\046\048-\126\128-\244]+$") then errors.tags = "must only contain printable ascii (except `,` and `/`) or valid utf-8" elseif #options.tags > 1 and options.tags_cond ~= "and" and options.tags_cond ~= "or" then errors.tags_cond = "must be a either 'and' or 'or' when more than one tag is specified" diff --git a/kong/db/schema/typedefs.lua b/kong/db/schema/typedefs.lua index 702a0d3fb641..04920a472caf 100644 --- a/kong/db/schema/typedefs.lua +++ b/kong/db/schema/typedefs.lua @@ -118,7 +118,7 @@ local function validate_tag(tag) -- printable ASCII (33-126 except ','(44) and '/'(47), -- plus non-ASCII utf8 (128-244) - if not match(tag, "^[\033-\043\045\046\048-\126\128-\244]+$") then + if not match(tag, "^[ \033-\043\045\046\048-\126\128-\244]+$") then return nil, "invalid tag '" .. tag .. "': expected printable ascii (except `,` and `/`) or valid utf-8 sequences" diff --git a/spec/02-integration/03-db/07-tags_spec.lua b/spec/02-integration/03-db/07-tags_spec.lua index c8f6556f992b..fa78bbcd5e41 100644 --- a/spec/02-integration/03-db/07-tags_spec.lua +++ b/spec/02-integration/03-db/07-tags_spec.lua @@ -34,7 +34,7 @@ for _, strategy in helpers.each_strategy() do local service = { host = "example-" .. i .. ".com", name = "service" .. i, - tags = { "team_a", "level_"..fmod(i, 5), "service"..i } + tags = { "team_ a", "level "..fmod(i, 5), "service"..i } } local row, err, err_t = bp.services:insert(service) assert.is_nil(err) @@ -62,15 +62,15 @@ for _, strategy in helpers.each_strategy() do end) it("list entity IDs by tag", function() - local rows, err, err_t, offset = db.tags:page_by_tag("team_a") + local rows, err, err_t, offset = db.tags:page_by_tag("team_ a") assert(is_valid_page(rows, err, err_t)) assert.is_nil(offset) assert.equal(test_entity_count, #rows) for _, row in ipairs(rows) do - assert.equal("team_a", row.tag) + assert.equal("team_ a", row.tag) end - rows, err, err_t, offset = db.tags:page_by_tag("team_alien") + rows, err, err_t, offset = db.tags:page_by_tag("team alien") assert(is_valid_page(rows, err, err_t)) assert.is_nil(offset) assert.equal(0, #rows) @@ -107,7 +107,7 @@ for _, strategy in helpers.each_strategy() do local func, key, removed_tag = unpack(scenario) it(func, function() - local tags = { "team_b_" .. func, "team_a" } + local tags = { "team_b_" .. func, "team_ a" } local row, err, err_t = db.services[func](db.services, key, { tags = tags, host = 'whatever.com' }) @@ -124,7 +124,7 @@ for _, strategy in helpers.each_strategy() do assert.is_nil(offset) assert.equal(test_entity_count*3 - removed_tags_count, #rows) - rows, err, err_t, offset = db.tags:page_by_tag("team_a") + rows, err, err_t, offset = db.tags:page_by_tag("team_ a") assert(is_valid_page(rows, err, err_t)) assert.is_nil(offset) assert.equal(test_entity_count, #rows) @@ -170,7 +170,7 @@ for _, strategy in helpers.each_strategy() do assert.is_nil(offset) assert.equal(test_entity_count*3 - removed_tags_count, #rows) - rows, err, err_t, offset = db.tags:page_by_tag("team_a") + rows, err, err_t, offset = db.tags:page_by_tag("team_ a") assert(is_valid_page(rows, err, err_t)) assert.is_nil(offset) assert.equal(test_entity_count - i, #rows) @@ -408,14 +408,6 @@ for _, strategy in helpers.each_strategy() do end) it("#db errors if tag value is invalid", function() - local ok, err = pcall(bp.services.insert, bp.services, { - host = "invalid-tag.com", - name = "service-invalid-tag", - tags = { "tag with spaces" } - }) - assert.is_falsy(ok) - assert.matches("invalid tag", err) - local ok, err = pcall(bp.services.insert, bp.services, { host = "invalid-tag.com", name = "service-invalid-tag", 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 8f987c52323c..cf25710910f8 100644 --- a/spec/02-integration/04-admin_api/14-tags_spec.lua +++ b/spec/02-integration/04-admin_api/14-tags_spec.lua @@ -19,7 +19,7 @@ describe("Admin API - tags", function() for i = 1, 2 do local consumer = { username = "adminapi-filter-by-tag-" .. i, - tags = { "corp_a", "consumer"..i, "🦍" } + tags = { "corp_ a", "consumer_ "..i, "🦍" } } local row, err, err_t = bp.consumers:insert(consumer) assert.is_nil(err) @@ -32,7 +32,7 @@ describe("Admin API - tags", function() config = { path = os.tmpname(), }, - tags = { "corp_a", "consumer" .. i } + tags = { "corp_ a", "consumer_ " .. i } }) end @@ -50,13 +50,13 @@ describe("Admin API - tags", function() it("filter by single tag", function() local res = assert(client:send { method = "GET", - path = "/consumers?tags=corp_a" + path = "/consumers?tags=corp_%20a" }) local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equals(2, #json.data) for i = 1, 2 do - assert.contains('corp_a', json.data[i].tags) + assert.contains('corp_ a', json.data[i].tags) end end) @@ -76,21 +76,21 @@ describe("Admin API - tags", function() it("filter by multiple tags with AND", function() local res = assert(client:send { method = "GET", - path = "/consumers?tags=corp_a,consumer1" + path = "/consumers?tags=corp_%20a,consumer_%201" }) local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equals(1, #json.data) assert.equals(3, #json.data[1].tags) - assert.contains('corp_a', json.data[1].tags) - assert.contains('consumer1', json.data[1].tags) + assert.contains('corp_ a', json.data[1].tags) + assert.contains('consumer_ 1', json.data[1].tags) assert.contains('🦍', json.data[1].tags) end) it("filter by multiple tags with OR", function() local res = assert(client:send { method = "GET", - path = "/consumers?tags=consumer2/consumer1" + path = "/consumers?tags=consumer_%202/consumer_%201" }) local body = assert.res_status(200, res) local json = cjson.decode(body) @@ -98,20 +98,20 @@ describe("Admin API - tags", function() end) it("ignores tags when filtering by multiple filters #6779", function() - local res = client:get("/consumers/adminapi-filter-by-tag-1/plugins?tags=consumer2") + local res = client:get("/consumers/adminapi-filter-by-tag-1/plugins?tags=consumer_%202") local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equals(1, #json.data) - assert.contains('corp_a', json.data[1].tags) - assert.contains('consumer1', json.data[1].tags) - assert.not_contains('consumer2', json.data[1].tags) + assert.contains('corp_ a', json.data[1].tags) + assert.contains('consumer_ 1', json.data[1].tags) + assert.not_contains('consumer_ 2', json.data[1].tags) end) it("errors if filter by mix of AND and OR", function() local res = assert(client:send { method = "GET", - path = "/consumers?tags=consumer3,consumer2/consumer1" + path = "/consumers?tags=consumer_%203,consumer_%202/consumer_%201" }) local body = assert.res_status(400, res) local json = cjson.decode(body) @@ -119,7 +119,7 @@ describe("Admin API - tags", function() local res = assert(client:send { method = "GET", - path = "/consumers?tags=consumer3/consumer2,consumer1" + path = "/consumers?tags=consumer_%203/consumer_%202,consumer_%201" }) local body = assert.res_status(400, res) local json = cjson.decode(body) @@ -127,14 +127,6 @@ describe("Admin API - tags", function() end) it("errors if filter by tag with invalid value", function() - local res = assert(client:send { - method = "GET", - path = "/consumers?tags=foo%20bar" - }) - local body = assert.res_status(400, res) - local json = cjson.decode(body) - assert.equals("invalid option (tags: invalid filter syntax)", json.message) - local res = assert(client:send { method = "GET", path = "/consumers?tags=" .. string.char(255) @@ -145,7 +137,7 @@ describe("Admin API - tags", function() end) it("returns the correct 'next' arg", function() - local tags_arg = 'tags=corp_a' + local tags_arg = 'tags=corp_%20a' local res = assert(client:send { method = "GET", path = "/consumers?" .. tags_arg .. "&size=1" @@ -153,7 +145,7 @@ describe("Admin API - tags", function() local body = assert.res_status(200, res) local json = cjson.decode(body) assert.equals(1, #json.data) - assert.match(tags_arg, json.next) + assert.match(tags_arg, json.next, 1, true) end) end) @@ -169,7 +161,7 @@ describe("Admin API - tags", function() for i = 1, 2 do local consumer = { username = "adminapi-filter-by-tag-" .. i, - tags = { "corp_a", "consumer"..i } + tags = { "corp_ a", "consumer_ "..i } } local row, err, err_t = bp.consumers:insert(consumer) assert.is_nil(err) @@ -201,7 +193,7 @@ describe("Admin API - tags", function() it("/tags/:tags", function() local res = assert(client:send { method = "GET", - path = "/tags/corp_a" + path = "/tags/corp_%20a" }) local body = assert.res_status(200, res) local json = cjson.decode(body) @@ -239,7 +231,7 @@ describe("Admin API - tags", function() it("/tags/:tags ignores ?tags= query", function() local res = assert(client:send { method = "GET", - path = "/tags/corp_a?tags=not_a_tag" + path = "/tags/corp_%20a?tags=not_a_tag" }) local body = assert.res_status(200, res) local json = cjson.decode(body) @@ -247,7 +239,7 @@ describe("Admin API - tags", function() local res = assert(client:send { method = "GET", - path = "/tags/corp_a?tags=invalid@tag" + path = "/tags/corp_%20a?tags=invalid@tag" }) local body = assert.res_status(200, res) local json = cjson.decode(body)