Merge remote branch 'lawrencepit/jsonp'

lib/rack/contrib/access.rb

@@ -70,7 +70,7 @@ def ipmasks_for_path(env)
     end
 
     def forbidden!
-      [403, { 'Content-Type' => 'text/html', 'Content-Length' => '0' }, '']
+      [403, { 'Content-Type' => 'text/html', 'Content-Length' => '0' }, []]
     end
 
     def ip_authorized?(ipmasks)

lib/rack/contrib/deflect.rb

@@ -63,7 +63,7 @@ def call env
     end
 
     def deflect!
-      [403, { 'Content-Type' => 'text/html', 'Content-Length' => '0' }, '']
+      [403, { 'Content-Type' => 'text/html', 'Content-Length' => '0' }, []]
     end
 
     def deflect? env

lib/rack/contrib/jsonp.rb

@@ -6,6 +6,11 @@ module Rack
   #
   class JSONP
 
+    CALLBACK_PARAM = 'callback'
+    HTTP_ACCEPT = 'HTTP_ACCEPT'.freeze
+    CONTENT_TYPE = 'Content-Type'.freeze
+    APPLICATION_JSON = /application\/json/.freeze
+
     def initialize(app)
       @app = app
     end
@@ -17,10 +22,13 @@ def initialize(app)
     #
     def call(env)
       status, headers, response = @app.call(env)
-      request = Rack::Request.new(env)
-      if request.params.include?('callback')
-        response = pad(request.params.delete('callback'), response)
-        headers['Content-Length'] = response.length.to_s
+      if env[HTTP_ACCEPT] =~ APPLICATION_JSON && headers[CONTENT_TYPE] =~ APPLICATION_JSON
+        request = Rack::Request.new(env)
+        if request.params.include?(CALLBACK_PARAM)
+          response = pad(request.params.delete(CALLBACK_PARAM), response)
+          headers['Content-Length'] = response.length.to_s
+          response = [response]
+        end
       end
       [status, headers, response]
     end

test/spec_rack_access.rb

@@ -5,7 +5,7 @@
 context "Rack::Access" do
 
   setup do
-    @app = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, 'hello'] }
+    @app = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['hello']] }
     @mock_addr_1 = '111.111.111.111'
     @mock_addr_2 = '192.168.1.222'
     @mock_addr_localhost = '127.0.0.1'
@@ -24,61 +24,61 @@ def middleware(options = {})
     app = middleware
     status, headers, body = app.call(mock_env(@mock_addr_1))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
   end
 
   specify "default configuration should allow requests from 127.0.0.1" do
     app = middleware
     status, headers, body = app.call(mock_env(@mock_addr_localhost))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
   end
 
   specify "should allow remote addresses in allow_ipmasking" do
     app = middleware('/' => [@mock_addr_1])
     status, headers, body = app.call(mock_env(@mock_addr_1))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
   end
 
   specify "should deny remote addresses not in allow_ipmasks" do
     app = middleware('/' => [@mock_addr_1])
     status, headers, body = app.call(mock_env(@mock_addr_2))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
   end
 
   specify "should allow remote addresses in allow_ipmasks range" do
     app = middleware('/' => [@mock_addr_range])
     status, headers, body = app.call(mock_env(@mock_addr_2))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
   end
 
   specify "should deny remote addresses not in allow_ipmasks range" do
     app = middleware('/' => [@mock_addr_range])
     status, headers, body = app.call(mock_env(@mock_addr_1))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
   end
 
   specify "should allow remote addresses in one of allow_ipmasking" do
     app = middleware('/' => [@mock_addr_range, @mock_addr_localhost])
 
     status, headers, body = app.call(mock_env(@mock_addr_2))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_localhost))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
   end
 
   specify "should deny remote addresses not in one of allow_ipmasks" do
     app = middleware('/' => [@mock_addr_range, @mock_addr_localhost])
     status, headers, body = app.call(mock_env(@mock_addr_1))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
   end
 
   specify "handles paths correctly" do
@@ -90,65 +90,65 @@ def middleware(options = {})
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/qux"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo/"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo/"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo/bar"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo/bar"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
     status, headers, body = app.call(mock_env(@mock_addr_2, "/foo/bar"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo/bar/"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo/bar/"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo///bar//quux"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo///bar//quux"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/foo/quux"))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/foo/quux"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
 
     status, headers, body = app.call(mock_env(@mock_addr_1, "/bar"))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
     status, headers, body = app.call(mock_env(@mock_addr_1, "/bar").merge('HTTP_HOST' => 'foo.org'))
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
     status, headers, body = app.call(mock_env(@mock_addr_localhost, "/bar").merge('HTTP_HOST' => 'foo.org'))
     status.should.equal 200
-    body.should.equal 'hello'
+    body.should.equal ['hello']
   end
 end

test/spec_rack_deflect.rb

@@ -5,7 +5,7 @@
 context "Rack::Deflect" do
 
   setup do
-    @app = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, 'cookies'] }
+    @app = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['cookies']] }
     @mock_addr_1 = '111.111.111.111'
     @mock_addr_2 = '222.222.222.222'
     @mock_addr_3 = '333.333.333.333'
@@ -23,7 +23,7 @@ def mock_deflect options = {}
     app = mock_deflect
     status, headers, body = app.call mock_env(@mock_addr_1)
     status.should.equal 200
-    body.should.equal 'cookies'
+    body.should.equal ['cookies']
   end
 
   specify "should deflect requests exceeding the request threshold" do
@@ -35,14 +35,14 @@ def mock_deflect options = {}
     5.times do
       status, headers, body = app.call env
       status.should.equal 200
-      body.should.equal 'cookies'
+      body.should.equal ['cookies']
     end
 
     # Remaining requests should fail for 10 seconds
     10.times do
       status, headers, body = app.call env
       status.should.equal 403
-      body.should.equal ''
+      body.should.equal []
     end
 
     # Log should reflect that we have blocked an address
@@ -58,13 +58,13 @@ def mock_deflect options = {}
     5.times do
       status, headers, body = app.call env
       status.should.equal 200
-      body.should.equal 'cookies'
+      body.should.equal ['cookies']
     end
 
     # Exceeds request threshold
     status, headers, body = app.call env
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
 
     # Allow block to expire
     sleep 3
@@ -73,7 +73,7 @@ def mock_deflect options = {}
     5.times do
       status, headers, body = app.call env
       status.should.equal 200
-      body.should.equal 'cookies'
+      body.should.equal ['cookies']
     end
 
     # Log should reflect block and release
@@ -88,7 +88,7 @@ def mock_deflect options = {}
     10.times do
       status, headers, body = app.call env
       status.should.equal 200
-      body.should.equal 'cookies'
+      body.should.equal ['cookies']
     end
   end
 
@@ -97,11 +97,11 @@ def mock_deflect options = {}
 
     status, headers, body = app.call mock_env(@mock_addr_1)
     status.should.equal 200
-    body.should.equal 'cookies'
+    body.should.equal ['cookies']
 
     status, headers, body = app.call mock_env(@mock_addr_2)
     status.should.equal 403
-    body.should.equal ''
+    body.should.equal []
   end
 
 end

test/spec_rack_jsonp.rb

@@ -8,27 +8,43 @@
     specify "should wrap the response body in the Javascript callback" do
       test_body = '{"bar":"foo"}'
       callback = 'foo'
-      app = lambda { |env| [200, {'Content-Type' => 'text/plain'}, [test_body]] }
-      request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+      app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+      request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}", 'HTTP_ACCEPT' => 'application/json')
       body = Rack::JSONP.new(app).call(request).last
-      body.should.equal "#{callback}(#{test_body})"
+      body.should.equal ["#{callback}(#{test_body})"]
     end
 
     specify "should modify the content length to the correct value" do
       test_body = '{"bar":"foo"}'
       callback = 'foo'
-      app = lambda { |env| [200, {'Content-Type' => 'text/plain'}, [test_body]] }
-      request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+      app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+      request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}", 'HTTP_ACCEPT' => 'application/json')
       headers = Rack::JSONP.new(app).call(request)[1]
       headers['Content-Length'].should.equal((test_body.length + callback.length + 2).to_s) # 2 parentheses
     end
   end
 
   specify "should not change anything if no callback param is provided" do
-    app = lambda { |env| [200, {'Content-Type' => 'text/plain'}, ['{"bar":"foo"}']] }
-    request = Rack::MockRequest.env_for("/", :params => "foo=bar")
+    app = lambda { |env| [200, {'Content-Type' => 'application/json'}, ['{"bar":"foo"}']] }
+    request = Rack::MockRequest.env_for("/", :params => "foo=bar", 'HTTP_ACCEPT' => 'application/json')
     body = Rack::JSONP.new(app).call(request).last
-    body.join.should.equal '{"bar":"foo"}'
+    body.should.equal ['{"bar":"foo"}']
+  end
+
+  specify "should not change anything if it's not a json request" do
+    test_body = '{"bar":"foo"}'
+    app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+    request = Rack::MockRequest.env_for("/", :params => "callback=foo", 'HTTP_ACCEPT' => 'application/html')
+    body = Rack::JSONP.new(app).call(request).last
+    body.should.equal [test_body]
+  end
+
+  specify "should not change anything if it's not a json response" do
+    test_body = '<html><body>404 Not Found</body></html>'
+    app = lambda { |env| [404, {'Content-Type' => 'text/html'}, [test_body]] }
+    request = Rack::MockRequest.env_for("/", :params => "callback=foo", 'HTTP_ACCEPT' => 'application/json')
+    body = Rack::JSONP.new(app).call(request).last
+    body.should.equal [test_body]
   end
 
 end