Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rate-limiting,response-ratelimiting) config option: hide/omit rate headers sent to client #2087

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions kong/plugins/rate-limiting/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,11 @@ function RateLimitingHandler:access(conf)

if usage then
-- Adding headers
for k, v in pairs(usage) do
ngx.header[RATELIMIT_LIMIT.."-"..k] = v.limit
ngx.header[RATELIMIT_REMAINING.."-"..k] = math.max(0, (stop == nil or stop == k) and v.remaining - 1 or v.remaining) -- -increment_value for this current request
if not conf.hide_client_headers then
for k, v in pairs(usage) do
ngx.header[RATELIMIT_LIMIT.."-"..k] = v.limit
ngx.header[RATELIMIT_REMAINING.."-"..k] = math.max(0, (stop == nil or stop == k) and v.remaining - 1 or v.remaining) -- -increment_value for this current request
end
end

-- If limit is exceeded, terminate the request
Expand Down
3 changes: 2 additions & 1 deletion kong/plugins/rate-limiting/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ return {
redis_port = { type = "number", default = 6379 },
redis_password = { type = "string" },
redis_timeout = { type = "number", default = 2000 },
redis_database = { type = "number", default = 0 }
redis_database = { type = "number", default = 0 },
hide_client_headers = { type = "boolean", default = false },
},
self_check = function(schema, plugin_t, dao, is_update)
local ordered_periods = { "second", "minute", "hour", "day", "month", "year"}
Expand Down
6 changes: 4 additions & 2 deletions kong/plugins/response-ratelimiting/header_filter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ function _M.execute(conf)
local stop
for limit_name, v in pairs(usage) do
for period_name, lv in pairs(usage[limit_name]) do
ngx.header[RATELIMIT_LIMIT.."-"..limit_name.."-"..period_name] = lv.limit
ngx.header[RATELIMIT_REMAINING.."-"..limit_name.."-"..period_name] = math_max(0, lv.remaining - (increments[limit_name] and increments[limit_name] or 0)) -- increment_value for this current request
if not conf.hide_client_headers then
ngx.header[RATELIMIT_LIMIT.."-"..limit_name.."-"..period_name] = lv.limit
ngx.header[RATELIMIT_REMAINING.."-"..limit_name.."-"..period_name] = math_max(0, lv.remaining - (increments[limit_name] and increments[limit_name] or 0)) -- increment_value for this current request
end

if increments[limit_name] and increments[limit_name] > 0 and lv.remaining <= 0 then
stop = true -- No more
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought here: should exit early out of this loop (via break) if we are omitting headers, and we've reached this condition? Once we've assigned stop = true, we're not doing any other work aside from assigning headers, so breakingnoit early would save some work. Don't know if this is worth it though, so this isn't a blocker to me, just a thought.

Expand Down
3 changes: 2 additions & 1 deletion kong/plugins/response-ratelimiting/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ return {
year = { type = "number" }
}
}
}
},
hide_client_headers = { type = "boolean", default = false },
},
self_check = function(schema, plugin_t, dao, is_update)
if not plugin_t.limits or (not next(plugin_t.limits)) then
Expand Down
36 changes: 36 additions & 0 deletions spec/03-plugins/24-rate-limiting/04-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,26 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
}
})

local api5 = assert(helpers.dao.apis:insert {
name = "api-5",
hosts = { "test5.com" },
upstream_url = "http://mockbin.com"
})
assert(helpers.dao.plugins:insert {
name = "rate-limiting",
api_id = api5.id,
config = {
policy = policy,
minute = 6,
hide_client_headers = true,
fault_tolerant = false,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})

assert(helpers.start_kong())
end)

Expand Down Expand Up @@ -359,6 +379,22 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
end)
end)

describe("Config with hide_client_headers", function()
it("does not send rate-limit headers when hide_client_headers==true", function()
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
["Host"] = "test5.com"
}
})

assert.res_status(200, res)
assert.falsy(res.headers["x-ratelimit-limit-minute"])
assert.falsy(res.headers["x-ratelimit-remaining-minute"])
end)
end)

if policy == "cluster" then
describe("Fault tolerancy", function()

Expand Down
36 changes: 36 additions & 0 deletions spec/03-plugins/25-response-rate-limiting/04-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,26 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
}
})

api = assert(helpers.dao.apis:insert {
name = "test9_com",
hosts = { "test9.com" },
upstream_url = "http://httpbin.org"
})
assert(helpers.dao.plugins:insert {
name = "response-ratelimiting",
api_id = api.id,
config = {
fault_tolerant = false,
policy = policy,
hide_client_headers = true,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE,
limits = {video = {minute = 6}}
}
})

assert(helpers.start_kong())
end)

Expand Down Expand Up @@ -477,6 +497,22 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.equal([[{"message":"API rate limit exceeded for 'image'"}]], body)
end)

describe("Config with hide_client_headers", function()
it("does not send rate-limit headers when hide_client_headers==true", function()
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "test9.com"
}
})

assert.res_status(200, res)
assert.falsy(res.headers["x-ratelimit-remaining-video-minute"])
assert.falsy(res.headers["x-ratelimit-limit-video-minute"])
end)
end)

if policy == "cluster" then
describe("Fault tolerancy", function()

Expand Down