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

README.md

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

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

# 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

# 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 rails/rails@6f6e754 covers it, it didn't end up in current stable/master branches.)