Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Proper mime negotiation in case of non-ajax request #7464

Closed
wants to merge 2 commits into from

7 participants

@LTe

Currently when client send request to rails with proper Accept header rails will return always HTML. When I need JSON or other format I can of course add .(:format) and rails will recognize this. Why is this the only option?

  • I do not want nor need to use :format in routes
  • I use jQuery.getJSON method with cross-domain option HTTP_X_REQUESTED_WITH is not set
  • There is a proper Accept header : application/json, text/javascript, */*; q=0.0
  • There is respond_to :json in controller definition
  • I use respond_with in action and give it an object that has to_json and as_json method implemented

When I ask for JSON rails should respond with JSON.

LTe added some commits
@LTe LTe Add specs for Accept header
Rails should recognize accept type and respond with proper format.
Request type should not matter. If a client asks for JSON, we deliver
him json.
d8e7d4b
@LTe LTe Update Accept header valid condition
Accept header is still valid when HTTP request is not XHR type
4848013
@josevalim
Owner

Browsers were used to send completely non-sense accept headers. For example, IE were used to ask for a png file with higher priority than text/html. We could eventually merge this pull request, but we would need to check first if browsers are sending proper accept headers, otherwise this change could make an application completely unusable.

@paneq

Let's just say that we find out that IE6 is doing something stupid. Maybe we do not care as this browser is not supported in our project and we would like to drop workarounds in favor of following the http spec. What do you think about adding config for browser compatibility into rails ex.:

  config.browsers.ie = 8

This could mean that we want to support IE >= 8 and disable hacks for ie6 and ie7.

Could that be helpful ?

@drogus
Collaborator

I'm fine with dropping support for IE6, as long as this behavior is extracted to a plugin - I don't like the idea of adding another setting and keeping such legacy things in core. This should be rather trivial to implement and we could point people to it in the CHANGELOG.

@rafaelfranca

:+1: for the @drogus proposal

@josevalim
Owner

For the purpose of this discussion, this change will screw safari and chrome since they give higher priority to xml, according to this link:

https://developer.mozilla.org/en-US/docs/HTTP/Content_negotiation

So I really can't see how we can trust browsers accept headers.

@josevalim josevalim closed this
@josevalim
Owner

Fun: We should probably convince browsers to send a X-Im-a-Browser: true header.

@steveklabnik
Collaborator

I really can't see how we can trust browsers accept headers.

:'( :cry:

More about the situation than about your statement.

@pawelpacana

@josevalim According to https://bugs.webkit.org/show_bug.cgi?id=27267 Safari and Chrome should be OK.

The default accept header now mirror's FireFox'. The meaningful change is that 'text/html' is now preferred over 'application/xml'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 28, 2012
  1. @LTe

    Add specs for Accept header

    LTe authored
    Rails should recognize accept type and respond with proper format.
    Request type should not matter. If a client asks for JSON, we deliver
    him json.
  2. @LTe

    Update Accept header valid condition

    LTe authored
    Accept header is still valid when HTTP request is not XHR type
This page is out of date. Refresh to see the latest.
View
5 actionpack/lib/action_dispatch/http/mime_negotiation.rb
@@ -118,11 +118,8 @@ def negotiate_mime(order)
protected
- BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/
-
def valid_accept_header
- (xhr? && (accept || content_mime_type)) ||
- (accept && accept !~ BROWSER_LIKE_ACCEPTS)
+ accept || content_mime_type
end
def use_accept_header
View
14 actionpack/test/controller/mime_responds_test.rb
@@ -248,12 +248,12 @@ def test_js_or_html
def test_json_or_yaml_with_leading_star_star
@request.accept = "*/*, application/json"
- get :json_xml_or_html
- assert_equal 'HTML', @response.body
+ get :json_or_yaml
+ assert_equal 'JSON', @response.body
@request.accept = "*/* , application/json"
- get :json_xml_or_html
- assert_equal 'HTML', @response.body
+ get :json_or_yaml
+ assert_equal 'JSON', @response.body
end
def test_json_or_yaml
@@ -411,7 +411,11 @@ def test_browser_check_with_any_any
@request.accept = "application/json, application/xml, */*"
get :json_xml_or_html
- assert_equal 'HTML', @response.body
+ assert_equal 'JSON', @response.body
+
+ @request.accept = "application/xml, application/json, */*"
+ get :json_xml_or_html
+ assert_equal 'XML', @response.body
end
def test_html_type_with_layout
View
6 actionpack/test/dispatch/mime_type_test.rb
@@ -96,6 +96,12 @@ class MimeTypeTest < ActiveSupport::TestCase
assert_equal expect, Mime::Type.parse(accept).collect { |c| c.to_s }
end
+ test "parse text with trailing star at the end" do
+ accept = "application/json, text/javascript, */*"
+ expect = [Mime::JSON, Mime::JS, Mime::ALL]
+ assert_equal expect, Mime::Type.parse(accept)
+ end
+
test "custom type" do
begin
Mime::Type.register("image/foo", :foo)
View
4 actionpack/test/dispatch/request_test.rb
@@ -569,6 +569,10 @@ def url_for(options = {})
request.expects(:parameters).at_least_once.returns({})
assert_equal with_set(Mime::XML), request.formats
+ request = stub_request 'HTTP_ACCEPT' => 'application/json, text/javascript, */*'
+ request.expects(:parameters).at_least_once.returns({})
+ assert_equal with_set(Mime::JSON, Mime::JS, Mime::ALL), request.formats
+
request = stub_request
request.expects(:parameters).at_least_once.returns({ :format => :txt })
assert_equal with_set(Mime::TEXT), request.formats
Something went wrong with that request. Please try again.