Ferreting out trouble
Posted on June 24th, 2008
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.