refactor(tools/http): simplify check_https() with ngx.var (#13167)

We used to call `ngx.req.get_headers()` to get all possible header values,
but since nginx 1.23.0 or OpenResty 1.25.3.1 `ngx.var` can get a combined value for all header values
with identical name (joined by comma), so I think that we could simplify these code.

There is one concern:
If header `X-Forwarded-Proto` has a comma separated value like `http, https`, 
This PR will think it is a multiple header request and report an error `Only one X-Forwarded-Proto header allowed`.
But I think that it is an unlikely case in the real world.

KAG-4673

kong/tools/http.lua

@@ -210,16 +210,19 @@ _M.check_https = function(trusted_ip, allow_terminated)
   -- otherwise, we fall back to relying on the client scheme
   -- (which was either validated earlier, or we fall through this block)
   if trusted_ip then
-    local scheme = ngx.req.get_headers()["x-forwarded-proto"]
+    local scheme = ngx.var.http_x_forwarded_proto
+    if not scheme then
+      return false
+    end
 
     -- we could use the first entry (lower security), or check the contents of
     -- each of them (slow). So for now defensive, and error
     -- out on multiple entries for the x-forwarded-proto header.
-    if type(scheme) == "table" then
+    if scheme:find(",", 1, true) then
       return nil, "Only one X-Forwarded-Proto header allowed"
     end
 
-    return tostring(scheme):lower() == "https"
+    return scheme:lower() == "https"
   end
 
   return false

spec/01-unit/05-utils_spec.lua

@@ -80,17 +80,14 @@ describe("Utils", function()
 
   describe("https_check", function()
     local old_ngx
-    local headers = {}
 
     lazy_setup(function()
       old_ngx = ngx
       _G.ngx = {
         var = {
-          scheme = nil
+          scheme = nil,
+          http_x_forwarded_proto = nil,
         },
-        req = {
-          get_headers = function() return headers end
-        }
       }
     end)
 
@@ -100,7 +97,7 @@ describe("Utils", function()
 
     describe("without X-Forwarded-Proto header", function()
       lazy_setup(function()
-        headers["x-forwarded-proto"] = nil
+        ngx.var.http_x_forwarded_proto = nil
       end)
 
       it("should validate an HTTPS scheme", function()
@@ -124,11 +121,11 @@ describe("Utils", function()
     describe("with X-Forwarded-Proto header", function()
 
       lazy_teardown(function()
-        headers["x-forwarded-proto"] = nil
+        ngx.var.http_x_forwarded_proto = nil
       end)
 
       it("should validate any scheme with X-Forwarded_Proto as HTTPS", function()
-        headers["x-forwarded-proto"] = "hTTPs"  -- check mixed casing for case insensitiveness
+        ngx.var.http_x_forwarded_proto = "hTTPs"  -- check mixed casing for case insensitiveness
         ngx.var.scheme = "hTTps"
         assert.is.truthy(tools_http.check_https(true, true))
         ngx.var.scheme = "hTTp"
@@ -138,7 +135,7 @@ describe("Utils", function()
       end)
 
       it("should validate only https scheme with X-Forwarded_Proto as non-HTTPS", function()
-        headers["x-forwarded-proto"] = "hTTP"
+        ngx.var.http_x_forwarded_proto = "hTTP"
         ngx.var.scheme = "hTTps"
         assert.is.truthy(tools_http.check_https(true, true))
         ngx.var.scheme = "hTTp"
@@ -148,7 +145,7 @@ describe("Utils", function()
       end)
 
       it("should return an error with multiple X-Forwarded_Proto headers", function()
-        headers["x-forwarded-proto"] = { "hTTP", "https" }
+        ngx.var.http_x_forwarded_proto = "hTTP, https"
         ngx.var.scheme = "hTTps"
         assert.is.truthy(tools_http.check_https(true, true))
         ngx.var.scheme = "hTTp"
@@ -157,27 +154,27 @@ describe("Utils", function()
       end)
 
       it("should not use X-Forwarded-Proto when the client is untrusted", function()
-        headers["x-forwarded-proto"] = "https"
+        ngx.var.http_x_forwarded_proto = "https"
         ngx.var.scheme = "http"
         assert.is_false(tools_http.check_https(false, false))
         assert.is_false(tools_http.check_https(false, true))
 
-        headers["x-forwarded-proto"] = "https"
+        ngx.var.http_x_forwarded_proto = "https"
         ngx.var.scheme = "https"
         assert.is_true(tools_http.check_https(false, false))
         assert.is_true(tools_http.check_https(false, true))
       end)
 
       it("should use X-Forwarded-Proto when the client is trusted", function()
-        headers["x-forwarded-proto"] = "https"
+        ngx.var.http_x_forwarded_proto = "https"
         ngx.var.scheme = "http"
 
         -- trusted client but do not allow terminated
         assert.is_false(tools_http.check_https(true, false))
 
         assert.is_true(tools_http.check_https(true, true))
 
-        headers["x-forwarded-proto"] = "https"
+        ngx.var.http_x_forwarded_proto = "https"
         ngx.var.scheme = "https"
         assert.is_true(tools_http.check_https(true, false))
         assert.is_true(tools_http.check_https(true, true))

t/01-pdk/05-client/08-get_protocol.t

@@ -301,3 +301,34 @@ qq{
 protocol=tls
 --- no_error_log
 [error]
+
+
+
+=== TEST 11: client.get_protocol() fails when kong receives an http request with multiple x-forwarded-proto headers
+
+--- http_config eval: $t::Util::HttpConfig
+--- config
+    location = /t {
+        access_by_lua_block {
+            ngx.ctx.route = {
+              protocols = { "http", "https" }
+            }
+
+            local PDK = require "kong.pdk"
+            local pdk = PDK.new()
+            pdk.ip.is_trusted = function() return true end -- mock
+
+            local ok, err = pdk.client.get_protocol(true)
+            assert(ok == nil)
+            ngx.say("err=", err)
+        }
+    }
+--- request
+GET /t
+--- more_headers
+X-Forwarded-Proto: https
+X-Forwarded-Proto: http
+--- response_body
+err=Only one X-Forwarded-Proto header allowed
+--- no_error_log
+[error]