diff options
Diffstat (limited to 'actionpack')
11 files changed, 152 insertions, 17 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index c9c347ea26..f477d18c6a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,4 +1,24 @@ -* Allow keys not found in RACK_KEY_TRANSLATION for setting the environment when rendering +* Fixes multiple calls to `logger.fatal` instead of a single call, + for every line in an exception backtrace, when printing trace + from `DebugExceptions` middleware. + + Fixes #26134 + + *Vipul A M* + +* 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* + +* Allow keys not found in `RACK_KEY_TRANSLATION` for setting the environment when rendering arbitrary templates. *Sammy Larbi* diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 8e588812f8..603c2e9ea7 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -215,7 +215,7 @@ module AbstractController # ==== Returns # * <tt>string</tt> - The name of the method that handles the action # * false - No valid method name could be found. - # Raise AbstractController::ActionNotFound. + # Raise +AbstractController::ActionNotFound+. def _find_action_name(action_name) _valid_action_name?(action_name) && method_for_action(action_name) end @@ -231,11 +231,11 @@ module AbstractController # with a template matching the action name is considered to exist. # # If you override this method to handle additional cases, you may - # also provide a method (like _handle_method_missing) to handle + # also provide a method (like +_handle_method_missing+) to handle # the case. # - # If none of these conditions are true, and method_for_action - # returns nil, an AbstractController::ActionNotFound exception will be raised. + # If none of these conditions are true, and +method_for_action+ + # returns +nil+, an +AbstractController::ActionNotFound+ exception will be raised. # # ==== Parameters # * <tt>action_name</tt> - An action name to find a method name for diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb index 6c103bb042..798564db96 100644 --- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -40,7 +40,7 @@ module ActionController end # Pick the template digest to include in the ETag. If the +:template+ option - # is present, use the named template. If +:template+ is nil or absent, use + # is present, use the named template. If +:template+ is +nil+ or absent, use # the default controller/action template. If +:template+ is false, omit the # template digest from the ETag. def pick_template_for_etag(options) diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 9c82abb640..e14da22e01 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({ @@ -382,14 +391,15 @@ module ActionController case filter when Symbol, String permitted_scalar_filter(params, filter) - when Hash then + when Hash hash_filter(params, filter) end end unpermitted_parameters!(params) if self.class.action_on_unpermitted_parameters - params.permit! + params.permitted = true + params end # Returns a parameter for the given +key+. If not found, @@ -539,7 +549,7 @@ module ActionController new_instance_with_inherited_permitted_status(@parameters.select(&block)) end - # Equivalent to Hash#keep_if, but returns nil if no changes were made. + # Equivalent to Hash#keep_if, but returns +nil+ if no changes were made. def select!(&block) @parameters.select!(&block) self @@ -573,6 +583,13 @@ module ActionController ) end + # Returns current <tt>ActionController::Parameters</tt> instance which + # +other_hash+ merges into current hash. + def merge!(other_hash) + @parameters.merge!(other_hash.to_h) + self + end + # This is required by ActiveModel attribute assignment, so that user can # pass +Parameters+ to a mass assignment methods in a model. It should not # matter as we are using +HashWithIndifferentAccess+ internally. @@ -759,6 +776,7 @@ module ActionController end EMPTY_ARRAY = [] + EMPTY_HASH = {} def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -772,6 +790,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| @@ -781,6 +804,39 @@ module ActionController end end + def permit_any_in_parameters(params) + self.class.new.tap do |sanitized| + params.each do |key, value| + case value + when ->(v) { permitted_scalar?(v) } + sanitized[key] = value + when Array + sanitized[key] = permit_any_in_array(value) + when Parameters + sanitized[key] = permit_any_in_parameters(value) + else + # Filter this one out. + end + end + sanitized.permitted = true + end + end + + def permit_any_in_array(array) + [].tap do |sanitized| + array.each do |element| + case element + when ->(e) { permitted_scalar?(e) } + sanitized << element + when 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/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 803ac52605..357ca56036 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -249,7 +249,7 @@ module ActionDispatch # :nodoc: end end - # Sets the HTTP character set. In case of nil parameter + # Sets the HTTP character set. In case of +nil+ parameter # it sets the charset to utf-8. # # response.charset = 'utf-16' # => 'utf-16' diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 4fc4df4463..956c53e813 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -179,7 +179,7 @@ module ActionDispatch # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed - # cookie was tampered with by the user (or a 3rd party), nil will be returned. + # cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # # If +secrets.secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, # legacy cookies signed with the old key generator will be transparently upgraded. @@ -202,7 +202,7 @@ module ActionDispatch end # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read. - # If the cookie was tampered with by the user (or a 3rd party), nil will be returned. + # If the cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # # If +secrets.secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, # legacy cookies signed with the old key generator will be transparently upgraded. diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 1a9018fe0c..1c720c5a8e 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -175,7 +175,11 @@ module ActionDispatch end def log_array(logger, array) - array.map { |line| logger.fatal line } + if logger.formatter && logger.formatter.respond_to?(:tags_text) + logger.fatal array.join("\n#{logger.formatter.tags_text}") + else + logger.fatal array.join("\n") + end end def logger(request) diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 6900934712..6dddcc6ee1 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -129,7 +129,7 @@ module ActionDispatch end # Builds a hash containing the flashes to keep for the next request. - # If there are none to keep, returns nil. + # If there are none to keep, returns +nil+. def to_session_value #:nodoc: flashes_to_keep = @flashes.except(*@discard) return nil if flashes_to_keep.empty? diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 8b98009efc..a2a80f39fc 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -85,7 +85,7 @@ module ActionDispatch end # Returns value of the key stored in the session or - # nil if the given key is not found in the session. + # +nil+ if the given key is not found in the session. def [](key) load_for_read! @delegate[key.to_s] diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 1cb7173aa0..60bfb66f2f 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -168,6 +168,44 @@ 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 permitted.permitted? + assert permitted[:preferences].permitted? + assert permitted[:preferences][:font].permitted? + assert permitted[:preferences][:dubious].all?(&:permitted?) + + 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 @@ -244,6 +282,23 @@ class ParametersPermitTest < ActiveSupport::TestCase assert merged_params[:id] end + test "not permitted is sticky beyond merge!" do + assert_not @params.merge!(a: "b").permitted? + end + + test "permitted is sticky beyond merge!" do + @params.permit! + assert @params.merge!(a: "b").permitted? + end + + test "merge! with parameters" do + other_params = ActionController::Parameters.new(id: "1234").permit! + @params.merge!(other_params) + + assert_equal "1234", @params[:id] + assert_equal "32", @params[:person][:age] + end + test "modifying the parameters" do @params[:person][:hometown] = "Chicago" @params[:person][:family] = { brother: "Jonas" } diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index e0dffc4be4..46a2ab8ccf 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -215,11 +215,11 @@ class ParamsWrapperTest < ActionController::TestCase def test_handles_empty_content_type with_default_wrapper_options do - @request.env["CONTENT_TYPE"] = '' + @request.env["CONTENT_TYPE"] = nil _controller_class.dispatch(:parse, @request, @response) assert_equal 200, @response.status - assert_equal '', @response.body + assert_equal "", @response.body end end end |