frog and code

The popular Rails search plugin acts_as_ferret has had, until recently, a nasty little bug affecting boolean attributes. Indexed attributes returning a value of false are indexed as *nil*, not false as expected. (True values are indexed correctly.) Thus searches that test whether an attribute is false omit matching records and, conversely and often more frighteningly, include records that should NOT match.

The bug resides in the following code in instance_methods.rb (from version 4.3, the latest stable release at this time):

def content_for_field_name(field, dynamic_boost = nil)
  ar_field = aaf_configuration[:ferret_fields][field][:via]
  field_data = self.send(ar_field) || self.instance_variable_get("@#{ar_field}")
  if (dynamic_boost && boost_value = self.send(dynamic_boost))
    field_data = Ferret::Field.new(field_data)
    field_data.boost = boost_value.to_i
  end
  field_data
end

On line three, the first half of the conditional expression

self.send(ar_field)

returns false if the indexed field is a method returning false. But instead assigning false to field_data (which is subsequently indexed), the second half of the condition

self.instance_variable_get("@#{ar_field}")

is run and checks for an instance variable with that field name. If none exists (typically the case for Rails attributes derived from database columns), the expression returns nil. So field_data is assigned nil, not false, and indexed as such.

Why is this a big deal? Boolean attributes such as 'deleted', 'activated', 'public' and 'suspended' are often used by applications to determine whether and when a user may participate and be visible. If these fields are indexed improperly, legitimate results can be dropped from searches and illegitimate users shown. (And if application logic relies on the attributes returned, further security issues can arise.) For applications that maintain sensitive user data, exposing a user as public who has asked to be private is a scary proposition.

About a month ago, I submitted a bug report http://projects.jkraemer.net/acts_as_ferret/ticket/220 with the following patch for line 3 above:

# Issue was that field_data was being set to nil for fields with value=false.
begin
  field_data = self.send(ar_field)
rescue
  field_data = self.instance_variable_get("@#{ar_field}")
end

Was happy to see that a fix has been recently committed on the trunk http://projects.jkraemer.net/acts_as_ferret/changeset/350 to address the issue. The following code was used to replace line 3:

field_data = (respond_to?(via) ? send(via) : instance_variable_get("@#{via}")).to_s

The respond_to? call tests for the presence of a method and skips the instance_variable_get if one is found. This is definitely slicker and more idiomatic ruby than the begin/rescue approach in my code (I'll conveniently blame years of Java for the exceptional approach). I'm happy to see this fix - thank you, Jens! - and encourage you to upgrade acts_as_ferret or patch your apps as soon as possible. You'll also need to re-index models with boolean fields if you've already been burned by the bug.

One of boons of using Ruby is its expressiveness and readability. A small detail that struck me when I started writing Ruby code was the ability to use words like and and or as keywords in conditional expressions (in lieu of && and ||). A nice touch that allows code to read more like English.

However, it turns out that the English forms of these conditional operators are not quite equivalent to their symbol counterparts. Consider the following exercise on the console:

>> username = 'frog'
>> password = nil
>> login_valid = !username.blank? && !password.blank?
>> puts login_valid
false

Login is valid if both username and password are not blank. Since password is nil, the value of login_valid is false as we would expect. Now let's try the same code, replacing && with and to represent the AND conditional operator.

>> username = 'frog'
>> password = nil
>> login_valid = !username.blank? and !password.blank?
>> puts login_valid
true

Whoa!? The login_valid variable now evaluates to true. What happened? Well, and is not simply an alias for &&, it's a different operator with a different level of precedence. Programming Ruby by Dave Thomas contains a table of operators in order of precedence and notes that "the word forms of these operators (and, or, and not) have a lower precedence than the corresponding symbol forms (&&, ||, and !)." More specifically, the word forms are lower in precedence than assignment operators (= in the example above), while the symbol forms are higher in precedence.

So in the second example, the variable login_valid is assigned the value of !username.blank? before the and condition !password.blank? is evaluated. Crazy, and a little scary - I'd expect both forms to work the same way, or at least both to have precedence over assignment operators. The solution in this case is either to use && as in the first example or wrap the full condition in parentheses to specify precedence explicitly:

>> username = 'frog'
>> password = nil
>> login_valid = (!username.blank? and !password.blank?)
>> puts login_valid
false

Rails 2.1 was released last week at RailsConf with a lot fanfare and sexy features like timestamped migrations, named scopes and better timezone handling. There was also a minor fix that I was happy to see: String#titleize now works with acronyms. Prior to 2.1:

>> "ALS community".titleize
=> "Als Community"

Not terribly desirable that "ALS" suddenly has lowercase letters because it's used in a title. But now in 2.1:

>> "ALS community".titleize
=> "ALS Community"

Ah, much better. I had patched String in my projects to correct this. Now the band-aid can come off. Interestingly, it looks like this bug was not reported (shame on me); instead the fix was made in response to an issue where the character following an apostrophe was being incorrectly capitalized:

>> "frog's code".titleize
=> "Frog'S Code"

The change to address this case also solves the acronym problem. Details of the fix can be found here:

http://dev.rubyonrails.org/changeset/8533