feat(core) allow spaces to be used in tag names FT-2680 (#9143)

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)

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"

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"

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",

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,65 +76,57 @@ 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)
         assert.equals(2, #json.data)
       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)
         assert.equals("invalid option (tags: invalid filter syntax)", json.message)
 
         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)
         assert.equals("invalid option (tags: invalid filter syntax)", json.message)
       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,15 +137,15 @@ 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"
         })
         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,15 +231,15 @@ 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)
         assert.equals(2, #json.data)
 
         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)