From 36245be10f984ebf3fed77c4717c6fdfe70c33d4 Mon Sep 17 00:00:00 2001 From: Nathaniel Watts <1141717+thewatts@users.noreply.github.com> Date: Sat, 7 May 2022 00:28:38 -0500 Subject: [PATCH 001/354] Fix for multiple default_scope all_queries options In our use case - we have a base model that has a default scope that we want enabled for all queries, ex: ```ruby class Developer < ApplicationRecord default_scope -> { where(firm_id: Current.firm_id) }, all_queries: true end ``` We're also leveraging a module that will add a default scope to only find soft-deleted records. ```ruby module SoftDeletable extend ActiveSupport::Concern included do default_scope { where(deleted_at: nil) } end ``` Through this, we've found that when using default scopes in combination, *specifically in the use case where the _non_ all queries scope is declared first*, that we would get an error when calling `.update`: ```ruby class Developer < ApplicationRecord include SoftDeletable default_scope -> { where(firm_id: Current.firm_id) }, all_queries: true ``` ```ruby Current.firm_id = 5 developer = Developer.new(name: "Steve") developer.update(name: "Stephen") NoMethodError: undefined method `where' for nil:NilClass ``` In digging into the code, this was due to there not being an `else` case for the `inject` used to combine `default_scopes` together (`inject` uses the return value of the block as the memoizer). Without the `else` case, if the block returned `nil`, `nil` was passed to the evaluation of the next `default_scope`. This commit adds the `else`, and also makes a minor adjustment in variable naming (`default_scope` to `combined_scope`) in an effort to add a little more readability, as we're iterating over an array of default scopes, but what we're building is _the_ default scope to be used in the query, etc. Co-authored-by: Will Cosgrove --- .../lib/active_record/scoping/default.rb | 6 ++++-- .../test/cases/scoping/default_scoping_test.rb | 17 +++++++++++++++++ activerecord/test/models/developer.rb | 15 ++++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index ba1d5a8ed0bda..f41e76c0afd76 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -150,11 +150,13 @@ def build_default_scope(relation = relation(), all_queries: nil) end elsif default_scopes.any? evaluate_default_scope do - default_scopes.inject(relation) do |default_scope, scope_obj| + default_scopes.inject(relation) do |combined_scope, scope_obj| if execute_scope?(all_queries, scope_obj) scope = scope_obj.scope.respond_to?(:to_proc) ? scope_obj.scope : scope_obj.scope.method(:call) - default_scope.instance_exec(&scope) || default_scope + combined_scope.instance_exec(&scope) || combined_scope + else + combined_scope end end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index a4a36aad86ace..d23663dd89fa3 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -80,6 +80,23 @@ def test_default_scope_with_multiple_calls assert_equal 50000, wheres["salary"] end + def test_combined_default_scope_without_and_with_all_queries_works + Mentor.create! + klass = DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries + + create_sql = capture_sql { klass.create!(name: "Steve") }.first + + assert_match(/mentor_id/, create_sql) + assert_match(/firm_id/, create_sql) + + developer = klass.find_by!(name: "Steve") + + update_sql = capture_sql { developer.update(name: "Stephen") }.first + + assert_no_match(/mentor_id/, update_sql) + assert_match(/firm_id/, update_sql) + end + def test_default_scope_runs_on_create Mentor.create! create_sql = capture_sql { DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") }.first diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index c84dfe4170c20..eea35dced8ec6 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -164,7 +164,20 @@ class DeveloperWithDefaultMentorScopeAllQueries < ActiveRecord::Base class DeveloperWithDefaultNilableMentorScopeAllQueries < ActiveRecord::Base self.table_name = "developers" - firm_id = nil # Could be something like Current.mentor_id + firm_id = nil # Could be something like Current.firm_id + default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true +end + +module MentorDefaultScopeNotAllQueries + extend ActiveSupport::Concern + + included { default_scope { where(mentor_id: 1) } } +end + +class DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries < ActiveRecord::Base + include MentorDefaultScopeNotAllQueries + self.table_name = "developers" + firm_id = 10 # Could be something like Current.firm_id default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true end From df829a3789ef74a3ec70353303f05a529767d68a Mon Sep 17 00:00:00 2001 From: eileencodes Date: Mon, 9 May 2022 16:29:38 -0400 Subject: [PATCH 002/354] Revert "Merge pull request #45048 from thewatts/nw-fix-default-scope-all-queries" This reverts commit a84de79dc8d82c4c4dc2e6926fe5da08f6d98274, reversing changes made to 3872bc0e54d32e8bf3a6299b0bfe173d94b072fc. Reverting as this was accidentally merged to 7-0-stable instead of main. --- .../lib/active_record/scoping/default.rb | 6 ++---- .../test/cases/scoping/default_scoping_test.rb | 17 ----------------- activerecord/test/models/developer.rb | 15 +-------------- 3 files changed, 3 insertions(+), 35 deletions(-) diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index e776b19502576..f6466947b54c4 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -146,13 +146,11 @@ def build_default_scope(relation = relation(), all_queries: nil) end elsif default_scopes.any? evaluate_default_scope do - default_scopes.inject(relation) do |combined_scope, scope_obj| + default_scopes.inject(relation) do |default_scope, scope_obj| if execute_scope?(all_queries, scope_obj) scope = scope_obj.scope.respond_to?(:to_proc) ? scope_obj.scope : scope_obj.scope.method(:call) - combined_scope.instance_exec(&scope) || combined_scope - else - combined_scope + default_scope.instance_exec(&scope) || default_scope end end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index d23663dd89fa3..a4a36aad86ace 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -80,23 +80,6 @@ def test_default_scope_with_multiple_calls assert_equal 50000, wheres["salary"] end - def test_combined_default_scope_without_and_with_all_queries_works - Mentor.create! - klass = DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries - - create_sql = capture_sql { klass.create!(name: "Steve") }.first - - assert_match(/mentor_id/, create_sql) - assert_match(/firm_id/, create_sql) - - developer = klass.find_by!(name: "Steve") - - update_sql = capture_sql { developer.update(name: "Stephen") }.first - - assert_no_match(/mentor_id/, update_sql) - assert_match(/firm_id/, update_sql) - end - def test_default_scope_runs_on_create Mentor.create! create_sql = capture_sql { DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") }.first diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index eea35dced8ec6..c84dfe4170c20 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -164,20 +164,7 @@ class DeveloperWithDefaultMentorScopeAllQueries < ActiveRecord::Base class DeveloperWithDefaultNilableMentorScopeAllQueries < ActiveRecord::Base self.table_name = "developers" - firm_id = nil # Could be something like Current.firm_id - default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true -end - -module MentorDefaultScopeNotAllQueries - extend ActiveSupport::Concern - - included { default_scope { where(mentor_id: 1) } } -end - -class DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries < ActiveRecord::Base - include MentorDefaultScopeNotAllQueries - self.table_name = "developers" - firm_id = 10 # Could be something like Current.firm_id + firm_id = nil # Could be something like Current.mentor_id default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true end From 0bbf34c614a8ae443285576b4379619ed7d194bb Mon Sep 17 00:00:00 2001 From: Nathaniel Watts <1141717+thewatts@users.noreply.github.com> Date: Mon, 9 May 2022 16:33:07 -0400 Subject: [PATCH 003/354] Fix for multiple default_scope all_queries options In our use case - we have a base model that has a default scope that we want enabled for all queries, ex: ```ruby class Developer < ApplicationRecord default_scope -> { where(firm_id: Current.firm_id) }, all_queries: true end ``` We're also leveraging a module that will add a default scope to only find soft-deleted records. ```ruby module SoftDeletable extend ActiveSupport::Concern included do default_scope { where(deleted_at: nil) } end ``` Through this, we've found that when using default scopes in combination, *specifically in the use case where the _non_ all queries scope is declared first*, that we would get an error when calling `.update`: ```ruby class Developer < ApplicationRecord include SoftDeletable default_scope -> { where(firm_id: Current.firm_id) }, all_queries: true ``` ```ruby Current.firm_id = 5 developer = Developer.new(name: "Steve") developer.update(name: "Stephen") NoMethodError: undefined method `where' for nil:NilClass ``` In digging into the code, this was due to there not being an `else` case for the `inject` used to combine `default_scopes` together (`inject` uses the return value of the block as the memoizer). Without the `else` case, if the block returned `nil`, `nil` was passed to the evaluation of the next `default_scope`. This commit adds the `else`, and also makes a minor adjustment in variable naming (`default_scope` to `combined_scope`) in an effort to add a little more readability, as we're iterating over an array of default scopes, but what we're building is _the_ default scope to be used in the query, etc. Co-authored-by: Will Cosgrove --- .../lib/active_record/scoping/default.rb | 6 ++++-- .../test/cases/scoping/default_scoping_test.rb | 17 +++++++++++++++++ activerecord/test/models/developer.rb | 15 ++++++++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/scoping/default.rb b/activerecord/lib/active_record/scoping/default.rb index f6466947b54c4..e776b19502576 100644 --- a/activerecord/lib/active_record/scoping/default.rb +++ b/activerecord/lib/active_record/scoping/default.rb @@ -146,11 +146,13 @@ def build_default_scope(relation = relation(), all_queries: nil) end elsif default_scopes.any? evaluate_default_scope do - default_scopes.inject(relation) do |default_scope, scope_obj| + default_scopes.inject(relation) do |combined_scope, scope_obj| if execute_scope?(all_queries, scope_obj) scope = scope_obj.scope.respond_to?(:to_proc) ? scope_obj.scope : scope_obj.scope.method(:call) - default_scope.instance_exec(&scope) || default_scope + combined_scope.instance_exec(&scope) || combined_scope + else + combined_scope end end end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index a4a36aad86ace..d23663dd89fa3 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -80,6 +80,23 @@ def test_default_scope_with_multiple_calls assert_equal 50000, wheres["salary"] end + def test_combined_default_scope_without_and_with_all_queries_works + Mentor.create! + klass = DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries + + create_sql = capture_sql { klass.create!(name: "Steve") }.first + + assert_match(/mentor_id/, create_sql) + assert_match(/firm_id/, create_sql) + + developer = klass.find_by!(name: "Steve") + + update_sql = capture_sql { developer.update(name: "Stephen") }.first + + assert_no_match(/mentor_id/, update_sql) + assert_match(/firm_id/, update_sql) + end + def test_default_scope_runs_on_create Mentor.create! create_sql = capture_sql { DeveloperwithDefaultMentorScopeNot.create!(name: "Eileen") }.first diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index c84dfe4170c20..eea35dced8ec6 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -164,7 +164,20 @@ class DeveloperWithDefaultMentorScopeAllQueries < ActiveRecord::Base class DeveloperWithDefaultNilableMentorScopeAllQueries < ActiveRecord::Base self.table_name = "developers" - firm_id = nil # Could be something like Current.mentor_id + firm_id = nil # Could be something like Current.firm_id + default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true +end + +module MentorDefaultScopeNotAllQueries + extend ActiveSupport::Concern + + included { default_scope { where(mentor_id: 1) } } +end + +class DeveloperWithIncludedMentorDefaultScopeNotAllQueriesAndDefaultScopeFirmWithAllQueries < ActiveRecord::Base + include MentorDefaultScopeNotAllQueries + self.table_name = "developers" + firm_id = 10 # Could be something like Current.firm_id default_scope -> { where(firm_id: firm_id) if firm_id }, all_queries: true end From fc8af155dd33c81d45745994c678e060907d510e Mon Sep 17 00:00:00 2001 From: Petrik Date: Wed, 11 May 2022 12:08:55 +0200 Subject: [PATCH 004/354] Fix misspelling of libraries --- activerecord/lib/active_record/middleware/database_selector.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/middleware/database_selector.rb b/activerecord/lib/active_record/middleware/database_selector.rb index d99440f109e0c..70b38bdfb763f 100644 --- a/activerecord/lib/active_record/middleware/database_selector.rb +++ b/activerecord/lib/active_record/middleware/database_selector.rb @@ -44,7 +44,7 @@ module Middleware # config.active_record.database_resolver_context = MyResolver::MySession # # Note: If you are using `rails new my_app --minimal` you will need to call - # `require "active_support/core_ext/integer/time"` to load the libaries + # `require "active_support/core_ext/integer/time"` to load the libraries # for +Time+. class DatabaseSelector def initialize(app, resolver_klass = nil, context_klass = nil, options = {}) From a6e15ddd89d165d4f10986c1294cad0758203afa Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Wed, 11 May 2022 07:41:37 -0400 Subject: [PATCH 005/354] Merge pull request #45067 from fatkodima/mysql-view-default Fix getting column default from VIEW in mysql --- .../connection_adapters/mysql/schema_statements.rb | 2 +- activerecord/test/cases/view_test.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index b1ae413e0550f..49d21055b1dc5 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -159,7 +159,7 @@ def create_table_definition(name, **options) end def default_type(table_name, field_name) - match = create_table_info(table_name).match(/`#{field_name}` (.+) DEFAULT ('|\d+|[A-z]+)/) + match = create_table_info(table_name)&.match(/`#{field_name}` (.+) DEFAULT ('|\d+|[A-z]+)/) default_pre = match[2] if match if default_pre == "'" diff --git a/activerecord/test/cases/view_test.rb b/activerecord/test/cases/view_test.rb index 36b9df7ba5139..e46cab0b39be7 100644 --- a/activerecord/test/cases/view_test.rb +++ b/activerecord/test/cases/view_test.rb @@ -21,7 +21,7 @@ def setup super @connection = ActiveRecord::Base.connection create_view "ebooks'", <<~SQL - SELECT id, name, status FROM books WHERE format = 'ebook' + SELECT id, name, cover, status FROM books WHERE format = 'ebook' SQL end @@ -58,11 +58,12 @@ def test_views_ara_valid_data_sources def test_column_definitions assert_equal([["id", :integer], ["name", :string], + ["cover", :string], ["status", :integer]], Ebook.columns.map { |c| [c.name, c.type] }) end def test_attributes - assert_equal({ "id" => 2, "name" => "Ruby for Rails", "status" => 0 }, + assert_equal({ "id" => 2, "name" => "Ruby for Rails", "cover" => "hard", "status" => 0 }, Ebook.first.attributes) end From 542f1ceaf3689e5fd63410bbe58689b1e1df5700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 18 May 2022 22:10:43 +0000 Subject: [PATCH 006/354] Upgrade bundler version --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index f87363ce58b5c..101658a44c847 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -642,4 +642,4 @@ DEPENDENCIES websocket-client-simple! BUNDLED WITH - 2.2.32 + 2.3.14 From d8e5d1c49d441d5c41959803aed4ad389b358ebc Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 6 Apr 2022 09:20:34 +0200 Subject: [PATCH 007/354] Handle renaming of `global_constant_state` in Ruby 3.2 Ref: https://github.com/ruby/ruby/pull/5766 --- activesupport/test/core_ext/enumerable_test.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 01b9d1c879516..84cc4bf5e74e8 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -362,11 +362,19 @@ def test_sole end def test_doesnt_bust_constant_cache - skip "Only applies to MRI" unless defined?(RubyVM.stat) && RubyVM.stat(:global_constant_state) + skip "Only applies to MRI" unless defined?(RubyVM.stat) object = Object.new - assert_no_difference -> { RubyVM.stat(:global_constant_state) } do + assert_no_difference -> { constant_cache_invalidations } do object.extend(Enumerable) end end + + private + + def constant_cache_invalidations + RubyVM.stat(:constant_cache_invalidations) + rescue ArgumentError + RubyVM.stat(:global_constant_state) # RUBY_VERSION < "3.2" + end end From bb2618d0b1952183ef75d3c1277435e13dcbbe54 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Wed, 6 Apr 2022 13:21:49 -0400 Subject: [PATCH 008/354] Fix style in test/core_ext/enumerable_test.rb --- activesupport/test/core_ext/enumerable_test.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 84cc4bf5e74e8..0dc4b64379bfd 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -371,10 +371,9 @@ def test_doesnt_bust_constant_cache end private - - def constant_cache_invalidations - RubyVM.stat(:constant_cache_invalidations) - rescue ArgumentError - RubyVM.stat(:global_constant_state) # RUBY_VERSION < "3.2" - end + def constant_cache_invalidations + RubyVM.stat(:constant_cache_invalidations) + rescue ArgumentError + RubyVM.stat(:global_constant_state) # RUBY_VERSION < "3.2" + end end From c82addc52058cefdb1a63032396cdc56fed928f4 Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Thu, 12 May 2022 20:29:00 -0500 Subject: [PATCH 009/354] Merge pull request #45083 from shouichi/doc-url-for-can-take-classes Document that url_for can take classes [ci-skip] (cherry picked from commit 25b126707152de3a1ec9762ee564f3c7623373b3) --- actionview/lib/action_view/routing_url_for.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/actionview/lib/action_view/routing_url_for.rb b/actionview/lib/action_view/routing_url_for.rb index a7d8ed54e0f99..60b2274eff6fc 100644 --- a/actionview/lib/action_view/routing_url_for.rb +++ b/actionview/lib/action_view/routing_url_for.rb @@ -47,6 +47,9 @@ module RoutingUrlFor # <%= url_for(action: 'jump', anchor: 'tax&ship') %> # # => /testing/jump/#tax&ship # + # <%= url_for(Workshop) %> + # # => /workshops + # # <%= url_for(Workshop.new) %> # # relies on Workshop answering a persisted? call (and in this case returning false) # # => /workshops From c15ee552d078258a562d836b998f768c2f2b25f1 Mon Sep 17 00:00:00 2001 From: Alex Ghiculescu Date: Thu, 19 May 2022 13:15:17 -0500 Subject: [PATCH 010/354] Clarify `cookies_serializer` wording in new framework defaults https://github.com/rails/rails/pull/45127 pointed out that the wording around how to update your `cookies_serializer` safely wasn't clear enough. This PR makes the wording a bit more stern. --- .../config/initializers/new_framework_defaults_7_0.rb.tt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt index a579326e2087f..ca8eafc021631 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt @@ -83,11 +83,13 @@ # Rails.application.config.active_storage.variant_processor = :vips # If you're upgrading and haven't set `cookies_serializer` previously, your cookie serializer -# was `:marshal`. Convert all cookies to JSON, using the `:hybrid` formatter. +# was `:marshal`. The default for new apps is `:json`. # -# If you're confident all your cookies are JSON formatted, you can switch to the `:json` formatter. +# You can convert all cookies to JSON using the `:hybrid` formatter. It is fine to use +#`:hybrid` long term; you should do that unless you're confident that *all* your cookies +# have been converted to JSON. # -# Continue to use `:marshal` for backward-compatibility with old cookies. +# You can also use `:marshal` for backward-compatibility with old cookies. # # If you have configured the serializer elsewhere, you can remove this. # From 0a6870822d63e91e79b4560b422101fa873b822a Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 16 May 2022 09:48:32 -0700 Subject: [PATCH 011/354] Merge pull request #45109 from yahonda/diag45108 Address QueryCacheTest#test_query_cache_does_not_allow_sql_key_mutation failure --- activerecord/test/cases/query_cache_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 1bc37024fc2a1..7907e2278b597 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -486,7 +486,7 @@ def test_cache_does_not_raise_exceptions def test_query_cache_does_not_allow_sql_key_mutation subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |_, _, _, _, payload| - payload[:sql].downcase! + payload[:sql].downcase! if payload[:name] == "Task Load" end ActiveRecord::Base.cache do From ef7c839b8b541e9c66a1275ca3c45bc410d1afce Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Thu, 19 May 2022 15:56:33 -0700 Subject: [PATCH 012/354] Separate new framework defaults with a blank line --- .../config/initializers/new_framework_defaults_7_0.rb.tt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt index a579326e2087f..89786a80ca2e2 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt @@ -71,7 +71,7 @@ # This default means that all columns will be referenced in INSERT queries # regardless of whether they have a default or not. # Rails.application.config.active_record.partial_inserts = false -# + # Protect from open redirect attacks in `redirect_back_or_to` and `redirect_to`. # Rails.application.config.action_controller.raise_on_open_redirects = true From 15582cbfac10956856d63893a55d05761beac94d Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Fri, 20 May 2022 21:40:12 +0100 Subject: [PATCH 013/354] Merge pull request #45141 from eugeneius/dont_call_headers Don't call controller's headers method internally --- .../lib/action_controller/metal/rendering.rb | 4 ++-- .../test/controller/integration_test.rb | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 9c3e292bca83c..3deb60fe19685 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -78,8 +78,8 @@ def _set_rendered_content_type(format) end def _set_vary_header - if self.headers["Vary"].blank? && request.should_apply_vary_header? - self.headers["Vary"] = "Accept" + if response.headers["Vary"].blank? && request.should_apply_vary_header? + response.headers["Vary"] = "Accept" end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 2e1a54fbf88da..170a8aa5d2baa 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -850,6 +850,30 @@ def app end end +class ControllerWithHeadersMethodIntegrationTest < ActionDispatch::IntegrationTest + class TestController < ActionController::Base + def index + render plain: "ok" + end + + def headers + {}.freeze + end + end + + test "doesn't call controller's headers method" do + with_routing do |routes| + routes.draw do + get "/ok" => "controller_with_headers_method_integration_test/test#index" + end + + get "/ok" + + assert_response 200 + end + end +end + class UrlOptionsIntegrationTest < ActionDispatch::IntegrationTest class FooController < ActionController::Base def index From 52fbe5b33b0ebffd53072305e96183aa82297009 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sun, 15 May 2022 17:07:06 -0400 Subject: [PATCH 014/354] Merge pull request #45103 from dorianmariefr/issue-45088-error-when-submitting-action-mailbox-conductor-form Fixes development Action Mailbox new mail form --- .../inbound_emails_controller.rb | 2 +- .../inbound_emails_controller_test.rb | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb index 62523cf4dc45d..eda58f7342054 100644 --- a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb +++ b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb @@ -22,7 +22,7 @@ def create def new_mail Mail.new(mail_params.except(:attachments).to_h).tap do |mail| mail[:bcc]&.include_in_headers = true - mail_params[:attachments].to_a.each do |attachment| + mail_params[:attachments]&.select(&:present?)&.each do |attachment| mail.add_file(filename: attachment.original_filename, content: attachment.read) end end diff --git a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb index 7a9f5d15a77b6..83c9e90dd5671 100644 --- a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb @@ -72,6 +72,29 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa end end + test "create inbound email with empty attachment" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "", + to: "", + cc: "", + bcc: "", + x_original_to: "", + subject: "", + in_reply_to: "", + body: "", + attachments: [ "" ], + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal 0, mail.attachments.count + end + end + private def with_rails_env(env) old_rails_env = Rails.env From cb76ede584f38291787be2814e22901d0436413b Mon Sep 17 00:00:00 2001 From: Alex Ghiculescu Date: Tue, 24 May 2022 15:40:45 -0500 Subject: [PATCH 015/354] `cookies_serializer` docs improvements This PR incorporates @p8's feedback on https://github.com/rails/rails/pull/45137 and @rafaelfranca's on https://github.com/rails/rails/pull/45127#issuecomment-1136402456 --- .../new_framework_defaults_7_0.rb.tt | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt index 3466e028c4eb5..b6fa3c0d8c22d 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_0.rb.tt @@ -82,20 +82,6 @@ # The `:mini_magick` option is not deprecated; it's fine to keep using it. # Rails.application.config.active_storage.variant_processor = :vips -# If you're upgrading and haven't set `cookies_serializer` previously, your cookie serializer -# was `:marshal`. The default for new apps is `:json`. -# -# You can convert all cookies to JSON using the `:hybrid` formatter. It is fine to use -#`:hybrid` long term; you should do that unless you're confident that *all* your cookies -# have been converted to JSON. -# -# You can also use `:marshal` for backward-compatibility with old cookies. -# -# If you have configured the serializer elsewhere, you can remove this. -# -# See https://guides.rubyonrails.org/action_controller_overview.html#cookies for more information. -# Rails.application.config.action_dispatch.cookies_serializer = :hybrid - # Enable parameter wrapping for JSON. # Previously this was set in an initializer. It's fine to keep using that initializer if you've customized it. # To disable parameter wrapping entirely, set this config to `false`. @@ -117,3 +103,29 @@ # "X-Permitted-Cross-Domain-Policies" => "none", # "Referrer-Policy" => "strict-origin-when-cross-origin" # } + +# Cookie serializer: 2 options +# +# If you're upgrading and haven't set `cookies_serializer` previously, your cookie serializer +# is `:marshal`. The default for new apps is `:json`. +# +# Rails.application.config.action_dispatch.cookies_serializer = :json +# +# +# To migrate an existing application to the `:json` serializer, use the `:hybrid` option. +# +# Rails transparently deserializes existing (Marshal-serialized) cookies on read and +# re-writes them in the JSON format. +# +# It is fine to use `:hybrid` long term; you should do that until you're confident *all* your cookies +# have been converted to JSON. To keep using `:hybrid` long term, move this config to its own +# initializer or to `config/application.rb`. +# +# Rails.application.config.action_dispatch.cookies_serializer = :hybrid +# +# +# If your cookies can't yet be serialized to JSON, keep using `:marshal` for backward-compatibility. +# +# If you have configured the serializer elsewhere, you can remove this section of the file. +# +# See https://guides.rubyonrails.org/action_controller_overview.html#cookies for more information. From e3fc6759e24df25df461893e0d0fd43e1bbe5ae0 Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Thu, 26 May 2022 11:46:15 -0400 Subject: [PATCH 016/354] Merge pull request #45178 from eileencodes/allow-setting-db-with-postgres Allow setting db with use_postgresql tests --- railties/test/application/rake/dbs_test.rb | 22 +++++-------------- .../test/application/rake/multi_dbs_test.rb | 14 ++++++++++++ railties/test/isolation/abstract_unit.rb | 10 ++++----- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 385f45d346de9..c533fcd241005 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -672,20 +672,6 @@ def db_fixtures_load(expected_database) end end - test "db:prepare setup the database even if schema does not exist" do - Dir.chdir(app_path) do - use_postgresql(multi_db: true) # bug doesn't exist with sqlite3 - output = rails("db:drop") - assert_match(/Dropped database/, output) - - rails "generate", "model", "recipe", "title:string" - output = rails("db:prepare") - assert_match(/CreateRecipes: migrated/, output) - end - ensure - rails "db:drop" rescue nil - end - test "db:prepare does not touch schema when dumping is disabled" do Dir.chdir(app_path) do rails "generate", "model", "book", "title:string" @@ -704,13 +690,15 @@ def db_fixtures_load(expected_database) test "db:prepare creates test database if it does not exist" do Dir.chdir(app_path) do - use_postgresql + use_postgresql(database_name: "railties_db") rails "db:drop", "db:create" - rails "runner", "ActiveRecord::Base.connection.drop_database(:railties_test)" + rails "runner", "ActiveRecord::Base.connection.drop_database(:railties_db_test)" output = rails("db:prepare") - assert_match(%r{Created database 'railties_test'}, output) + assert_match(%r{Created database 'railties_db_test'}, output) end + ensure + rails "db:drop" rescue nil end test "lazily loaded schema cache isn't read when reading the schema migrations table" do diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index b04dedd62e5b8..568b5c71f28c4 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -753,6 +753,20 @@ class TwoMigration < ActiveRecord::Migration::Current db_migrate_and_schema_cache_dump end + test "db:prepare setup the database even if schema does not exist" do + Dir.chdir(app_path) do + use_postgresql(multi_db: true) # bug doesn't exist with sqlite3 + output = rails("db:drop") + assert_match(/Dropped database/, output) + + rails "generate", "model", "recipe", "title:string" + output = rails("db:prepare") + assert_match(/CreateRecipes: migrated/, output) + end + ensure + rails "db:drop" rescue nil + end + # Note that schema cache loader depends on the connection and # does not work for all connections. test "schema_cache is loaded on primary db in multi-db app" do diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index d1d8aab5e5ca8..e1ef038f152ff 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -458,7 +458,7 @@ def use_frameworks(arr) $:.reject! { |path| path =~ %r'/(#{to_remove.join('|')})/' } end - def use_postgresql(multi_db: false) + def use_postgresql(multi_db: false, database_name: "railties_#{Process.pid}") if multi_db File.open("#{app_path}/config/database.yml", "w") do |f| f.puts <<-YAML @@ -468,10 +468,10 @@ def use_postgresql(multi_db: false) development: primary: <<: *default - database: railties_test + database: #{database_name}_test animals: <<: *default - database: railties_animals_test + database: #{database_name}_animals_test migrations_paths: db/animals_migrate YAML end @@ -483,10 +483,10 @@ def use_postgresql(multi_db: false) pool: 5 development: <<: *default - database: railties_development + database: #{database_name}_development test: <<: *default - database: railties_test + database: #{database_name}_test YAML end end From 5b6bc8f5cf81e52ab81f1e295f7202c42dec1efb Mon Sep 17 00:00:00 2001 From: Ed Sharp Date: Tue, 31 May 2022 09:03:19 +0100 Subject: [PATCH 017/354] Fix hstore deserialize regression https://github.com/rails/rails/commit/98bf64bcb9648f88bff4cb59a7ae4db2b6410241 introduced a StringScanner to ensure that value is a valid hstore document. However, the negative lookbehind in the regex used to find the final double-quote of both the key and the value (/(?"\\"'::hstore; hstore ------------ "\\"=>"\\" will be incorrectly deemed invalid. This commit aims to rectify that by switching from scan_until to scan and lazily matching zero or more of either: - an escaped pair (either \\ or \"); or - a character that doesn't need escaping until the positive lookahead matches a double-quote. The tests have been updated to include both a key and value that end in a backslash. --- activerecord/CHANGELOG.md | 4 ++++ .../connection_adapters/postgresql/oid/hstore.rb | 4 ++-- activerecord/test/cases/adapters/postgresql/hstore_test.rb | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1c95d0e60d385..5e98881a55e42 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix Hstore deserialize regression. + + *edsharp* + ## Rails 7.0.3 (May 09, 2022) ## * Some internal housekeeping on reloads could break custom `respond_to?` diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb index 9c838d6310309..d6f60f563e76c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/hstore.rb @@ -26,7 +26,7 @@ def deserialize(value) raise(ArgumentError, ERROR % scanner.string.inspect) end - unless key = scanner.scan_until(/(? 'b\\ar', '1"foo' => "2") assert_cycle('a\\"' => 'b\\ar', '1"foo' => "2") + assert_cycle("a\\" => "bar\\", '1"foo' => "2") end def test_comma From d15a694b40922f15c81042acaeede9e7df7bbb75 Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Tue, 31 May 2022 10:25:57 -0400 Subject: [PATCH 018/354] Merge pull request #45218 from flavorjones/flavorjones-rhs124-html-safe Strings returned from `strip_tags` are correctly tagged `html_safe?` --- actionview/CHANGELOG.md | 10 ++++++++++ actionview/lib/action_view/helpers/sanitize_helper.rb | 2 +- actionview/test/template/sanitize_helper_test.rb | 6 ++++++ activesupport/test/safe_buffer_test.rb | 3 ++- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index cd4cdf94a4e50..06e38a200e1a1 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,13 @@ +* Strings returned from `strip_tags` are correctly tagged `html_safe?` + + Because these strings contain no HTML elements and the basic entities are escaped, they are safe + to be included as-is as PCDATA in HTML content. Tagging them as html-safe avoids double-escaping + entities when being concatenated to a SafeBuffer during rendering. + + Fixes [rails/rails-html-sanitizer#124](https://github.com/rails/rails-html-sanitizer/issues/124) + + *Mike Dalessio* + ## Rails 7.0.3 (May 09, 2022) ## * Ensure models passed to `form_for` attempt to call `to_model`. diff --git a/actionview/lib/action_view/helpers/sanitize_helper.rb b/actionview/lib/action_view/helpers/sanitize_helper.rb index 336d20abcce54..5fbfb85b2c87b 100644 --- a/actionview/lib/action_view/helpers/sanitize_helper.rb +++ b/actionview/lib/action_view/helpers/sanitize_helper.rb @@ -101,7 +101,7 @@ def sanitize_css(style) # strip_tags("> A quote from Smith & Wesson") # # => > A quote from Smith & Wesson def strip_tags(html) - self.class.full_sanitizer.sanitize(html) + self.class.full_sanitizer.sanitize(html)&.html_safe end # Strips all link tags from +html+ leaving just the link text. diff --git a/actionview/test/template/sanitize_helper_test.rb b/actionview/test/template/sanitize_helper_test.rb index 4641fd1fdcccd..ce092f9c686aa 100644 --- a/actionview/test/template/sanitize_helper_test.rb +++ b/actionview/test/template/sanitize_helper_test.rb @@ -37,6 +37,12 @@ def test_strip_tags_will_not_encode_special_characters assert_equal "test\r\n\r\ntest", strip_tags("test\r\n\r\ntest") end + def test_strip_tags_is_marked_safe + frag = "
<script>xss();</script>
" + assert_equal("<script>xss();</script>", strip_tags(frag)) # this string is safe for use as pcdata in html + assert_predicate(strip_tags(frag), :html_safe?) + end + def test_sanitize_is_marked_safe assert_predicate sanitize(""), :html_safe? end diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb index e7352d4343bf0..b8f0786f8ac45 100644 --- a/activesupport/test/safe_buffer_test.rb +++ b/activesupport/test/safe_buffer_test.rb @@ -25,7 +25,8 @@ def test_titleize test "Should NOT escape a safe value passed to it" do @buffer << "