Permalink
Browse files

Revert "Key the attributes hash with symbols"

This reverts commit 86c3dfb.

Conflicts:
	activerecord/lib/active_record/attribute_methods/read.rb

Reason: whilst this increased performance, it also presents a DoS risk
via memory exhaustion if users were allowing user input to dictate the
arguments of read/write_attribute. I will investigate alternative ways
to cut down on string allocations here.
  • Loading branch information...
1 parent 36360e9 commit 9e5f7cc62e9ef1e865c6eba28be69e935434f8e6 @jonleighton jonleighton committed Oct 5, 2012
@@ -46,7 +46,7 @@ def cache_attribute?(attr_name)
def define_method_attribute(attr_name)
generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
def __temp__
- read_attribute(:'#{attr_name}') { |n| missing_attribute(n, caller) }
+ read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) }
end
alias_method '#{attr_name}', :__temp__
undef_method :__temp__
@@ -70,17 +70,13 @@ def cacheable_column?(column)
# it has been typecast (for example, "2004-12-12" in a data column is cast
# to a date object, like Date.new(2004, 12, 12)).
def read_attribute(attr_name)
- return unless attr_name
- name_sym = attr_name.to_sym
-
# If it's cached, just return it
# We use #[] first as a perf optimization for non-nil values. See https://gist.github.com/3552829.
- @attributes_cache[name_sym] || @attributes_cache.fetch(name_sym) {
- name = attr_name.to_s
-
+ name = attr_name.to_s
+ @attributes_cache[name] || @attributes_cache.fetch(name) {
column = @columns_hash.fetch(name) {
return @attributes.fetch(name) {
- if name_sym == :id && self.class.primary_key != name
+ if name == 'id' && self.class.primary_key != name
read_attribute(self.class.primary_key)
end
}
@@ -91,7 +87,7 @@ def read_attribute(attr_name)
}
if self.class.cache_attribute?(name)
- @attributes_cache[name_sym] = column.type_cast(value)
+ @attributes_cache[name] = column.type_cast(value)
else
column.type_cast value
end
@@ -50,7 +50,7 @@ def #{attr_name}=(original_time)
if (rounded_value != rounded_time) || (!rounded_value && original_time)
write_attribute("#{attr_name}", original_time)
#{attr_name}_will_change!
- @attributes_cache[:"#{attr_name}"] = zoned_time
+ @attributes_cache["#{attr_name}"] = zoned_time
end
end
EOV
@@ -26,13 +26,13 @@ def define_method_attribute=(attr_name)
def write_attribute(attr_name, value)
attr_name = attr_name.to_s
attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key
- @attributes_cache.delete(attr_name.to_sym)
+ @attributes_cache.delete(attr_name)
column = column_for_attribute(attr_name)
# If we're dealing with a binary column, write the data to the cache
# so we don't attempt to typecast multiple times.
if column && column.binary?
- @attributes_cache[attr_name.to_sym] = value
+ @attributes_cache[attr_name] = value
end
if column || @attributes.has_key?(attr_name)
@@ -542,10 +542,10 @@ def test_accessing_cached_attributes_caches_the_converted_values_and_nothing_els
val = t.send attr_name unless attr_name == "type"
if attribute_gets_cached
assert cached_columns.include?(attr_name)
- assert_equal val, cache[attr_name.to_sym]
+ assert_equal val, cache[attr_name]
else
assert uncached_columns.include?(attr_name)
- assert !cache.include?(attr_name.to_sym)
+ assert !cache.include?(attr_name)
end
end
end

0 comments on commit 9e5f7cc

Please sign in to comment.