diff options
Diffstat (limited to 'actionpack/lib')
25 files changed, 387 insertions, 74 deletions
diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 3a98931167..d1ff62a032 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "action_pack" +require "active_support" require "active_support/rails" require "active_support/i18n" 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.rb b/actionpack/lib/action_controller.rb index 29d61c3ceb..22dc229599 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/rails" require "abstract_controller" require "action_dispatch" require "action_controller/metal/live" @@ -28,6 +27,7 @@ module ActionController autoload :DefaultHeaders autoload :EtagWithTemplateDigest autoload :EtagWithFlash + autoload :FeaturePolicy autoload :Flash autoload :ForceSSL autoload :Head diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 2e565d5d44..63c138af55 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -226,6 +226,7 @@ module ActionController FormBuilder, RequestForgeryProtection, ContentSecurityPolicy, + FeaturePolicy, ForceSSL, Streaming, DataStreaming, diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 9ef4f50df1..879745a895 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -53,7 +53,7 @@ module ActionController #:nodoc: # # Show a 404 page in the browser: # - # send_file '/path/to/404.html', type: 'text/html; charset=utf-8', status: 404 + # send_file '/path/to/404.html', type: 'text/html; charset=utf-8', disposition: 'inline', status: 404 # # Read about the other Content-* HTTP headers if you'd like to # provide the user with more information (such as Content-Description) in diff --git a/actionpack/lib/action_controller/metal/feature_policy.rb b/actionpack/lib/action_controller/metal/feature_policy.rb new file mode 100644 index 0000000000..a627eabea6 --- /dev/null +++ b/actionpack/lib/action_controller/metal/feature_policy.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module ActionController #:nodoc: + # HTTP Feature Policy is a web standard for defining a mechanism to + # allow and deny the use of browser features in its own context, and + # in content within any <iframe> elements in the document. + # + # Full details of HTTP Feature Policy specification and guidelines can + # be found at MDN: + # + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy + # + # Examples of usage: + # + # # Global policy + # Rails.application.config.feature_policy do |f| + # f.camera :none + # f.gyroscope :none + # f.microphone :none + # f.usb :none + # f.fullscreen :self + # f.payment :self, "https://secure.example.com" + # end + # + # # Controller level policy + # class PagesController < ApplicationController + # feature_policy do |p| + # p.geolocation "https://example.com" + # end + # end + module FeaturePolicy + extend ActiveSupport::Concern + + module ClassMethods + def feature_policy(**options, &block) + before_action(options) do + if block_given? + policy = request.feature_policy.clone + yield policy + request.feature_policy = policy + end + end + end + end + end +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_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 6a07a73d94..920ae52f2b 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -259,6 +259,11 @@ module ActionController @parameters == other end end + alias eql? == + + def hash + [@parameters.hash, @permitted].hash + end # Returns a safe <tt>ActiveSupport::HashWithIndifferentAccess</tt> # representation of the parameters with all unpermitted keys removed. @@ -744,6 +749,18 @@ module ActionController end alias_method :delete_if, :reject! + # Returns a new instance of <tt>ActionController::Parameters</tt> without the blank values. + # Uses Object#blank? for determining if a value is blank. + def compact_blank + reject { |_k, v| v.blank? } + end + + # Removes all blank values in place and returns self. + # Uses Object#blank? for determining if a value is blank. + def compact_blank! + reject! { |_k, v| v.blank? } + end + # Returns values that were assigned to the given +keys+. Note that all the # +Hash+ objects will be converted to <tt>ActionController::Parameters</tt>. def values_at(*keys) diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 6a4ba9af4a..67d303a368 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -43,6 +43,7 @@ module ActionDispatch eager_autoload do autoload_under "http" do autoload :ContentSecurityPolicy + autoload :FeaturePolicy autoload :Request autoload :Response end diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 5c6fa2dfa7..9c430b57e3 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -22,8 +22,9 @@ module ActionDispatch #:nodoc: if policy = request.content_security_policy nonce = request.content_security_policy_nonce + nonce_directives = request.content_security_policy_nonce_directives context = request.controller_instance || request - headers[header_name(request)] = policy.build(context, nonce) + headers[header_name(request)] = policy.build(context, nonce, nonce_directives) end response @@ -54,6 +55,7 @@ module ActionDispatch #:nodoc: POLICY_REPORT_ONLY = "action_dispatch.content_security_policy_report_only" NONCE_GENERATOR = "action_dispatch.content_security_policy_nonce_generator" NONCE = "action_dispatch.content_security_policy_nonce" + NONCE_DIRECTIVES = "action_dispatch.content_security_policy_nonce_directives" def content_security_policy get_header(POLICY) @@ -79,6 +81,14 @@ module ActionDispatch #:nodoc: set_header(NONCE_GENERATOR, generator) end + def content_security_policy_nonce_directives + get_header(NONCE_DIRECTIVES) + end + + def content_security_policy_nonce_directives=(generator) + set_header(NONCE_DIRECTIVES, generator) + end + def content_security_policy_nonce if content_security_policy_nonce_generator if nonce = get_header(NONCE) @@ -127,13 +137,17 @@ module ActionDispatch #:nodoc: object_src: "object-src", prefetch_src: "prefetch-src", script_src: "script-src", + script_src_attr: "script-src-attr", + script_src_elem: "script-src-elem", style_src: "style-src", + style_src_attr: "style-src-attr", + style_src_elem: "style-src-elem", worker_src: "worker-src" }.freeze - NONCE_DIRECTIVES = %w[script-src style-src].freeze + DEFAULT_NONCE_DIRECTIVES = %w[script-src style-src].freeze - private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES + private_constant :MAPPINGS, :DIRECTIVES, :DEFAULT_NONCE_DIRECTIVES attr_reader :directives @@ -202,8 +216,9 @@ module ActionDispatch #:nodoc: end end - def build(context = nil, nonce = nil) - build_directives(context, nonce).compact.join("; ") + def build(context = nil, nonce = nil, nonce_directives = nil) + nonce_directives = DEFAULT_NONCE_DIRECTIVES if nonce_directives.nil? + build_directives(context, nonce, nonce_directives).compact.join("; ") end private @@ -226,10 +241,10 @@ module ActionDispatch #:nodoc: end end - def build_directives(context, nonce) + def build_directives(context, nonce, nonce_directives) @directives.map do |directive, sources| if sources.is_a?(Array) - if nonce && nonce_directive?(directive) + if nonce && nonce_directive?(directive, nonce_directives) "#{directive} #{build_directive(sources, context).join(' ')} 'nonce-#{nonce}'" else "#{directive} #{build_directive(sources, context).join(' ')}" @@ -264,8 +279,8 @@ module ActionDispatch #:nodoc: end end - def nonce_directive?(directive) - NONCE_DIRECTIVES.include?(directive) + def nonce_directive?(directive, nonce_directives) + nonce_directives.include?(directive) end end end diff --git a/actionpack/lib/action_dispatch/http/feature_policy.rb b/actionpack/lib/action_dispatch/http/feature_policy.rb new file mode 100644 index 0000000000..592b6e4393 --- /dev/null +++ b/actionpack/lib/action_dispatch/http/feature_policy.rb @@ -0,0 +1,168 @@ +# frozen_string_literal: true + +require "active_support/core_ext/object/deep_dup" + +module ActionDispatch #:nodoc: + class FeaturePolicy + class Middleware + CONTENT_TYPE = "Content-Type" + POLICY = "Feature-Policy" + + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new(env) + _, headers, _ = response = @app.call(env) + + return response unless html_response?(headers) + return response if policy_present?(headers) + + if policy = request.feature_policy + headers[POLICY] = policy.build(request.controller_instance) + end + + if policy_empty?(policy) + headers.delete(POLICY) + end + + response + end + + private + def html_response?(headers) + if content_type = headers[CONTENT_TYPE] + content_type =~ /html/ + end + end + + def policy_present?(headers) + headers[POLICY] + end + + def policy_empty?(policy) + policy.try(:directives) && policy.directives.empty? + end + end + + module Request + POLICY = "action_dispatch.feature_policy" + + def feature_policy + get_header(POLICY) + end + + def feature_policy=(policy) + set_header(POLICY, policy) + end + end + + MAPPINGS = { + self: "'self'", + none: "'none'", + }.freeze + + # List of available features can be found at + # https://github.com/WICG/feature-policy/blob/master/features.md#policy-controlled-features + DIRECTIVES = { + accelerometer: "accelerometer", + ambient_light_sensor: "ambient-light-sensor", + autoplay: "autoplay", + camera: "camera", + encrypted_media: "encrypted-media", + fullscreen: "fullscreen", + geolocation: "geolocation", + gyroscope: "gyroscope", + magnetometer: "magnetometer", + microphone: "microphone", + midi: "midi", + payment: "payment", + picture_in_picture: "picture-in-picture", + speaker: "speaker", + usb: "usb", + vibrate: "vibrate", + vr: "vr", + }.freeze + + private_constant :MAPPINGS, :DIRECTIVES + + attr_reader :directives + + def initialize + @directives = {} + yield self if block_given? + end + + def initialize_copy(other) + @directives = other.directives.deep_dup + end + + DIRECTIVES.each do |name, directive| + define_method(name) do |*sources| + if sources.first + @directives[directive] = apply_mappings(sources) + else + @directives.delete(directive) + end + end + end + + def build(context = nil) + build_directives(context).compact.join("; ") + end + + private + def apply_mappings(sources) + sources.map do |source| + case source + when Symbol + apply_mapping(source) + when String, Proc + source + else + raise ArgumentError, "Invalid HTTP feature policy source: #{source.inspect}" + end + end + end + + def apply_mapping(source) + MAPPINGS.fetch(source) do + raise ArgumentError, "Unknown HTTP feature policy source mapping: #{source.inspect}" + end + end + + def build_directives(context) + @directives.map do |directive, sources| + if sources.is_a?(Array) + "#{directive} #{build_directive(sources, context).join(' ')}" + elsif sources + directive + else + nil + end + end + end + + def build_directive(sources, context) + sources.map { |source| resolve_source(source, context) } + end + + def resolve_source(source, context) + case source + when String + source + when Symbol + source.to_s + when Proc + if context.nil? + raise RuntimeError, "Missing context for the dynamic feature policy source: #{source.inspect}" + else + context.instance_exec(&source) + end + else + raise RuntimeError, "Unexpected feature policy source: #{source.inspect}" + end + end + end +end diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a2cac49082..6bf4e652d3 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,12 +147,22 @@ 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) + (accept.present? && !accept.match?(BROWSER_LIKE_ACCEPTS)) end def use_accept_header # :doc: diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 44f23940d3..4ac7c5c2bd 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -23,6 +23,7 @@ module ActionDispatch include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::URL include ActionDispatch::ContentSecurityPolicy::Request + include ActionDispatch::FeaturePolicy::Request include Rack::Request::Env autoload :Session, "action_dispatch/request/session" diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3b0f6378ea..225ae0a497 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -133,7 +133,7 @@ module ActionDispatch end def named_host?(host) - IP_HOST_REGEXP !~ host + !IP_HOST_REGEXP.match?(host) end def normalize_protocol(protocol) 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..96bdf570af 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.match?(/^[\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/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index f8937a2faf..e546d1c11f 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -137,9 +137,7 @@ module ActionDispatch return unless logger exception = wrapper.exception - - trace = wrapper.application_trace - trace = wrapper.framework_trace if trace.empty? + trace = wrapper.exception_trace ActiveSupport::Deprecation.silence do message = [] diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 2da0ef9600..e4a2a51c57 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -36,18 +36,23 @@ module ActionDispatch "ActionView::Template::Error" ] + cattr_accessor :silent_exceptions, default: [ + "ActionController::RoutingError" + ] + attr_reader :backtrace_cleaner, :exception, :wrapped_causes, :line_number, :file def initialize(backtrace_cleaner, exception) @backtrace_cleaner = backtrace_cleaner @exception = exception + @exception_class_name = @exception.class.name @wrapped_causes = wrapped_causes_for(exception, backtrace_cleaner) expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError) end def unwrapped_exception - if wrapper_exceptions.include?(exception.class.to_s) + if wrapper_exceptions.include?(@exception_class_name) exception.cause else exception @@ -55,13 +60,19 @@ module ActionDispatch end def rescue_template - @@rescue_templates[@exception.class.name] + @@rescue_templates[@exception_class_name] end def status_code self.class.status_code_for_exception(unwrapped_exception.class.name) end + def exception_trace + trace = application_trace + trace = framework_trace if trace.empty? && !silent_exceptions.include?(@exception_class_name) + trace + end + def application_trace clean_backtrace(:silent) end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 66f90980b9..2e09aed41d 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -54,11 +54,5 @@ module ActionDispatch ActionDispatch.test_app = app end - - initializer "action_dispatch.system_tests" do |app| - ActiveSupport.on_load(:action_dispatch_system_test_case) do - include app.routes.url_helpers - end - end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d1100089b1..e3090e7ba2 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -112,7 +112,7 @@ module ActionDispatch end def self.optional_format?(path, format) - format != false && path !~ OPTIONAL_FORMAT_REGEX + format != false && !path.match?(OPTIONAL_FORMAT_REGEX) end def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:, options_constraints:, anchor:, scope_params:, options:) @@ -1833,7 +1833,7 @@ module ActionDispatch # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. if as.nil? - candidate unless candidate !~ /\A[_a-z]/i || has_named_route?(candidate) + candidate unless !candidate.match?(/\A[_a-z]/i) || has_named_route?(candidate) else candidate end diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index a7fb5fa330..aae96975c7 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true -gem "capybara", ">= 2.15" +gem "capybara", ">= 3.26" require "capybara/dsl" require "capybara/minitest" +require "selenium/webdriver" require "action_controller" require "action_dispatch/system_testing/driver" require "action_dispatch/system_testing/browser" @@ -158,12 +159,33 @@ module ActionDispatch driven_by :selenium - def url_options # :nodoc: - default_url_options.merge(host: Capybara.app_host) - end + private + def url_helpers + @url_helpers ||= + if ActionDispatch.test_app + Class.new do + include ActionDispatch.test_app.routes.url_helpers - ActiveSupport.run_load_hooks(:action_dispatch_system_test_case, self) - end + def url_options + default_url_options.reverse_merge(host: Capybara.app_host || Capybara.current_session.server_url) + end + end.new + end + end - SystemTestCase.start_application + def method_missing(name, *args, &block) + if url_helpers.respond_to?(name) + url_helpers.public_send(name, *args, &block) + else + super + end + end + + def respond_to_missing?(name, include_private = false) + url_helpers.respond_to?(name) + end + end end + +ActiveSupport.run_load_hooks :action_dispatch_system_test_case, ActionDispatch::SystemTestCase +ActionDispatch::SystemTestCase.start_application diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb index c34907b6cb..e861e52f09 100644 --- a/actionpack/lib/action_dispatch/system_testing/browser.rb +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -39,6 +39,29 @@ module ActionDispatch end end + # driver_path can be configured as a proc. The webdrivers gem uses this + # proc to update web drivers. Running this proc early allows us to only + # update the webdriver once and avoid race conditions when using + # parallel tests. + def preload + case type + when :chrome + if ::Selenium::WebDriver::Service.respond_to? :driver_path= + ::Selenium::WebDriver::Chrome::Service.driver_path.try(:call) + else + # Selenium <= v3.141.0 + ::Selenium::WebDriver::Chrome.driver_path + end + when :firefox + if ::Selenium::WebDriver::Service.respond_to? :driver_path= + ::Selenium::WebDriver::Firefox::Service.driver_path.try(:call) + else + # Selenium <= v3.141.0 + ::Selenium::WebDriver::Firefox.driver_path + end + end + end + private def headless_chrome_browser_options capabilities.args << "--headless" diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 25a09dd918..15943a55ea 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -9,6 +9,8 @@ module ActionDispatch @screen_size = options[:screen_size] @options = options[:options] @capabilities = capabilities + + @browser.preload end def use diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb index 20f6a7634f..30dc21ebb9 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -4,15 +4,12 @@ module ActionDispatch module SystemTesting module TestHelpers module SetupAndTeardown # :nodoc: - DEFAULT_HOST = "http://127.0.0.1" - def host!(host) - Capybara.app_host = host - end + ActiveSupport::Deprecation.warn \ + "ActionDispatch::SystemTestCase#host! is deprecated with no replacement. " \ + "Set Capybara.app_host directly or rely on Capybara's default host." - def before_setup - host! DEFAULT_HOST - super + Capybara.app_host = host end def before_teardown diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index bb8b43ad4d..9e7b4301a8 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -3,7 +3,6 @@ require "stringio" require "uri" require "active_support/core_ext/kernel/singleton_class" -require "active_support/core_ext/object/try" require "rack/test" require "minitest" @@ -50,11 +49,16 @@ module ActionDispatch # Follow a single redirect response. If the last response was not a # redirect, an exception will be raised. Otherwise, the redirect is - # performed on the location header. Any arguments are passed to the - # underlying call to `get`. + # performed on the location header. If the redirection is a 307 redirect, + # the same HTTP verb will be used when redirecting, otherwise a GET request + # will be performed. Any arguments are passed to the + # underlying request. def follow_redirect!(**args) raise "not a redirect! #{status} #{status_message}" unless redirect? - get(response.location, **args) + + method = response.status == 307 ? request.method.downcase : :get + public_send(method, response.location, **args) + status end end |