Permalink
Browse files

Use equal? to compare form_input to rack.input

Using equal? provides consistent results of equality between 1.8, 1.9,
and 2.0 when comparing Tempfile objects.

In 1.8, == will change the position of the Tempfile.
In 1.9+, == compares Tempfiles correctly.

In 1.8, eql? compares Tempfiles correctly
In 1.9+, t.eql?(t) always returns false
  • Loading branch information...
statianzo committed Jul 18, 2013
1 parent 6829a8a commit b0593078ce792a380779528a6a135c066aa03515
Showing with 16 additions and 1 deletion.
  1. +1 −1 lib/rack/request.rb
  2. +15 −0 test/spec_request.rb
View
@@ -204,7 +204,7 @@ def GET
def POST
if @env["rack.input"].nil?
raise "Missing rack.input"
- elsif @env["rack.request.form_input"].eql? @env["rack.input"]
+ elsif @env["rack.request.form_input"].equal? @env["rack.input"]
@env["rack.request.form_hash"]
elsif form_data? || parseable_data?
unless @env["rack.request.form_hash"] = parse_multipart(env)
View
@@ -878,6 +878,21 @@
lambda{ req.POST }.should.not.raise("input re-processed!")
end
+ should "use form_hash when form_input is a Tempfile" do
+ input = "{foo: 'bar'}"
+
+ rack_input = Tempfile.new("rackspec")
+ rack_input.write(input)
+ rack_input.rewind
+
+ req = Rack::Request.new Rack::MockRequest.env_for("/",
+ "rack.request.form_hash" => {'foo' => 'bar'},
+ "rack.request.form_input" => rack_input,
+ :input => rack_input)
+
+ req.POST.should.equal(req.env['rack.request.form_hash'])
+ end
+
should "conform to the Rack spec" do
app = lambda { |env|
content = Rack::Request.new(env).POST["file"].inspect

6 comments on commit b059307

@gucki

This comment has been minimized.

Show comment Hide comment
@gucki

gucki Jan 21, 2014

Please release 1.5.4 with this fixed. Withou this fix, the request body for all requests which use a temp file (all requests larger 128 kb?) fails to be properly parsed. One project hit by this bug was our gitlab ci installation for example. Pinning rack to master in the Gemfile does not work, as rails depends on 1.5.x but master has version 1.6.0.alpha.

Please release 1.5.4 with this fixed. Withou this fix, the request body for all requests which use a temp file (all requests larger 128 kb?) fails to be properly parsed. One project hit by this bug was our gitlab ci installation for example. Pinning rack to master in the Gemfile does not work, as rails depends on 1.5.x but master has version 1.6.0.alpha.

@myitcv

This comment has been minimized.

Show comment Hide comment
@myitcv

myitcv Jan 27, 2014

+1 on an updated release to fix this

+1 on an updated release to fix this

@mikkabouzu

This comment has been minimized.

Show comment Hide comment
@mikkabouzu

mikkabouzu Feb 21, 2014

+1 on an updated release to fix this

+1 on an updated release to fix this

@spastorino

This comment has been minimized.

Show comment Hide comment
@spastorino

spastorino Feb 23, 2014

Member

I think we should backport this fix to rack-1.5 /cc @raggi

Member

spastorino replied Feb 23, 2014

I think we should backport this fix to rack-1.5 /cc @raggi

@filipegiusti

This comment has been minimized.

Show comment Hide comment
@filipegiusti

filipegiusti Nov 13, 2014

@spastorino please just merge the backport #756

@spastorino please just merge the backport #756

@arangamani

This comment has been minimized.

Show comment Hide comment
@arangamani

arangamani Jan 8, 2015

Thanks for this patch! Took me a while to figure out why I lose all my large POST's payload. I see that the backport was merged. Is it possible to get a new 1.5.x version that includes this patch?

Thanks for this patch! Took me a while to figure out why I lose all my large POST's payload. I see that the backport was merged. Is it possible to get a new 1.5.x version that includes this patch?

Please sign in to comment.