Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Content Negotiation Regression

#3541

A recent change in Rails3 rendering/respond_to code completely broke content negotiation. This commit[1] causes any request that 1) does not have a .format param, 2) is not xhr, and 3) CONTAINS A COMMA in the Accept header to return an html document.

So if, for example, you have a resource that provides html, xml, and json, and a web service client that accepts both xml and json, you get html. Also, if you have a web service similar to Sun's Cloud offering which uses custom mime-types to indicate various flavors of json, you would get html. If the resource does not provide an html content-type at all, you would get a 406 Not Acceptable response!

I discussed this with Yehuda, and this change was intended, to work around a bug in Safari's Accept header, which prefers xml over html. The way this was avoiding in Rails 2.x was to make sure the html block occured first in the respond_to block. I'm not sure if the recent changes to support respond_with prompted this change, but I hope that it be reverted to the old behaviour.

The easiest solution is to just ignore the quality parameter of the accept header, and just serve the first acceptable response. This would still require users to specify the html first, but it should not be a problem if it is well-documented. Another solution, that is more magical, would be to always prefer html over everything else, if the client accepts html or /.

I have attached[2] a failing test to demonstrate this behavior.

Reported by Paul Sadauskas (Rando) · December 5th, 2009 @ 12:18 AM

State: committed
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Paul Sadauskas (Rando)
    Paul Sadauskas (Rando)

    Looks like LH swallowed my markdown. Here's the links referenced above:

    1: http://github.com/rails/rails/commit/1310231c15742bf7d99e2f143d88b3...
    2: http://gist.github.com/237075

    February 8th, 2010 @ 06:05 PM

  2. Paul Sadauskas (Rando)
    Paul Sadauskas (Rando)

    And here's a patch that hacks around the hack. It pretty much is just a better was to detect browsers without also catching web-service clients.

    Commit: http://github.com/paul/rails/commit/680762155921bbc42b0030807f34cb0...
    Topic branch, for merging: http://github.com/paul/rails/commits/accepts

    February 8th, 2010 @ 07:15 PM

  3. Repository
    Repository
    • State changed from new to resolved

    (from [dc5300adb6d46252c26e239ac67e3ca6e5e2d77b]) Slightly less annoying check for acceptable mime_types. This allows Accept: application/json, application/jsonp (and the like), but still blacklists browsers. Essentially, we use normal content negotiation unless you include / in your list, in which case we assume you're a browser and send HTML [#3541 state:resolved] http://github.com/rails/rails/commit/dc5300adb6d46252c26e239ac67e3c...

    April 2nd, 2010 @ 01:42 AM

  4. Jeremy Kemper
    Jeremy Kemper
    • Milestone cleared.
    • State changed from resolved to incomplete

    Missing tests.

    April 2nd, 2010 @ 09:40 PM

  5. Dan Pickett
    Dan Pickett
    • Tag changed from content_type, rails3 to bugmash, content_type, rails3

    Can a bugmasher add some tests?

    May 15th, 2010 @ 01:55 AM

  6. Patrik Stenmark
    Patrik Stenmark

    I've created a patched which add tests which I think tests the right thing. My first patch ever, so I hope it's useful.

    May 15th, 2010 @ 01:12 PM

  7. José Valim
    José Valim
    • Importance changed from to Low

    Patrik, could you please create a patch using the guidelines specified here:

    http://rails.lighthouseapp.com/projects/8994/sending-patches

    This way it will preserve you as the author! Thanks!

    June 29th, 2010 @ 07:51 PM

  8. Santiago Pastorino
    Santiago Pastorino
    • Tag changed from bugmash, content_type, rails3 to content_type, rails3
    • State changed from incomplete to verified
    • Assigned user changed from Yehuda Katz (wycats) to José Valim

    José, Patrik patch applies clean for me, and all the tests pass

    July 1st, 2010 @ 03:33 AM

  9. Santiago Pastorino
    Santiago Pastorino
    • State changed from verified to committed

    July 4th, 2010 @ 07:26 PM

  10. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2

    October 15th, 2010 @ 11:01 PM

  11. Russell Garner
    Russell Garner

    Hi,

    Have opened a ticket at https://rails.lighthouseapp.com/projects/8994/tickets/5991-406-unac... - I'm still not sure this is behaving in a way which is reasonable HTTP (or whether or not I've opened a duplicate ticket ;)). If we have an HTML form which POSTs to an action which can only return application/atom+xml we end up with a 406, even though the browser has said it will accept */*. A patch which demonstrates this is attached at the link above.

    This 406 happens whether or not respond_with or the old-style respond_to is used.

    I'm not sure I understand the line from Paul's commit message above "Essentially, we use normal content negotiation unless you include / in your list, in which case we assume you're a browser and send HTML" - that seems to say that we're assuming that browsers can only handle HTML?

    November 19th, 2010 @ 08:57 AM

  12. Fjan
    Fjan

    I'm also not convinced this is how we want this to behave. Here are a few Accept Headers I found in my production log over the last few days that get served a "Missing Template" error:

    • HTTP_ACCEPT: text/*
    • HTTP_ACCEPT: /, auth/sicily
    • HTTP_ACCEPT: /, application/youtube-client

    As far as I can tell those should have been served an HTML page. If I set my browser header to "/" it works as expected, but if I add garbage to the header it won't work. perhaps we should have an option somewhere that says "when in doubt serve HTML" that you could switch on when you are certain you are not serving web services? Should I put in a new ticket for this?

    November 20th, 2010 @ 08:15 PM

  13. Fjan
    Fjan

    Sorry, LH ate the asterisks on the posting above, wherever it says "/" please read star / star

    November 20th, 2010 @ 08:16 PM

  14. José Valim
    José Valim

    Fjan, your case indeed seems to be a regression. Whenever rails sees "star / star", it should deliver HTML if available. Feel free to open a new ticket. A patch with tests (or even a fix) is extremely welcome.

    November 20th, 2010 @ 08:18 PM

  15. Fjan
    Fjan

    Ok, new ticket is here. I don't have time for proper patch I'm afraid, but I'll add the monkey patch that I created for myself.

    November 20th, 2010 @ 08:38 PM