Re: Bug in Rack::JSONP?

[Gaius <james.a.rosen@gmail.com>]

+1, Magnus

On Jun 22, 8:32 am, Magnus Holm <judo...@gmail.com> wrote:
> What about something like this:
>
> diff --git a/lib/rack/contrib/jsonp.rb b/lib/rack/contrib/jsonp.rb
> index 723d63a..7eefe00 100644
> --- a/lib/rack/contrib/jsonp.rb
> +++ b/lib/rack/contrib/jsonp.rb
> @@ -19,6 +19,10 @@ module Rack
>      #
>      def call(env)
>        status, headers, response = @app.call(env)
> +
> +      if STATUS_WITH_NO_ENTITY_BODY.include?(status)
> +        return status, headers, response
> +      end
>
>        headers = HeaderHash.new(headers)
>        request = Rack::Request.new(env)
> diff --git a/test/spec_rack_jsonp.rb b/test/spec_rack_jsonp.rb
> index eb6570f..c1abc59 100644
> --- a/test/spec_rack_jsonp.rb
> +++ b/test/spec_rack_jsonp.rb
> @@ -69,5 +69,15 @@ context "Rack::JSONP" do
>      body = Rack::JSONP.new(app).call(request).last
>      body.should.equal [test_body]
>    end
> -
> +
> +  specify "should not change anything if the request doesn't have a body" do
> +    app1 = lambda { |env| [100, {}, []] }
> +    app2 = lambda { |env| [204, {}, []] }
> +    app3 = lambda { |env| [304, {}, []] }
> +    request = Rack::MockRequest.env_for("/", :params =>
> "callback=foo", 'HTTP_ACCEPT' => 'application/json')
> +
> +    Rack::JSONP.new(app1).call(request).should.equal app1.call({})
> +    Rack::JSONP.new(app2).call(request).should.equal app2.call({})
> +    Rack::JSONP.new(app3).call(request).should.equal app3.call({})
> +  end
>  end
> \ No newline at end of file
>
> // Magnus Holm
>
>
>
> On Tue, Jun 22, 2010 at 14:22, Magnus Holm <judo...@gmail.com> wrote:
> > From the spec: "There must be a Content-Type, except when the Status
> > is 1xx, 204 or 304, in which case there must be none given."
>
> > I guess we should rather make JSONP a noop when the status is 1xx, 204 or 304.
>
> > // Magnus Holm
>
> > On Tue, Jun 22, 2010 at 14:08, Aslak Hellesøy <aslak.helle...@gmail.com> wrote:
> >> Hi all,
>
> >> I'm using Rack::JSONP from rack-contrib in a Sinatra project. When I
> >> issue a GET request that's not JSON (and that doesn't set a Content-
> >> Type header, I get:
>
> >> NoMethodError: undefined method `include?' for nil:NilClass
> >>        /Users/aslakhellesoy/.rvm/gems/ruby-1.8.7-p249@cukepatch/gems/rack-
> >> contrib-1.0.1/lib/rack/contrib/jsonp.rb:44:in `is_json?'
>
> >> Am I the only one running into this? Should I patch with:
>
> >> (headers['Content-Type'] || []).include?('application/json') in rack-
> >> contrib-1.0.1/lib/rack/contrib/jsonp.rb:44
>
> >> Or is it better to patch rack to return an empty array instead of nil
> >> in #headers.
>
> >> I'm on rack 1.2.1 and Sinatra 1.0.
>
> >> Cheers,
> >> Aslak