From d442336c0bbd65d254f3959e361ab7533a2f94a5 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Wed, 5 Apr 2017 16:35:20 -0700 Subject: [PATCH] fix(router) ensure preserve_host when no `hosts` The router avoids grabbing the headers if not absolutely necessary (at least one API configured with a `hosts` rule. This caused an edge-case and mislead the router into believing the headers were already grabbed, and returned a `nil` value instead of the request's Host header. * add a unit test case * add an integration test case, although more generic * use the `ngx.var.http_*` API to grab the request's Host --- kong/core/router.lua | 6 +---- spec/01-unit/11-router_spec.lua | 22 ++++++++++++++++++- .../05-proxy/01-router_spec.lua | 19 ++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/kong/core/router.lua b/kong/core/router.lua index ba34b948d866..477bb1ffa316 100644 --- a/kong/core/router.lua +++ b/kong/core/router.lua @@ -615,11 +615,7 @@ function _M.new(apis) if api_t.preserve_host then - if not headers then - headers = ngx.req.get_headers() - end - - host_header = headers["host"] + host_header = ngx.var.http_host end diff --git a/spec/01-unit/11-router_spec.lua b/spec/01-unit/11-router_spec.lua index bad59299889a..957f5d39f1ae 100644 --- a/spec/01-unit/11-router_spec.lua +++ b/spec/01-unit/11-router_spec.lua @@ -588,7 +588,8 @@ describe("Router", function() _ngx = { re = ngx.re, var = { - uri = uri + uri = uri, + http_host = headers["host"] or headers["Host"], }, req = { set_uri = function(uri) @@ -860,6 +861,25 @@ describe("Router", function() assert.same(use_case_apis[1], api) assert.equal("httpbin.org", upstream.host) end) + + it("uses the request's Host header when `grab_header` is disabled", function() + local use_case_apis = { + { + name = "api-1", + upstream_url = "http://httpbin.org", + preserve_host = true, + uris = { "/foo" }, + } + } + + local router = assert(Router.new(use_case_apis)) + + local _ngx = mock_ngx("GET", "/foo", { ["host"] = "preserve.com" }) + + local api, _, host_header = router.exec(_ngx) + assert.same(use_case_apis[1], api) + assert.equal("preserve.com", host_header) + end) end) describe("when preserve_host is false", function() diff --git a/spec/02-integration/05-proxy/01-router_spec.lua b/spec/02-integration/05-proxy/01-router_spec.lua index c62a8b6f5fac..c20ff721ef32 100644 --- a/spec/02-integration/05-proxy/01-router_spec.lua +++ b/spec/02-integration/05-proxy/01-router_spec.lua @@ -250,6 +250,13 @@ describe("Router", function() upstream_url = "http://localhost:9999/headers-inspect", hosts = "discarded.com", }, + { + name = "api-3", + strip_uri = false, + preserve_host = true, + upstream_url = "http://localhost:9999", + uris = "/headers-inspect", + } } assert(helpers.start_kong { @@ -323,6 +330,18 @@ describe("Router", function() local json = cjson.decode(body) assert.equal("preserved.com:123", json.host) end) + + it("forwards request Host even if not matched by [hosts]", function() + local res = assert(client:send { + method = "GET", + path = "/headers-inspect", + headers = { ["Host"] = "preserved.com" }, + }) + + local body = assert.res_status(200, res) + local json = cjson.decode(body) + assert.equal("preserved.com", json.host) + end) end) end)