aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb115
-rw-r--r--actionpack/lib/action_dispatch/routing.rb9
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb6
-rw-r--r--actionpack/lib/action_dispatch/routing/url_for.rb13
-rw-r--r--actionpack/test/controller/parameters/parameters_permit_test.rb65
-rw-r--r--actionpack/test/controller/redirect_test.rb4
-rw-r--r--actionpack/test/controller/required_params_test.rb24
-rw-r--r--actionpack/test/controller/url_for_test.rb2
-rw-r--r--actionpack/test/dispatch/routing/custom_url_helpers_test.rb8
-rw-r--r--actionpack/test/dispatch/routing_test.rb2
-rw-r--r--actionview/lib/action_view/helpers/url_helper.rb5
-rw-r--r--actionview/test/template/url_helper_test.rb6
12 files changed, 168 insertions, 91 deletions
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index baf15570d5..7864f9decd 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -43,6 +43,18 @@ module ActionController
end
end
+ # Raised when a Parameters instance is not marked as permitted and
+ # an operation to transform it to hash is called.
+ #
+ # params = ActionController::Parameters.new(a: "123", b: "456")
+ # params.to_h
+ # # => ActionController::UnfilteredParameters: unable to convert unpermitted parameters to hash
+ class UnfilteredParameters < ArgumentError
+ def initialize # :nodoc:
+ super("unable to convert unpermitted parameters to hash")
+ end
+ end
+
# == Action Controller \Parameters
#
# Allows you to choose which attributes should be whitelisted for mass updating
@@ -53,9 +65,9 @@ module ActionController
#
# params = ActionController::Parameters.new({
# person: {
- # name: 'Francesco',
+ # name: "Francesco",
# age: 22,
- # role: 'admin'
+ # role: "admin"
# }
# })
#
@@ -103,7 +115,7 @@ module ActionController
# You can fetch values of <tt>ActionController::Parameters</tt> using either
# <tt>:key</tt> or <tt>"key"</tt>.
#
- # params = ActionController::Parameters.new(key: 'value')
+ # params = ActionController::Parameters.new(key: "value")
# params[:key] # => "value"
# params["key"] # => "value"
class Parameters
@@ -203,13 +215,13 @@ module ActionController
# class Person < ActiveRecord::Base
# end
#
- # params = ActionController::Parameters.new(name: 'Francesco')
+ # params = ActionController::Parameters.new(name: "Francesco")
# params.permitted? # => false
# Person.new(params) # => ActiveModel::ForbiddenAttributesError
#
# ActionController::Parameters.permit_all_parameters = true
#
- # params = ActionController::Parameters.new(name: 'Francesco')
+ # params = ActionController::Parameters.new(name: "Francesco")
# params.permitted? # => true
# Person.new(params) # => #<Person id: nil, name: "Francesco">
def initialize(parameters = {})
@@ -228,13 +240,14 @@ module ActionController
end
# Returns a safe <tt>ActiveSupport::HashWithIndifferentAccess</tt>
- # representation of this parameter with all unpermitted keys removed.
+ # representation of the parameters with all unpermitted keys removed.
#
# params = ActionController::Parameters.new({
- # name: 'Senjougahara Hitagi',
- # oddity: 'Heavy stone crab'
+ # name: "Senjougahara Hitagi",
+ # oddity: "Heavy stone crab"
# })
- # params.to_h # => {}
+ # params.to_h
+ # # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash
#
# safe_params = params.permit(:name)
# safe_params.to_h # => {"name"=>"Senjougahara Hitagi"}
@@ -242,17 +255,61 @@ module ActionController
if permitted?
convert_parameters_to_hashes(@parameters, :to_h)
else
- slice(*self.class.always_permitted_parameters).permit!.to_h
+ raise UnfilteredParameters
end
end
+ # Returns a safe <tt>Hash</tt> representation of the parameters
+ # with all unpermitted keys removed.
+ #
+ # params = ActionController::Parameters.new({
+ # name: "Senjougahara Hitagi",
+ # oddity: "Heavy stone crab"
+ # })
+ # params.to_hash
+ # # => ActionController::UnfilteredParameters: unable to convert unfiltered parameters to hash
+ #
+ # safe_params = params.permit(:name)
+ # safe_params.to_hash # => {"name"=>"Senjougahara Hitagi"}
+ def to_hash
+ to_h.to_hash
+ end
+
+ # Returns a string representation of the receiver suitable for use as a URL
+ # query string:
+ #
+ # params = ActionController::Parameters.new({
+ # name: "David",
+ # nationality: "Danish"
+ # })
+ # params.to_query
+ # # => "name=David&nationality=Danish"
+ #
+ # An optional namespace can be passed to enclose key names:
+ #
+ # params = ActionController::Parameters.new({
+ # name: "David",
+ # nationality: "Danish"
+ # })
+ # params.to_query("user")
+ # # => "user%5Bname%5D=David&user%5Bnationality%5D=Danish"
+ #
+ # The string pairs "key=value" that conform the query string
+ # are sorted lexicographically in ascending order.
+ #
+ # This method is also aliased as +to_param+.
+ def to_query(*args)
+ to_h.to_query(*args)
+ end
+ alias_method :to_param, :to_query
+
# Returns an unsafe, unfiltered
- # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this
- # parameter.
+ # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of the
+ # parameters.
#
# params = ActionController::Parameters.new({
- # name: 'Senjougahara Hitagi',
- # oddity: 'Heavy stone crab'
+ # name: "Senjougahara Hitagi",
+ # oddity: "Heavy stone crab"
# })
# params.to_unsafe_h
# # => {"name"=>"Senjougahara Hitagi", "oddity" => "Heavy stone crab"}
@@ -297,7 +354,7 @@ module ActionController
# class Person < ActiveRecord::Base
# end
#
- # params = ActionController::Parameters.new(name: 'Francesco')
+ # params = ActionController::Parameters.new(name: "Francesco")
# params.permitted? # => false
# Person.new(params) # => ActiveModel::ForbiddenAttributesError
# params.permit!
@@ -319,7 +376,7 @@ module ActionController
# When passed a single key, if it exists and its associated value is
# either present or the singleton +false+, returns said value:
#
- # ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person)
+ # ActionController::Parameters.new(person: { name: "Francesco" }).require(:person)
# # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
#
# Otherwise raises <tt>ActionController::ParameterMissing</tt>:
@@ -352,7 +409,7 @@ module ActionController
# Technically this method can be used to fetch terminal values:
#
# # CAREFUL
- # params = ActionController::Parameters.new(person: { name: 'Finn' })
+ # params = ActionController::Parameters.new(person: { name: "Finn" })
# name = params.require(:person).require(:name) # CAREFUL
#
# but take into account that at some point those ones have to be permitted:
@@ -382,7 +439,7 @@ module ActionController
# for the object to +true+. This is useful for limiting which attributes
# should be allowed for mass updating.
#
- # params = ActionController::Parameters.new(user: { name: 'Francesco', age: 22, role: 'admin' })
+ # params = ActionController::Parameters.new(user: { name: "Francesco", age: 22, role: "admin" })
# permitted = params.require(:user).permit(:name, :age)
# permitted.permitted? # => true
# permitted.has_key?(:name) # => true
@@ -402,7 +459,7 @@ module ActionController
# You may declare that the parameter should be an array of permitted scalars
# by mapping it to an empty array:
#
- # params = ActionController::Parameters.new(tags: ['rails', 'parameters'])
+ # params = ActionController::Parameters.new(tags: ["rails", "parameters"])
# params.permit(tags: [])
#
# Sometimes it is not possible or convenient to declare the valid keys of
@@ -418,11 +475,11 @@ module ActionController
#
# params = ActionController::Parameters.new({
# person: {
- # name: 'Francesco',
+ # name: "Francesco",
# age: 22,
# pets: [{
- # name: 'Purplish',
- # category: 'dogs'
+ # name: "Purplish",
+ # category: "dogs"
# }]
# }
# })
@@ -441,8 +498,8 @@ module ActionController
# params = ActionController::Parameters.new({
# person: {
# contact: {
- # email: 'none@test.com',
- # phone: '555-1234'
+ # email: "none@test.com",
+ # phone: "555-1234"
# }
# }
# })
@@ -475,7 +532,7 @@ module ActionController
# Returns a parameter for the given +key+. If not found,
# returns +nil+.
#
- # params = ActionController::Parameters.new(person: { name: 'Francesco' })
+ # params = ActionController::Parameters.new(person: { name: "Francesco" })
# params[:person] # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
# params[:none] # => nil
def [](key)
@@ -494,11 +551,11 @@ module ActionController
# if more arguments are given, then that will be returned; if a block
# is given, then that will be run and its result returned.
#
- # params = ActionController::Parameters.new(person: { name: 'Francesco' })
+ # params = ActionController::Parameters.new(person: { name: "Francesco" })
# params.fetch(:person) # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false>
# params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none
- # params.fetch(:none, 'Francesco') # => "Francesco"
- # params.fetch(:none) { 'Francesco' } # => "Francesco"
+ # params.fetch(:none, "Francesco") # => "Francesco"
+ # params.fetch(:none) { "Francesco" } # => "Francesco"
def fetch(key, *args)
convert_value_to_parameters(
@parameters.fetch(key) {
@@ -715,8 +772,6 @@ module ActionController
end
end
- undef_method :to_param
-
# Returns duplicate of object including all parameters.
def deep_dup
self.class.new(@parameters.deep_dup).tap do |duplicate|
diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb
index 60d4789a63..87dd1eba38 100644
--- a/actionpack/lib/action_dispatch/routing.rb
+++ b/actionpack/lib/action_dispatch/routing.rb
@@ -254,14 +254,5 @@ module ActionDispatch
SEPARATORS = %w( / . ? ) #:nodoc:
HTTP_METHODS = [:get, :head, :post, :patch, :put, :delete, :options] #:nodoc:
-
- #:stopdoc:
- INSECURE_URL_PARAMETERS_MESSAGE = <<-MSG.squish
- Attempting to generate a URL from non-sanitized request parameters!
-
- An attacker can inject malicious data into the generated URL, such as
- changing the host. Whitelist and sanitize passed parameters to be secure.
- MSG
- #:startdoc:
end
end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 129e90037e..e1f9fc9ecc 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -318,11 +318,7 @@ module ActionDispatch
when Hash
args.pop
when ActionController::Parameters
- if last.permitted?
- args.pop.to_h
- else
- raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE
- end
+ args.pop.to_h
end
helper.call self, args, options
end
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 008216cc80..a9bdefa775 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -171,17 +171,10 @@ module ActionDispatch
case options
when nil
_routes.url_for(url_options.symbolize_keys)
- when Hash
+ when Hash, ActionController::Parameters
route_name = options.delete :use_route
- _routes.url_for(options.symbolize_keys.reverse_merge!(url_options),
- route_name)
- when ActionController::Parameters
- unless options.permitted?
- raise ArgumentError.new(ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE)
- end
- route_name = options.delete :use_route
- _routes.url_for(options.to_h.symbolize_keys.
- reverse_merge!(url_options), route_name)
+ merged_url_options = options.to_h.symbolize_keys.reverse_merge!(url_options)
+ _routes.url_for(merged_url_options, route_name)
when String
options
when Symbol
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 3e067314d6..daef9ad892 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -377,17 +377,17 @@ class ParametersPermitTest < ActiveSupport::TestCase
assert_equal "32", @params[:person].permit([ :age ])[:age]
end
- test "to_h returns empty hash on unpermitted params" do
- assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
- assert_not @params.to_h.is_a? ActionController::Parameters
- assert @params.to_h.empty?
+ test "to_h raises UnfilteredParameters on unfiltered params" do
+ assert_raises(ActionController::UnfilteredParameters) do
+ @params.to_h
+ end
end
test "to_h returns converted hash on permitted params" do
@params.permit!
- assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
- assert_not @params.to_h.is_a? ActionController::Parameters
+ assert_instance_of ActiveSupport::HashWithIndifferentAccess, @params.to_h
+ assert_not_kind_of ActionController::Parameters, @params.to_h
end
test "to_h returns converted hash when .permit_all_parameters is set" do
@@ -395,39 +395,64 @@ class ParametersPermitTest < ActiveSupport::TestCase
ActionController::Parameters.permit_all_parameters = true
params = ActionController::Parameters.new(crab: "Senjougahara Hitagi")
- assert params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess
- assert_not @params.to_h.is_a? ActionController::Parameters
+ assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h
+ assert_not_kind_of ActionController::Parameters, params.to_h
assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h)
ensure
ActionController::Parameters.permit_all_parameters = false
end
end
- test "to_h returns always permitted parameter on unpermitted params" do
- params = ActionController::Parameters.new(
- controller: "users",
- action: "create",
- user: {
- name: "Sengoku Nadeko"
- }
- )
+ test "to_hash raises UnfilteredParameters on unfiltered params" do
+ assert_raises(ActionController::UnfilteredParameters) do
+ @params.to_hash
+ end
+ end
+
+ test "to_hash returns converted hash on permitted params" do
+ @params.permit!
+
+ assert_instance_of Hash, @params.to_hash
+ assert_not_kind_of ActionController::Parameters, @params.to_hash
+ end
+
+ test "to_hash returns converted hash when .permit_all_parameters is set" do
+ begin
+ ActionController::Parameters.permit_all_parameters = true
+ params = ActionController::Parameters.new(crab: "Senjougahara Hitagi")
- assert_equal({ "controller" => "users", "action" => "create" }, params.to_h)
+ assert_instance_of Hash, params.to_hash
+ assert_not_kind_of ActionController::Parameters, params.to_hash
+ assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash)
+ assert_equal({ "crab" => "Senjougahara Hitagi" }, params)
+ ensure
+ ActionController::Parameters.permit_all_parameters = false
+ end
end
test "to_unsafe_h returns unfiltered params" do
- assert @params.to_unsafe_h.is_a? ActiveSupport::HashWithIndifferentAccess
- assert_not @params.to_unsafe_h.is_a? ActionController::Parameters
+ assert_instance_of ActiveSupport::HashWithIndifferentAccess, @params.to_unsafe_h
+ assert_not_kind_of ActionController::Parameters, @params.to_unsafe_h
end
test "to_unsafe_h returns unfiltered params even after accessing few keys" do
params = ActionController::Parameters.new("f" => { "language_facet" => ["Tibetan"] })
expected = { "f" => { "language_facet" => ["Tibetan"] } }
- assert params["f"].is_a? ActionController::Parameters
+ assert_instance_of ActionController::Parameters, params["f"]
assert_equal expected, params.to_unsafe_h
end
+ test "to_unsafe_h does not mutate the parameters" do
+ params = ActionController::Parameters.new("f" => { "language_facet" => ["Tibetan"] })
+ params[:f]
+
+ params.to_unsafe_h
+
+ assert_not_predicate params, :permitted?
+ assert_not_predicate params[:f], :permitted?
+ end
+
test "to_h only deep dups Ruby collections" do
company = Class.new do
attr_reader :dupped
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index f06a1f4d23..5b16af78c4 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -285,10 +285,10 @@ class RedirectTest < ActionController::TestCase
end
def test_redirect_to_params
- error = assert_raise(ArgumentError) do
+ error = assert_raise(ActionController::UnfilteredParameters) do
get :redirect_to_params
end
- assert_equal ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE, error.message
+ assert_equal "unable to convert unpermitted parameters to hash", error.message
end
def test_redirect_to_with_block
diff --git a/actionpack/test/controller/required_params_test.rb b/actionpack/test/controller/required_params_test.rb
index dd07c2486b..46bb374b3f 100644
--- a/actionpack/test/controller/required_params_test.rb
+++ b/actionpack/test/controller/required_params_test.rb
@@ -72,9 +72,27 @@ class ParametersRequireTest < ActiveSupport::TestCase
assert params.value?("cinco")
end
- test "to_query is not supported" do
- assert_raises(NoMethodError) do
- ActionController::Parameters.new(foo: "bar").to_param
+ test "to_param works like in a Hash" do
+ params = ActionController::Parameters.new(nested: { key: "value" }).permit!
+ assert_equal({ nested: { key: "value" } }.to_param, params.to_param)
+
+ params = { root: ActionController::Parameters.new(nested: { key: "value" }).permit! }
+ assert_equal({ root: { nested: { key: "value" } } }.to_param, params.to_param)
+
+ assert_raise(ActionController::UnfilteredParameters) do
+ ActionController::Parameters.new(nested: { key: "value" }).to_param
+ end
+ end
+
+ test "to_query works like in a Hash" do
+ params = ActionController::Parameters.new(nested: { key: "value" }).permit!
+ assert_equal({ nested: { key: "value" } }.to_query, params.to_query)
+
+ params = { root: ActionController::Parameters.new(nested: { key: "value" }).permit! }
+ assert_equal({ root: { nested: { key: "value" } } }.to_query, params.to_query)
+
+ assert_raise(ActionController::UnfilteredParameters) do
+ ActionController::Parameters.new(nested: { key: "value" }).to_query
end
end
end
diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb
index 862dcf01c3..2afe67ed91 100644
--- a/actionpack/test/controller/url_for_test.rb
+++ b/actionpack/test/controller/url_for_test.rb
@@ -386,7 +386,7 @@ module AbstractController
def test_url_action_controller_parameters
add_host!
- assert_raise(ArgumentError) do
+ assert_raise(ActionController::UnfilteredParameters) do
W.new.url_for(ActionController::Parameters.new(controller: "c", action: "a", protocol: "javascript", f: "%0Aeval(name)"))
end
end
diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
index cb5ca5888b..338992dda5 100644
--- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
+++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
@@ -165,8 +165,8 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
assert_equal "/", params_path(@safe_params)
assert_equal "/", Routes.url_helpers.params_path(@safe_params)
- assert_raises(ArgumentError) { params_path(@unsafe_params) }
- assert_raises(ArgumentError) { Routes.url_helpers.params_path(@unsafe_params) }
+ assert_raises(ActionController::UnfilteredParameters) { params_path(@unsafe_params) }
+ assert_raises(ActionController::UnfilteredParameters) { Routes.url_helpers.params_path(@unsafe_params) }
assert_equal "/basket", symbol_path
assert_equal "/basket", Routes.url_helpers.symbol_path
@@ -208,8 +208,8 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
assert_equal "http://www.example.com/", params_url(@safe_params)
assert_equal "http://www.example.com/", Routes.url_helpers.params_url(@safe_params)
- assert_raises(ArgumentError) { params_url(@unsafe_params) }
- assert_raises(ArgumentError) { Routes.url_helpers.params_url(@unsafe_params) }
+ assert_raises(ActionController::UnfilteredParameters) { params_url(@unsafe_params) }
+ assert_raises(ActionController::UnfilteredParameters) { Routes.url_helpers.params_url(@unsafe_params) }
assert_equal "http://www.example.com/basket", symbol_url
assert_equal "http://www.example.com/basket", Routes.url_helpers.symbol_url
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index fdc47743fa..d64917e0d3 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -3633,7 +3633,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
end
params = ActionController::Parameters.new(id: "1")
- assert_raises ArgumentError do
+ assert_raises ActionController::UnfilteredParameters do
root_path(params)
end
end
diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb
index 304db38060..70fc57c35f 100644
--- a/actionview/lib/action_view/helpers/url_helper.rb
+++ b/actionview/lib/action_view/helpers/url_helper.rb
@@ -621,11 +621,6 @@ module ActionView
# # => [{name: 'country[name]', value: 'Denmark'}]
def to_form_params(attribute, namespace = nil)
attribute = if attribute.respond_to?(:permitted?)
- unless attribute.permitted?
- raise ArgumentError, "Attempting to generate a button from non-sanitized request parameters!" \
- " Whitelist and sanitize passed parameters to be secure."
- end
-
attribute.to_h
else
attribute
diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb
index 09454b32cc..a6444a1686 100644
--- a/actionview/test/template/url_helper_test.rb
+++ b/actionview/test/template/url_helper_test.rb
@@ -231,7 +231,11 @@ class UrlHelperTest < ActiveSupport::TestCase
end
def to_h
- { foo: :bar, baz: "quux" }
+ if permitted?
+ { foo: :bar, baz: "quux" }
+ else
+ raise ArgumentError
+ end
end
end