public
Last active

Monkey patches for a couple Rails Mime::Type.parse bugs.

  • Download Gist
README.md
Markdown

Rails Mime::Type.parse Patches

There are two Rails issues in it's handling of the HTTP Accept header which cause a number of spurious exception emails via Airbrake. I am encountering this on Rails 3.0.7. One of these is fixed in a later version of Rails, but for other reasons I can't upgrade right now. The other bug is still present in Rails 3.2 and in master at the time of this writing. This gist includes some monkey patches you can apply to fix these issues until such time that they are fixed in Rails properly.

Rails Issue #736

Issue #736 is that Rails does not correctly parse a q-value in an Accept header when there is only one content-type specified. For example:

Accept: text/html;q=0.9

Will likely cause a MissingTemplate exception because the request will not properly set formats=[:html]. This bug has existed since at least Rails 2.3, and still exists in Rails 3.2 and in master as of this writing.

Rails Issue #860

Issue #860 is that Rails does not properly parse wildcard content-types such as text/*, application/*, and image/*. For example:

Accept: text/*

will likely cause a MissingTemplate exception because the request will not properly set formats[:html,:text,:js,:css,:ics,:csv,:xml,:yaml:json]. This bug was fixed sometime later than Rails 3.0.7. That fix, however, only solved the cases for text/* and application/*. The monkey-patch for this is largely based off of their solution, but also includes a fix for image/*.

Testing This Patch

This gist includes a file named mime_type_spec.rb, which is an RSpec example that you can drop into your rspec directory somewhere and run with the rest of your tests. It tests for both issues. Try it before dropping in the other files and see the failure. Then, add in the other files and see the tests pass.

When you change to a different version of Rails, the patches will be disabled, and the tests may fail. If they do, edit the patches to change the Rails version they are constrained to, and see if the tests pass. If not, you may need to do more investigation as the internals of Rails may have changed. On the other hand, if you upgrade to a new version of Rails and the tests still pass, then this means that bugs have been fixed in your version of Rails and you can delete these patches.

Applying This Patch

This gist contains files named rails_issue_736.rb and rails_issue_860.rb. These are the monkey-patches for these two issues, respectively. Place them somewhere in your Rails project where they will be loaded. I put them in config/initializers, but you should be able to put them anywhere that will get them loaded in your project.

You can take one or both files at your discretion.

You'll notice at the top of each of these is a test against the version of Rails you are running, that looks like:

if Rails.version == "3.0.7"

You will want to edit this to be equal to your version of Rails. This makes it so that the monkey-patch gets applied, but only for your current version of Rails. This way, when you upgrade Rails, the patch is disabled. Then, if your tests no longer fail, you know Rails has fixed it and you can delete this monkey-patch. Otherwise, you can go back and edit that version number again and see if it makes the tests pass again.

The Future

In my opinion, some simple refactoring of Mime::Types.parse would eliminate the maintenance hazard that causes these issues. Currently there are two blocks of code in this method: one that handles a single content-type, and one that handles a list of multiple content-types. This should really just treat the single case as a list of one content-type. This way, fixes made to one don't have to be applied to the other -- which is likely the main cause of these problems. Both these issues are non-issues in the multiple content-type case, but are not addressed in the single content-type case.

I'll probably work up a pull request to address this, so hopefully these will be fixed in the mainline Rails someday.

Original blog post: http://scottwb.com/blog/2012/02/06/fix-rails-mishandling-of-http-accept-header/

mime_type_spec.rb
Ruby
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43
require 'spec_helper'
 
describe Mime::Type do
describe ".parse" do
 
# Test for rails issue:
# https://github.com/rails/rails/issues/736
#
# This issue exists in Rails 3.0.7 and we have a monkey patch for it in:
# config/initializers/rails_issue_736.rb
#
# If this test is failing, it is likely because an upgrade to rails
# disables the monkey patch, but Rails hasn't fixed the problem yet.
#
it "should correctly handle media range in Accepts header when single type is specified" do
Mime::Type.parse("*/*;q=0.9").should == [Mime::ALL]
Mime::Type.parse("text/html;q=0.9").should == [Mime::HTML]
end
# Test for rails issue:
# https://github.com/rails/rails/issues/860
#
# This issue exists in Rails 3.0.7 and we have a monkey patch for it in:
# config/initializers/rails_issue_860.rb
#
# If this test is failing, it is likely because an upgrade to rails
# disables the monkey patch, but Rails hasn't fixed the problem yet.
#
it "should correctly match prefix/* types" do
actual_types = Mime::Type.parse("text/*").sort_by(&:to_s)
expected_types = Mime::SET.select{|t| t =~ 'text'}.sort_by(&:to_s)
actual_types.should == expected_types
 
actual_types = Mime::Type.parse("application/*").sort_by(&:to_s)
expected_types = Mime::SET.select{|t| t =~ 'application'}.sort_by(&:to_s)
actual_types.should == expected_types
 
actual_types = Mime::Type.parse("image/*").sort_by(&:to_s)
expected_types = Mime::SET.select{|t| t =~ 'image'}.sort_by(&:to_s)
actual_types.should == expected_types
end
end
end
rails_issue_736.rb
Ruby
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
# This is a workaround for Rails issue:
# https://github.com/rails/rails/issues/736
#
# We are hitting this as of Rails 3.0.7, and will focus this patch only
# on that version. Upgrades to newer versions of Rails will disable this
# patch, and the test for this may fail if said version of Rails hasn't
# fixed this yet. At that time, this patch may need re-investigation since
# it take advantage of internals of this class.
if Rails.version == "3.0.7"
module Mime
class Type
def self.lookup(string)
LOOKUP[string.split(';').first]
end
end
end
end
rails_issue_860.rb
Ruby
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107
# This is a workaround for Rail issue:
# https://github.com/rails/rails/issues/860
#
# We are hitting this as of Rails 3.0.7, and will focus this patch only
# on that version. Upgrades to newer versions of Rails will disable this
# patch, and the test for this may fail if said version of Rails hasn't
# fixed this yet. At that time, this patch may need re-investigation since
# it takes advantage of internals of this class.
#
# Allegedly, this *has* been fixed in later versions of Rails.
#
# This patch is mainly taken from the implementation of Mime::Type.parse
# and Mime::Type.parse_data_with_trailing_star from the HEAD of Rails master
# on 2012-02-06:
# https://github.com/rails/rails/blob/4d5266e2706195888c9f72fdc9ebde22f89a08df/actionpack/lib/action_dispatch/http/mime_type.rb
# With the one small addition that this patch also correctly handles
# image/* types.
#
if Rails.version == "3.0.7"
module Mime
class Type
 
TRAILING_STAR_REGEXP = /(text|application|image)\/\*/
def self.parse(accept_header)
if accept_header !~ /,/
if accept_header =~ TRAILING_STAR_REGEXP
parse_data_with_trailing_star($1)
else
[Mime::Type.lookup(accept_header)]
end
else
# keep track of creation order to keep the subsequent sort stable
list, index = [], 0
accept_header.split(/,/).each do |header|
params, q = header.split(/;\s*q=/)
if params.present?
params.strip!
 
if params =~ TRAILING_STAR_REGEXP
parse_data_with_trailing_star($1).each do |m|
list << AcceptItem.new(index, m.to_s, q)
index += 1
end
else
list << AcceptItem.new(index, params, q)
index += 1
end
end
end
list.sort!
 
# Take care of the broken text/xml entry by renaming or deleting it
text_xml = list.index("text/xml")
app_xml = list.index(Mime::XML.to_s)
 
if text_xml && app_xml
# set the q value to the max of the two
list[app_xml].q = [list[text_xml].q, list[app_xml].q].max
 
# make sure app_xml is ahead of text_xml in the list
if app_xml > text_xml
list[app_xml], list[text_xml] = list[text_xml], list[app_xml]
app_xml, text_xml = text_xml, app_xml
end
 
# delete text_xml from the list
list.delete_at(text_xml)
 
elsif text_xml
list[text_xml].name = Mime::XML.to_s
end
 
# Look for more specific XML-based types and sort them ahead of app/xml
 
if app_xml
idx = app_xml
app_xml_type = list[app_xml]
 
while(idx < list.length)
type = list[idx]
break if type.q < app_xml_type.q
if type.name =~ /\+xml$/
list[app_xml], list[idx] = list[idx], list[app_xml]
app_xml = idx
end
idx += 1
end
end
 
list.map! { |i| Mime::Type.lookup(i.name) }.uniq!
list
end
end
 
# input: 'text'
# returned value: [Mime::JSON, Mime::XML, Mime::ICS, Mime::HTML, Mime::CSS, Mime::CSV, Mime::JS, Mime::YAML, Mime::TEXT]
#
# input: 'application'
# returned value: [Mime::HTML, Mime::JS, Mime::XML, Mime::YAML, Mime::ATOM, Mime::JSON, Mime::RSS, Mime::URL_ENCODED_FORM]
def self.parse_data_with_trailing_star(input)
Mime::SET.select { |m| m =~ input }
end
 
end
end
end

For Rails 3.2.17 it seems we don't have to apply the patch for #736 if the patch for #860 is updated as follows. It does give a warning of redefining the constant, not sure how to improve this patch without having to reimplement (duplicate) existing methods:

if Rails.version == "3.2.17"
  module Mime
    class Type
      class << self
        TRAILING_STAR_REGEXP = /(text|application|image)\/\*/
      end
    end
  end
end

Leaves me with the question why the image/* case keeps to be seemingly overlooked. (Although commit https://github.com/rails/rails/commit/6f6e754bac4c78f657feb0ea119447546aa87197 covers it, it didn't end up in current stable/master branches.)

Please sign in to comment on this gist.

Something went wrong with that request. Please try again.