diff options
21 files changed, 585 insertions, 100 deletions
@@ -14,8 +14,6 @@ gem 'turbolinks', github: 'rails/turbolinks', branch: 'master' gem 'coffee-rails', '~> 4.0.0' gem 'rails-html-sanitizer', github: 'rails/rails-html-sanitizer' gem 'rails-deprecated_sanitizer', github: 'rails/rails-deprecated_sanitizer' -#temporary gem until a new version of loofah is released -gem 'loofah', github: 'kaspth/loofah', branch: 'single-scrub' gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' # require: false so bcrypt is loaded only when has_secure_password is used. diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1c84bac3ff..e2731d0ee5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,38 @@ +* `ActionController::Parameters` will stop inheriting from `Hash` and + `HashWithIndifferentAccess` in the next major release. If you use any method + that is not available on `ActionController::Parameters` you should consider + calling `#to_h` to convert it to a `Hash` first before calling that method. + + *Prem Sichanugrist* + +* `ActionController::Parameters#to_h` now returns a `Hash` with unpermitted + keys removed. This change is to reflect on a security concern where some + method performed on an `ActionController::Parameters` may yield a `Hash` + object which does not maintain `permitted?` status. If you would like to + get a `Hash` with all the keys intact, duplicate and mark it as permitted + before calling `#to_h`. + + params = ActionController::Parameters.new({ + name: 'Senjougahara Hitagi', + oddity: 'Heavy stone crab' + }) + params.to_h + # => {} + + unsafe_params = params.dup.permit! + unsafe_params.to_h + # => {"name"=>"Senjougahara Hitagi", "oddity"=>"Heavy stone crab"} + + safe_params = params.permit(:name) + safe_params.to_h + # => {"name"=>"Senjougahara Hitagi"} + + This change is consider a stopgap as we cannot change the code to stop + `ActionController::Parameters` to inherit from `HashWithIndifferentAccess` + in the next minor release. + + *Prem Sichanugrist* + * Deprecated TagAssertions. *Kasper Timm Hansen* diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index bc27ecaa20..7038f0997f 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -141,6 +141,37 @@ module ActionController @permitted = self.class.permit_all_parameters end + # Returns a safe +Hash+ representation of this parameter with all + # unpermitted keys removed. + # + # params = ActionController::Parameters.new({ + # name: 'Senjougahara Hitagi', + # oddity: 'Heavy stone crab' + # }) + # params.to_h # => {} + # + # safe_params = params.permit(:name) + # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} + def to_h + if permitted? + to_hash + else + slice(*self.class.always_permitted_parameters).permit!.to_h + end + end + + # Convert all hashes in values into parameters, then yield each pair like + # the same way as <tt>Hash#each_pair</tt> + def each_pair(&block) + super do |key, value| + convert_hashes_to_parameters(key, value) + end + + super + end + + alias_method :each, :each_pair + # Attribute that keeps track of converted arrays, if any, to avoid double # looping in the common use case permit + mass-assignment. Defined in a # method to instantiate it only if needed. @@ -176,7 +207,6 @@ module ActionController # Person.new(params) # => #<Person id: nil, name: "Francesco"> def permit! each_pair do |key, value| - value = convert_hashes_to_parameters(key, value) Array.wrap(value).each do |v| v.permit! if v.respond_to? :permit! end @@ -331,11 +361,56 @@ module ActionController # params.slice(:a, :b) # => {"a"=>1, "b"=>2} # params.slice(:d) # => {} def slice(*keys) - self.class.new(super).tap do |new_instance| - new_instance.permitted = @permitted + new_instance_with_inherited_permitted_status(super) + end + + # Removes and returns the key/value pairs matching the given keys. + # + # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) + # params.extract!(:a, :b) # => {"a"=>1, "b"=>2} + # params # => {"c"=>3} + def extract!(*keys) + new_instance_with_inherited_permitted_status(super) + end + + # Returns a new <tt>ActionController::Parameters</tt> with the results of + # running +block+ once for every value. The keys are unchanged. + # + # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) + # params.transform_values { |x| x * 2 } + # # => {"a"=>2, "b"=>4, "c"=>6} + def transform_values + if block_given? + new_instance_with_inherited_permitted_status(super) + else + super + end + end + + # This method is here only to make sure that the returned object has the + # correct +permitted+ status. It should not matter since the parent of + # this object is +HashWithIndifferentAccess+ + def transform_keys # :nodoc: + if block_given? + new_instance_with_inherited_permitted_status(super) + else + super end end + # Deletes and returns a key-value pair from +Parameters+ whose key is equal + # to key. If the key is not found, returns the default value. If the + # optional code block is given and the key is not found, pass in the key + # and return the result of block. + def delete(key, &block) + convert_hashes_to_parameters(key, super, false) + end + + # Equivalent to Hash#keep_if, but returns nil if no changes were made. + def select!(&block) + convert_value_to_parameters(super) + end + # Returns an exact copy of the <tt>ActionController::Parameters</tt> # instance. +permitted+ state is kept on the duped object. # @@ -356,6 +431,12 @@ module ActionController end private + def new_instance_with_inherited_permitted_status(hash) + self.class.new(hash).tap do |new_instance| + new_instance.permitted = @permitted + end + end + def convert_hashes_to_parameters(key, value, assign_if_converted=true) converted = convert_value_to_parameters(value) self[key] = converted if assign_if_converted && !converted.equal?(value) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 8c035c3c6c..f35289253b 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -292,7 +292,7 @@ module ActionDispatch # Override Rack's GET method to support indifferent access def GET @env["action_dispatch.request.query_parameters"] ||= Utils.deep_munge(normalize_encode_params(super || {})) - rescue TypeError => e + rescue TypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new(:query, e) end alias :query_parameters :GET @@ -300,7 +300,7 @@ module ActionDispatch # Override Rack's POST method to support indifferent access def POST @env["action_dispatch.request.request_parameters"] ||= Utils.deep_munge(normalize_encode_params(super || {})) - rescue TypeError => e + rescue TypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new(:request, e) end alias :request_parameters :POST diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb new file mode 100644 index 0000000000..97875c3cbb --- /dev/null +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -0,0 +1,125 @@ +require 'abstract_unit' +require 'action_controller/metal/strong_parameters' +require 'active_support/core_ext/hash/transform_values' + +class ParametersAccessorsTest < ActiveSupport::TestCase + setup do + @params = ActionController::Parameters.new( + person: { + age: '32', + name: { + first: 'David', + last: 'Heinemeier Hansson' + }, + addresses: [{city: 'Chicago', state: 'Illinois'}] + } + ) + end + + test "[] retains permitted status" do + @params.permit! + assert @params[:person].permitted? + assert @params[:person][:name].permitted? + end + + test "[] retains unpermitted status" do + assert_not @params[:person].permitted? + assert_not @params[:person][:name].permitted? + end + + test "each carries permitted status" do + @params.permit! + @params.each { |key, value| assert(value.permitted?) if key == "person" } + end + + test "each carries unpermitted status" do + @params.each { |key, value| assert_not(value.permitted?) if key == "person" } + end + + test "each_pair carries permitted status" do + @params.permit! + @params.each_pair { |key, value| assert(value.permitted?) if key == "person" } + end + + test "each_pair carries unpermitted status" do + @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } + end + + test "except retains permitted status" do + @params.permit! + assert @params.except(:person).permitted? + assert @params[:person].except(:name).permitted? + end + + test "except retains unpermitted status" do + assert_not @params.except(:person).permitted? + assert_not @params[:person].except(:name).permitted? + end + + test "fetch retains permitted status" do + @params.permit! + assert @params.fetch(:person).permitted? + assert @params[:person].fetch(:name).permitted? + end + + test "fetch retains unpermitted status" do + assert_not @params.fetch(:person).permitted? + assert_not @params[:person].fetch(:name).permitted? + end + + test "reject retains permitted status" do + assert_not @params.reject { |k| k == "person" }.permitted? + end + + test "reject retains unpermitted status" do + @params.permit! + assert @params.reject { |k| k == "person" }.permitted? + end + + test "select retains permitted status" do + @params.permit! + assert @params.select { |k| k == "person" }.permitted? + end + + test "select retains unpermitted status" do + assert_not @params.select { |k| k == "person" }.permitted? + end + + test "slice retains permitted status" do + @params.permit! + assert @params.slice(:person).permitted? + end + + test "slice retains unpermitted status" do + assert_not @params.slice(:person).permitted? + end + + test "transform_keys retains permitted status" do + @params.permit! + assert @params.transform_keys { |k| k }.permitted? + end + + test "transform_keys retains unpermitted status" do + assert_not @params.transform_keys { |k| k }.permitted? + end + + test "transform_values retains permitted status" do + @params.permit! + assert @params.transform_values { |v| v }.permitted? + end + + test "transform_values retains unpermitted status" do + assert_not @params.transform_values { |v| v }.permitted? + end + + test "values_at retains permitted status" do + @params.permit! + assert @params.values_at(:person).first.permitted? + assert @params[:person].values_at(:name).first.permitted? + end + + test "values_at retains unpermitted status" do + assert_not @params.values_at(:person).first.permitted? + assert_not @params[:person].values_at(:name).first.permitted? + end +end diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb new file mode 100644 index 0000000000..744d8664be --- /dev/null +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -0,0 +1,99 @@ +require 'abstract_unit' +require 'action_controller/metal/strong_parameters' +require 'active_support/core_ext/hash/transform_values' + +class ParametersMutatorsTest < ActiveSupport::TestCase + setup do + @params = ActionController::Parameters.new( + person: { + age: '32', + name: { + first: 'David', + last: 'Heinemeier Hansson' + }, + addresses: [{city: 'Chicago', state: 'Illinois'}] + } + ) + end + + test "delete retains permitted status" do + @params.permit! + assert @params.delete(:person).permitted? + end + + test "delete retains unpermitted status" do + assert_not @params.delete(:person).permitted? + end + + test "delete_if retains permitted status" do + @params.permit! + assert @params.delete_if { |k| k == "person" }.permitted? + end + + test "delete_if retains unpermitted status" do + assert_not @params.delete_if { |k| k == "person" }.permitted? + end + + test "extract! retains permitted status" do + @params.permit! + assert @params.extract!(:person).permitted? + end + + test "extract! retains unpermitted status" do + assert_not @params.extract!(:person).permitted? + end + + test "keep_if retains permitted status" do + @params.permit! + assert @params.keep_if { |k,v| k == "person" }.permitted? + end + + test "keep_if retains unpermitted status" do + assert_not @params.keep_if { |k,v| k == "person" }.permitted? + end + + test "reject! retains permitted status" do + @params.permit! + assert @params.reject! { |k| k == "person" }.permitted? + end + + test "reject! retains unpermitted status" do + assert_not @params.reject! { |k| k == "person" }.permitted? + end + + test "select! retains permitted status" do + @params.permit! + assert @params.select! { |k| k != "person" }.permitted? + end + + test "select! retains unpermitted status" do + assert_not @params.select! { |k| k != "person" }.permitted? + end + + test "slice! retains permitted status" do + @params.permit! + assert @params.slice!(:person).permitted? + end + + test "slice! retains unpermitted status" do + assert_not @params.slice!(:person).permitted? + end + + test "transform_keys! retains permitted status" do + @params.permit! + assert @params.transform_keys! { |k| k }.permitted? + end + + test "transform_keys! retains unpermitted status" do + assert_not @params.transform_keys! { |k| k }.permitted? + end + + test "transform_values! retains permitted status" do + @params.permit! + assert @params.transform_values! { |v| v }.permitted? + end + + test "transform_values! retains unpermitted status" do + assert_not @params.transform_values! { |v| v }.permitted? + end +end diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index aa894ffa17..ba98ad7605 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -194,42 +194,6 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "monkey", @params.fetch(:foo) { "monkey" } end - test "not permitted is sticky on accessors" do - assert !@params.slice(:person).permitted? - assert !@params[:person][:name].permitted? - assert !@params[:person].except(:name).permitted? - - @params.each { |key, value| assert(!value.permitted?) if key == "person" } - - assert !@params.fetch(:person).permitted? - - assert !@params.values_at(:person).first.permitted? - end - - test "permitted is sticky on accessors" do - @params.permit! - assert @params.slice(:person).permitted? - assert @params[:person][:name].permitted? - assert @params[:person].except(:name).permitted? - - @params.each { |key, value| assert(value.permitted?) if key == "person" } - - assert @params.fetch(:person).permitted? - - assert @params.values_at(:person).first.permitted? - end - - test "not permitted is sticky on mutators" do - assert !@params.delete_if { |k| k == "person" }.permitted? - assert !@params.keep_if { |k,v| k == "person" }.permitted? - end - - test "permitted is sticky on mutators" do - @params.permit! - assert @params.delete_if { |k| k == "person" }.permitted? - assert @params.keep_if { |k,v| k == "person" }.permitted? - end - test "not permitted is sticky beyond merges" do assert !@params.merge(a: "b").permitted? end @@ -277,4 +241,43 @@ class ParametersPermitTest < ActiveSupport::TestCase test "permitting parameters as an array" do 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? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + assert @params.to_h.empty? + end + + test "to_h returns converted hash on permitted params" do + @params.permit! + + assert @params.to_h.is_a? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + assert_equal @params.to_hash, @params.to_h + end + + test "to_h 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 params.to_h.is_a? Hash + assert_not @params.to_h.is_a? ActionController::Parameters + 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" + } + ) + + assert_equal({ "controller" => "users", "action" => "create" }, params.to_h) + end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index fe9ee6f73d..84bd392fd9 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -909,6 +909,31 @@ class RequestParameters < BaseRequestTest end end + test "parameters not accessible after rack parse error of invalid UTF8 character" do + request = stub_request("QUERY_STRING" => "foo%81E=1") + + 2.times do + assert_raises(ActionController::BadRequest) do + # rack will raise a Rack::Utils::InvalidParameterError when parsing this query string + request.parameters + end + end + end + + test "parameters not accessible after rack parse error 1" do + request = stub_request( + 'REQUEST_METHOD' => 'POST', + 'CONTENT_LENGTH' => "a%=".length, + 'CONTENT_TYPE' => 'application/x-www-form-urlencoded; charset=utf-8', + 'rack.input' => StringIO.new("a%=") + ) + + assert_raises(ActionController::BadRequest) do + # rack will raise a TypeError when parsing this query string + request.parameters + end + end + test "we have access to the original exception" do request = stub_request("QUERY_STRING" => "x[y]=1&x[y][][w]=2") diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 9e07e9a5c4..92ac607a3c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -2,6 +2,7 @@ require 'date' require 'set' require 'bigdecimal' require 'bigdecimal/util' +require 'active_support/core_ext/string/strip' module ActiveRecord module ConnectionAdapters #:nodoc: diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index a6847e28c2..659c5e3bbb 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -836,21 +836,20 @@ module ActiveRecord SchemaMigration.table_name end - def get_all_versions - SchemaMigration.all.map { |x| x.version.to_i }.sort + def get_all_versions(connection = Base.connection) + if connection.table_exists?(schema_migrations_table_name) + SchemaMigration.all.map { |x| x.version.to_i }.sort + else + [] + end end def current_version(connection = Base.connection) - sm_table = schema_migrations_table_name - if connection.table_exists?(sm_table) - get_all_versions.max || 0 - else - 0 - end + get_all_versions(connection).max || 0 end def needs_migration?(connection = Base.connection) - current_version(connection) < last_version + (migrations(migrations_paths).collect(&:version) - get_all_versions(connection)).size > 0 end def last_version diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 11338e1fb6..f9d1edc340 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -81,6 +81,21 @@ class MigrationTest < ActiveRecord::TestCase assert_equal 0, ActiveRecord::Migrator.current_version assert_equal 3, ActiveRecord::Migrator.last_version assert_equal true, ActiveRecord::Migrator.needs_migration? + + ActiveRecord::SchemaMigration.create!(:version => ActiveRecord::Migrator.last_version) + assert_equal true, ActiveRecord::Migrator.needs_migration? + ensure + ActiveRecord::Migrator.migrations_paths = old_path + end + + def test_migration_detection_without_schema_migration_table + ActiveRecord::Base.connection.drop_table :schema_migrations + + migrations_path = MIGRATIONS_ROOT + "/valid" + old_path = ActiveRecord::Migrator.migrations_paths + ActiveRecord::Migrator.migrations_paths = migrations_path + + assert_equal true, ActiveRecord::Migrator.needs_migration? ensure ActiveRecord::Migrator.migrations_paths = old_path end diff --git a/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb b/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb index 901c2e05a8..c55600a02d 100644 --- a/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb +++ b/activesupport/lib/active_support/cache/strategy/local_cache_middleware.rb @@ -28,6 +28,9 @@ module ActiveSupport LocalCacheRegistry.set_cache_for(local_cache_key, nil) end response + rescue Rack::Utils::InvalidParameterError + LocalCacheRegistry.set_cache_for(local_cache_key, nil) + [400, {}, []] rescue Exception LocalCacheRegistry.set_cache_for(local_cache_key, nil) raise diff --git a/guides/source/4_2_release_notes.md b/guides/source/4_2_release_notes.md index 1c14519506..9937a947a5 100644 --- a/guides/source/4_2_release_notes.md +++ b/guides/source/4_2_release_notes.md @@ -75,22 +75,24 @@ Please refer to the [Changelog][railties] for detailed changes. * Introduced an `after_bundle` callback for use in Rails templates. ([Pull Request](https://github.com/rails/rails/pull/16359)) -* Introduced the `x` namespace for defining custom configuration options: +* Custom configuration options can be chained: ```ruby # config/environments/production.rb - config.x.payment_processing.schedule = :daily - config.x.payment_processing.retries = 3 - config.x.super_debugger = true + config.payment_processing.schedule = :daily + config.payment_processing.retries = 3 + config.resque = { timeout: 60, inline_jobs: :always } + config.super_debugger = true ``` These options are then available through the configuration object: ```ruby - Rails.configuration.x.payment_processing.schedule # => :daily - Rails.configuration.x.payment_processing.retries # => 3 - Rails.configuration.x.super_debugger # => true - Rails.configuration.x.super_debugger.not_set # => nil + Rails.configuration.payment_processing.schedule # => :daily + Rails.configuration.payment_processing.retries # => 3 + Rails.configuration.resque.timeout # => 60 + Rails.configuration.resque.inline_jobs # => :always + Rails.configuration.super_debugger # => true ``` ([Commit](https://github.com/rails/rails/commit/611849772dd66c2e4d005dcfe153f7ce79a8a7db)) diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 801cef5ca6..6922dd681a 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1006,16 +1006,18 @@ Custom configuration You can configure your own code through the Rails configuration object with custom configuration. It works like this: ```ruby - config.x.payment_processing.schedule = :daily - config.x.payment_processing.retries = 3 - config.x.super_debugger = true + config.payment_processing.schedule = :daily + config.payment_processing.retries = 3 + config.resque = { timeout: 60, inline_jobs: :always } + config.super_debugger = true ``` These configuration points are then available through the configuration object: ```ruby - Rails.configuration.x.payment_processing.schedule # => :daily - Rails.configuration.x.payment_processing.retries # => 3 - Rails.configuration.x.super_debugger # => true - Rails.configuration.x.super_debugger.not_set # => nil + Rails.configuration.payment_processing.schedule # => :daily + Rails.configuration.payment_processing.retries # => 3 + Rails.configuration.resque.timeout # => 60 + Rails.configuration.resque.inline_jobs # => :always + Rails.configuration.super_debugger # => true ``` diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 5e2b82c3e9..72bf73cb26 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -13,16 +13,18 @@ configure your own code through the Rails configuration object with custom configuration: # config/environments/production.rb - config.x.payment_processing.schedule = :daily - config.x.payment_processing.retries = 3 - config.x.super_debugger = true + config.payment_processing.schedule = :daily + config.payment_processing.retries = 3 + config.resque = { timeout: 60, inline_jobs: :always } + config.super_debugger = true These configuration points are then available through the configuration object: - Rails.configuration.x.payment_processing.schedule # => :daily - Rails.configuration.x.payment_processing.retries # => 3 - Rails.configuration.x.super_debugger # => true - Rails.configuration.x.super_debugger.not_set # => nil + Rails.configuration.payment_processing.schedule # => :daily + Rails.configuration.payment_processing.retries # => 3 + Rails.configuration.resque.timeout # => 60 + Rails.configuration.resque.inline_jobs # => :always + Rails.configuration.super_debugger # => true *DHH* diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 782bc4b0f1..5e8f4de847 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -13,7 +13,7 @@ module Rails :railties_order, :relative_url_root, :secret_key_base, :secret_token, :serve_static_assets, :ssl_options, :static_cache_control, :session_options, :time_zone, :reload_classes_only_on_change, - :beginning_of_week, :filter_redirect, :x + :beginning_of_week, :filter_redirect attr_writer :log_level attr_reader :encoding @@ -48,7 +48,6 @@ module Rails @eager_load = nil @secret_token = nil @secret_key_base = nil - @x = Custom.new @assets = ActiveSupport::OrderedOptions.new @assets.enabled = true @@ -155,17 +154,6 @@ module Rails def annotations SourceAnnotationExtractor::Annotation end - - private - class Custom - def initialize - @configurations = Hash.new - end - - def method_missing(method, *args) - @configurations[method] ||= ActiveSupport::OrderedOptions.new - end - end end end end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index d68f0dd851..7ea08685fb 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -17,8 +17,6 @@ source 'https://rubygems.org' # Use Rails Html Sanitizer for HTML sanitization gem 'rails-html-sanitizer', github: 'rails/rails-html-sanitizer', branch: 'master' -#temporary gem until a new version of loofah is released -gem 'loofah', github: 'kaspth/loofah', branch: 'single-scrub' # Use Unicorn as the app server # gem 'unicorn' diff --git a/railties/lib/rails/railtie/configuration.rb b/railties/lib/rails/railtie/configuration.rb index eb3b2d8ef4..406ed5ac61 100644 --- a/railties/lib/rails/railtie/configuration.rb +++ b/railties/lib/rails/railtie/configuration.rb @@ -88,11 +88,49 @@ module Rails def method_missing(name, *args, &blk) if name.to_s =~ /=$/ - @@options[$`.to_sym] = args.first + key = $`.to_sym + value = args.first + + if value.is_a?(Hash) + @@options[key] = ChainedConfigurationOptions.new value + else + @@options[key] = value + end elsif @@options.key?(name) @@options[name] else - super + @@options[name] = ActiveSupport::OrderedOptions.new + end + end + + class ChainedConfigurationOptions < ActiveSupport::OrderedOptions # :nodoc: + def initialize(value = nil) + if value.is_a?(Hash) + value.each_pair { |k, v| set_value k, v } + else + super + end + end + + def method_missing(meth, *args) + if meth =~ /=$/ + key = $`.to_sym + value = args.first + + set_value key, value + else + self.fetch(meth) { super } + end + end + + private + + def set_value(key, value) + if value.is_a?(Hash) + value = self.class.new(value) + end + + self[key] = value end end end diff --git a/railties/test/application/configuration/base_test.rb b/railties/test/application/configuration/base_test.rb index d6a82b139d..6e2b618160 100644 --- a/railties/test/application/configuration/base_test.rb +++ b/railties/test/application/configuration/base_test.rb @@ -5,6 +5,8 @@ require 'env_helpers' module ApplicationTests module ConfigurationTests class BaseTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + def setup build_app boot_rails @@ -30,8 +32,8 @@ module ApplicationTests end def require_environment - require "#{app_path}/config/environment" + require "#{app_path}/config/environment" end end end -end
\ No newline at end of file +end diff --git a/railties/test/application/configuration/custom_test.rb b/railties/test/application/configuration/custom_test.rb index 045537fc28..e8c7a37913 100644 --- a/railties/test/application/configuration/custom_test.rb +++ b/railties/test/application/configuration/custom_test.rb @@ -1,15 +1,84 @@ require 'application/configuration/base_test' class ApplicationTests::ConfigurationTests::CustomTest < ApplicationTests::ConfigurationTests::BaseTest - test 'access custom configuration point' do + test 'configuration top level can be chained' do add_to_config <<-RUBY - config.x.resque.inline_jobs = :always - config.x.resque.timeout = 60 + config.resque.inline_jobs = :always + config.resque.timeout = 60 RUBY require_environment - assert_equal :always, Rails.configuration.x.resque.inline_jobs - assert_equal 60, Rails.configuration.x.resque.timeout - assert_nil Rails.configuration.x.resque.nothing + assert_equal :always, Rails.configuration.resque.inline_jobs + assert_equal 60, Rails.configuration.resque.timeout + assert_nil Rails.configuration.resque.nothing + end + + test 'configuration top level accept normal values' do + add_to_config <<-RUBY + config.timeout = 60 + config.something_nil = nil + config.something_false = false + config.something_true = true + RUBY + require_environment + + assert_equal 60, Rails.configuration.timeout + assert_equal nil, Rails.configuration.something_nil + assert_equal false, Rails.configuration.something_false + assert_equal true, Rails.configuration.something_true + end + + test 'configuration top level builds options from hashes' do + add_to_config <<-RUBY + config.resque = { timeout: 60, inline_jobs: :always } + RUBY + require_environment + + assert_equal :always, Rails.configuration.resque.inline_jobs + assert_equal 60, Rails.configuration.resque.timeout + assert_nil Rails.configuration.resque.nothing + end + + test 'configuration top level builds options from hashes with string keys' do + add_to_config <<-RUBY + config.resque = { 'timeout' => 60, 'inline_jobs' => :always } + RUBY + require_environment + + assert_equal :always, Rails.configuration.resque.inline_jobs + assert_equal 60, Rails.configuration.resque.timeout + assert_nil Rails.configuration.resque.nothing + end + + test 'configuration top level builds nested options from hashes with symbol keys' do + add_to_config <<-RUBY + config.resque = { timeout: 60, inline_jobs: :always, url: { host: 'localhost', port: 8080 } } + config.resque.url.protocol = 'https' + config.resque.queues = { production: ['low_priority'] } + RUBY + require_environment + + assert_equal(:always, Rails.configuration.resque.inline_jobs) + assert_equal(60, Rails.configuration.resque.timeout) + assert_equal({ host: 'localhost', port: 8080, protocol: 'https' }, Rails.configuration.resque.url) + assert_equal('localhost', Rails.configuration.resque.url.host) + assert_equal(8080, Rails.configuration.resque.url.port) + assert_equal('https', Rails.configuration.resque.url.protocol) + assert_equal(['low_priority'], Rails.configuration.resque.queues.production) + assert_nil(Rails.configuration.resque.nothing) + end + + test 'configuration top level builds nested options from hashes with string keys' do + add_to_config <<-RUBY + config.resque = { 'timeout' => 60, 'inline_jobs' => :always, 'url' => { 'host' => 'localhost', 'port' => 8080 } } + RUBY + require_environment + + assert_equal(:always, Rails.configuration.resque.inline_jobs) + assert_equal(60, Rails.configuration.resque.timeout) + assert_equal({ host: 'localhost', port: 8080 }, Rails.configuration.resque.url) + assert_equal('localhost', Rails.configuration.resque.url.host) + assert_equal(8080, Rails.configuration.resque.url.port) + assert_nil(Rails.configuration.resque.nothing) end end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index ec64ce5941..da4eccd2b7 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -840,7 +840,7 @@ YAML Rails.application.load_seed assert Rails.application.config.app_seeds_loaded - assert_raise(NoMethodError) { Bukkits::Engine.config.bukkits_seeds_loaded } + assert_empty Bukkits::Engine.config.bukkits_seeds_loaded Bukkits::Engine.load_seed assert Bukkits::Engine.config.bukkits_seeds_loaded |