diff --git a/kong/core/router.lua b/kong/core/router.lua index 059bac58f99..89dce1134e7 100644 --- a/kong/core/router.lua +++ b/kong/core/router.lua @@ -15,7 +15,6 @@ local tonumber = tonumber local ipairs = ipairs local pairs = pairs local type = type -local next = next local band = bit.band local bor = bit.bor local ERR = ngx.ERR @@ -588,9 +587,6 @@ function _M.new(apis) end) - local grab_host = #wildcard_hosts > 0 or next(plain_indexes.hosts) ~= nil - - local function find_api(req_method, req_uri, req_host, ngx) if type(req_method) ~= "string" then return error("arg #1 method must be a string") @@ -598,7 +594,7 @@ function _M.new(apis) if type(req_uri) ~= "string" then return error("arg #2 uri must be a string") end - if req_host and type(req_host) ~= "string" then + if type(req_host) ~= "string" then return error("arg #3 host must be a string") end @@ -831,13 +827,7 @@ function _M.new(apis) end end - --print("grab host header: ", grab_host) - - local req_host - - if grab_host then - req_host = ngx.var.http_host - end + local req_host = ngx.var.http_host local match_t = find_api(method, uri, req_host, ngx) if not match_t then diff --git a/spec/01-unit/010-router_spec.lua b/spec/01-unit/010-router_spec.lua index 706e3e7c6c9..b185e5cdbd7 100644 --- a/spec/01-unit/010-router_spec.lua +++ b/spec/01-unit/010-router_spec.lua @@ -95,14 +95,14 @@ describe("Router", function() it("[uri]", function() -- uri - local match_t = router.select("GET", "/my-api") + local match_t = router.select("GET", "/my-api", "domain.org") assert.truthy(match_t) assert.same(use_case[3], match_t.api) end) it("[method]", function() -- method - local match_t = router.select("TRACE", "/") + local match_t = router.select("TRACE", "/", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -123,7 +123,7 @@ describe("Router", function() it("[uri + method]", function() -- uri + method - local match_t = router.select("PUT", "/api-6") + local match_t = router.select("PUT", "/api-6", "domain.org") assert.truthy(match_t) assert.same(use_case[6], match_t.api) end) @@ -138,7 +138,7 @@ describe("Router", function() describe("[uri prefix]", function() it("matches when given [uri] is in request URI prefix", function() -- uri prefix - local match_t = router.select("GET", "/my-api/some/path") + local match_t = router.select("GET", "/my-api/some/path", "domain.org") assert.truthy(match_t) assert.same(use_case[3], match_t.api) end) @@ -157,19 +157,19 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/my-api/hello") + local match_t = router.select("GET", "/my-api/hello", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) - match_t = router.select("GET", "/my-api/hello/world") + match_t = router.select("GET", "/my-api/hello/world", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) - match_t = router.select("GET", "/my-api") + match_t = router.select("GET", "/my-api", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) - match_t = router.select("GET", "/my-api/world") + match_t = router.select("GET", "/my-api/world", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -190,19 +190,19 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/my-api/hello") + local match_t = router.select("GET", "/my-api/hello", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) - match_t = router.select("GET", "/my-api/hello/world") + match_t = router.select("GET", "/my-api/hello/world", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) - match_t = router.select("GET", "/my-api") + match_t = router.select("GET", "/my-api", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) - match_t = router.select("GET", "/my-api/world") + match_t = router.select("GET", "/my-api/world", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) @@ -279,7 +279,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/users/123/profile") + local match_t = router.select("GET", "/users/123/profile", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) @@ -302,7 +302,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/api/persons/456") + local match_t = router.select("GET", "/api/persons/456", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) @@ -321,7 +321,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/api/persons/123/profile") + local match_t = router.select("GET", "/api/persons/123/profile", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -346,7 +346,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) it("matches leftmost wildcards", function() - local match_t = router.select("GET", "/", "foo.api.com") + local match_t = router.select("GET", "/", "foo.api.com", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) @@ -463,7 +463,7 @@ describe("Router", function() assert.same(use_case[1], match_t.api) -- uri - local match_t = router.select("TRACE", "/my-api") + local match_t = router.select("TRACE", "/my-api", "domain.org") assert.truthy(match_t) assert.same(use_case[3], match_t.api) end) @@ -539,11 +539,11 @@ describe("Router", function() } local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/users/123/profile") + local match_t = router.select("GET", "/users/123/profile", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) - match_t = router.select("POST", "/users/123/profile") + match_t = router.select("POST", "/users/123/profile", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -561,11 +561,11 @@ describe("Router", function() } local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/example") + local match_t = router.select("GET", "/example", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) - match_t = router.select("GET", "/example/status/200") + match_t = router.select("GET", "/example/status/200", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -585,7 +585,7 @@ describe("Router", function() } local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/") + local match_t = router.select("GET", "/", "nothing.com") assert.truthy(match_t) assert.same(use_case[1], match_t.api) @@ -608,7 +608,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/a/bb/foobar") + local match_t = router.select("GET", "/a/bb/foobar", "domain.org") assert.truthy(match_t) assert.same(use_case[2], match_t.api) end) @@ -627,25 +627,25 @@ describe("Router", function() it("request with [method]", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/") + local match_t = router.select("GET", "/", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) it("does not supersede another API", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/my-api") + local match_t = router.select("GET", "/my-api", "domain.org") assert.truthy(match_t) assert.same(use_case[4], match_t.api) - match_t = router.select("GET", "/my-api/hello/world") + match_t = router.select("GET", "/my-api/hello/world", "domain.org") assert.truthy(match_t) assert.same(use_case[4], match_t.api) end) it("acts as a catch-all API", function() local router = assert(Router.new(use_case)) - local match_t = router.select("GET", "/foobar/baz") + local match_t = router.select("GET", "/foobar/baz", "domain.org") assert.truthy(match_t) assert.same(use_case[1], match_t.api) end) @@ -707,7 +707,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local api_t = router.select("GET", "/a/bb/foobar") + local api_t = router.select("GET", "/a/bb/foobar", "domain.org") assert.truthy(api_t) assert.same(use_case[2], api_t.api) end) @@ -735,7 +735,7 @@ describe("Router", function() end) it("does not match when given [uri] is in URI but not in prefix", function() - local match_t = router.select("GET", "/some-other-prefix/my-api") + local match_t = router.select("GET", "/some-other-prefix/my-api", "domain.org") assert.is_nil(match_t) end) end) @@ -877,7 +877,7 @@ describe("Router", function() end, "arg #2 uri must be a string", nil, true) assert.error_matches(function() - router.select("GET", "/", 1) + router.select("GET", "/") end, "arg #3 host must be a string", nil, true) end) end) @@ -932,7 +932,7 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) @@ -945,7 +945,7 @@ describe("Router", function() assert.is_nil(match_t.upstream_host) -- only when `preserve_host = true` assert.equal("/my-api", match_t.upstream_uri) - _ngx = mock_ngx("GET", "/my-api-2", {}) + _ngx = mock_ngx("GET", "/my-api-2", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[2], match_t.api) @@ -1018,7 +1018,7 @@ describe("Router", function() assert.is_nil(match_t.matches.uri) assert.is_nil(match_t.matches.method) - _ngx = mock_ngx("GET", "/users/123/profile", {}) + _ngx = mock_ngx("GET", "/users/123/profile", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[4], match_t.api) assert.is_nil(match_t.matches.host) @@ -1036,7 +1036,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local _ngx = mock_ngx("GET", "/users/1984/profile", {}) + local _ngx = mock_ngx("GET", "/users/1984/profile", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.equal("1984", match_t.matches.uri_captures[1]) assert.equal("1984", match_t.matches.uri_captures.user_id) @@ -1060,7 +1060,7 @@ describe("Router", function() assert.is_nil(match_t.matches.uri_captures.stripped_uri) assert.equal(2, #match_t.matches.uri_captures) - _ngx = mock_ngx("GET", "/users/1984/profile/email", {}) + _ngx = mock_ngx("GET", "/users/1984/profile/email", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.equal("1984", match_t.matches.uri_captures[1]) assert.equal("1984", match_t.matches.uri_captures.user_id) @@ -1084,7 +1084,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local _ngx = mock_ngx("GET", "/hello/world", {}) + local _ngx = mock_ngx("GET", "/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.equal("/world", match_t.upstream_uri) assert.is_nil(match_t.matches.uri_captures) @@ -1100,7 +1100,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local _ngx = mock_ngx("GET", "/users/1984/profile", {}) + local _ngx = mock_ngx("GET", "/users/1984/profile", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.is_nil(match_t.matches.uri_captures) end) @@ -1116,7 +1116,7 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/get", match_t.upstream_url_t.path) @@ -1138,11 +1138,11 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.equal(8080, match_t.upstream_url_t.port) - _ngx = mock_ngx("GET", "/my-api-2", {}) + _ngx = mock_ngx("GET", "/my-api-2", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.equal(8443, match_t.upstream_url_t.port) end) @@ -1157,52 +1157,12 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("GET", "/endel%C3%B8st", {}) + local _ngx = mock_ngx("GET", "/endel%C3%B8st", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/endel%C3%B8st", match_t.upstream_uri) end) - describe("grab_headers", function() - local use_case_apis = { - { - name = "api-1", - uris = { "/my-api" }, - } - } - - it("does not read Host header if not required", function() - local _ngx = mock_ngx("GET", "/my-api", {}) - - spy.on(spy_stub, "nop") - - local router = assert(Router.new(use_case_apis)) - - local match_t = router.exec(_ngx) - assert.same(use_case_apis[1], match_t.api) - assert.spy(spy_stub.nop).was.not_called() - end) - - it("reads Host header if required", function() - table.insert(use_case_apis, { - name = "api-2", - uris = { "/my-api" }, - headers = { - host = { "my-api.com" }, - } - }) - - local _ngx = mock_ngx("GET", "/my-api", { host = "my-api.com" }) - spy.on(spy_stub, "nop") - - local router = assert(Router.new(use_case_apis)) - - local match_t = router.exec(_ngx) - assert.same(use_case_apis[2], match_t.api) - assert.spy(spy_stub.nop).was.called(1) - end) - end) - describe("stripped uris", function() local router local use_case_apis = { @@ -1224,7 +1184,7 @@ describe("Router", function() end) it("strips the specified uris from the given uri if matching", function() - local _ngx = mock_ngx("GET", "/my-api/hello/world", {}) + local _ngx = mock_ngx("GET", "/my-api/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) @@ -1232,7 +1192,7 @@ describe("Router", function() end) it("strips if matched URI is plain (not a prefix)", function() - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) @@ -1240,7 +1200,7 @@ describe("Router", function() end) it("doesn't strip if 'strip_uri' is not enabled", function() - local _ngx = mock_ngx("POST", "/my-api/hello/world", {}) + local _ngx = mock_ngx("POST", "/my-api/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[2], match_t.api) @@ -1258,7 +1218,7 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("POST", "/my-api/hello/world", {}) + local _ngx = mock_ngx("POST", "/my-api/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) @@ -1266,35 +1226,35 @@ describe("Router", function() end) it("can find an API with stripped URI several times in a row", function() - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) - _ngx = mock_ngx("GET", "/my-api", {}) + _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) end) it("can proxy an API with stripped URI with different URIs in a row", function() - local _ngx = mock_ngx("GET", "/my-api", {}) + local _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) - _ngx = mock_ngx("GET", "/this-api", {}) + _ngx = mock_ngx("GET", "/this-api", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) - _ngx = mock_ngx("GET", "/my-api", {}) + _ngx = mock_ngx("GET", "/my-api", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) - _ngx = mock_ngx("GET", "/this-api", {}) + _ngx = mock_ngx("GET", "/this-api", { ["host"] = "domain.org" }) match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) @@ -1311,7 +1271,7 @@ describe("Router", function() local router = assert(Router.new(use_case_apis)) - local _ngx = mock_ngx("GET", "/endel%C3%B8st", {}) + local _ngx = mock_ngx("GET", "/endel%C3%B8st", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal("/", match_t.upstream_uri) @@ -1328,7 +1288,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local _ngx = mock_ngx("GET", "/users/123/profile/hello/world", {}) + local _ngx = mock_ngx("GET", "/users/123/profile/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.equal("/hello/world", match_t.upstream_uri) end) @@ -1344,7 +1304,7 @@ describe("Router", function() local router = assert(Router.new(use_case)) - local _ngx = mock_ngx("GET", "/users/123/profile/hello/world", {}) + local _ngx = mock_ngx("GET", "/users/123/profile/hello/world", { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.equal("/hello/world", match_t.upstream_uri) end) @@ -1422,6 +1382,36 @@ describe("Router", function() assert.same(use_case_apis[1], match_t.api) assert.equal("preserve.com", match_t.upstream_host) end) + + it("uses the request's Host header if an API with no host was cached", function() + -- This is a regression test for: + -- https://github.com/Mashape/kong/issues/2825 + -- Ensure cached APIs (in the LRU cache) still get proxied with the + -- correct Host header when preserve_host = true and no registered + -- API has a `hosts` property. + + local use_case_apis = { + { + name = "no-host", + uris = { "/nohost" }, + preserve_host = true, + } + } + + local router = assert(Router.new(use_case_apis)) + + local _ngx = mock_ngx("GET", "/nohost", { ["host"] = "domain1.com" }) + + local match_t = router.exec(_ngx) + assert.same(use_case_apis[1], match_t.api) + assert.equal("domain1.com", match_t.upstream_host) + + _ngx = mock_ngx("GET", "/nohost", { ["host"] = "domain2.com" }) + + match_t = router.exec(_ngx) + assert.same(use_case_apis[1], match_t.api) + assert.equal("domain2.com", match_t.upstream_host) + end) end) describe("when preserve_host is false", function() @@ -1502,7 +1492,7 @@ describe("Router", function() local router = assert(Router.new(use_case_apis) ) - local _ngx = mock_ngx("GET", args[3], {}) + local _ngx = mock_ngx("GET", args[3], { ["host"] = "domain.org" }) local match_t = router.exec(_ngx) assert.same(use_case_apis[1], match_t.api) assert.equal(args[1], match_t.upstream_url_t.path)