JSONP middleware may invalidate Content-Length
ajvondrak opened this issue · 3 comments
Basic problem: when using the JSONP middleware and specifying a callback
parameter, there are cases where the Content-Length
will reflect the length of the body before applying the callback; thus, the returned value is actually truncated.
I'm not entirely sure who to open this issue for, because it came up for me when using Grape under Goliath, as demonstrated by @dblock here, and I cannot reproduce this without Grape.
I looked around, and I'm not the only one who has had this problem:
- This StackOverflow question is a couple years old, but indicated this may be a bug.
- This old pull request was closed without being merged.
Minimal working example:
require 'goliath'
require 'grape'
class API < Grape::API
format :json
get do
{ content_length: 21 }
end
end
class App < Goliath::API
use Goliath::Rack::Params
use Goliath::Rack::JSONP
def response(env)
API.call(env)
end
end
Without the callback
param, the content-length is exactly 21, and the Content-Length
header is correct:
$ curl -i 'localhost:9000' && echo
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 21
Server: Goliath
Date: Thu, 11 Dec 2014 23:38:27 GMT
{"content_length":21}
With the callback
param, the content-length is actually 21 + 2 (for the parens) + the length of the callback argument. However, the Content-Length
header is still set to 21:
$ curl -i 'localhost:9000?callback=callback' && echo
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 21
CONTENT_TYPE: application/javascript
Server: Goliath
Date: Thu, 11 Dec 2014 23:38:32 GMT
callback({"content_le
Setting up breakpoints, I see that by the time the Goliath::Rack::JSONP
middleware gets invoked, we have already set a Content-Length
header. I'm having troubles tracing exactly why the header's present specifically when using Grape (and not with plain-old-Goliath itself), but whatever way it's happening, it's happening.
If we study Rack's JSONP middleware, we see that they adjust the Content-Length
if mutated: https://github.com/rack/rack-contrib/blob/master/lib/rack/contrib/jsonp.rb#L51-L55 So, I think it stands to reason that Goliath should do something similar.
+1 If you modify the body you have to adjust the content-length.
Goliath uses AsyncRack::ContentLength (which, under the hood) uses Rack::ContentLength to set the length header. Rack::ContentLength will only set the length if it hasn't been set. So, there is the assumption that if you're setting the length, you're setting it correctly, it doesn't get changed.
The ContentLength middleware is injected as the second one in the chain, which means it will get executed after the JSONP middleware and do the right thing, unless a content length has been set in the header.
So, probably the right solution is to do the same as JSON Rack in this case and force modify the header if it's been set, otherwise, leave it for the content length middleware to handle.