From df51142a2d06a5207592ee2b1eea87884613ddf0 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Fri, 24 Mar 2017 18:16:27 -0700 Subject: [PATCH 01/10] feat(proxy) add configuration options to hide server tokens and latency tokens * Add server_tokens and latency_tokens Kong configuration properties. Fix #1009 --- kong/conf_loader.lua | 4 +- kong/core/handler.lua | 33 +- kong/templates/kong_defaults.lua | 2 + .../05-proxy/10-server_tokens_spec.lua | 286 ++++++++++++++++++ 4 files changed, 315 insertions(+), 10 deletions(-) create mode 100644 spec/02-integration/05-proxy/10-server_tokens_spec.lua diff --git a/kong/conf_loader.lua b/kong/conf_loader.lua index 831d386f224a..685916a72f13 100644 --- a/kong/conf_loader.lua +++ b/kong/conf_loader.lua @@ -61,6 +61,8 @@ local CONF_INFERENCES = { cluster_advertise = {typ = "string"}, nginx_worker_processes = {typ = "string"}, upstream_keepalive = {typ = "number"}, + server_tokens = {typ = "boolean"}, + latency_tokens = {typ = "boolean"}, database = {enum = {"postgres", "cassandra"}}, pg_port = {typ = "number"}, @@ -424,7 +426,7 @@ local function load(path, custom_conf) -- initialize the dns client, so the globally patched tcp.connect method -- will work from here onwards. assert(require("kong.tools.dns")(conf)) - + return setmetatable(conf, nil) -- remove Map mt end diff --git a/kong/core/handler.lua b/kong/core/handler.lua index da9e139880ca..de3e09fec097 100644 --- a/kong/core/handler.lua +++ b/kong/core/handler.lua @@ -119,7 +119,7 @@ return { end -- if set `host_header` is the original header to be preserved - var.upstream_host = host_header or + var.upstream_host = host_header or balancer_address.hostname..":"..balancer_address.port end, @@ -137,19 +137,34 @@ return { }, header_filter = { before = function() - if ngx.ctx.KONG_PROXIED then + local ctx = ngx.ctx + + if ctx.KONG_PROXIED then local now = get_now() - ngx.ctx.KONG_WAITING_TIME = now - ngx.ctx.KONG_ACCESS_ENDED_AT -- time spent waiting for a response from upstream - ngx.ctx.KONG_HEADER_FILTER_STARTED_AT = now + ctx.KONG_WAITING_TIME = now - ctx.KONG_ACCESS_ENDED_AT -- time spent waiting for a response from upstream + ctx.KONG_HEADER_FILTER_STARTED_AT = now end end, after = function() - if ngx.ctx.KONG_PROXIED then - ngx.header[constants.HEADERS.UPSTREAM_LATENCY] = ngx.ctx.KONG_WAITING_TIME - ngx.header[constants.HEADERS.PROXY_LATENCY] = ngx.ctx.KONG_PROXY_LATENCY - ngx.header["Via"] = server_header + local ctx, header = ngx.ctx, ngx.header + + if ctx.KONG_PROXIED then + if singletons.configuration.latency_tokens then + header[constants.HEADERS.UPSTREAM_LATENCY] = ctx.KONG_WAITING_TIME + header[constants.HEADERS.PROXY_LATENCY] = ctx.KONG_PROXY_LATENCY + end + + if singletons.configuration.server_tokens then + header["Via"] = server_header + end + else - ngx.header["Server"] = server_header + if singletons.configuration.server_tokens then + header["Server"] = server_header + + else + header["Server"] = nil + end end end }, diff --git a/kong/templates/kong_defaults.lua b/kong/templates/kong_defaults.lua index 6e3851c3d537..c679866122dc 100644 --- a/kong/templates/kong_defaults.lua +++ b/kong/templates/kong_defaults.lua @@ -19,6 +19,8 @@ admin_ssl = on admin_ssl_cert = NONE admin_ssl_cert_key = NONE upstream_keepalive = 60 +server_tokens = on +latency_tokens = on database = postgres pg_host = 127.0.0.1 diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua new file mode 100644 index 000000000000..f3fbfb27bd49 --- /dev/null +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -0,0 +1,286 @@ +local helpers = require "spec.helpers" +local constants = require "kong.constants" + + +local default_server_header = _KONG._NAME.."/".._KONG._VERSION + + +local function start(config) + return function() + helpers.dao.apis:insert { + name = "api-1", + upstream_url = "http://localhost:9999/headers-inspect", + hosts = "inexistent.com", + } + + config = config or {} + config.nginx_conf = "spec/fixtures/custom_nginx.template" + + assert(helpers.start_kong(config)) + end +end + + +describe("Server Tokens", function() + local client + + before_each(function() + client = helpers.proxy_client() + end) + + after_each(function() + if client then + client:close() + end + end) + + describe("(with default configration values)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + }) + + teardown(helpers.stop_kong) + + it("should return Kong 'Via' header but not change the 'Server' header", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.not_equal(default_server_header, res.headers["server"]) + assert.equal(default_server_header, res.headers["via"]) + end) + + it("should return Kong 'Server' header but not the Kong 'Via' header", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) + end) + + end) + + describe("(with server_tokens = on)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + server_tokens = "on", + }) + + teardown(helpers.stop_kong) + + it("should return Kong 'Via' header but not change the 'Server' header", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.not_equal(default_server_header, res.headers["server"]) + assert.equal(default_server_header, res.headers["via"]) + end) + + it("should return Kong 'Server' header but not the Kong 'Via' header", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) + end) + + end) + + describe("(with server_tokens = off)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + server_tokens = "off", + }) + + teardown(helpers.stop_kong) + + it("should not return Kong 'Via' header but it should forward the 'Server' header", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.not_equal(nil, res.headers["server"]) + assert.not_equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) + end) + + it("should not return Kong 'Server' or 'Via' headers", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(nil, res.headers["server"]) + assert.equal(nil, res.headers["via"]) + end) + + end) +end) + + +describe("Latency Tokens", function() + local client + + before_each(function() + client = helpers.proxy_client() + end) + + after_each(function() + if client then + client:close() + end + end) + + describe("(with default configration values)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + }) + + teardown(helpers.stop_kong) + + it("should be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + it("should not be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + end) + + describe("(with latency_tokens = on)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + latency_tokens = "on", + }) + + teardown(helpers.stop_kong) + + it("should be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + it("should not be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + end) + + describe("(with latency_tokens = off)", function() + + setup(start { + nginx_conf = "spec/fixtures/custom_nginx.template", + latency_tokens = "off", + }) + + teardown(function() + helpers.stop_kong() + end) + + it("should not be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "inexistent.com" + } + }) + + assert.res_status(200, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + it("should not be returned", function() + local res = assert(client:send { + method = "GET", + path = "/get", + headers = { + host = "404.com" + } + }) + + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + end) + + end) +end) From 1b766c2d9947820329739b4cda84e51cc5023331 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 14:45:03 +0300 Subject: [PATCH 02/10] Fix https://github.com/Mashape/kong/pull/2259#discussion_r108142953 --- .../05-proxy/10-server_tokens_spec.lua | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua index f3fbfb27bd49..832bbaa92328 100644 --- a/spec/02-integration/05-proxy/10-server_tokens_spec.lua +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -51,7 +51,7 @@ describe("Server Tokens", function() } }) - assert.res_status(200, res) + assert.response(res).has.status(200) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) @@ -65,9 +65,9 @@ describe("Server Tokens", function() } }) - assert.res_status(404, res) + assert.response(res).has.status(404) + assert.response(res).has_not.header "via" assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) end) end) @@ -90,7 +90,7 @@ describe("Server Tokens", function() } }) - assert.res_status(200, res) + assert.response(res).has.status(200) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) @@ -104,9 +104,9 @@ describe("Server Tokens", function() } }) - assert.res_status(404, res) + assert.response(res).has.status(404) + assert.response(res).has_not.header "via" assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) end) end) @@ -129,10 +129,10 @@ describe("Server Tokens", function() } }) - assert.res_status(200, res) - assert.not_equal(nil, res.headers["server"]) + assert.response(res).has.status(200) + assert.response(res).has.header "server" + assert.response(res).has_not.header "via" assert.not_equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) end) it("should not return Kong 'Server' or 'Via' headers", function() @@ -144,9 +144,9 @@ describe("Server Tokens", function() } }) - assert.res_status(404, res) - assert.equal(nil, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.response(res).has.status(404) + assert.response(res).has_not.header "server" + assert.response(res).has_not.header "via" end) end) @@ -183,9 +183,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(200) + assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) end) it("should not be returned", function() @@ -197,9 +197,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(404) + assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) end) end) @@ -222,9 +222,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(200) + assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) end) it("should not be returned", function() @@ -236,9 +236,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(404) + assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) end) end) @@ -263,9 +263,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(200, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(200) + assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) end) it("should not be returned", function() @@ -277,9 +277,9 @@ describe("Latency Tokens", function() } }) - assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.response(res).has.status(404) + assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) + assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) end) end) From f62edfd225a6a101c41514dcbd31d45a9ddabadf Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 14:48:37 +0300 Subject: [PATCH 03/10] Fix https://github.com/Mashape/kong/pull/2259#discussion_r108143106 --- kong.conf.default | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/kong.conf.default b/kong.conf.default index 5acc2d75bb22..9a9947ebb331 100644 --- a/kong.conf.default +++ b/kong.conf.default @@ -115,6 +115,16 @@ # process. When this number is exceeded, the # least recently used connections are closed. +#server_tokens = on # Enables or disables emitting Kong version on + # error pages and in the "Server" or "Via" + # (in case the request was proxied) response + # header field. + +#latency_tokens = on # Enables or disables emitting Kong latency + # information in the "X-Kong-Proxy-Latency" + # and "X-Kong-Upstream-Latency" response + # header fields. + #------------------------------------------------------------------------------ # DATASTORE #------------------------------------------------------------------------------ From f4c607cdffb1c925209f5690e6e3e9b60555060e Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 21:56:06 +0300 Subject: [PATCH 04/10] Fix issues based on review comments by @thibault See: https://github.com/Mashape/kong/pull/2259#discussion_r108231658 https://github.com/Mashape/kong/pull/2259#discussion_r108232468 https://github.com/Mashape/kong/pull/2259#discussion_r108233416 https://github.com/Mashape/kong/pull/2259#discussion_r108233189 https://github.com/Mashape/kong/pull/2259#discussion_r108233055 https://github.com/Mashape/kong/pull/2259#discussion_r108232918 https://github.com/Mashape/kong/pull/2259#discussion_r108232863 --- .../05-proxy/10-server_tokens_spec.lua | 108 +++++++++--------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua index 832bbaa92328..9d34f61477a4 100644 --- a/spec/02-integration/05-proxy/10-server_tokens_spec.lua +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -10,7 +10,9 @@ local function start(config) helpers.dao.apis:insert { name = "api-1", upstream_url = "http://localhost:9999/headers-inspect", - hosts = "inexistent.com", + hosts = { + "header-inspect.com", + } } config = config or {} @@ -42,32 +44,32 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should return Kong 'Via' header but not change the 'Server' header", function() + it("should return Kong 'Via' header but not change the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) - it("should return Kong 'Server' header but not the Kong 'Via' header", function() + it("should return Kong 'Server' header but not the Kong 'Via' header when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "via" + assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -81,32 +83,32 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should return Kong 'Via' header but not change the 'Server' header", function() + it("should return Kong 'Via' header but not change the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) - it("should return Kong 'Server' header but not the Kong 'Via' header", function() + it("should return Kong 'Server' header but not the Kong 'Via' header when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "via" + assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -120,33 +122,33 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should not return Kong 'Via' header but it should forward the 'Server' header", function() + it("should not return Kong 'Via' header but it should forward the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.response(res).has.header "server" assert.response(res).has_not.header "via" assert.not_equal(default_server_header, res.headers["server"]) end) - it("should not return Kong 'Server' or 'Via' headers", function() + it("should not return Kong 'Server' or 'Via' headers when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "server" - assert.response(res).has_not.header "via" + assert.res_status(404, res) + assert.equal(nil, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -174,32 +176,32 @@ describe("Latency Tokens", function() teardown(helpers.stop_kong) - it("should be returned", function() + it("should be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) - assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -213,32 +215,32 @@ describe("Latency Tokens", function() teardown(helpers.stop_kong) - it("should be returned", function() + it("should be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com" } }) - assert.response(res).has.status(200) - assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -254,32 +256,32 @@ describe("Latency Tokens", function() helpers.stop_kong() end) - it("should not be returned", function() + it("should not be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) From 1a3c9bd7e3e1505f69155a5004337160322d84ae Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 21:56:06 +0300 Subject: [PATCH 05/10] Fix issues based on review comments by @thibaultcha See: https://github.com/Mashape/kong/pull/2259#discussion_r108231658 https://github.com/Mashape/kong/pull/2259#discussion_r108232468 https://github.com/Mashape/kong/pull/2259#discussion_r108233416 https://github.com/Mashape/kong/pull/2259#discussion_r108233189 https://github.com/Mashape/kong/pull/2259#discussion_r108233055 https://github.com/Mashape/kong/pull/2259#discussion_r108232918 https://github.com/Mashape/kong/pull/2259#discussion_r108232863 --- .../05-proxy/10-server_tokens_spec.lua | 110 +++++++++--------- 1 file changed, 56 insertions(+), 54 deletions(-) diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua index 832bbaa92328..5fe821e6fcd9 100644 --- a/spec/02-integration/05-proxy/10-server_tokens_spec.lua +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -2,7 +2,7 @@ local helpers = require "spec.helpers" local constants = require "kong.constants" -local default_server_header = _KONG._NAME.."/".._KONG._VERSION +local default_server_header = _KONG._NAME .. "/" .. _KONG._VERSION local function start(config) @@ -10,7 +10,9 @@ local function start(config) helpers.dao.apis:insert { name = "api-1", upstream_url = "http://localhost:9999/headers-inspect", - hosts = "inexistent.com", + hosts = { + "header-inspect.com", + } } config = config or {} @@ -42,32 +44,32 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should return Kong 'Via' header but not change the 'Server' header", function() + it("should return Kong 'Via' header but not change the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) - it("should return Kong 'Server' header but not the Kong 'Via' header", function() + it("should return Kong 'Server' header but not the Kong 'Via' header when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "via" + assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -81,32 +83,32 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should return Kong 'Via' header but not change the 'Server' header", function() + it("should return Kong 'Via' header but not change the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.not_equal(default_server_header, res.headers["server"]) assert.equal(default_server_header, res.headers["via"]) end) - it("should return Kong 'Server' header but not the Kong 'Via' header", function() + it("should return Kong 'Server' header but not the Kong 'Via' header when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "via" + assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -120,33 +122,33 @@ describe("Server Tokens", function() teardown(helpers.stop_kong) - it("should not return Kong 'Via' header but it should forward the 'Server' header", function() + it("should not return Kong 'Via' header but it should forward the 'Server' header when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) + assert.res_status(200, res) assert.response(res).has.header "server" assert.response(res).has_not.header "via" assert.not_equal(default_server_header, res.headers["server"]) end) - it("should not return Kong 'Server' or 'Via' headers", function() + it("should not return Kong 'Server' or 'Via' headers when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header "server" - assert.response(res).has_not.header "via" + assert.res_status(404, res) + assert.equal(nil, res.headers["server"]) + assert.equal(nil, res.headers["via"]) end) end) @@ -174,32 +176,32 @@ describe("Latency Tokens", function() teardown(helpers.stop_kong) - it("should be returned", function() + it("should be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) - assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -213,32 +215,32 @@ describe("Latency Tokens", function() teardown(helpers.stop_kong) - it("should be returned", function() + it("should be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com" } }) - assert.response(res).has.status(200) - assert.response(res).has.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -254,32 +256,32 @@ describe("Latency Tokens", function() helpers.stop_kong() end) - it("should not be returned", function() + it("should not be returned when request was proxied", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "inexistent.com" + host = "header-inspect.com", } }) - assert.response(res).has.status(200) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(200, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) - it("should not be returned", function() + it("should not be returned when no API matched (no proxy)", function() local res = assert(client:send { method = "GET", path = "/get", headers = { - host = "404.com" + host = "404.com", } }) - assert.response(res).has.status(404) - assert.response(res).has_not.header(constants.HEADERS.UPSTREAM_LATENCY) - assert.response(res).has_not.header(constants.HEADERS.PROXY_LATENCY) + assert.res_status(404, res) + assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) From 1870c7945b59f342566ce566521eaf56d25d2036 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 22:06:03 +0300 Subject: [PATCH 06/10] Fix issues based on review comments by @thibaultcha See: https://github.com/Mashape/kong/pull/2259#commitcomment-21506437 --- .../05-proxy/10-server_tokens_spec.lua | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua index 5fe821e6fcd9..f60764044275 100644 --- a/spec/02-integration/05-proxy/10-server_tokens_spec.lua +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -69,7 +69,7 @@ describe("Server Tokens", function() assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["via"]) end) end) @@ -108,7 +108,7 @@ describe("Server Tokens", function() assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["via"]) end) end) @@ -147,8 +147,8 @@ describe("Server Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["server"]) + assert.is_nil(res.headers["via"]) end) end) @@ -186,8 +186,8 @@ describe("Latency Tokens", function() }) assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -200,8 +200,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -225,8 +225,8 @@ describe("Latency Tokens", function() }) assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -239,8 +239,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -266,8 +266,8 @@ describe("Latency Tokens", function() }) assert.res_status(200, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -280,8 +280,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) From 59bfa286907feef49d87057e5950211b0c8faf91 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Mon, 27 Mar 2017 22:06:03 +0300 Subject: [PATCH 07/10] Fix issues based on review comments by @thibaultcha See: https://github.com/Mashape/kong/pull/2259#commitcomment-21506437 --- .../05-proxy/10-server_tokens_spec.lua | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/spec/02-integration/05-proxy/10-server_tokens_spec.lua b/spec/02-integration/05-proxy/10-server_tokens_spec.lua index 5fe821e6fcd9..fc3081f6c354 100644 --- a/spec/02-integration/05-proxy/10-server_tokens_spec.lua +++ b/spec/02-integration/05-proxy/10-server_tokens_spec.lua @@ -11,7 +11,7 @@ local function start(config) name = "api-1", upstream_url = "http://localhost:9999/headers-inspect", hosts = { - "header-inspect.com", + "headers-inspect.com", } } @@ -49,7 +49,7 @@ describe("Server Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com", + host = "headers-inspect.com", } }) @@ -69,7 +69,7 @@ describe("Server Tokens", function() assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["via"]) end) end) @@ -88,7 +88,7 @@ describe("Server Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com", + host = "headers-inspect.com", } }) @@ -108,7 +108,7 @@ describe("Server Tokens", function() assert.res_status(404, res) assert.equal(default_server_header, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["via"]) end) end) @@ -127,7 +127,7 @@ describe("Server Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com", + host = "headers-inspect.com", } }) @@ -147,8 +147,8 @@ describe("Server Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers["server"]) - assert.equal(nil, res.headers["via"]) + assert.is_nil(res.headers["server"]) + assert.is_nil(res.headers["via"]) end) end) @@ -181,13 +181,13 @@ describe("Latency Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com", + host = "headers-inspect.com", } }) assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -200,8 +200,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -220,13 +220,13 @@ describe("Latency Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com" + host = "headers-inspect.com" } }) assert.res_status(200, res) - assert.not_equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.not_equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_not_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -239,8 +239,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) @@ -261,13 +261,13 @@ describe("Latency Tokens", function() method = "GET", path = "/get", headers = { - host = "header-inspect.com", + host = "headers-inspect.com", } }) assert.res_status(200, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) it("should not be returned when no API matched (no proxy)", function() @@ -280,8 +280,8 @@ describe("Latency Tokens", function() }) assert.res_status(404, res) - assert.equal(nil, res.headers[constants.HEADERS.UPSTREAM_LATENCY]) - assert.equal(nil, res.headers[constants.HEADERS.PROXY_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.UPSTREAM_LATENCY]) + assert.is_nil(res.headers[constants.HEADERS.PROXY_LATENCY]) end) end) From 7f10580f9991f5f4f4c2da59f7cee8f1c449e8b3 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Tue, 28 Mar 2017 09:29:24 +0300 Subject: [PATCH 08/10] Hide "Server" header also from possible error page. --- kong/core/error_handlers.lua | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/kong/core/error_handlers.lua b/kong/core/error_handlers.lua index 751ee683b169..69f6e2b79bbc 100644 --- a/kong/core/error_handlers.lua +++ b/kong/core/error_handlers.lua @@ -1,3 +1,5 @@ +local singletons = require "kong.singletons" + local find = string.find local format = string.format @@ -52,7 +54,10 @@ return function(ngx) local status = ngx.status message = BODIES["s"..status] and BODIES["s"..status] or format(BODIES.default, status) - ngx.header["Server"] = SERVER_HEADER + if singletons.configuration.server_tokens then + ngx.header["Server"] = SERVER_HEADER + end + ngx.header["Content-Type"] = content_type ngx.say(format(template, message)) -end \ No newline at end of file +end From 0372b3898b9c21382f7c0c317c4b93630d4d2c49 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Tue, 28 Mar 2017 15:44:00 +0300 Subject: [PATCH 09/10] Add `server_tokens` and `latency_tokens` to changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 458436335c3b..d0b456f70fbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ### Added +- :fireworks: `server_tokens` and `latency_tokens` configuration fields. + Check the [0.10 Configuration Guide](https://getkong.org/docs/0.10.x/configuration/#server_tokens) + to learn more. + [#2259](https://github.com/Mashape/kong/pull/2259) - Plugins: - cors: Support for configuring multiple Origin domains. [#2203](https://github.com/Mashape/kong/pull/2203) From d769504d54ed2af8923f0e78dba22740516bebf1 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Thu, 30 Mar 2017 01:24:10 +0300 Subject: [PATCH 10/10] Fix review comment: https://github.com/Mashape/kong/pull/2259#pullrequestreview-29855996 --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0b456f70fbe..cdae14ba9e15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,9 @@ ### Added -- :fireworks: `server_tokens` and `latency_tokens` configuration fields. - Check the [0.10 Configuration Guide](https://getkong.org/docs/0.10.x/configuration/#server_tokens) - to learn more. +- Ability to hide Kong-specific response headers. Two new configuration fields: + `server_tokens` and `latency_tokens` will respectively toggle whether the + `Server` and `X-Kong-*-Latency` headers should be sent to downstream clients. [#2259](https://github.com/Mashape/kong/pull/2259) - Plugins: - cors: Support for configuring multiple Origin domains.