Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making proxy to be compliant with Sinatra 1.4.2 release #73

Closed
wants to merge 2 commits into from

Conversation

lzap
Copy link
Member

@lzap lzap commented Mar 29, 2013

This change in Sinatra 1.4.2

https://github.com/sinatra/sinatra/pull/626/files

introduces new class AcceptEntry which is used for comparison of the accept
flag. Since we use request.accept.include? it does not work anymore on my
system:

(rdb:3) pp request.accept
[application/json]

(rdb:3) pp request.accept.include?("application/json")
false

(rdb:3) pp request.accept[0].class
Sinatra::Request::AcceptEntry

This patch changes all places where we use this comparison and changes it to
the standard way. This should be compatible across old Sinatra versions, but
please confirm this explicitly.

@lzap
Copy link
Member Author

lzap commented Mar 29, 2013

Ok I have tracked down what is our minimum version for my patch (search for "accept?"):

= 1.3.0 / 2011-09-30

https://github.com/sinatra/sinatra/blob/master/CHANGES

Is this acceptable?

@domcleal
Copy link
Contributor

+1, fixes it for me on sinatra 1.4.3 and 1.3.2.

@domcleal
Copy link
Contributor

1.3.0's not going to be old enough for our RHEL 6 install, which has rubygem-sinatra 1.0.2 from EPEL.

@lzap
Copy link
Member Author

lzap commented Mar 29, 2013

Let me do a monkey patch for older Sinatra versions.

@lzap
Copy link
Member Author

lzap commented Mar 29, 2013

Here it comes, tested with Sinatra 1.2.9 and 1.4.2.

@domcleal
Copy link
Contributor

domcleal commented Apr 3, 2013

[test]

@ohadlevy
Copy link
Member

ohadlevy commented Apr 8, 2013

@lzap does it mean that 1.2.9 also represents 1.02?

accept.include? type
end
end
Request.send :include, MonkeyRequest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt this be restricted based on sinatra version?

@lzap
Copy link
Member Author

lzap commented Apr 9, 2013

Right that is better. Rebased. [test]

@ohadlevy
Copy link
Member

merged - thanks

@ohadlevy ohadlevy closed this Apr 10, 2013
@lzap lzap deleted the accept-header-fix branch July 23, 2014 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants