diff options
34 files changed, 350 insertions, 103 deletions
diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index bca3d1d9ed..0d4799a7b8 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix Bcc header not being included with emails from `create_inbound_email_from` test helpers. + + *jduff* + * Add `ApplicationMailbox.mailbox_for` to expose mailbox routing. *James Dabbs* 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 8713f545f5..cd8b12dcde 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 @@ -23,7 +23,7 @@ module Rails Mail.new(params.require(:mail).permit(:from, :to, :cc, :bcc, :in_reply_to, :subject, :body).to_h).tap do |mail| mail[:bcc]&.include_in_headers = true params[:mail][:attachments].to_a.each do |attachment| - mail.add_file(filename: attachment.path, content: attachment.read) + mail.add_file(filename: attachment.original_filename, content: attachment.read) end end end diff --git a/actionmailbox/lib/action_mailbox/test_helper.rb b/actionmailbox/lib/action_mailbox/test_helper.rb index d248fa8734..dc6a0229f7 100644 --- a/actionmailbox/lib/action_mailbox/test_helper.rb +++ b/actionmailbox/lib/action_mailbox/test_helper.rb @@ -14,7 +14,11 @@ module ActionMailbox # # create_inbound_email_from_mail(from: "david@loudthinking.com", subject: "Hello!") def create_inbound_email_from_mail(status: :processing, **mail_options) - create_inbound_email_from_source Mail.new(mail_options).to_s, status: status + mail = Mail.new(mail_options) + # Bcc header is not encoded by default + mail[:bcc].include_in_headers = true if mail[:bcc] + + create_inbound_email_from_source mail.to_s, status: status end # Create an +InboundEmail+ using the raw rfc822 +source+ as text. 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 6fc39c2433..33282d36f7 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 @@ -30,6 +30,27 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa end end + test "create inbound email with bcc" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "Jason Fried <jason@37signals.com>", + bcc: "Replies <replies@example.com>", + subject: "Hey there", + body: "How's it going?" + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal %w[ jason@37signals.com ], mail.from + assert_equal %w[ replies@example.com ], mail.bcc + assert_equal "Hey there", mail.subject + assert_equal "How's it going?", mail.body.decoded + end + end + test "create inbound email with attachments" do with_rails_env("development") do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do @@ -47,6 +68,7 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa mail = ActionMailbox::InboundEmail.last.mail assert_equal "Let's talk about these images:", mail.text_part.decoded assert_equal 2, mail.attachments.count + assert_equal %w[ avatar1.jpeg avatar2.jpeg ], mail.attachments.collect(&:filename) end end diff --git a/actionmailbox/test/unit/router_test.rb b/actionmailbox/test/unit/router_test.rb index 4dd3730604..7eb8e04a73 100644 --- a/actionmailbox/test/unit/router_test.rb +++ b/actionmailbox/test/unit/router_test.rb @@ -51,6 +51,15 @@ module ActionMailbox assert_equal inbound_email.mail, $processed_mail end + test "single string routing on bcc" do + @router.add_routes("first@example.com" => :first) + + inbound_email = create_inbound_email_from_mail(to: "someone@example.com", bcc: "first@example.com", subject: "This is a reply") + @router.route inbound_email + assert_equal "FirstMailbox", $processed_by + assert_equal inbound_email.mail, $processed_mail + end + test "single string routing case-insensitively" do @router.add_routes("first@example.com" => :first) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index ca7ae6621b..8bb2c73e52 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,18 @@ +* Add `Vary: Accept` header when using `Accept` header for response + + For some requests like `/users/1`, Rails uses requests' `Accept` + header to determine what to return. And if we don't add `Vary` + in the response header, browsers might accidentally cache different + types of content, which would cause issues: e.g. javascript got displayed + instead of html content. This PR fixes these issues by adding `Vary: Accept` + in these types of requests. For more detailed problem description, please read: + + https://github.com/rails/rails/pull/36213 + + Fixes #25842 + + *Stan Lo* + * Fix IntegrationTest `follow_redirect!` to follow redirection using the same HTTP verb when following a 307 redirection. diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 8ba2b25552..280af50a44 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -28,6 +28,7 @@ module AbstractController else _set_rendered_content_type rendered_format end + _set_vary_header self.response_body = rendered_body end @@ -109,6 +110,9 @@ module AbstractController def _set_html_content_type # :nodoc: end + def _set_vary_header # :nodoc: + end + def _set_rendered_content_type(format) # :nodoc: end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index efa5de313c..fd22c4fa64 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -77,6 +77,10 @@ module ActionController end end + def _set_vary_header + self.headers["Vary"] = "Accept" if request.should_apply_vary_header? + end + # Normalize arguments by catching blocks and setting them on :update. def _normalize_args(action = nil, options = {}, &blk) options = super diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a2cac49082..ac0ff133eb 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -62,13 +62,7 @@ module ActionDispatch def formats fetch_header("action_dispatch.request.formats") do |k| - params_readable = begin - parameters[:format] - rescue *RESCUABLE_MIME_FORMAT_ERRORS - false - end - - v = if params_readable + v = if params_readable? Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts @@ -153,9 +147,19 @@ module ActionDispatch order.include?(Mime::ALL) ? format : nil end + def should_apply_vary_header? + !params_readable? && use_accept_header && valid_accept_header + end + private BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ + def params_readable? # :doc: + parameters[:format] + rescue *RESCUABLE_MIME_FORMAT_ERRORS + false + end + def valid_accept_header # :doc: (xhr? && (accept.present? || content_mime_type)) || (accept.present? && accept !~ BROWSER_LIKE_ACCEPTS) diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 4aee7a6f83..9184676801 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -148,7 +148,7 @@ module ActionDispatch end def glob? - !path.spec.grep(Nodes::Star).empty? + path.spec.any?(Nodes::Star) end def dispatcher? diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 642f155085..9b5a5cf2b0 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -346,28 +346,6 @@ module ActionDispatch @cookies.map { |k, v| "#{escape(k)}=#{escape(v)}" }.join "; " end - def handle_options(options) # :nodoc: - if options[:expires].respond_to?(:from_now) - options[:expires] = options[:expires].from_now - end - - options[:path] ||= "/" - - if options[:domain] == :all || options[:domain] == "all" - # If there is a provided tld length then we use it otherwise default domain regexp. - domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP - - # If host is not ip and matches domain regexp. - # (ip confirms to domain regexp so we explicitly check for ip) - options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) - ".#{$&}" - end - elsif options[:domain].is_a? Array - # If host matches one of the supplied domains without a dot in front of it. - options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } - end - end - # Sets the cookie named +name+. The second argument may be the cookie's # value or a hash of options as documented above. def []=(name, options) @@ -447,6 +425,28 @@ module ActionDispatch def write_cookie?(cookie) request.ssl? || !cookie[:secure] || always_write_cookie end + + def handle_options(options) + if options[:expires].respond_to?(:from_now) + options[:expires] = options[:expires].from_now + end + + options[:path] ||= "/" + + if options[:domain] == :all || options[:domain] == "all" + # If there is a provided tld length then we use it otherwise default domain regexp. + domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP + + # If host is not ip and matches domain regexp. + # (ip confirms to domain regexp so we explicitly check for ip) + options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) + ".#{$&}" + end + elsif options[:domain].is_a? Array + # If host matches one of the supplied domains without a dot in front of it. + options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } + end + end end class AbstractCookieJar # :nodoc: diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index caacd66bd6..cb7c2467ac 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -543,6 +543,32 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_setting_vary_header_when_request_is_xhr_with_accept_header + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_equal "Accept", response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_format_is_provided + with_test_route_set do + get "/get", params: { format: "json" } + assert_nil response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_ignore_accept_header_is_set + original_ignore_accept_header = ActionDispatch::Request.ignore_accept_header + ActionDispatch::Request.ignore_accept_header = true + + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_nil response.headers["Vary"] + end + ensure + ActionDispatch::Request.ignore_accept_header = original_ignore_accept_header + end + private def with_default_headers(headers) original = ActionDispatch::Response.default_headers diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index a2a6c69dd3..dd76824413 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -4,6 +4,11 @@ require "abstract_unit" require "controller/fake_models" class TestControllerWithExtraEtags < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "test/with_implicit_template.erb" => "Hello explicitly!", + "test/hello_world.erb" => "Hello world!" + )] + def self.controller_name; "test"; end def self.controller_path; "test"; end @@ -37,6 +42,11 @@ class TestControllerWithExtraEtags < ActionController::Base end class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "implicit_render_test/hello_world.erb" => "Hello world!", + "implicit_render_test/empty_action_with_template.html.erb" => "<h1>Empty action rendered this implicitly.</h1>\n" + )] + def empty_action end @@ -46,6 +56,10 @@ end module Namespaced class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "namespaced/implicit_render_test/hello_world.erb" => "Hello world!" + )] + def hello_world fresh_when(etag: "abc") end @@ -293,13 +307,15 @@ end module TemplateModificationHelper private def modify_template(name) - path = File.expand_path("../fixtures/#{name}.erb", __dir__) - original = File.read(path) - File.write(path, "#{original} Modified!") + hash = @controller.view_paths.first.instance_variable_get(:@hash) + key = name + ".erb" + original = hash[key] + hash[key] = "#{original} Modified!" ActionView::LookupContext::DetailsKey.clear yield ensure - File.write(path, original) + hash[key] = original + ActionView::LookupContext::DetailsKey.clear end end diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee index 736cab08db..9e11bfa7ed 100644 --- a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee @@ -11,6 +11,7 @@ Rails.serializeElement = (element, additionalParam) -> inputs.forEach (input) -> return if !input.name || input.disabled + return if input.closest('fieldset[disabled]') if matches(input, 'select') toArray(input.options).forEach (option) -> params.push(name: input.name, value: option.value) if option.selected diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 97a6da3634..cdf436ccae 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -9,10 +9,11 @@ module ActionView class << self # Supported options: # - # * <tt>name</tt> - Template name - # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> - # * <tt>dependencies</tt> - An array of dependent views - def digest(name:, format:, finder:, dependencies: nil) + # * <tt>name</tt> - Template name + # * <tt>format</tt> - Template format + # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> + # * <tt>dependencies</tt> - An array of dependent views + def digest(name:, format: nil, finder:, dependencies: nil) if dependencies.nil? || dependencies.empty? cache_key = "#{name}.#{format}" else diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 99443663dd..03eac29bb4 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -7,10 +7,15 @@ module ActionView #:nodoc: # file system. This is used internally by Rails' own test suite, and is # useful for testing extensions that have no way of knowing what the file # system will look like at runtime. - class FixtureResolver < PathResolver + class FixtureResolver < OptimizedFileSystemResolver def initialize(hash = {}, pattern = nil) - super(pattern) + super("") + if pattern + ActiveSupport::Deprecation.warn "Specifying a custom path for #{self.class} is deprecated. Implement a custom Resolver subclass instead." + @pattern = pattern + end @hash = hash + @path = "" end def data @@ -23,25 +28,32 @@ module ActionView #:nodoc: private def query(path, exts, _, locals, cache:) - query = +"" - EXTENSIONS.each do |ext, prefix| - query << "(" << exts[ext].map { |e| e && Regexp.escape("#{prefix}#{e}") }.join("|") << "|)" - end - query = /^(#{Regexp.escape(path)})#{query}$/ + regex = build_regex(path, exts) - templates = [] - @hash.each do |_path, source| - next unless query.match?(_path) + @hash.select do |_path, _| + ("/" + _path).match?(regex) + end.map do |_path, source| handler, format, variant = extract_handler_and_format_and_variant(_path) - templates << Template.new(source, _path, handler, + + Template.new(source, _path, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals ) + end.sort_by do |t| + match = ("/" + t.identifier).match(regex) + EXTENSIONS.keys.reverse.map do |ext| + if ext == :variants && exts[ext] == :any + match[ext].nil? ? 0 : 1 + elsif match[ext].nil? + exts[ext].length + else + found = match[ext].to_sym + exts[ext].index(found) + end + end end - - templates.sort_by { |t| -t.identifier.match(/^#{query}$/).captures.compact_blank.size } end end diff --git a/actionview/test/template/testing/fixture_resolver_test.rb b/actionview/test/template/testing/fixture_resolver_test.rb index 6d0317e0c9..a6066e94c6 100644 --- a/actionview/test/template/testing/fixture_resolver_test.rb +++ b/actionview/test/template/testing/fixture_resolver_test.rb @@ -27,4 +27,26 @@ class FixtureResolverTest < ActiveSupport::TestCase assert_equal :html, templates.first.format assert_equal "variant", templates.first.variant end + + def test_should_match_locales + resolver = ActionView::FixtureResolver.new("arbitrary/path.erb" => "this text", "arbitrary/path.fr.erb" => "ce texte") + en = resolver.find_all("path", "arbitrary", false, locale: [:en], formats: [:html], variants: [], handlers: [:erb]) + fr = resolver.find_all("path", "arbitrary", false, locale: [:fr], formats: [:html], variants: [], handlers: [:erb]) + + assert_equal 1, en.size + assert_equal 2, fr.size + + assert_equal "this text", en[0].source + assert_equal "ce texte", fr[0].source + assert_equal "this text", fr[1].source + end + + def test_should_return_all_variants_for_any + resolver = ActionView::FixtureResolver.new("arbitrary/path.html.erb" => "this html", "arbitrary/path.html+varient.erb" => "this text") + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: [], handlers: [:erb]) + assert_equal 1, templates.size, "expected one template" + assert_equal "this html", templates.first.source + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: :any, handlers: [:erb]) + assert_equal 2, templates.size, "expected all templates" + end end diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 16ea114f3b..dbbb383995 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -490,4 +490,22 @@ asyncTest('changing a select option without "data-url" attribute still fires aja setTimeout(function() { start() }, 20) }) +asyncTest('inputs inside disabled fieldset are not submited on remote forms', 3, function() { + $('form') + .append('<fieldset>\ + <input name="description" value="A wise man" />\ + </fieldset>') + .append('<fieldset disabled="disabled">\ + <input name="age" />\ + </fieldset>') + .bindNative('ajax:success', function(e, data, status, xhr) { + equal(data.params.user_name, 'john') + equal(data.params.description, 'A wise man') + equal(data.params.age, undefined) + + start() + }) + .triggerNative('submit') +}) + })() diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 2f0d72ccb9..542a4eb2db 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,6 @@ +* `assert_enqueued_with` and `assert_performed_with` can now test jobs with relative delay. + + *Vlado Cingel* Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activejob/CHANGELOG.md) for previous changes. diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 463020a332..5b1af7da22 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -630,7 +630,7 @@ module ActiveJob def prepare_args_for_assertion(args) args.dup.tap do |arguments| - arguments[:at] = arguments[:at].to_f if arguments[:at] + arguments[:at] = round_time_arguments(arguments[:at]) if arguments[:at] arguments[:args] = round_time_arguments(arguments[:args]) if arguments[:args] end end @@ -650,6 +650,7 @@ module ActiveJob def deserialize_args_for_assertion(job) job.dup.tap do |new_job| + new_job[:at] = round_time_arguments(Time.at(new_job[:at])) if new_job[:at] new_job[:args] = ActiveJob::Arguments.deserialize(new_job[:args]) if new_job[:args] end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 32471cfacc..34dfd8eb56 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -621,6 +621,12 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end + def test_assert_enqueued_with_with_relative_at_option + assert_enqueued_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + end + def test_assert_enqueued_with_with_no_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later assert_enqueued_with(job: HelloJob, at: Date.tomorrow.noon) @@ -1663,6 +1669,18 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_performed_with_with_relative_at_option + assert_performed_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, at: 2.minutes.from_now) do + HelloJob.set(wait: 1.minute).perform_later + end + end + end + def test_assert_performed_with_without_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 415f1f679b..1a4e0b8e59 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -352,11 +352,7 @@ module ActiveModel def attribute_method_matchers_matching(method_name) attribute_method_matchers_cache.compute_if_absent(method_name) do - # Bump plain matcher to last place so that only methods that do not - # match any other pattern match the actual attribute name. - # This is currently only needed to support legacy usage. - matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) - matchers.map { |matcher| matcher.match(method_name) }.compact + attribute_method_matchers.map { |matcher| matcher.match(method_name) }.compact end end @@ -406,10 +402,6 @@ module ActiveModel def method_name(attr_name) @method_name % attr_name end - - def plain? - prefix.empty? && suffix.empty? - end end end diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 6ad4c75fb5..2072f93194 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -62,7 +62,7 @@ module ActiveRecord::Associations::Builder # :nodoc: def middle_reflection(join_model) middle_name = [lhs_model.name.downcase.pluralize, - association_name].join("_").gsub("::", "_").to_sym + association_name.to_s].sort.join("_").gsub("::", "_").to_sym middle_options = middle_options join_model HasMany.create_reflection(lhs_model, diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index e122628b05..3e387782f6 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -91,6 +91,19 @@ module ActiveRecord end alias :blank? :empty? + def each + throw_getter_deprecation(:each) + configurations.each { |config| + yield [config.env_name, config.config] + } + end + + def first + throw_getter_deprecation(:first) + config = configurations.first + [config.env_name, config.config] + end + private def env_with_configs(env = nil) if env @@ -105,18 +118,20 @@ module ActiveRecord return configs if configs.is_a?(Array) db_configs = configs.flat_map do |env_name, config| - if config.is_a?(Hash) && config.all? { |k, v| v.is_a?(Hash) } + if config.is_a?(Hash) && config.all? { |_, v| v.is_a?(Hash) } walk_configs(env_name.to_s, config) else build_db_config_from_raw_config(env_name.to_s, "primary", config) end - end.compact + end - if url = ENV["DATABASE_URL"] - merge_url_with_configs(url, db_configs) - else - db_configs + current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s + + unless db_configs.find(&:for_current_env?) + db_configs << environment_url_config(current_env, "primary", {}) end + + merge_db_environment_variables(current_env, db_configs.compact) end def walk_configs(env_name, config) @@ -156,27 +171,24 @@ module ActiveRecord end end - def merge_url_with_configs(url, configs) - env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s + def merge_db_environment_variables(current_env, configs) + configs.map do |config| + next config if config.url_config? || config.env_name != current_env - if configs.find(&:for_current_env?) - configs.map do |config| - if config.url_config? - config - else - ActiveRecord::DatabaseConfigurations::UrlConfig.new(config.env_name, config.spec_name, url, config.config) - end - end - else - configs + [ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, "primary", url)] + url_config = environment_url_config(current_env, config.spec_name, config.config) + url_config || config end end + def environment_url_config(env, spec_name, config) + url = ENV["DATABASE_URL"] + return unless url + + ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, spec_name, url, config) + end + def method_missing(method, *args, &blk) case method - when :each, :first - throw_getter_deprecation(method) - configurations.send(method, *args, &blk) when :fetch throw_getter_deprecation(method) configs_for(env_name: args.first) 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 25cfa0a723..de9742b250 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 @@ -700,10 +700,17 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal ["id"], developers(:david).projects.select(:id).first.attributes.keys end + def test_join_middle_table_alias + assert_equal( + 2, + Project.includes(:developers_projects).where.not("developers_projects.joined_on": nil).to_a.size + ) + end + def test_join_table_alias assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).to_a.size ) end @@ -716,7 +723,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).group(group.join(",")).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).group(group.join(",")).to_a.size ) end diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 15a045ed23..95e57f42e3 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -347,6 +347,22 @@ module ActiveRecord assert_equal expected, actual end end + + def test_does_not_change_other_environments + ENV["DATABASE_URL"] = "postgres://localhost/foo" + config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" }, "default_env" => {} } + + actual = resolve_spec(:production, config) + assert_equal config["production"].merge("name" => "production"), actual + + actual = resolve_spec(:default_env, config) + assert_equal({ + "host" => "localhost", + "database" => "foo", + "adapter" => "postgresql", + "name" => "default_env" + }, actual) + end end end end diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb index ed8151f01a..725d1b5d1b 100644 --- a/activerecord/test/cases/database_configurations_test.rb +++ b/activerecord/test/cases/database_configurations_test.rb @@ -80,17 +80,20 @@ class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase def test_each_is_deprecated assert_deprecated do - ActiveRecord::Base.configurations.each do |db_config| - assert_equal "primary", db_config.spec_name + all_configs = ActiveRecord::Base.configurations.values + ActiveRecord::Base.configurations.each do |env_name, config| + assert_includes ["arunit", "arunit2", "arunit_without_prepared_statements"], env_name + assert_includes all_configs, config end end end def test_first_is_deprecated + first_config = ActiveRecord::Base.configurations.values.first assert_deprecated do - db_config = ActiveRecord::Base.configurations.first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name + env_name, config = ActiveRecord::Base.configurations.first + assert_equal "arunit", env_name + assert_equal first_config, config end end diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb index 4ca24028b0..4894193665 100644 --- a/activesupport/lib/active_support/core_ext/string/access.rb +++ b/activesupport/lib/active_support/core_ext/string/access.rb @@ -44,7 +44,7 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def from(position) - self[position..-1] + self[position, length] end # Returns a substring from the beginning of the string to the given position. @@ -61,7 +61,8 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def to(position) - self[0..position] + position = [position + length, -1].max if position < 0 + self[0, position + 1] end # Returns the first character. If a limit is supplied, returns a substring diff --git a/activesupport/lib/active_support/secure_compare_rotator.rb b/activesupport/lib/active_support/secure_compare_rotator.rb index 14a0aee947..97110d41f7 100644 --- a/activesupport/lib/active_support/secure_compare_rotator.rb +++ b/activesupport/lib/active_support/secure_compare_rotator.rb @@ -37,7 +37,7 @@ module ActiveSupport @value = value end - def secure_compare!(other_value, on_rotation: @rotation) + def secure_compare!(other_value, on_rotation: @on_rotation) secure_compare(@value, other_value) || run_rotations(on_rotation) { |wrapper| wrapper.secure_compare!(other_value) } || raise(InvalidMatch) diff --git a/activesupport/test/secure_compare_rotator_test.rb b/activesupport/test/secure_compare_rotator_test.rb index 8acf13e38f..d80faea128 100644 --- a/activesupport/test/secure_compare_rotator_test.rb +++ b/activesupport/test/secure_compare_rotator_test.rb @@ -41,4 +41,17 @@ class SecureCompareRotatorTest < ActiveSupport::TestCase assert_equal(true, wrapper.secure_compare!("and_another_one", on_rotation: -> { @witness = true })) end end + + test "#secure_compare! calls the on_rotation proc that given in constructor" do + @witness = nil + + wrapper = ActiveSupport::SecureCompareRotator.new("old_secret", on_rotation: -> { @witness = true }) + wrapper.rotate("new_secret") + wrapper.rotate("another_secret") + wrapper.rotate("and_another_one") + + assert_changes(:@witness, from: nil, to: true) do + assert_equal(true, wrapper.secure_compare!("and_another_one")) + end + end end diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 8cf8efefd0..b740e933ba 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -14,6 +14,7 @@ After reading this guide, you will know: * How Rails' built-in helpers assist you. * How to handle Ajax on the server side. * The Turbolinks gem. +* How to include your Cross-Site Request Forgery token in request headers ------------------------------------------------------------------------------- @@ -524,6 +525,23 @@ For more details, including other events you can bind to, check out [the Turbolinks README](https://github.com/turbolinks/turbolinks/blob/master/README.md). +Cross-Site Request Forgery (CSRF) token in Ajax +---- + +When using another library to make Ajax calls, it is necessary to add +the security token as a default header for Ajax calls in your library. To get +the token: + +```javascript +var token = document.getElementsByName('csrf-token')[0].content +``` + +You can then submit this token as a X-CSRF-Token in your header for your +Ajax requst. You do not need to add a CSRF for GET requests, only non-GET +requests. + +You can read more about about Cross-Site Request Forgery in [Security](https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf) + Other Resources --------------- diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt index ffe53497bf..2510ab906f 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_0.rb.tt @@ -38,3 +38,8 @@ # MailDeliveryJob to ensure all delivery jobs are processed properly. # Make sure your entire app is migrated and stable on 6.0 before using this setting. # Rails.application.config.action_mailer.delivery_job = "ActionMailer::MailDeliveryJob" + +# Enable the same cache key to be reused when the object being cached of type +# `ActiveRecord::Relation` changes by moving the volatile information (max updated at and count) +# of the relation's cache key into the cache version to support recycling cache key. +# Rails.application.config.active_record.collection_cache_versioning = true diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index 838fe55acc..0664338e0b 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -223,12 +223,10 @@ module Rails private def files_in(path) - Dir.chdir(path) do - files = Dir.glob(@glob) - files -= @exclude if @exclude - files.map! { |file| File.join(path, file) } - files.sort - end + files = Dir.glob(@glob, base: path) + files -= @exclude if @exclude + files.map! { |file| File.join(path, file) } + files.sort end end end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index fe56e3d076..e8456e8c19 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -162,7 +162,6 @@ module ApplicationTests rails "generate", "scaffold", "user", "username:string", "password:string" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") @@ -194,7 +193,6 @@ module ApplicationTests rails "generate", "scaffold", "LineItems", "product:references", "cart:belongs_to" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") |