fix(proxy) include port in upstream Host header

Fix #2045

kong/core/balancer.lua

@@ -237,7 +237,7 @@ end
 -- Main entry point when resolving
 --===========================================================
 
--- Resolves the target structure in-place (fields `ip` and `port`).
+-- Resolves the target structure in-place (fields `ip`, `port`, and `hostname`).
 --
 -- If the hostname matches an 'upstream' pool, then it must be balanced in that
 -- pool, in this case any port number provided will be ignored, as the pool provides it.
@@ -249,6 +249,7 @@ local function execute(target)
     -- it's an ip address (v4 or v6), so nothing we can do...
     target.ip = target.host
     target.port = target.port or 80 -- TODO: remove this fallback value
+    target.hostname = target.host
     return true
   end
 
@@ -309,6 +310,7 @@ local function execute(target)
 
   target.ip = ip
   target.port = port
+  target.hostname = target.host
   return true
 end
 

kong/core/handler.lua

@@ -79,7 +79,8 @@ return {
 
       ctx.KONG_ACCESS_START = get_now()
 
-      local api, upstream_scheme, upstream_host, upstream_port = router.exec(ngx)
+      local api, upstream_scheme, upstream_host, 
+                 upstream_port, host_header = router.exec(ngx)
       if not api then
         return responses.send_HTTP_NOT_FOUND("no API found with those values")
       end
@@ -92,8 +93,8 @@ return {
       end
 
       local balancer_address = {
-        type                 = utils.hostname_type(upstream_host),  -- the type of `upstream.host`; ipv4, ipv6 or name
-        host                 = upstream_host,  -- supposed target host
+        type                 = utils.hostname_type(upstream_host),  -- the type of `host`; ipv4, ipv6 or name
+        host                 = upstream_host,  -- target host per `upstream_url`
         port                 = upstream_port,  -- final target port
         tries                = 0,              -- retry counter
         retries              = api.retries,    -- number of retries for the balancer
@@ -103,6 +104,7 @@ return {
         -- ip                = nil,            -- final target IP address
         -- failures          = nil,            -- for each failure an entry { name = "...", code = xx }
         -- balancer          = nil,            -- the balancer object, in case of a balancer
+        -- hostname          = nil,            -- the hostname belonging to the final target IP
       }
 
       var.upstream_scheme = upstream_scheme
@@ -117,14 +119,12 @@ return {
           "' with: "..tostring(err))
       end
 
-      if balancer_address.hostname and not api.preserve_host then
-        var.upstream_host = balancer_address.hostname
+      -- if set `host_header` is the original header to be preserved
+      var.upstream_host = host_header or 
+          balancer_address.hostname..":"..balancer_address.port
 
-      else
-        var.upstream_host = upstream_host
-      end
     end,
-    -- Only executed if the `resolver` module found an API and allows nginx to proxy it.
+    -- Only executed if the `router` module found an API and allows nginx to proxy it.
     after = function()
       local ctx = ngx.ctx
       local now = get_now()

kong/core/router.lua

@@ -9,6 +9,7 @@ local re_sub = ngx.re.sub
 local insert = table.insert
 local upper = string.upper
 local lower = string.lower
+local match = string.match
 local fmt = string.format
 local tonumber = tonumber
 local ipairs = ipairs
@@ -492,7 +493,11 @@ function _M.new(apis)
     end
 
 
-    local host = headers["Host"] or headers["host"]
+    local host = headers["host"]
+    if host then
+      host = match(host,"^([^:]+)") -- strip port number if given
+    end
+
     method = upper(method)
 
 
@@ -598,7 +603,7 @@ function _M.new(apis)
   function self.exec(ngx)
     local method = ngx.req.get_method()
     local uri = ngx.var.uri
-    local upstream_host
+    local host_header
     local headers
 
 
@@ -619,9 +624,6 @@ function _M.new(apis)
     end
 
 
-    upstream_host = api_t.upstream_host
-
-
     if api_t.strip_uri_regex then
       local stripped_uri = re_sub(uri, api_t.strip_uri_regex, "/$1", "jo")
       ngx.req.set_uri(stripped_uri)
@@ -633,7 +635,7 @@ function _M.new(apis)
         headers = ngx.req.get_headers()
       end
 
-      upstream_host = headers["host"]
+      host_header = headers["host"]
     end
 
 
@@ -642,7 +644,8 @@ function _M.new(apis)
     end
 
 
-    return api_t.api, api_t.upstream_scheme, upstream_host, api_t.upstream_port
+    return api_t.api, api_t.upstream_scheme, api_t.upstream_host, 
+           api_t.upstream_port, host_header
   end
 
 

spec/01-unit/12-router_spec.lua

@@ -86,6 +86,13 @@ describe("Router", function()
       assert.same(use_case[1], api_t.api)
     end)
 
+    it("[host] ignores port", function()
+      -- host
+      local api_t = router.select("GET", "/", { ["host"] = "domain-1.org:123" })
+      assert.truthy(api_t)
+      assert.same(use_case[1], api_t.api)
+    end)
+
     it("[uri]", function()
       -- uri
       local api_t = router.select("GET", "/my-api", {})
@@ -767,17 +774,28 @@ describe("Router", function()
       it("uses the request's Host header if preserve_host", function()
         local _ngx = mock_ngx("GET", "/", { ["host"] = "preserve.com" })
 
-        local api, _, upstream_host = router.exec(_ngx)
+        local api, _, upstream_host, _, host_header = router.exec(_ngx)
         assert.same(use_case_apis[1], api)
-        assert.equal("preserve.com", upstream_host)
+        assert.equal("httpbin.org", upstream_host)
+        assert.equal("preserve.com", host_header)
+      end)
+
+      it("uses the request's Host header incl. port if preserve_host", function()
+        local _ngx = mock_ngx("GET", "/", { ["host"] = "preserve.com:123" })
+
+        local api, _, upstream_host, _, host_header = router.exec(_ngx)
+        assert.same(use_case_apis[1], api)
+        assert.equal("httpbin.org", upstream_host)
+        assert.equal("preserve.com:123", host_header)
       end)
 
       it("uses the API's upstream_url host if not preserve_host", function()
         local _ngx = mock_ngx("GET", "/", { ["host"] = "discard.com" })
 
-        local api, _, upstream_host = router.exec(_ngx)
+        local api, _, upstream_host, _, host_header = router.exec(_ngx)
         assert.same(use_case_apis[2], api)
         assert.equal("httpbin.org", upstream_host)
+        assert.is_nil(host_header)
       end)
     end)
   end)

spec/02-integration/05-proxy/01-router_spec.lua

@@ -230,8 +230,37 @@ describe("Router", function()
         headers = { ["Host"] = "preserved.com" },
       })
 
-      local body = assert.res_status(404, res)
-      assert.matches("Server at preserved.com", body, nil, true)
+      assert.res_status(200, res)
+      local json = cjson.decode((res:read_body()))
+      assert.equal("preserved.com", json.headers.Host)
+    end)
+
+    pending("preserves downstream host+port if enabled", function()
+      -- pending because httpbin.org seems to not return the exact header
+      -- but a parsed one, with the port stripped.
+      local res = assert(client:send {
+        method = "GET",
+        path = "/get",
+        headers = { ["Host"] = "preserved.com:80" },
+      })
+
+      assert.res_status(200, res)
+      local json = cjson.decode((res:read_body()))
+      assert.equal("preserved.com:80", json.headers.Host)
+    end)
+
+    pending("preserves downstream host+non_default_port if enabled", function()
+      -- pending because httpbin.org seems to not return the exact header
+      -- but a parsed one, with the port stripped.
+      local res = assert(client:send {
+        method = "GET",
+        path = "/get",
+        headers = { ["Host"] = "preserved.com:123" },
+      })
+
+      assert.res_status(200, res)
+      local json = cjson.decode((res:read_body()))
+      assert.equal("preserved.com:123", json.headers.Host)
     end)
 
     it("discards downstream host if disabled", function()
@@ -241,7 +270,8 @@ describe("Router", function()
         headers = { ["Host"] = "discarded.com" },
       })
 
-      assert.res_status(200, res)
+      assert.response(res).has.status(200)
+      assert.equal("httpbin.org", assert.request(res).has.header("Host"))
     end)
   end)