Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

forces atomic values where expected

  • Loading branch information...
commit 4b47270a32d0269b8bc4fc6ac6e2d612f5c80d3f 1 parent 548d0e4
@fxn fxn authored
View
16 README.rdoc
@@ -29,9 +29,23 @@ In addition, parameters can be marked as required and flow through a predefined
end
end
+== Atomic Values
+
+Given
+
+ params.permit(:id)
+
+they key +:id+ will pass the whitelisting if it appears in +params+ and it has a string or integer associated. Otherwise the key is going to be filtered out, so arrays, hashes, or any other objects cannot be injected.
+
+To declare that the value in +params+ must be an array of strings or integers map the key to an empty array:
+
+ params.permit(:id => [])
+
+== Nested Parameters
+
You can also use permit on nested parameters, like:
- params.permit(:name, friends: [ :name, { family: [ :name ] }])
+ params.permit(:name, :emails => [], friends: [ :name, { family: [ :name ] }])
Thanks to Nick Kallen for the permit idea!
View
107 lib/action_controller/parameters.rb
@@ -12,7 +12,79 @@ def initialize(param)
end
end
+ module Filtering
+ def atomic?(value)
+ # We allow Integer for backwards compatibility. Parameters coming from web
+ # requests are strings, but it is not uncommon that users set integer IDs
+ # in controller tests, where the hash is passed as is to the controller.
+ #
+ # Note that we short-circuit the common case first.
+ value.is_a?(String) || value.is_a?(Integer)
+ end
+
+ def array_of_atomics?(value)
+ if value.is_a?(Array)
+ value.all? {|_| atomic?(_)}
+ end
+ end
+
+ def atomic_filter(params, key)
+ if has_key?(key) && atomic?(self[key])
+ params[key] = self[key]
+ end
+
+ keys.grep(/\A#{Regexp.escape(key.to_s)}\(\d+[if]?\)\z/).each do |key|
+ if atomic?(self[key])
+ params[key] = self[key]
+ end
+ end
+ end
+
+ def hash_filter(params, filter)
+ filter = filter.with_indifferent_access
+
+ # Slicing filters out non-declared keys.
+ slice(*filter.keys).each do |key, value|
+ return unless value
+
+ if filter[key] == []
+ # Declaration {:coment_ids => []}.
+ array_of_atomics_filter(params, key)
+ else
+ # Declaration {:user => :name} or {:user => [:name, :age, {:adress => ...}]}.
+ params[key] = each_element(value) do |element|
+ if element.is_a?(Hash)
+ element = self.class.new(element) unless element.respond_to?(:permit)
+ element.permit(*Array.wrap(filter[key]))
+ end
+ end
+ end
+ end
+ end
+
+ def array_of_atomics_filter(params, key)
+ if has_key?(key) && array_of_atomics?(self[key])
+ params[key] = self[key]
+ end
+ end
+
+ def each_element(value)
+ if value.is_a?(Array)
+ value.map { |el| yield el }.compact
+ # fields_for on an array of records uses numeric hash keys.
+ elsif value.is_a?(Hash) && value.keys.all? { |k| k =~ /\A-?\d+\z/ }
+ hash = value.class.new
+ value.each { |k,v| hash[k] = yield v }
+ hash
+ else
+ yield value
+ end
+ end
+ end
+
class Parameters < ActiveSupport::HashWithIndifferentAccess
+ include Filtering
+
attr_accessor :permitted
alias :permitted? :permitted
@@ -42,26 +114,10 @@ def permit(*filters)
filters.each do |filter|
case filter
- when Symbol, String then
- params[filter] = self[filter] if has_key?(filter)
- keys.grep(/\A#{Regexp.escape(filter.to_s)}\(\d+[if]?\)\z/).each { |key| params[key] = self[key] }
+ when Symbol, String
+ atomic_filter(params, filter)
when Hash then
- filter = filter.with_indifferent_access
-
- self.slice(*filter.keys).each do |key, value|
- return unless value
-
- key = key.to_sym
-
- params[key] = each_element(value) do |element|
- # filters are a Hash, so we expect value to be a Hash too
- next if filter.is_a?(Hash) && !element.is_a?(Hash)
-
- element = self.class.new(element) if !element.respond_to?(:permit)
-
- element.permit(*Array.wrap(filter[key]))
- end
- end
+ hash_filter(params, filter)
end
end
@@ -111,19 +167,6 @@ def convert_hashes_to_parameters(key, value)
self[key] = self.class.new(value)
end
end
-
- def each_element(object)
- if object.is_a?(Array)
- object.map { |el| yield el }.compact
- # fields_for on an array of records uses numeric hash keys
- elsif object.is_a?(Hash) && object.keys.all? { |k| k =~ /\A-?\d+\z/ }
- hash = object.class.new
- object.each { |k,v| hash[k] = yield v }
- hash
- else
- yield object
- end
- end
end
module StrongParameters
View
133 test/nested_parameters_test.rb
@@ -3,9 +3,125 @@
class NestedParametersTest < ActiveSupport::TestCase
def assert_filtered_out(params, key)
- assert !params.has_key?(key), "the key #{key.inspect} has not been ignored"
+ assert !params.has_key?(key), "key #{key.inspect} has not been filtered out"
end
+ #
+ # -- Basic interface ---------------------------------------------------------
+ #
+
+ # --- nothing ----------------------------------------------------------------
+
+ test 'if nothing is permitted, the hash becomes empty' do
+ params = ActionController::Parameters.new(:id => '1234')
+ permitted = params.permit
+ permitted.permitted?
+ permitted.empty?
+ end
+
+ # --- key --------------------------------------------------------------------
+
+ test 'key: atomic values' do
+ params = ActionController::Parameters.new(:id => '1234')
+ permitted = params.permit(:id)
+ assert_equal '1234', permitted[:id]
+
+ %w(i f).each do |suffix|
+ params = ActionController::Parameters.new("foo(000#{suffix})" => '5678')
+ permitted = params.permit(:foo)
+ assert_equal '5678', permitted["foo(000#{suffix})"]
+ end
+ end
+
+ test 'key: unknown keys are filtered out' do
+ params = ActionController::Parameters.new(:id => '1234', :injected => 'injected')
+ permitted = params.permit(:id)
+ assert_equal '1234', permitted[:id]
+ assert_filtered_out permitted, :injected
+ end
+
+ test 'key: arrays are filtered out' do
+ [[], [1], ['1']].each do |array|
+ params = ActionController::Parameters.new(:id => array)
+ permitted = params.permit(:id)
+ assert_filtered_out permitted, :id
+
+ %w(i f).each do |suffix|
+ params = ActionController::Parameters.new("foo(000#{suffix})" => array)
+ permitted = params.permit(:foo)
+ assert_filtered_out permitted, "foo(000#{suffix})"
+ end
+ end
+ end
+
+ test 'key: hashes are filtered out' do
+ [{}, {:foo => 1}, {:foo => 'bar'}].each do |hash|
+ params = ActionController::Parameters.new(:id => hash)
+ permitted = params.permit(:id)
+ assert_filtered_out permitted, :id
+
+ %w(i f).each do |suffix|
+ params = ActionController::Parameters.new("foo(000#{suffix})" => hash)
+ permitted = params.permit(:foo)
+ assert_filtered_out permitted, "foo(000#{suffix})"
+ end
+ end
+ end
+
+ test 'key: non-atomic objects are filtered out' do
+ params = ActionController::Parameters.new(:id => Object.new)
+ permitted = params.permit(:id)
+ assert_filtered_out permitted, :id
+
+ %w(i f).each do |suffix|
+ params = ActionController::Parameters.new("foo(000#{suffix})" => Object.new)
+ permitted = params.permit(:foo)
+ assert_filtered_out permitted, "foo(000#{suffix})"
+ end
+ end
+
+ test 'key: it is not assigned if not present in params' do
+ params = ActionController::Parameters.new(:name => 'Joe')
+ permitted = params.permit(:id)
+ assert !permitted.has_key?(:id)
+ end
+
+ # --- key to empty array -----------------------------------------------------
+
+ test 'key to empty array: empty arrays pass' do
+ params = ActionController::Parameters.new(:id => [])
+ permitted = params.permit(:id => [])
+ assert_equal [], permitted[:id]
+ end
+
+ test 'key to empty array: arrays of atomics pass' do
+ [['foo'], [1], ['foo', 'bar'], [1, 2, 3]].each do |array|
+ params = ActionController::Parameters.new(:id => array)
+ permitted = params.permit(:id => [])
+ assert_equal array, permitted[:id]
+ end
+ end
+
+ test 'key to empty array: atomic values do not pass' do
+ ['foo', 1].each do |atomic|
+ params = ActionController::Parameters.new(:id => atomic)
+ permitted = params.permit(:id => [])
+ assert_filtered_out permitted, :id
+ end
+ end
+
+ test 'key to empty array: arrays of non-atomic do not pass' do
+ [[Object.new], [[]], [[1]], [{}], [{:id => '1'}]].each do |non_atomic|
+ params = ActionController::Parameters.new(:id => non_atomic)
+ permitted = params.permit(:id => [])
+ assert_filtered_out permitted, :id
+ end
+ end
+
+ #
+ # --- Nesting ----------------------------------------------------------------
+ #
+
test "permitted nested parameters" do
params = ActionController::Parameters.new({
:book => {
@@ -15,6 +131,8 @@ def assert_filtered_out(params, key)
:born => "1564-04-26"
}, {
:name => "Christopher Marlowe"
+ }, {
+ :name => %w(malicious injected names)
}],
:details => {
:pages => 200,
@@ -32,9 +150,11 @@ def assert_filtered_out(params, key)
assert_equal "Christopher Marlowe", permitted[:book][:authors][1][:name]
assert_equal 200, permitted[:book][:details][:pages]
+ assert_filtered_out permitted[:book][:authors][2], :name
+
assert_filtered_out permitted, :magazine
assert_filtered_out permitted[:book][:details], :genre
- assert_filtered_out permitted[:book][:authors][1], :born
+ assert_filtered_out permitted[:book][:authors][0], :born
end
test "permitted nested parameters with a string or a symbol as a key" do
@@ -69,7 +189,7 @@ def assert_filtered_out(params, key)
}
})
- permitted = params.permit :book => :genres
+ permitted = params.permit :book => {:genres => []}
assert_equal ["Tragedy"], permitted[:book][:genres]
end
@@ -128,7 +248,8 @@ def assert_filtered_out(params, key)
:book => {
:authors_attributes => {
:'0' => { :name => 'William Shakespeare', :age_of_death => '52' },
- :'1' => { :name => 'Unattributed Assistant' }
+ :'1' => { :name => 'Unattributed Assistant' },
+ :'2' => { :name => %w(injected names)}
}
}
})
@@ -136,9 +257,11 @@ def assert_filtered_out(params, key)
assert_not_nil permitted[:book][:authors_attributes]['0']
assert_not_nil permitted[:book][:authors_attributes]['1']
- assert_nil permitted[:book][:authors_attributes]['0'][:age_of_death]
+ assert_empty permitted[:book][:authors_attributes]['2']
assert_equal 'William Shakespeare', permitted[:book][:authors_attributes]['0'][:name]
assert_equal 'Unattributed Assistant', permitted[:book][:authors_attributes]['1'][:name]
+
+ assert_filtered_out permitted[:book][:authors_attributes]['0'], :age_of_death
end
test "fields_for_style_nested_params with negative numbers" do
Please sign in to comment.
Something went wrong with that request. Please try again.