Add a 'jsonp force 200' setting to force an HTTP 200 JSONP response #1412

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@chuyeow
chuyeow commented Nov 12, 2012
app.enable('jsonp force 200');
app.use(function(req, res){
  res.statusCode = 500;
  res.jsonp({ error: 'message' });
});

and

curl -is http://localhost:3000/?callback=something

HTTP/1.1 200 OK

something && something({
  "error": "message"
});

This is useful because browsers are unable to respond to non-200 JSONP responses. A good way to inform browsers of the actual HTTP status code is to include it in the JSON response, e.g.

cb && cb({"code":400,"message":"Invalid params"})`.
@chuyeow chuyeow Add 'jsonp force 200' setting to force an HTTP 200 response when resp…
…onding with JSONP.

This is useful because browsers are unable to respond to non-200 JSONP responses. A good way to inform browsers of the actual HTTP status code is to include it in the JSON response, e.g. `cb && cb({"code":400,"message":"Invalid params"})`.
519a259
@chuyeow
chuyeow commented Nov 12, 2012

The current workaround I'm using in my app looks something like this:

  var callback = req.query[req.app.get('jsonp callback name')];

  if (callback) {
    // Always send HTTP 200 response for JSONP so that browsers can handle it.
    res.jsonp(200, err);
  } else {
    res.json(err)
  }

With this new setting, I hope to remove the conditional and be able to just use res.jsonp().

@tj
Member
tj commented Nov 12, 2012

doesn't make too much sense to me, jsonp is fundamentally flawed from the get go, I'd rather not bake in more weird support for it, ideally remove it all together once CORs is widespread, this seems odd though. If you still want it for your case i'd just do:

if (req.query.callback) res.status(200);
next();

in middleware

@chuyeow
chuyeow commented Nov 15, 2012

It's a pretty common thing, you can see it in Foursquare's API and also in Github's API.

But fair enough, I respect an opinion - since you think baking in more weird support for something that's going to die anyway is a bad idea, I'm just gonna close my pull request :)

@chuyeow chuyeow closed this Nov 15, 2012
@strk strk referenced this pull request in CartoDB/Windshaft Sep 5, 2013
Closed

layergroup should always return 200 when the call is jsonp #84

@jirikopsa

I came over this issue when migrating part of our back-end to nodejs. Our previous infrastructure passes non-200 HTTP status code in JSONP as a second callback parameter. This is neat as:

  1. server side response API stays the same res.send 500, { msg: "my error" }
  2. SCRIPT element get's executed in the browser, function get's called, msg processed
  3. There is no nesting/wrapping of the metadata into the payload, it's just a second/optional parameter (looks just like "standard" JSONP)

Please note, that although JSONP is a hack, it sometimes remains the only way to do cross-domain requests - e.g. in Windows Phone < 10.
http://caniuse.com/cors

Also note that passing status code as a second JSONP callback parameter is not just my idea ;-) #not-a-fan-of-microsoft
http://msdn.microsoft.com/en-us/library/ee816916.aspx

The fix goes like this - not setting statusCode on response, but rather including it as a second parameter:

res.jsonp = function(obj){
  var statusCode = null;

  // allow status / body
  if (2 == arguments.length) {
    // res.json(body, status) backwards compat
    if ('number' == typeof arguments[1]) {
      statusCode = arguments[1];
    } else {
      statusCode = obj;
      obj = arguments[1];
    }
  }

  // settings
  var app = this.app;
  var replacer = app.get('json replacer');
  var spaces = app.get('json spaces');
  var body = JSON.stringify(obj, replacer, spaces)
    .replace(/\u2028/g, '\\u2028')
    .replace(/\u2029/g, '\\u2029');
  var callback = this.req.query[app.get('jsonp callback name')];

  // content-type
  this.charset = this.charset || 'utf-8';
  this.set('Content-Type', 'application/json');

  // jsonp
  if (callback) {
    if (Array.isArray(callback)) callback = callback[0];
    this.set('Content-Type', 'text/javascript');
    var cb = callback.replace(/[^\[\]\w$.]/g, '');
    body = cb + ' && ' + cb + '(' + body;
    if (statusCode)
      body += ', ' + statusCode
    body += ');';
  }

  return this.send(body);

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment