fix(router): http headers value match should be case sensitive in `ex…

…pressions` flavor (#11905) `traditional_compatible` flavor remains case insensitive to stay compatible with `traditional` flavor. This change allow `expressions` route authors to pick whether they want case sensitive or insensitive matches. KAG-2905 --------- Co-authored-by: Datong Sun <[email protected]>
Kong · Feb 26, 2024 · ab300ab · ab300ab
1 parent 8141909
commit ab300ab
Show file tree

Hide file tree

Showing 4 changed files with 182 additions and 11 deletions.
diff --git a/changelog/unreleased/kong/expression_http_headers_sensitive.yml b/changelog/unreleased/kong/expression_http_headers_sensitive.yml
@@ -0,0 +1,6 @@
+message: |
+    Header value matching (`http.headers.*`) in `expressions` router flavor are now case sensitive.
+    This change does not affect on `traditional_compatible` mode
+    where header value match are always performed ignoring the case.
+type: bugfix
+scope: Core
diff --git a/kong/router/atc.lua b/kong/router/atc.lua
@@ -471,14 +471,14 @@ function _M:select(req_method, req_uri, req_host, req_scheme,
       local v = req_headers[h]
 
       if type(v) == "string" then
-        local res, err = c:add_value(field, v:lower())
+        local res, err = c:add_value(field, v)
         if not res then
           return nil, err
         end
 
       elseif type(v) == "table" then
         for _, v in ipairs(v) do
-          local res, err = c:add_value(field, v:lower())
+          local res, err = c:add_value(field, v)
           if not res then
             return nil, err
           end
@@ -584,14 +584,8 @@ do
       local name = replace_dashes_lower(name)
 
       if type(value) == "table" then
-        for i, v in ipairs(value) do
-          value[i] = v:lower()
-        end
         tb_sort(value)
         value = tb_concat(value, ", ")
-
-      else
-        value = value:lower()
       end
 
       str_buf:putf("|%s=%s", name, value)

diff --git a/kong/router/compat.lua b/kong/router/compat.lua
@@ -252,7 +252,7 @@ local function get_expression(route)
       single_header_buf:reset():put("(")
 
       for i, value in ipairs(v) do
-        local name = "any(http.headers." .. replace_dashes_lower(h) .. ")"
+        local name = "any(lower(http.headers." .. replace_dashes_lower(h) .. "))"
         local op = OP_EQUAL
 
         -- value starts with "~*"

diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua
@@ -2249,7 +2249,32 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
             assert(new_router(use_case))
           end)
         end)
-      end
+
+        describe("match http.headers.*", function()
+          local use_case
+          local get_expression = atc_compat.get_expression
+
+          before_each(function()
+            use_case = {
+              {
+                service = service,
+                route = {
+                  id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
+                  methods = { "GET" },
+                },
+              },
+            }
+          end)
+
+          it("should always add lower()", function()
+            use_case[1].route.headers = { test = { "~*Quote" }, }
+
+            assert.equal([[(http.method == r#"GET"#) && (any(lower(http.headers.test)) ~ r#"quote"#)]],
+                         get_expression(use_case[1].route))
+            assert(new_router(use_case))
+          end)
+        end)
+      end   -- if flavor ~= "traditional"
 
       describe("normalization stopgap measurements", function()
         local use_case, router
@@ -4890,6 +4915,65 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible" }) do
   end)
 end
 
+do
+  local flavor = "traditional_compatible"
+
+  describe("Router (flavor = " .. flavor .. ")", function()
+    reload_router(flavor)
+
+    local use_case, router
+
+    lazy_setup(function()
+      use_case = {
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
+            paths = {
+              "/foo",
+            },
+            headers = {
+              test1 = { "Quote" },
+            },
+          },
+        },
+      }
+    end)
+
+    it("[cache hit should be case sensitive]", function()
+      router = assert(new_router(use_case))
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo", { test1 = "QUOTE", })
+      router._set_ngx(_ngx)
+
+      -- first match, case insensitive
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.falsy(ctx.route_match_cached)
+
+      -- cache hit
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.same(ctx.route_match_cached, "pos")
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo", { test1 = "QuoTe", })
+      router._set_ngx(_ngx)
+
+      -- case insensitive match
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+
+      -- cache miss, case sensitive
+      assert.falsy(ctx.route_match_cached)
+    end)
+  end)
+end   -- local flavor = "traditional_compatible"
+
 do
   local flavor = "expressions"
 
@@ -5063,5 +5147,92 @@ do
     end)
 
   end)
-end
+
+  describe("Router (flavor = " .. flavor .. ") [http]", function()
+    reload_router(flavor)
+
+    local use_case, router
+
+    lazy_setup(function()
+      use_case = {
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
+            expression = [[http.path == "/foo/bar" && http.headers.test1 == "Quote"]],
+            priority = 100,
+          },
+        },
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8102",
+            expression = [[http.path == "/foo/bar" && lower(http.headers.test2) == "quote"]],
+            priority = 100,
+          },
+        },
+      }
+    end)
+
+    it("select() should match with case sensitivity", function()
+      router = assert(new_router(use_case))
+
+      local match_t = router:select("GET", "/foo/bar", nil, nil, nil, nil, nil, nil, nil, {test1 = "quote"})
+      assert.falsy(match_t)
+
+      local match_t = router:select("GET", "/foo/bar", nil, nil, nil, nil, nil, nil, nil, {test1 = "quoTe"})
+      assert.falsy(match_t)
+
+      local match_t = router:select("GET", "/foo/bar", nil, nil, nil, nil, nil, nil, nil, {test1 = "Quote"})
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+    end)
+
+    it("select() should match with lower() (case insensitive)", function()
+      router = assert(new_router(use_case))
+
+      local match_t = router:select("GET", "/foo/bar", nil, nil, nil, nil, nil, nil, nil, {test2 = "QuoTe"})
+      assert.truthy(match_t)
+      assert.same(use_case[2].route, match_t.route)
+
+      local match_t = router:select("GET", "/foo/bar", nil, nil, nil, nil, nil, nil, nil, {test2 = "QUOTE"})
+      assert.truthy(match_t)
+      assert.same(use_case[2].route, match_t.route)
+    end)
+
+    it("exec() should hit cache with case sensitive", function()
+      router = assert(new_router(use_case))
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo/bar", { test1 = "Quote", })
+      router._set_ngx(_ngx)
+
+      -- first match
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.falsy(ctx.route_match_cached)
+
+      -- cache hit pos
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.same(ctx.route_match_cached, "pos")
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo/bar", { test1 = "QUOTE", })
+      router._set_ngx(_ngx)
+
+      -- case sensitive not match
+      local match_t = router:exec(ctx)
+      assert.falsy(match_t)
+      assert.falsy(ctx.route_match_cached)
+
+      -- cache hit neg
+      local match_t = router:exec(ctx)
+      assert.falsy(match_t)
+      assert.same(ctx.route_match_cached, "neg")
+    end)
+  end)
+end   -- local flavor = "expressions"