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

Make multipart parser not depend on having a Content-Length header #432

Merged
merged 1 commit into from Sep 17, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions lib/rack/multipart/parser.rb
Expand Up @@ -48,13 +48,15 @@ def setup_parse
@buf = ""
@params = Utils::KeySpaceConstrainedParams.new

@content_length = @env['CONTENT_LENGTH'].to_i
@io = @env['rack.input']
@io.rewind

@boundary_size = Utils.bytesize(@boundary) + EOL.size

@content_length -= @boundary_size
if @content_length = @env['CONTENT_LENGTH']
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you use

@content_length = @env['CONTENT_LENGTH']
@content_length = @content_length.to_i - @boundary_size unless @content_length.nil?

Using if together with assignment is very confusing.

@content_length = @content_length.to_i
@content_length -= @boundary_size
end
true
end

Expand Down Expand Up @@ -104,11 +106,11 @@ def get_current_head_and_filename_and_content_type_and_name_and_body
body << @buf.slice!(0, @buf.size - (@boundary_size+4))
end

content = @io.read(BUFSIZE < @content_length ? BUFSIZE : @content_length)
content = @io.read(@content_length && BUFSIZE >= @content_length ? @content_length : BUFSIZE)
Copy link
Contributor

Choose a reason for hiding this comment

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

The comparison is getting quite complex; could you simplify the code into

acceptable_length = @content_length && BUFSIZE >= @content_length 
content = @io.read(acceptable_length ? @content_length : BUFSIZE)

raise EOFError, "bad content body" if content.nil? || content.empty?

@buf << content
@content_length -= content.size
@content_length -= content.size if @content_length
end

[head, filename, content_type, name, body]
Expand Down
7 changes: 7 additions & 0 deletions test/spec_multipart.rb
Expand Up @@ -360,4 +360,11 @@ def multipart_file(name)
params.should.equal({"description"=>"Very very blue"})
end

should "parse multipart upload with no content-length header" do
env = Rack::MockRequest.env_for '/', multipart_fixture(:webkit)
env['CONTENT_TYPE'] = "multipart/form-data; boundary=----WebKitFormBoundaryWLHCs9qmcJJoyjKR"
env.delete 'CONTENT_LENGTH'
params = Rack::Multipart.parse_multipart(env)
params['profile']['bio'].should.include 'hello'
end
end