We build Web & Mobile Applications.
UPDATE! After Rails 2.3.9 and 2.3.10 were released without fixing the issue described in this post I submitted a patch which didn’t get merged resulting in Rails 2.3.11 also being released broken. After several months Rack 1.1.1 was released and that finally fixed the issue.
If you’re using Rails 2.3.8 for your application and thought that you were safe after May’s comedy of errors produced three point updates in as many days, think again. Unfortunately there’s a little bug that can lead to parameters being altered or potentially even truncated without warning.
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.
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.
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:
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)
Add the following code to it:
Dir[File.join(Rails.root, "lib", "patches", "**", "*.rb")].sort.each { |patch| require(patch) }
Create a folder called patches in the lib directory of your Rails application (unless it already exists)
Create a file in this lib/patches directory called rack.rb
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
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.
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?!