As Rails is a Rack application it delegates the parsing of request parameters to the Rack::Utils module. This worked fine until for some bizarre reason the Rack team decided that the parse_query and normalize_params methods should “properly” parse quoted string values. I haven't been able to find the discussion that led to this commit, so I don't know what the original intention was.

Although this change was made way back in December last year and was released as part of Rack 1.1 in January, it didn't cause a problem for Rails applications until version 2.3.6 bumped the Rack dependency from 1.0 to 1.1. Applications using other Rack frameworks like Sinatra would likely have been affected sooner as they're able to take advantage of a new Rack release more easily.

We first noticed a problem when deploying a Rails 2.3.5 app with JRuby (which was a real pain to track down as something in JRuby was activating Rack 1.1 even though Rails 2.3.5 uses 1.0) and then it cropped up again in one of our Sinatra apps.

So what's the problem?

It's probably best to show a couple of examples:

Rack::Utils.parse_query("foo=bar")
=> {"foo"=>"bar"} # correct
Rack::Utils.parse_query("foo=\"bar\"")
=> {"foo"=>"bar"} # incorrect! should be {"foo"=>"\"bar\""}

It may not be immediately obvious why this removal of quotes could be a problem. For us, our application was providing search functionality using ThinkingSphinx and we wanted to allow users to search for phrases. Rack's interference meant this wasn't possible:

Rack::Utils.parse_query("search=\"I%20am%20a%20search%20phrase\"")
{"search"=>"I am a search phrase"}

The quotes around the phrase are gone, changing the meaning of the ThinkingSphinx search and causing us headaches.

A ticket has also been created on the Rails tracker that suggests that text submitted in a form can also be prematurely truncated as a result of this bug.

How can it be fixed?

The good news is that the changes that caused this bug were reverted in time for Rack 1.2, the bad news is that at the time of writing Rails is still dependent on the broken 1.1 release. Hopefully this will be changed before Rails 2.3.9 is made available: if it isn't expect 2.3.10 to come out a day or so later!

For existing Rails 2.3.8 applications you won't be able to change the Rack version used by Rails but you will be able to monkey-patch the code. Follow the steps below:

  1. Create an initializer called _run_first.rb in the config/initializers directory of your app (note that the leading underscore is important as Rails loads initializers in alphabetical order, the underscore forces this one to the top of the list)

  2. Add the following code to it:

    Dir[File.join(Rails.root, "lib", "patches", "**", "*.rb")].sort.each { |patch| require(patch) }
  3. Create a folder called patches in the lib directory of your Rails application (unless it already exists)

  4. Create a file in this lib/patches directory called rack.rb

  5. Add the following code (or get the gist):

    module Rack
      module Utils
        def parse_query(qs, d = nil)
          params = {}
    
          (qs || '').split(d ? /[#{d}] */n : DEFAULT_SEP).each do |p|
            k, v = p.split('=', 2).map { |x| unescape(x) }
            if cur = params[k]
              if cur.class == Array
                params[k] << v
              else
                params[k] = [cur, v]
              end
            else
              params[k] = v
            end
          end
    
          return params
        end
        module_function :parse_query
    
        def normalize_params(params, name, v = nil)
          name =~ %r(\A[\[\]]*([^\[\]]+)\]*)
          k = $1 || ''
          after = $' || ''
    
          return if k.empty?
    
          if after == ""
            params[k] = v
          elsif after == "[]"
            params[k] ||= []
            raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
            params[k] << v
          elsif after =~ %r(^\[\]\[([^\[\]]+)\]$) || after =~ %r(^\[\](.+)$)
            child_key = $1
            params[k] ||= []
            raise TypeError, "expected Array (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Array)
            if params[k].last.is_a?(Hash) && !params[k].last.key?(child_key)
              normalize_params(params[k].last, child_key, v)
            else
              params[k] << normalize_params({}, child_key, v)
            end
          else
            params[k] ||= {}
            raise TypeError, "expected Hash (got #{params[k].class.name}) for param `#{k}'" unless params[k].is_a?(Hash)
            params[k] = normalize_params(params[k], after, v)
          end
    
          return params
        end
        module_function :normalize_params
      end
    end
  6. Restart your server for the patch to take effect.

There's two things going on here:

  • You've now got a nice, simple pattern for adding patches to your Rails application. For example if you later need to patch a bug in ActionView then you could simply make a lib/patches/action_view.rb file containing the code for the patch and it will automatically be loaded when you next start your app.
  • The patch file overwrites the broken parse_query and normalize_params methods using the fixed code from Rack 1.2.

You can easily verify that the patch is working by firing up a Rails console (using script/console) and trying one of the examples given above. You should get the correct results, if you don't then run through the above steps again making sure you've done everything exactly as described.

And the moral of the story is?

Even with an extensive test suite it is possible for bugs to slip through: in this case the Rack tests were changed to ensure they passed even though the behaviour was wrong, and the Rails tests didn't cover it because the code should already have been tested in the Rack tests. I guess the question is who tests the tests?!