diff options
8 files changed, 149 insertions, 41 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d4b8d9b456..b539c893e3 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add support for arbitrary hashes in strong parameters: + + ```ruby + params.permit(preferences: {}) + ``` + + *Xavier Noria* + * Add `ActionController::Parameters#merge!`, which behaves the same as `Hash#merge!`. *Yuji Yaginuma* diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 98aacd53ab..4d9ed976d6 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -334,6 +334,15 @@ module ActionController # params = ActionController::Parameters.new(tags: ['rails', 'parameters']) # params.permit(tags: []) # + # Sometimes it is not possible or convenient to declare the valid keys of + # a hash parameter or its internal structure. Just map to an empty hash: + # + # params.permit(preferences: {}) + # + # but be careful because this opens the door to arbitrary input. In this + # case, +permit+ ensures values in the returned structure are permitted + # scalars and filters out anything else. + # # You can also use +permit+ on nested parameters, like: # # params = ActionController::Parameters.new({ @@ -766,6 +775,7 @@ module ActionController end EMPTY_ARRAY = [] + EMPTY_HASH = {} def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -779,6 +789,11 @@ module ActionController array_of_permitted_scalars?(self[key]) do |val| params[key] = val end + elsif filter[key] == EMPTY_HASH + # Declaration { preferences: {} }. + if value.is_a?(Parameters) + params[key] = permit_any_in_parameters(value) + end elsif non_scalar?(value) # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| @@ -788,6 +803,36 @@ module ActionController end end + def permit_any_in_parameters(params) + self.class.new.tap do |sanitized| + params.each do |key, value| + if permitted_scalar?(value) + sanitized[key] = value + elsif value.is_a?(Array) + sanitized[key] = permit_any_in_array(value) + elsif value.is_a?(Parameters) + sanitized[key] = permit_any_in_parameters(value) + else + # Filter this one out. + end + end + end + end + + def permit_any_in_array(array) + [].tap do |sanitized| + array.each do |element| + if permitted_scalar?(element) + sanitized << element + elsif element.is_a?(Parameters) + sanitized << permit_any_in_parameters(element) + else + # Filter this one out. + end + end + end + end + def initialize_copy(source) super @parameters = @parameters.dup diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 2b9a95d17c..16637d330b 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -168,6 +168,39 @@ class ParametersPermitTest < ActiveSupport::TestCase end end + test "key to empty hash: arbitrary hashes are permitted" do + params = ActionController::Parameters.new( + username: "fxn", + preferences: { + scheme: "Marazul", + font: { + name: "Source Code Pro", + size: 12 + }, + tabstops: [4, 8, 12, 16], + suspicious: [true, Object.new, false, /yo!/], + dubious: [{a: :a, b: /wtf!/}, {c: :c}], + injected: Object.new + }, + hacked: 1 # not a hash + ) + + permitted = params.permit(:username, preferences: {}, hacked: {}) + + assert_equal "fxn", permitted[:username] + assert_equal "Marazul", permitted[:preferences][:scheme] + assert_equal "Source Code Pro", permitted[:preferences][:font][:name] + assert_equal 12, permitted[:preferences][:font][:size] + assert_equal [4, 8, 12, 16], permitted[:preferences][:tabstops] + assert_equal [true, false], permitted[:preferences][:suspicious] + assert_equal :a, permitted[:preferences][:dubious][0][:a] + assert_equal :c, permitted[:preferences][:dubious][1][:c] + + assert_filtered_out permitted[:preferences][:dubious][0], :b + assert_filtered_out permitted[:preferences], :injected + assert_filtered_out permitted, :hacked + end + test "fetch raises ParameterMissing exception" do e = assert_raises(ActionController::ParameterMissing) do @params.fetch :foo diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 137396a677..237367c8b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -161,6 +161,14 @@ module ActiveRecord SchemaCreation.new self end + # Returns an array of +Column+ objects for the table specified by +table_name+. + def columns(table_name) # :nodoc: + table_name = table_name.to_s + column_definitions(table_name).map do |field| + new_column_from_field(table_name, field) + end + end + # this method must only be called while holding connection pool's mutex def lease if in_use? diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 6f334f5c8d..cbbba5b1a5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -398,18 +398,14 @@ module ActiveRecord indexes end - # Returns an array of +Column+ objects for the table specified by +table_name+. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - column_definitions(table_name).map do |field| - type_metadata = fetch_type_metadata(field[:Type], field[:Extra]) - if type_metadata.type == :datetime && field[:Default] == "CURRENT_TIMESTAMP" - default, default_function = nil, field[:Default] - else - default, default_function = field[:Default], nil - end - new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) + def new_column_from_field(table_name, field) # :nodoc: + type_metadata = fetch_type_metadata(field[:Type], field[:Extra]) + if type_metadata.type == :datetime && field[:Default] == "CURRENT_TIMESTAMP" + default, default_function = nil, field[:Default] + else + default, default_function = field[:Default], nil end + new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) end def table_comment(table_name) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 69f797da3a..9e7487b27f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -221,21 +221,23 @@ module ActiveRecord end.compact end - # Returns the list of all column definitions for a table. - def columns(table_name) # :nodoc: - table_name = table_name.to_s - column_definitions(table_name).map do |column_name, type, default, notnull, oid, fmod, collation, comment| - oid = oid.to_i - fmod = fmod.to_i - type_metadata = fetch_type_metadata(column_name, type, oid, fmod) - default_value = extract_value_from_default(default) - default_function = extract_default_function(default_value, default) - new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation, comment: comment.presence) - end - end - - def new_column(*args) # :nodoc: - PostgreSQLColumn.new(*args) + def new_column_from_field(table_name, field) # :nondoc: + column_name, type, default, notnull, oid, fmod, collation, comment = field + oid = oid.to_i + fmod = fmod.to_i + type_metadata = fetch_type_metadata(column_name, type, oid, fmod) + default_value = extract_value_from_default(default) + default_function = extract_default_function(default_value, default) + PostgreSQLColumn.new( + column_name, + default_value, + type_metadata, + !notnull, + table_name, + default_function, + collation, + comment: comment.presence + ) end def table_options(table_name) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 2c50321048..6b215a7374 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -308,20 +308,24 @@ module ActiveRecord def columns(table_name) # :nodoc: table_name = table_name.to_s table_structure(table_name).map do |field| - case field["dflt_value"] - when /^null$/i - field["dflt_value"] = nil - when /^'(.*)'$/m - field["dflt_value"] = $1.gsub("''", "'") - when /^"(.*)"$/m - field["dflt_value"] = $1.gsub('""', '"') - end + new_column_from_field(table_name, field) + end + end - collation = field["collation"] - sql_type = field["type"] - type_metadata = fetch_type_metadata(sql_type) - new_column(field["name"], field["dflt_value"], type_metadata, field["notnull"].to_i == 0, table_name, nil, collation) + def new_column_from_field(table_name, field) # :nondoc: + case field["dflt_value"] + when /^null$/i + field["dflt_value"] = nil + when /^'(.*)'$/m + field["dflt_value"] = $1.gsub("''", "'") + when /^"(.*)"$/m + field["dflt_value"] = $1.gsub('""', '"') end + + collation = field["collation"] + sql_type = field["type"] + type_metadata = fetch_type_metadata(sql_type) + new_column(field["name"], field["dflt_value"], type_metadata, field["notnull"].to_i == 0, table_name, nil, collation) end # Returns an array of indexes for the given table. diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 7b1138c7d4..40eb838d32 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -258,6 +258,17 @@ scalar values, map the key to an empty array: params.permit(id: []) ``` +Sometimes it is not possible or convenient to declare the valid keys of +a hash parameter or its internal structure. Just map to an empty hash: + +```ruby +params.permit(preferences: {}) +``` + +but be careful because this opens the door to arbitrary input. In this +case, `permit` ensures values in the returned structure are permitted +scalars and filters out anything else. + To whitelist an entire hash of parameters, the `permit!` method can be used: @@ -265,9 +276,10 @@ used: params.require(:log_entry).permit! ``` -This will mark the `:log_entry` parameters hash and any sub-hash of it as -permitted. Extreme care should be taken when using `permit!`, as it -will allow all current and future model attributes to be mass-assigned. +This marks the `:log_entry` parameters hash and any sub-hash of it as +permitted and does not check for permitted scalars, anything is accepted. +Extreme care should be taken when using `permit!`, as it will allow all current +and future model attributes to be mass-assigned. #### Nested Parameters |