diff --git a/javascript/adapters/node_adapter.js b/javascript/adapters/node_adapter.js index 047d9214..6988b35f 100644 --- a/javascript/adapters/node_adapter.js +++ b/javascript/adapters/node_adapter.js @@ -22,6 +22,8 @@ Faye.NodeAdapter = Faye.Class({ TYPE_SCRIPT: {'Content-Type': 'text/javascript; charset=utf-8'}, TYPE_TEXT: {'Content-Type': 'text/plain; charset=utf-8'}, + VALID_JSONP_CALLBACK: /^[a-z_\$][a-z0-9_\$]*(\.[a-z_\$][a-z0-9_\$]*)*$/i, + initialize: function(options) { this._options = options || {}; this._endpoint = this._options.mount || this.DEFAULT_ENDPOINT; @@ -135,12 +137,21 @@ Faye.NodeAdapter = Faye.Class({ headers = Faye.extend({}, type), origin = request.headers.origin; + if (!this.VALID_JSONP_CALLBACK.test(jsonp)) + return this._returnError(response, {message: 'Invalid JSON-P callback: ' + jsonp}); + if (origin) headers['Access-Control-Allow-Origin'] = origin; headers['Cache-Control'] = 'no-cache, no-store'; + headers['X-Content-Type-Options'] = 'nosniff'; this._server.process(message, request, function(replies) { var body = Faye.toJSON(replies); - if (isGet) body = jsonp + '(' + this._jsonpEscape(body) + ');'; + + if (isGet) { + body = '/**/' + jsonp + '(' + this._jsonpEscape(body) + ');'; + headers['Content-Disposition'] = 'attachment; filename=f.txt'; + } + headers['Content-Length'] = new Buffer(body, 'utf8').length.toString(); headers['Connection'] = 'close'; diff --git a/lib/faye/adapters/rack_adapter.rb b/lib/faye/adapters/rack_adapter.rb index 8a13e75c..eba69021 100644 --- a/lib/faye/adapters/rack_adapter.rb +++ b/lib/faye/adapters/rack_adapter.rb @@ -15,6 +15,8 @@ class RackAdapter TYPE_SCRIPT = {'Content-Type' => 'text/javascript; charset=utf-8'} TYPE_TEXT = {'Content-Type' => 'text/plain; charset=utf-8'} + VALID_JSONP_CALLBACK = /^[a-z_\$][a-z0-9_\$]*(\.[a-z_\$][a-z0-9_\$]*)*$/i + # This header is passed by Rack::Proxy during testing. Rack::Proxy seems to # set content-length for you, and setting it in here really slows the tests # down. Better suggestions welcome. @@ -94,22 +96,33 @@ def handle_request(request) debug("Received message via HTTP #{request.request_method}: ?", json_msg) - message = MultiJson.load(json_msg) - request.env['rack.hijack'].call if request.env['rack.hijack'] - + message = MultiJson.load(json_msg) jsonp = request.params['jsonp'] || JSONP_CALLBACK headers = request.get? ? TYPE_SCRIPT.dup : TYPE_JSON.dup origin = request.env['HTTP_ORIGIN'] callback = request.env['async.callback'] - hijack = request.env['rack.hijack_io'] + + if jsonp !~ VALID_JSONP_CALLBACK + error 'Invalid JSON-P callback: ?', jsonp + return [400, TYPE_TEXT, ['Bad request']] + end headers['Access-Control-Allow-Origin'] = origin if origin headers['Cache-Control'] = 'no-cache, no-store' + headers['X-Content-Type-Options'] = 'nosniff' + + request.env['rack.hijack'].call if request.env['rack.hijack'] + hijack = request.env['rack.hijack_io'] EventMachine.next_tick do @server.process(message, request) do |replies| response = Faye.to_json(replies) - response = "#{ jsonp }(#{ jsonp_escape(response) });" if request.get? + + if request.get? + response = "/**/#{ jsonp }(#{ jsonp_escape(response) });" + headers['Content-Disposition'] = 'attachment; filename=f.txt' + end + headers['Content-Length'] = response.bytesize.to_s unless request.env[HTTP_X_NO_CONTENT_LENGTH] headers['Connection'] = 'close' debug 'HTTP response: ?', response diff --git a/spec/javascript/node_adapter_spec.js b/spec/javascript/node_adapter_spec.js index a10313d4..d6fdfd9c 100644 --- a/spec/javascript/node_adapter_spec.js +++ b/spec/javascript/node_adapter_spec.js @@ -246,8 +246,8 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this) get("/bayeux", params) check_status(200) check_content_type("text/javascript") - check_content_length("42") - check_body('callback([{"channel":"/meta/handshake"}]);') + check_content_length("46") + check_body('/**/callback([{"channel":"/meta/handshake"}]);') }}) it("does not let the client cache the response", function() { with(this) { @@ -267,8 +267,8 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this) get("/bayeux", params) check_status(200) check_content_type("text/javascript") - check_content_length("47") - check_body('jsonpcallback([{"channel":"/meta/handshake"}]);') + check_content_length("51") + check_body('/**/jsonpcallback([{"channel":"/meta/handshake"}]);') }}) }}) @@ -292,6 +292,13 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this) behavesLike("bad GET request") }}) + describe("with a callback that's not a JS identifier", function() { with(this) { + before(function() { with(this) { + params.jsonp = "42" + }}) + behavesLike("bad GET request") + }}) + describe("missing message", function() { with(this) { before(function() { with(this) { delete params.message diff --git a/spec/ruby/rack_adapter_spec.rb b/spec/ruby/rack_adapter_spec.rb index 2231dce3..5b04c97f 100644 --- a/spec/ruby/rack_adapter_spec.rb +++ b/spec/ruby/rack_adapter_spec.rb @@ -160,8 +160,8 @@ get "/bayeux", params status.should == 200 content_type.should == "text/javascript; charset=utf-8" - content_length.should == "42" - body.should == 'callback([{"channel":"/meta/handshake"}]);' + content_length.should == "46" + body.should == '/**/callback([{"channel":"/meta/handshake"}]);' end it "does not let the client cache the response" do @@ -190,8 +190,8 @@ get "/bayeux", params status.should == 200 content_type.should == "text/javascript; charset=utf-8" - content_length.should == "47" - body.should == 'jsonpcallback([{"channel":"/meta/handshake"}]);' + content_length.should == "51" + body.should == '/**/jsonpcallback([{"channel":"/meta/handshake"}]);' end end @@ -213,6 +213,11 @@ it_should_behave_like "bad GET request" end + describe "with a callback that's not a JS identifier" do + before { params[:jsonp] = "42" } + it_should_behave_like "bad GET request" + end + describe "missing message" do before { params.delete(:message) } it_should_behave_like "bad GET request"