Duplicate temp files created when handling uploads

[wkrsz]

https://github.com/rack/rack/blob/master/lib/rack/methodoverride.rb#L25

In Rack::MethodOverride#method_override a Rack::Request instance is being created, which involves storing uploaded file in a temp file. This Rack::Request instance is only used to check for request_method input and then is abandoned, along with temp file.

Rails then creates its own Rack::Request instance (with a new temp file). So each uploaded file causes two identical temp files to appear.

Only upon next Garbage Collection Rack::Request instances are swept and temp files deleted (https://groups.google.com/d/topic/rack-devel/brK8eh-MByw/discussion).

[raggi]

Yeah, we've wanted to memoize Rack::Request objects for a while. This was removed some time ago because it was no implemented correctly. We can likely move to a model where Rack::Request memoizes into self.class.name, but we'd have to be sure we're not breaking existing frameworks. I will defer until 2.0 unless someone sends through a working patch tested against common frameworks.

[janko]

In Rack::MethodOverride#method_override a Rack::Request instance is being created, which involves storing uploaded file in a temp file. This Rack::Request instance is only used to check for request_method input and then is abandoned, along with temp file.

I don't know what was in 2014, but now the parsed multipart body is cached in the env hash (in rack.request.form_hash), so the multipart parsing happens only once even if you instantiate multiple Rack::Request objects. The following script proves it:

gem "rack-test_app"

require "rack"
require "rack/test_app"

app = Rack::Builder.new do
  use Rack::MethodOverride
  run ->(env) {
    request = Rack::Request.new(env)
    p request.env["rack.tempfiles"]
    request.params
    p request.env["rack.tempfiles"]

    [200, {}, []]
  }
end

test_app = Rack::TestApp.wrap(app)
test_app.post("/", multipart: {file: File.open(__FILE__)})

[#<Tempfile:/var/folders/k7/6zx6dx6x7ys3rv3srh0nyfj00000gn/T/RackMultipart20170412-93100-praly8.rb>]
[#<Tempfile:/var/folders/k7/6zx6dx6x7ys3rv3srh0nyfj00000gn/T/RackMultipart20170412-93100-praly8.rb>]

Only upon next Garbage Collection Rack::Request instances are swept and temp files deleted

This isn't the behaviour anymore since the addition of the Rack::TempfileReaper middleware, which deletes tempfiles created during multipart parsing at the end of the request.

Since Rack doesn't have the problems mentioned in this issue anymore, I think this issue can be closed.