Merge branch 'fix/csrf_missing_close' of https://github.com/finnlabs/…

…rack-protection into finnlabs-fix/csrf_missing_close

lib/rack/protection/json_csrf.rb

@@ -17,9 +17,10 @@ def call(env)
         request               = Request.new(env)
         status, headers, body = app.call(env)
 
-        if has_vector? request, headers
+        if has_vector?(request, headers)
           warn env, "attack prevented by #{self.class}"
-          react(env) or [status, headers, body]
+
+          react_and_close(env, body) or [status, headers, body]
         else
           [status, headers, body]
         end
@@ -30,6 +31,18 @@ def has_vector?(request, headers)
         return false unless headers['Content-Type'].to_s.split(';', 2).first =~ /^\s*application\/json\s*$/
         origin(request.env).nil? and referrer(request.env) != request.host
       end
+
+      def react_and_close(env, body)
+        reaction = react(env)
+
+        close_body(body) if reaction
+
+        reaction
+      end
+
+      def close_body(body)
+        body.close if body.respond_to?(:close)
+      end
     end
   end
 end

spec/lib/rack/protection/json_csrf_spec.rb

@@ -1,6 +1,31 @@
 describe Rack::Protection::JsonCsrf do
   it_behaves_like "any rack application"
 
+  module DummyAppWithBody
+    module Closeable
+      def close
+        @closed = true
+      end
+
+      def closed?
+        @closed
+      end
+    end
+
+    def self.body
+      @body ||= begin
+        body = ['ok']
+        body.extend(Closeable)
+        body
+      end
+    end
+
+    def self.call(env)
+      Thread.current[:last_env] = env
+      [200, {'Content-Type' => 'application/json'}, body]
+    end
+  end
+
   describe 'json response' do
     before do
       mock_app { |e| [200, {'Content-Type' => 'application/json'}, []]}
@@ -10,6 +35,16 @@
       expect(get('/', {}, 'HTTP_REFERER' => 'http://evil.com')).not_to be_ok
     end
 
+    it "closes the body returned by the app if it denies the get request" do
+      mock_app DummyAppWithBody do |e|
+        [200, {'Content-Type' => 'application/json'}, []]
+      end
+
+      get('/', {}, 'HTTP_REFERER' => 'http://evil.com')
+
+      DummyAppWithBody.body.should be_closed
+    end
+
     it "accepts requests with json responses with a remote referrer when there's an origin header set" do
       expect(get('/', {}, 'HTTP_REFERER' => 'http://good.com', 'HTTP_ORIGIN' => 'http://good.com')).to be_ok
     end