#3541 Content Negotiation Regression - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#3541 ✓committed

Content Negotiation Regression

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

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.

Comments and changes to this ticket

You flagged this item as spam.
Paul Sadauskas (Rando) February 8th, 2010 @ 06:05 PM
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
You flagged this item as spam.
Paul Sadauskas (Rando) February 8th, 2010 @ 07:15 PM
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
Repository April 2nd, 2010 @ 01:42 AM
- 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...
Jeremy Kemper April 2nd, 2010 @ 09:40 PM
- Milestone cleared.
- State changed from “resolved” to “incomplete”
Missing tests.
Dan Pickett May 15th, 2010 @ 01:55 AM
- Tag changed from “content_type, rails3” to “bugmash, content_type, rails3”
Can a bugmasher add some tests?
You flagged this item as spam.
Patrik Stenmark May 15th, 2010 @ 01:12 PM
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.
- content-negotiation-tests.patch 1.6 KB
José Valim June 29th, 2010 @ 07:51 PM
- 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!
Santiago Pastorino July 1st, 2010 @ 03:33 AM
- 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
Santiago Pastorino July 4th, 2010 @ 07:26 PM
- State changed from “verified” to “committed”
Applied tests here http://github.com/rails/rails/commit/7f7480f6fc1e88ce19bee8ac7f6fb2...
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
You flagged this item as spam.
Russell Garner November 19th, 2010 @ 08:57 AM
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?
You flagged this item as spam.
Fjan November 20th, 2010 @ 08:15 PM
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?
You flagged this item as spam.
Fjan November 20th, 2010 @ 08:16 PM
Sorry, LH ate the asterisks on the posting above, wherever it says "/" please read star / star
José Valim November 20th, 2010 @ 08:18 PM
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.
You flagged this item as spam.
Fjan November 20th, 2010 @ 08:38 PM
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.