Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Rack::JSONP checks the callback parameters on non-JSON response types #43

Merged
merged 2 commits into from

2 participants

@benmanns

Currently, if Rack::JSONP is being used, a request to /widgets.html?callback=* will return a 400 "Bad Request" error. This is because the check for a valid callback is run before the response's content-type is checked. This patch moves the callback test after the JSON content-type test. If accepted, a request to /widgets.html?callback=* will return the content of the response, but a request to /widgets.json?callback=* (provided that /widgets.json returns application/json) will return 400 "Bad Request".

benmanns added some commits
@benmanns benmanns Add test cases for invalid callback with JSON and non-JSON content-ty…
…pes.
2a9014f
@benmanns benmanns Only check for invalid callbacks after finding response to be JSON.
Previously, a request to /widgets.html?callback=* would send a 400 "Bad Request" response to the client. This change only sends 400 "Bad Request" if the response type is JSON.
a80437b
@rkh rkh merged commit 551ae79 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 25, 2011
  1. @benmanns
  2. @benmanns

    Only check for invalid callbacks after finding response to be JSON.

    benmanns authored
    Previously, a request to /widgets.html?callback=* would send a 400 "Bad Request" response to the client. This change only sends 400 "Bad Request" if the response type is JSON.
This page is out of date. Refresh to see the latest.
Showing with 50 additions and 6 deletions.
  1. +4 −6 lib/rack/contrib/jsonp.rb
  2. +46 −0 test/spec_rack_jsonp.rb
View
10 lib/rack/contrib/jsonp.rb
@@ -23,16 +23,14 @@ def initialize(app)
def call(env)
request = Rack::Request.new(env)
- if has_callback?(request)
- callback = request.params['callback']
- return bad_request unless valid_callback?(callback)
- end
-
status, headers, response = @app.call(env)
headers = HeaderHash.new(headers)
- if is_json?(headers) && callback
+ if is_json?(headers) && has_callback?(request)
+ callback = request.params['callback']
+ return bad_request unless valid_callback?(callback)
+
response = pad(callback, response)
# No longer json, its javascript!
View
46 test/spec_rack_jsonp.rb
@@ -72,6 +72,52 @@
end
end
+ context 'but is invalid' do
+ context 'with content-type application/json' do
+ specify 'should return "Bad Request"' do
+ test_body = '{"bar":"foo"}'
+ callback = '*'
+ content_type = 'application/json'
+ app = lambda { |env| [200, {'Content-Type' => content_type}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ body = Rack::JSONP.new(app).call(request).last
+ body.should.equal ['Bad Request']
+ end
+
+ specify 'should return set the response code to 400' do
+ test_body = '{"bar":"foo"}'
+ callback = '*'
+ content_type = 'application/json'
+ app = lambda { |env| [200, {'Content-Type' => content_type}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ response_code = Rack::JSONP.new(app).call(request).first
+ response_code.should.equal 400
+ end
+ end
+
+ context 'with content-type text/plain' do
+ specify 'should return "Good Request"' do
+ test_body = 'Good Request'
+ callback = '*'
+ content_type = 'text/plain'
+ app = lambda { |env| [200, {'Content-Type' => content_type}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ body = Rack::JSONP.new(app).call(request).last
+ body.should.equal ['Good Request']
+ end
+
+ specify 'should not change the response code from 200' do
+ test_body = '{"bar":"foo"}'
+ callback = '*'
+ content_type = 'text/plain'
+ app = lambda { |env| [200, {'Content-Type' => content_type}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ response_code = Rack::JSONP.new(app).call(request).first
+ response_code.should.equal 200
+ end
+ end
+ end
+
context "with XSS vulnerability attempts" do
def request(callback, body = '{"bar":"foo"}')
app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [body]] }
Something went wrong with that request. Please try again.