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.

Sign up for GitHub

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

fix(plugin) fixes a wrong constant for the request size limit plugin #1416

Merged
merged 4 commits into from
Jul 20, 2016
Merged

fix(plugin) fixes a wrong constant for the request size limit plugin #1416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36ac7c6
fix(plugin) fixes a wrong constant for the request size limit plugin
Tieske Jul 18, 2016
3bfa718
purified a constant, excorcism completed
Tieske Jul 18, 2016
0b51f89
Merge branch 'next' of https://github.com/Mashape/kong into fix/reque…
Tieske Jul 18, 2016
c6545d2
fix linter errors
Tieske Jul 18, 2016
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
13 changes: 7 additions & 6 deletions kong/plugins/request-size-limiting/handler.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

local BasePlugin = require "kong.plugins.base_plugin"
local responses = require "kong.tools.responses"
local stringy = require "stringy"
local strip = require("pl.stringx").strip

local RequestSizeLimitingHandler = BasePlugin:extend()

RequestSizeLimitingHandler.PRIORITY = 950

local function check_size(length, allowed_size, headers)
local allowed_bytes_size = allowed_size * 100000
local allowed_bytes_size = allowed_size * 1000000
Copy link
Member

Choose a reason for hiding this comment

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

I find it much more convenient and requiring less cognitive load (as well as much less error prone) to declare constants representing size (including MBs) like so:

local CONST_NAME = X * 10^6 -- here X being 1

Then use CONST_NAME in the module's function.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated, including a linter fix to make ci run

if length > allowed_bytes_size then
if headers.expect and stringy.strip(headers.expect:lower()) == "100-continue" then
if headers.expect and strip(headers.expect:lower()) == "100-continue" then
return responses.send(417, "Request size limit exceeded")
else
return responses.send(413, "Request size limit exceeded")
Expand All @@ -26,14 +26,15 @@ end
function RequestSizeLimitingHandler:access(conf)
RequestSizeLimitingHandler.super.access(self)
local headers = ngx.req.get_headers()
if headers["content-length"] then
check_size(tonumber(headers["content-length"]), conf.allowed_payload_size, headers)
local cl = headers["content-length"]
if cl then
check_size(tonumber(cl), conf.allowed_payload_size, headers)
else
-- If the request body is too big, this could consume too much memory (to check)
ngx.req.read_body()
local data = ngx.req.get_body_data()
if data then
check_size(string.len(data), conf.allowed_payload_size, headers)
check_size(#data, conf.allowed_payload_size, headers)
end
end
end
Expand Down
12 changes: 7 additions & 5 deletions spec/03-plugins/request-size-limiting/01-access_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
local helpers = require "spec.helpers"

local TEST_SIZE = 1

describe("Plugin: request-size-limiting (access)", function()
local client
setup(function()
Expand All @@ -15,7 +17,7 @@ describe("Plugin: request-size-limiting (access)", function()
name = "request-size-limiting",
api_id = api.id,
config = {
allowed_payload_size = 10
allowed_payload_size = TEST_SIZE
}
})

Expand All @@ -29,7 +31,7 @@ describe("Plugin: request-size-limiting (access)", function()

describe("with Content-Length set", function()
it("allows request of lower size", function()
local body = "foo=test&bar=foobar"
local body = string.rep("a", TEST_SIZE * 1000000)

local res = assert(client:request {
method = "POST",
Expand All @@ -44,7 +46,7 @@ describe("Plugin: request-size-limiting (access)", function()
assert.res_status(200, res)
end)
it("blocks request exceeding size limit", function()
local body = string.rep("a", 11 * 2^20)
local body = string.rep("a", TEST_SIZE * 1000000 + 1)

local res = assert(client:send {
method = "POST",
Expand All @@ -63,7 +65,7 @@ describe("Plugin: request-size-limiting (access)", function()

describe("without Content-Length", function()
it("allows request of lower size", function()
local body = "foo=test&bar=foobar"
local body = string.rep("a", TEST_SIZE * 1000000)

local res = assert(client:request {
method = "POST",
Expand All @@ -77,7 +79,7 @@ describe("Plugin: request-size-limiting (access)", function()
assert.res_status(200, res)
end)
it("blocks request exceeding size limit", function()
local body = string.rep("a", 11 * 2^20)
local body = string.rep("a", TEST_SIZE * 1000000 + 1)

local res = assert(client:send {
method = "POST",
Expand Down