提交 1ecada20 编写于 作者: X Xavier Noria

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 e63be276.
上级 a39c88b5
......@@ -130,7 +130,7 @@ def initialize(attributes = nil)
# 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 @@ def permitted=(new_permitted)
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
......
......@@ -169,7 +169,7 @@ def assert_filtered_out(params, key)
test 'arrays are converted at most once' do
params = ActionController::Parameters.new(foo: [{}])
assert_same params[:foo], params[:foo]
assert params[:foo].equal?(params[:foo])
end
test "fetch doesnt raise ParameterMissing exception if there is a default" do
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册