This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

permits more scalar types

  • Loading branch information...
fxn committed Jan 20, 2013
1 parent cdea33c commit 4c21c676c1f7ea8895a59c2431df9a0339de21f0
Showing with 124 additions and 92 deletions.
  1. +5 −3 README.rdoc
  2. +94 −72 lib/action_controller/parameters.rb
  3. +25 −17 test/parameters_permit_test.rb
View
@@ -29,15 +29,17 @@ In addition, parameters can be marked as required and flow through a predefined
end
end
-== Atomic Values
+== Permitted Scalar Values
Given
params.permit(:id)
-the 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.
+the key +:id+ will pass the whitelisting if it appears in +params+ and it has a permitted scalar value 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:
+The permitted scalar types are +String+, +Symbol+, +NilClass+, +Numeric+, +TrueClass+, +FalseClass+, +Date+, +Time+, +DateTime+, +StringIO+, and +IO+.
+
+To declare that the value in +params+ must be an array of permitted scalar values map the key to an empty array:
params.permit(:id => [])
@@ -1,3 +1,7 @@
+require 'date'
+require 'bigdecimal'
+require 'stringio'
+
require 'active_support/concern'
require 'active_support/core_ext/hash/indifferent_access'
require 'action_controller'
@@ -12,79 +16,8 @@ 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 {:comment_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
@@ -115,7 +48,7 @@ def permit(*filters)
filters.each do |filter|
case filter
when Symbol, String
- atomic_filter(params, filter)
+ permitted_scalar_filter(params, filter)
when Hash then
hash_filter(params, filter)
end
@@ -159,6 +92,7 @@ def convert_value(value)
end
private
+
def convert_hashes_to_parameters(key, value)
if value.is_a?(Parameters) || !value.is_a?(Hash)
value
@@ -167,6 +101,94 @@ def convert_hashes_to_parameters(key, value)
self[key] = self.class.new(value)
end
end
+
+ #
+ # --- Filtering ----------------------------------------------------------
+ #
+
+ # This is a white list of permitted scalar types that includes the ones
+ # supported in XML and JSON requests.
+ #
+ # This list is in particular used to filter ordinary requests, String goes
+ # as first element to quickly short-circuit the common case.
+ #
+ # If you modify this collection please update the README.
+ PERMITTED_SCALAR_TYPES = [
+ String,
+ Symbol,
+ NilClass,
+ Numeric,
+ TrueClass,
+ FalseClass,
+ Date,
+ Time,
+ # DateTimes are Dates, we document the type but avoid the redundant check.
+ StringIO,
+ IO,
+ ]
+
+ def permitted_scalar?(value)
+ PERMITTED_SCALAR_TYPES.any? {|type| value.is_a?(type)}
+ end
+
+ def array_of_permitted_scalars?(value)
+ if value.is_a?(Array)
+ value.all? {|element| permitted_scalar?(element)}
+ end
+ end
+
+ def permitted_scalar_filter(params, key)
+ if has_key?(key) && permitted_scalar?(self[key])
+ params[key] = self[key]
+ end
+
+ keys.grep(/\A#{Regexp.escape(key.to_s)}\(\d+[if]?\)\z/).each do |key|
+ if permitted_scalar?(self[key])
+ params[key] = self[key]
+ end
+ end
+ end
+
+ def array_of_permitted_scalars_filter(params, key)
+ if has_key?(key) && array_of_permitted_scalars?(self[key])
+ params[key] = self[key]
+ 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 {:comment_ids => []}.
+ array_of_permitted_scalars_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 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
module StrongParameters
@@ -7,7 +7,7 @@ def assert_filtered_out(params, key)
end
#
- # -- Basic interface ---------------------------------------------------------
+ # --- Basic interface --------------------------------------------------------
#
# --- nothing ----------------------------------------------------------------
@@ -21,15 +21,23 @@ def assert_filtered_out(params, key)
# --- key --------------------------------------------------------------------
- test 'key: atomic values' do
- params = ActionController::Parameters.new(:id => '1234')
- permitted = params.permit(:id)
- assert_equal '1234', permitted[:id]
+ test 'key: permitted scalar values' do
+ values = ['a', :a, nil]
+ values += [0, 1.0, 2**128, BigDecimal.new(1)]
+ values += [true, false]
+ values += [Date.today, Time.now, DateTime.now]
+ values += [StringIO.new]
- %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})"]
+ values.each do |value|
+ params = ActionController::Parameters.new(:id => value)
+ permitted = params.permit(:id)
+ assert_equal value, permitted[:id]
+
+ %w(i f).each do |suffix|
+ params = ActionController::Parameters.new("foo(000#{suffix})" => value)
+ permitted = params.permit(:foo)
+ assert_equal value, permitted["foo(000#{suffix})"]
+ end
end
end
@@ -68,7 +76,7 @@ def assert_filtered_out(params, key)
end
end
- test 'key: non-atomic objects are filtered out' do
+ test 'key: non-permitted scalar values are filtered out' do
params = ActionController::Parameters.new(:id => Object.new)
permitted = params.permit(:id)
assert_filtered_out permitted, :id
@@ -94,25 +102,25 @@ def assert_filtered_out(params, key)
assert_equal [], permitted[:id]
end
- test 'key to empty array: arrays of atomics pass' do
+ test 'key to empty array: arrays of permitted scalars 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)
+ test 'key to empty array: permitted scalar values do not pass' do
+ ['foo', 1].each do |permitted_scalar|
+ params = ActionController::Parameters.new(:id => permitted_scalar)
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)
+ test 'key to empty array: arrays of non-permitted scalar do not pass' do
+ [[Object.new], [[]], [[1]], [{}], [{:id => '1'}]].each do |non_permitted_scalar|
+ params = ActionController::Parameters.new(:id => non_permitted_scalar)
permitted = params.permit(:id => [])
assert_filtered_out permitted, :id
end

0 comments on commit 4c21c67

Please sign in to comment.