Skip to content

Commit

Permalink
Make TempfileReaper delete temp files if application raises an exception
Browse files Browse the repository at this point in the history
Previously, tempfiles were only deleted if the exception occurred
when iterating over the body, not when raised during app.call.

Fixes rack#1679.
  • Loading branch information
jeremyevans committed Jul 13, 2020
1 parent e15eafc commit 488820c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. For info on

### Fixed

- TempfileReaper now deletes temp files if application raises an exception. ([#1679](https://github.com/rack/rack/issues/1679), [@jeremyevans](https://github.com/jeremyevans))
- Fix using Rack::Session::Cookie with coder: Rack::Session::Cookie::Base64::{JSON,Zip}. ([#1666](https://github.com/rack/rack/issues/1666), [@jeremyevans](https://github.com/jeremyevans))
- Avoid NoMethodError when accessing Rack::Session::Cookie without requiring delegate first. ([#1610](https://github.com/rack/rack/issues/1610), [@onigra](https://github.com/onigra))
- Handle cookies with values that end in '=' ([#1645](https://github.com/rack/rack/pull/1645), [@lukaso](https://github.com/lukaso))
Expand Down
9 changes: 8 additions & 1 deletion lib/rack/tempfile_reaper.rb
Expand Up @@ -12,7 +12,14 @@ def initialize(app)

def call(env)
env[RACK_TEMPFILES] ||= []
status, headers, body = @app.call(env)

begin
status, headers, body = @app.call(env)
rescue Exception
env[RACK_TEMPFILES].each(&:close!) unless env[RACK_TEMPFILES].nil?
raise
end

body_proxy = BodyProxy.new(body) do
env[RACK_TEMPFILES].each(&:close!) unless env[RACK_TEMPFILES].nil?
end
Expand Down
18 changes: 18 additions & 0 deletions test/spec_tempfile_reaper.rb
Expand Up @@ -30,6 +30,24 @@ def call(app)
response[0].must_equal 200
end

it 'close env[rack.tempfiles] when app raises an error' do
tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new
@env['rack.tempfiles'] = [ tempfile1, tempfile2 ]
app = lambda { |_| raise 'foo' }
proc{call(app)}.must_raise RuntimeError
tempfile1.closed.must_equal true
tempfile2.closed.must_equal true
end

it 'close env[rack.tempfiles] when app raises an non-StandardError' do
tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new
@env['rack.tempfiles'] = [ tempfile1, tempfile2 ]
app = lambda { |_| raise LoadError, 'foo' }
proc{call(app)}.must_raise LoadError
tempfile1.closed.must_equal true
tempfile2.closed.must_equal true
end

it 'close env[rack.tempfiles] when body is closed' do
tempfile1, tempfile2 = MockTempfile.new, MockTempfile.new
@env['rack.tempfiles'] = [ tempfile1, tempfile2 ]
Expand Down

0 comments on commit 488820c

Please sign in to comment.