From 488820cd79d15a1b1a6d6c1b1d68a9e680f7654f Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 13 Jul 2020 14:35:36 -0700 Subject: [PATCH] Make TempfileReaper delete temp files if application raises an exception Previously, tempfiles were only deleted if the exception occurred when iterating over the body, not when raised during app.call. Fixes #1679. --- CHANGELOG.md | 1 + lib/rack/tempfile_reaper.rb | 9 ++++++++- test/spec_tempfile_reaper.rb | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4faeef9d..6f024e744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/lib/rack/tempfile_reaper.rb b/lib/rack/tempfile_reaper.rb index 9b04fefc2..12500f0a9 100644 --- a/lib/rack/tempfile_reaper.rb +++ b/lib/rack/tempfile_reaper.rb @@ -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 diff --git a/test/spec_tempfile_reaper.rb b/test/spec_tempfile_reaper.rb index 063687a09..4756e53ea 100644 --- a/test/spec_tempfile_reaper.rb +++ b/test/spec_tempfile_reaper.rb @@ -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 ]