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

bugfix: read the request body from the temporary file if it was cached. #1863

Merged
merged 7 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
9 changes: 6 additions & 3 deletions apisix/admin/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,13 @@ local function run()
core.response.exit(404)
end

ngx.req.read_body()
local req_body = ngx.req.get_body_data()

local req_body = core.request.get_body()
if req_body then
if #req_body > 1024 * 1024 * 1.5 then
Copy link
Member

Choose a reason for hiding this comment

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

We need to detect the size (use stat, for example) before reading it. It doesn't make sense to do the check if we have already read the very BIG file.

Copy link
Member

Choose a reason for hiding this comment

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

agreed.

core.log.error("maximum request body is 1.5mb, but got ", #req_body)
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, we can use MiB to indicate the size is 1024 base.

core.response.exit(400, {error_msg = "request body is too large"})
end

local data, err = core.json.decode(req_body)
if not data then
core.log.error("invalid request body: ", req_body, " err: ", err)
Expand Down
38 changes: 36 additions & 2 deletions apisix/core/request.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ local tonumber = tonumber
local error = error
local type = type
local str_fmt = string.format
local io_open = io.open
local req_read_body = ngx.req.read_body
local req_get_body_data = ngx.req.get_body_data
local req_get_body_file = ngx.req.get_body_file

local _M = {version = 0.1}

local _M = {}


local function _headers(ctx)
Expand Down Expand Up @@ -94,7 +99,36 @@ function _M.get_remote_client_port(ctx)
ctx = ngx.ctx.api_ctx
end
return tonumber(ctx.var.remote_port)
end
end


local function get_file(file_name)
local f = io_open(file_name, 'r')
Copy link
Member

Choose a reason for hiding this comment

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

Better to check the second error returned value before using the f.

if f then
local req_body = f:read("*all")
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

f:close()
return req_body
end

return
end


function _M.get_body()
req_read_body()

local req_body = req_get_body_data()
if req_body then
return req_body
end

local file_name = req_get_body_file()
if file_name then
req_body = get_file(file_name)
end

return req_body
end


return _M
2 changes: 1 addition & 1 deletion rockspec/apisix-master-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ dependencies = {
"luafilesystem = 1.7.0-2",
"lua-tinyyaml = 0.1",
"lua-resty-prometheus = 1.1",
"jsonschema = 0.8",
"jsonschema = 0.9",
Copy link
Member

Choose a reason for hiding this comment

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

should not modify this line

"lua-resty-ipmatcher = 0.6",
"lua-resty-kafka = 0.07",
"lua-resty-logger-socket = 2.0-0",
Expand Down
2 changes: 1 addition & 1 deletion t/admin/health-check.t
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ GET /t
GET /t
--- error_code: 400
--- response_body
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"healthy\" validation failed: property \"http_statuses\" validation failed: expected unique items but items 2 and 1 are equal"}
{"error_msg":"invalid configuration: property \"upstream\" validation failed: property \"checks\" validation failed: property \"active\" validation failed: property \"healthy\" validation failed: property \"http_statuses\" validation failed: expected unique items but items 1 and 2 are equal"}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this test case?

--- no_error_log
[error]

Expand Down
79 changes: 78 additions & 1 deletion t/admin/routes.t
Original file line number Diff line number Diff line change
Expand Up @@ -1882,6 +1882,83 @@ GET /t
}
}
--- request
GET /t
GET /t
--- response_headers
Content-Type: application/json



=== TEST 52: set route with size 35k
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test

local core = require("apisix.core")
local uris = {}
for i = 1, 1000 * 4 do
uris[i] = "/" .. i
end
local req_body = [[{
"upstream": {
"nodes": {
"127.0.0.1:8080": 1
},
"type": "roundrobin"
},
"uris": ]] .. core.json.encode(uris) .. [[
}]]

local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT, req_body)

if code >= 300 then
ngx.status = code
end

ngx.say("req size: ", #req_body)
ngx.say(body)
}
}
--- request
GET /t
--- response_body
req size: 35121
passed
--- error_log
a client request body is buffered to a temporary file
--- timeout: 10



=== TEST 53: route size more than 1.5mb
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local s = string.rep( "a", 1024 * 1024 * 1.6 )
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"upstream": {
"nodes": {
"127.0.0.1:8080": 1
},
"type": "roundrobin"
},
"desc": "]] .. s .. [[",
"uri": "/index.html"
}]]
)

ngx.status = code
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"request body is too large"}
--- error_log
maximum request body is 1.5mb, but got 1678025