aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib/action_controller/metal/strong_parameters.rb
diff options
context:
space:
mode:
authorXavier Noria <fxn@hashref.com>2014-06-07 13:04:40 +0200
committerXavier Noria <fxn@hashref.com>2014-06-07 13:19:16 +0200
commit1ecada20d163ec1a3b0a3b6b51922da1dd7f089e (patch)
treecdede81a976ab194a53c32b38ba60338e4a17f95 /actionpack/lib/action_controller/metal/strong_parameters.rb
parenta39c88b5c9ddaf7df6768db0171846a5db1402fd (diff)
downloadrails-1ecada20d163ec1a3b0a3b6b51922da1dd7f089e.tar.gz
rails-1ecada20d163ec1a3b0a3b6b51922da1dd7f089e.tar.bz2
rails-1ecada20d163ec1a3b0a3b6b51922da1dd7f089e.zip
Revert "Convert StrongParameters cache to a hash. This fixes an unbounded"
We cannot cache keys because arrays are mutable. We rather want to cache the arrays. This behaviour is tailor-made for the usage pattern strongs params is designed for. In a forthcoming commit I am going to add a test that covers why we need to cache by value. Every strong params instance has a live span of a request, the cache goes away with the object. Since strong params have such a concrete intention, it would be interesting to see if there are actually any real-world use cases that are an actual leak, one that practically may matter. I am not convinced that the theoretical leak has any practical consequences, but if it can be shown there are, then I believe we should either get rid of the cache (which is an optimization), or else wipe it in the mutating API. This reverts commit e63be2769c039e4e9ada523a8497ce3206cc8a9b.
Diffstat (limited to 'actionpack/lib/action_controller/metal/strong_parameters.rb')
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb12
1 files changed, 6 insertions, 6 deletions
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index 0236af4a19..55954d0f37 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -130,7 +130,7 @@ module ActionController
# looping in the common use case permit + mass-assignment. Defined in a
# method to instantiate it only if needed.
def converted_arrays
- @converted_arrays ||= {}
+ @converted_arrays ||= Set.new
end
# Returns +true+ if the parameter is permitted, +false+ otherwise.
@@ -333,15 +333,15 @@ module ActionController
private
def convert_hashes_to_parameters(key, value, assign_if_converted=true)
- converted = convert_value_to_parameters(key, value)
+ converted = convert_value_to_parameters(value)
self[key] = converted if assign_if_converted && !converted.equal?(value)
converted
end
- def convert_value_to_parameters(key, value)
- if value.is_a?(Array) && !converted_arrays.member?(key)
- converted = value.map { |v| convert_value_to_parameters(nil, v) }
- converted_arrays[key] = converted if key
+ def convert_value_to_parameters(value)
+ if value.is_a?(Array) && !converted_arrays.member?(value)
+ converted = value.map { |_| convert_value_to_parameters(_) }
+ converted_arrays << converted
converted
elsif value.is_a?(Parameters) || !value.is_a?(Hash)
value