Skip to content

Added request body buffer regeneration #43

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

Closed
wants to merge 8 commits into from
Closed

Added request body buffer regeneration #43

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
74bfc12
Added request body buffer regeneration
dylandreimerink Mar 29, 2018
4ce90bb
Fixed missing \r\n's and first boundary
dylandreimerink Mar 29, 2018
ce8ddd8
Fixed typo and removed unnecessary concatenation
dylandreimerink Apr 2, 2018
f9c3a30
Replaced multiple body append by a string buffer and single body append
dylandreimerink Apr 4, 2018
ef5cb46
Cleaned up unnecessary code
dylandreimerink Apr 7, 2018
458fb3a
Removed body_buffer variable
dylandreimerink Apr 7, 2018
79836a0
Made code standards changes
dylandreimerink Apr 28, 2018
873f235
Changed elseif to else if
dylandreimerink Apr 28, 2018
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
84 changes: 82 additions & 2 deletions lib/resty/upload.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ local match = string.match
local setmetatable = setmetatable
local type = type
local ngx_var = ngx.var
local ngx_init_body = ngx.req.init_body
local ngx_finish_body = ngx.req.finish_body
local ngx_append_body = ngx.req.append_body
-- local print = print


Expand Down Expand Up @@ -46,7 +49,7 @@ local function get_boundary()
end


function _M.new(self, chunk_size, max_line_size)
function _M.new(self, chunk_size, max_line_size, restore_body_buffer)
local boundary = get_boundary()

-- print("boundary: ", boundary)
Expand All @@ -62,20 +65,34 @@ function _M.new(self, chunk_size, max_line_size)
return nil, err
end

if restore_body_buffer then
ngx_init_body(chunk_size)
end

local read2boundary, err = sock:receiveuntil("--" .. boundary)
if not read2boundary then
if restore_body_buffer then
ngx_finish_body()
end

return nil, err
end

local read_line, err = sock:receiveuntil("\r\n")
if not read_line then

if restore_body_buffer then
ngx_finish_body()
Copy link
Member

Choose a reason for hiding this comment

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

The previous read, on line 72, was successful, but we didn't write the request body back, is this intended? Maybe there are many similar cases in code, but no easy way to solve? E.g. on errors we don't write the request body back.

Copy link
Author

Choose a reason for hiding this comment

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

The sock:receiveuntil returns an iterator Lua function and doesn't actually read data from the buffer so there is noting to restore.

end

return nil, err
end

return setmetatable({
sock = sock,
size = chunk_size or CHUNK_SIZE,
line_size = max_line_size or MAX_LINE_SIZE,
restore_body_buffer = restore_body_buffer,
read2boundary = read2boundary,
read_line = read_line,
boundary = boundary,
Expand All @@ -99,15 +116,30 @@ local function discard_line(self)

local line, err = read_line(self.line_size)
if not line then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, err
end

if self.restore_body_buffer then
ngx_append_body(line .. "\r\n")
Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine, but you could also have:

ngx_append_body(line)
ngx_append_body("\r\n")

Copy link
Author

Choose a reason for hiding this comment

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

I don't see any point in not concatenating the string here, the same goes for the rest of the remarks regarding concatenation.

end

local dummy, err = read_line(1)
if dummy then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, "line too long: " .. line .. dummy .. "..."
end

if err then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.

return nil, err
end

Expand All @@ -122,6 +154,10 @@ local function discard_rest(self)
while true do
local dummy, err = sock:receive(size)
if err and err ~= 'closed' then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, err
end

Expand All @@ -137,6 +173,10 @@ local function read_body_part(self)

local chunk, err = read2boundary(self.size)
if err then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, nil, err
end

Expand All @@ -145,6 +185,10 @@ local function read_body_part(self)

local data = sock:receive(2)
if data == "--" then
if self.restore_body_buffer then
ngx_append_body("\r\n--" .. self.boundary .. "--\r\n")
Copy link
Member

Choose a reason for hiding this comment

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

Same here:

ngx_append_body("\r\n--")
ngx_append_body(self.boundary)
ngx_append_body(ngx_append_body("--\r\n"))

Not a big deal.

end

local ok, err = discard_rest(self)
if not ok then
return nil, nil, err
Expand All @@ -161,10 +205,18 @@ local function read_body_part(self)
end
end

if self.restore_body_buffer then
ngx_append_body("\r\n--" .. self.boundary .. "\r\n")
Copy link
Member

Choose a reason for hiding this comment

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

This coud be cached before the while loop:

local boundary_delimiter = "\r\n--" .. self.boundary .. "\r\n"

And then here:

ngx_append_body(boundary_delimiter)

Copy link
Author

Choose a reason for hiding this comment

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

There is no while loop surrounding this line

end

self.state = STATE_READING_HEADER
return "part_end"
end

if self.restore_body_buffer then
ngx_append_body(chunk)
end

return "body", chunk
end

Expand All @@ -174,15 +226,31 @@ local function read_header(self)

local line, err = read_line(self.line_size)
if err then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, nil, err
end

if self.restore_body_buffer then
ngx_append_body(line .. "\r\n")
end

local dummy, err = read_line(1)
if dummy then
if self.restore_body_buffer then
ngx_finish_body()
Copy link
Member

Choose a reason for hiding this comment

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

do we also need to restore that dummy?

Copy link
Author

Choose a reason for hiding this comment

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

The dummy is there to detect if the line we are trying to read exceeded the size of self.line_size. When this happens the request can't be processed further. This means that the remainder of the request will not be restored anyway. So there is no point is appending the one byte.

end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, nil, "line too long: " .. line .. dummy .. "..."
end

if err then
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return nil, nil, err
end

Expand All @@ -203,7 +271,11 @@ local function read_header(self)
end


local function eof()
local function eof(self)
if self.restore_body_buffer then
ngx_finish_body()
end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


return "eof", nil
end

Expand Down Expand Up @@ -232,7 +304,15 @@ local function read_preamble(self)
while true do
local preamble = read2boundary(size)
if not preamble then
if self.restore_body_buffer then
ngx_append_body("--" .. self.boundary)
Copy link
Member

Choose a reason for hiding this comment

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

Ditto here:

ngx_append_body("--")
ngx_append_body(self.boundary)

Yes, bikeshedding :-).

end
Copy link
Member

Choose a reason for hiding this comment

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

new line after this.


break

else if self.restore_body_buffer then
ngx_append_body(preamble)
end
end

-- discard the preamble data chunk
Expand Down