Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Merged
merged 1 commit into from

3 participants

@spastorino
Owner

Related ...

rails/rails#7556
and
#418

@rkh rkh merged commit ccfdaf3 into rack:master

1 check passed

Details default The Travis build passed
@gioele gioele commented on the diff
lib/rack/multipart/parser.rb
((5 lines not shown))
@io = @env['rack.input']
@io.rewind
@boundary_size = Utils.bytesize(@boundary) + EOL.size
- @content_length -= @boundary_size
+ if @content_length = @env['CONTENT_LENGTH']
@gioele
gioele added a note

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gioele gioele commented on the diff
lib/rack/multipart/parser.rb
@@ -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)
@gioele
gioele added a note

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 13 additions and 4 deletions.
  1. +6 −4 lib/rack/multipart/parser.rb
  2. +7 −0 test/spec_multipart.rb
View
10 lib/rack/multipart/parser.rb
@@ -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']
@gioele
gioele added a note

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ @content_length = @content_length.to_i
+ @content_length -= @boundary_size
+ end
true
end
@@ -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)
@gioele
gioele added a note

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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]
View
7 test/spec_multipart.rb
@@ -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
Something went wrong with that request. Please try again.