From def2ccb8e3534e06ec1bb9c33aeee35a52b40138 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Wed, 4 Jul 2012 12:37:31 +1200 Subject: Add ActiveSupport::KeyGenerator as a simple wrapper around PBKDF2 This will be used to derive keys from the secret and a salt, in order to allow us to do things like encrypted cookie stores without using the secret for multiple purposes directly. --- activesupport/lib/active_support.rb | 1 + activesupport/lib/active_support/key_generator.rb | 23 ++++++++++++++++ activesupport/test/key_generator_test.rb | 32 +++++++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 activesupport/lib/active_support/key_generator.rb create mode 100644 activesupport/test/key_generator_test.rb diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 41d77ab6c1..4e397ea110 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -48,6 +48,7 @@ module ActiveSupport autoload :Gzip autoload :Inflector autoload :JSON + autoload :KeyGenerator autoload :MessageEncryptor autoload :MessageVerifier autoload :Multibyte diff --git a/activesupport/lib/active_support/key_generator.rb b/activesupport/lib/active_support/key_generator.rb new file mode 100644 index 0000000000..04d170f801 --- /dev/null +++ b/activesupport/lib/active_support/key_generator.rb @@ -0,0 +1,23 @@ +require 'openssl' + +module ActiveSupport + # KeyGenerator is a simple wrapper around OpenSSL's implementation of PBKDF2 + # It can be used to derive a number of keys for various purposes from a given secret. + # This lets rails applications have a single secure secret, but avoid reusing that + # key in multiple incompatible contexts. + class KeyGenerator + def initialize(secret, options = {}) + @secret = secret + # The default iterations are higher than required for our key derivation uses + # on the off chance someone uses this for password storage + @iterations = options[:iterations] || 2**16 + end + + # Returns a derived key suitable for use. The default key_size is chosen + # to be compatible with the default settings of ActiveSupport::MessageVerifier. + # i.e. OpenSSL::Digest::SHA1#block_length + def generate_key(salt, key_size=64) + OpenSSL::PKCS5.pbkdf2_hmac_sha1(@secret, salt, @iterations, key_size) + end + end +end diff --git a/activesupport/test/key_generator_test.rb b/activesupport/test/key_generator_test.rb new file mode 100644 index 0000000000..525082d313 --- /dev/null +++ b/activesupport/test/key_generator_test.rb @@ -0,0 +1,32 @@ +require 'abstract_unit' + +begin + require 'openssl' + OpenSSL::PKCS5 +rescue LoadError, NameError + $stderr.puts "Skipping KeyGenerator test: broken OpenSSL install" +else + +require 'active_support/time' +require 'active_support/json' + +class KeyGeneratorTest < ActiveSupport::TestCase + def setup + @secret = SecureRandom.hex(64) + @generator = ActiveSupport::KeyGenerator.new(@secret, :iterations=>2) + end + + test "Generating a key of the default length" do + derived_key = @generator.generate_key("some_salt") + assert_kind_of String, derived_key + assert_equal OpenSSL::Digest::SHA1.new.block_length, derived_key.length, "Should have generated a key of the default size" + end + + test "Generating a key of an alternative length" do + derived_key = @generator.generate_key("some_salt", 32) + assert_kind_of String, derived_key + assert_equal 32, derived_key.length, "Should have generated a key of the right size" + end +end + +end -- cgit v1.2.3 From 0479bff32dfb26a420b9ab4e2c2e6c2ed17550a3 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 1 Oct 2012 15:34:12 +1300 Subject: Provide access to the application's KeyGenerator Available both as an env entry for rack and an instance method on Rails::Application for other uses --- railties/lib/rails/application.rb | 11 ++++++++++- railties/test/application/configuration_test.rb | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 050190cba6..b92b65214f 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -101,6 +101,14 @@ module Rails routes_reloader.reload! end + + # Return the application's KeyGenerator + def key_generator + # number of iterations selected based on consultation with the google security + # team. Details at https://github.com/rails/rails/pull/6952#issuecomment-7661220 + @key_generator ||= ActiveSupport::KeyGenerator.new(config.secret_token, :iterations=>1000) + end + # Stores some of the Rails initial environment parameters which # will be used by middlewares and engines to configure themselves. # Currently stores: @@ -121,7 +129,8 @@ module Rails "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions, "action_dispatch.show_detailed_exceptions" => config.consider_all_requests_local, "action_dispatch.logger" => Rails.logger, - "action_dispatch.backtrace_cleaner" => Rails.backtrace_cleaner + "action_dispatch.backtrace_cleaner" => Rails.backtrace_cleaner, + "action_dispatch.key_generator" => key_generator }) end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index d014e5e362..d8d022c265 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -634,6 +634,7 @@ module ApplicationTests assert_equal app.env_config['action_dispatch.show_exceptions'], app.config.action_dispatch.show_exceptions assert_equal app.env_config['action_dispatch.logger'], Rails.logger assert_equal app.env_config['action_dispatch.backtrace_cleaner'], Rails.backtrace_cleaner + assert_equal app.env_config['action_dispatch.key_generator'], Rails.application.key_generator end test "config.colorize_logging default is true" do -- cgit v1.2.3 From 26fe77b27dac099241f73e35d9dda896ca08dc1b Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 2 Oct 2012 20:44:02 -0400 Subject: Make Rails.public_path return a Pathname --- railties/CHANGELOG.md | 2 ++ railties/lib/rails.rb | 2 +- railties/test/application/configuration_test.rb | 10 +++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 8c2b64d543..a422c5fe39 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 4.0.0 (unreleased) ## +* `Rails.public_path` now returns a Pathname object. *Prem Sichanugrist* + * Remove highly uncommon `config.assets.manifest` option for moving the manifest path. This option is now unsupported in sprockets-rails. diff --git a/railties/lib/rails.rb b/railties/lib/rails.rb index a15965a9da..d7e22cc839 100644 --- a/railties/lib/rails.rb +++ b/railties/lib/rails.rb @@ -127,7 +127,7 @@ module Rails end def public_path - application && application.paths["public"].first + application && Pathname.new(application.paths["public"].first) end end end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index d8d022c265..ac41e01616 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -139,6 +139,14 @@ module ApplicationTests assert_instance_of Pathname, Rails.root end + test "Rails.public_path should be a Pathname" do + add_to_config <<-RUBY + config.paths["public"] = "somewhere" + RUBY + require "#{app_path}/config/environment" + assert_instance_of Pathname, Rails.public_path + end + test "initialize an eager loaded, cache classes app" do add_to_config <<-RUBY config.eager_load = true @@ -227,7 +235,7 @@ module ApplicationTests RUBY require "#{app_path}/config/application" - assert_equal File.join(app_path, "somewhere"), Rails.public_path + assert_equal Pathname.new(app_path).join("somewhere"), Rails.public_path end test "config.secret_token is sent in env" do -- cgit v1.2.3 From 4f9b59dba0fe9d90f53417f0d4f1bc63679d556a Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 2 Oct 2012 21:45:09 -0400 Subject: Make `.validators_on` accept `:kind` option This will filter out the validators on a particular attribute based on its kind. --- activemodel/CHANGELOG.md | 10 ++++++++++ activemodel/lib/active_model/validations.rb | 13 ++++++++++++- activemodel/test/cases/validations_test.rb | 8 ++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2c966943ee..2a5598e1b2 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,15 @@ ## Rails 4.0.0 (unreleased) ## +* `ActiveModel::Validators#validators_on` now accepts a `:kind` option which will filter out the + validators on a particular attribute based on its kind. + + Person.validators_on(:name) + # => [#, + # #0..99}>] + + Person.validators_on(:name, kind: :presence) + # => [#] + * Add `ActiveModel::ForbiddenAttributesProtection`, a simple module to protect attributes from mass assignment when non-permitted attributes are passed. diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 4762f39044..5a3225a7e6 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -187,10 +187,21 @@ module ActiveModel # # #, # # #0..99}> # # ] + # + # You can also pass a +:kind+ option to filter the validators based on their kind. + # + # Person.validators_on(:name, kind: :presence) + # # => [#] def validators_on(*attributes) + options = attributes.extract_options! + attributes.map do |attribute| _validators[attribute.to_sym] - end.flatten + end.flatten.tap do |validators| + if options[:kind] + validators.select! { |validator| validator.kind == options[:kind] } + end + end end # Returns +true+ if +attribute+ is an attribute method, +false+ otherwise. diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index a9d32808da..66d9e51854 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -287,6 +287,14 @@ class ValidationsTest < ActiveModel::TestCase assert_equal [], Topic.validators_on(:author_name) end + def test_list_of_validators_on_an_attribute_based_on_kind + Topic.validates_presence_of :title, :content + Topic.validates_length_of :title, :minimum => 2 + + assert_equal Topic.validators_on(:title).select { |v| v.kind == :presence }, + Topic.validators_on(:title, kind: :presence) + end + def test_validations_on_the_instance_level auto = Automobile.new -- cgit v1.2.3 From 454d820bf0a18fe1db4c55b0145197d70fef1f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 2 Oct 2012 23:24:42 -0300 Subject: Don't use tap in this case. The use of tap in this case is very confusing since we are mutating the return value inside the block --- activemodel/lib/active_model/validations.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 5a3225a7e6..be780d570b 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -195,12 +195,14 @@ module ActiveModel def validators_on(*attributes) options = attributes.extract_options! - attributes.map do |attribute| + validators = attributes.map do |attribute| _validators[attribute.to_sym] - end.flatten.tap do |validators| - if options[:kind] - validators.select! { |validator| validator.kind == options[:kind] } - end + end.flatten + + if options[:kind] + validators.select! { |validator| validator.kind == options[:kind] } + else + validators end end -- cgit v1.2.3 From 86062005a73589dc4919cfac401ce061f061c6a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 2 Oct 2012 23:56:12 -0300 Subject: Revert "Merge pull request #7826 from sikachu/master-validators-kind" This reverts commit 4e9f53f9736544f070e75e516c71137b7eb49a7a, reversing changes made to 6b802cdb4f5b84e1bf49aaeb0e994b3be6028af9. Revert "Don't use tap in this case." This reverts commit 454d820bf0a18fe1db4c55b0145197d70fef1f82. Reason: Is not a good idea to add options to this method since we can do the same thing using method composition. Person.validators_on(:name).select { |v| v.kind == :presence } Also it avoids to change the method again to add more options. --- activemodel/CHANGELOG.md | 10 ---------- activemodel/lib/active_model/validations.rb | 15 +-------------- activemodel/test/cases/validations_test.rb | 8 -------- 3 files changed, 1 insertion(+), 32 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2a5598e1b2..2c966943ee 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,15 +1,5 @@ ## Rails 4.0.0 (unreleased) ## -* `ActiveModel::Validators#validators_on` now accepts a `:kind` option which will filter out the - validators on a particular attribute based on its kind. - - Person.validators_on(:name) - # => [#, - # #0..99}>] - - Person.validators_on(:name, kind: :presence) - # => [#] - * Add `ActiveModel::ForbiddenAttributesProtection`, a simple module to protect attributes from mass assignment when non-permitted attributes are passed. diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index be780d570b..4762f39044 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -187,23 +187,10 @@ module ActiveModel # # #, # # #0..99}> # # ] - # - # You can also pass a +:kind+ option to filter the validators based on their kind. - # - # Person.validators_on(:name, kind: :presence) - # # => [#] def validators_on(*attributes) - options = attributes.extract_options! - - validators = attributes.map do |attribute| + attributes.map do |attribute| _validators[attribute.to_sym] end.flatten - - if options[:kind] - validators.select! { |validator| validator.kind == options[:kind] } - else - validators - end end # Returns +true+ if +attribute+ is an attribute method, +false+ otherwise. diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 66d9e51854..a9d32808da 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -287,14 +287,6 @@ class ValidationsTest < ActiveModel::TestCase assert_equal [], Topic.validators_on(:author_name) end - def test_list_of_validators_on_an_attribute_based_on_kind - Topic.validates_presence_of :title, :content - Topic.validates_length_of :title, :minimum => 2 - - assert_equal Topic.validators_on(:title).select { |v| v.kind == :presence }, - Topic.validators_on(:title, kind: :presence) - end - def test_validations_on_the_instance_level auto = Automobile.new -- cgit v1.2.3 From d56b5dacb1fbaeef9d48d57594b2f1f9c32a21bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 3 Oct 2012 00:17:10 -0300 Subject: Use the `flat_map` method. Thanks to @jeremy to teach me this one. --- activemodel/lib/active_model/validations.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 4762f39044..243d911f71 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -188,9 +188,9 @@ module ActiveModel # # #0..99}> # # ] def validators_on(*attributes) - attributes.map do |attribute| + attributes.flat_map do |attribute| _validators[attribute.to_sym] - end.flatten + end end # Returns +true+ if +attribute+ is an attribute method, +false+ otherwise. -- cgit v1.2.3 From e4e84fee8b9ecd63e1cc7b62beb577f8fe7ce35d Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 3 Oct 2012 00:48:28 -0300 Subject: Refactor --- activerecord/lib/active_record/counter_cache.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index d28cd560d9..57838ff984 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -23,12 +23,11 @@ module ActiveRecord has_many_association = reflect_on_association(association.to_sym) if has_many_association.is_a? ActiveRecord::Reflection::ThroughReflection - foreign_key = has_many_association.through_reflection.foreign_key.to_s - child_class = has_many_association.through_reflection.klass - else - foreign_key = has_many_association.foreign_key.to_s - child_class = has_many_association.klass + has_many_association = has_many_association.through_reflection end + + foreign_key = has_many_association.foreign_key.to_s + child_class = has_many_association.klass belongs_to = child_class.reflect_on_all_associations(:belongs_to) reflection = belongs_to.find { |e| e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? } counter_name = reflection.counter_cache_column -- cgit v1.2.3 From aa202adf6c78468c8e03efd11d84b71478de7b03 Mon Sep 17 00:00:00 2001 From: Francesco Rodriguez Date: Wed, 3 Oct 2012 18:02:14 -0500 Subject: Count returns 0 without querying if parent is not saved Patches `CollectionAssociation#count` to return 0 without querying if the parent record is new. Consider the following code: class Account has_many :dossiers end class Dossier belongs_to :account end a = Account.new a.dossiers.build # before patch a.dossiers.count # SELECT COUNT(*) FROM "dossiers" WHERE "dossiers"."account_id" IS NULL # => 0 # after a.dosiers.count # fires without sql query # => 0 Fixes #1856. --- activerecord/CHANGELOG.md | 17 +++++++++++++++++ .../associations/collection_association.rb | 2 ++ .../has_and_belongs_to_many_associations_test.rb | 6 ++++++ .../cases/associations/has_many_associations_test.rb | 6 ++++++ .../associations/has_many_through_associations_test.rb | 6 ++++++ 5 files changed, 37 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 8ff4c4706c..2a0f1bf92a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,22 @@ ## Rails 4.0.0 (unreleased) ## +* `CollectionAssociation#count` returns 0 without querying if the + parent record is new. + + Before: + + person.pets + # SELECT COUNT(*) FROM "pets" WHERE "pets"."person_id" IS NULL + # => 0 + + After: + + person.pets + # fires without sql query + # => 0 + + *Francesco Rodriguez* + * Fix `reset_counters` crashing on `has_many :through` associations. Fix #7822. diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index fe3e5b00f7..96270ec0e9 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -174,6 +174,8 @@ module ActiveRecord # association, it will be used for the query. Otherwise, construct options and pass them with # scope to the target class's +count+. def count(column_name = nil, count_options = {}) + return 0 if owner.new_record? + column_name, count_options = nil, column_name if column_name.is_a?(Hash) if options[:counter_sql] || options[:finder_sql] diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f3520d43e0..42f5b69d4e 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -799,6 +799,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, developer.projects.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Developer.new.projects.count + end + end + unless current_adapter?(:PostgreSQLAdapter) def test_count_with_finder_sql assert_equal 3, projects(:active_record).developers_with_finder_sql.count diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 4b56037a08..50c23c863f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -262,6 +262,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal firm.limited_clients.length, firm.limited_clients.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Person.new.readers.count + end + end + def test_finding assert_equal 2, Firm.all.merge!(:order => "id").first.clients.length end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index d4ceae6f80..f0582a3090 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -767,6 +767,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal 1, authors(:mary).categories.general.count end + def test_counting_should_not_fire_sql_if_parent_is_unsaved + assert_no_queries do + assert_equal 0, Person.new.posts.count + end + end + def test_has_many_through_belongs_to_should_update_when_the_through_foreign_key_changes post = posts(:eager_other) -- cgit v1.2.3 From d7d228402efd1ddbbdb2b15d2e91ec22693b9298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 4 Oct 2012 10:23:27 -0300 Subject: Fix CHANGELOG entry [ci skip] --- activerecord/CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2a0f1bf92a..714263faea 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,17 +1,17 @@ ## Rails 4.0.0 (unreleased) ## -* `CollectionAssociation#count` returns 0 without querying if the - parent record is new. +* `CollectionAssociation#count` returns `0` without querying if the + parent record is not persisted. Before: - person.pets + person.pets.count # SELECT COUNT(*) FROM "pets" WHERE "pets"."person_id" IS NULL # => 0 After: - person.pets + person.pets.count # fires without sql query # => 0 -- cgit v1.2.3