diff options
Diffstat (limited to 'actionpack')
114 files changed, 1607 insertions, 1089 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 81201795d4..9931a0de81 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,100 @@ +* Raise an `ArgumentError` if a resource custom param contains a colon (`:`). + + After this change it's not possible anymore to configure routes like this: + + ``` + routes.draw do + resources :users, param: 'name/:sneaky' + end + ``` + + Fixes #30467. + + *Josua Schmid* + + +## Rails 6.0.0.beta3 (March 11, 2019) ## + +* No changes. + + +## Rails 6.0.0.beta2 (February 25, 2019) ## + +* Make debug exceptions works in an environment where ActiveStorage is not loaded. + + *Tomoyuki Kurosawa* + +* `ActionDispatch::SystemTestCase.driven_by` can now be called with a block + to define specific browser capabilities. + + *Edouard Chin* + + +## Rails 6.0.0.beta1 (January 18, 2019) ## + +* Remove deprecated `fragment_cache_key` helper in favor of `combined_fragment_cache_key`. + + *Rafael Mendonça França* + +* Remove deprecated methods in `ActionDispatch::TestResponse`. + + `#success?`, `missing?` and `error?` were deprecated in Rails 5.2 in favor of + `#successful?`, `not_found?` and `server_error?`. + + *Rafael Mendonça França* + +* Introduce ActionDispatch::HostAuthorization + + This is a new middleware that guards against DNS rebinding attacks by + explicitly permitting the hosts a request can be made to. + + Each host is checked with the case operator (`#===`) to support `RegExp`, + `Proc`, `IPAddr` and custom objects as host allowances. + + *Genadi Samokovarov* + +* Allow using `parsed_body` in `ActionController::TestCase`. + + In addition to `ActionDispatch::IntegrationTest`, allow using + `parsed_body` in `ActionController::TestCase`: + + ``` + class SomeControllerTest < ActionController::TestCase + def test_some_action + post :action, body: { foo: 'bar' } + assert_equal({ "foo" => "bar" }, response.parsed_body) + end + end + ``` + + Fixes #34676. + + *Tobias Bühlmann* + +* Raise an error on root route naming conflicts. + + Raises an ArgumentError when multiple root routes are defined in the + same context instead of assigning nil names to subsequent roots. + + *Gannon McGibbon* + +* Allow rescue from parameter parse errors: + + ``` + rescue_from ActionDispatch::Http::Parameters::ParseError do + head :unauthorized + end + ``` + + *Gannon McGibbon*, *Josh Cheek* + +* Reset Capybara sessions if failed system test screenshot raising an exception. + + Reset Capybara sessions if `take_failed_screenshot` raise exception + in system test `after_teardown`. + + *Maxim Perepelitsa* + * Use request object for context if there's no controller There is no controller instance when using a redirect route or a @@ -11,7 +108,7 @@ * Apply mapping to symbols returned from dynamic CSP sources Previously if a dynamic source returned a symbol such as :self it - would be converted to a string implicity, e.g: + would be converted to a string implicitly, e.g: policy.default_src -> { :self } @@ -33,10 +130,6 @@ *Yoshiyuki Kinjo* -* Remove undocumented `params` option from `url_for` helper. - - *Ilkka Oksanen* - * Encode Content-Disposition filenames on `send_data` and `send_file`. Previously, `send_data 'data', filename: "\u{3042}.txt"` sends `"filename=\"\u{3042}.txt\""` as Content-Disposition and it can be @@ -68,7 +161,7 @@ *Assain Jaleel* -* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. +* Raises `ActionController::RespondToMismatchError` with conflicting `respond_to` invocations. `respond_to` can match multiple types and lead to undefined behavior when multiple invocations are made and the types do not match: @@ -93,7 +186,7 @@ *Aaron Kromer* -* Pass along arguments to underlying `get` method in `follow_redirect!`. +* Pass along arguments to underlying `get` method in `follow_redirect!` Now all arguments passed to `follow_redirect!` are passed to the underlying `get` method. This for example allows to set custom headers for the @@ -141,9 +234,9 @@ *Derek Prior* -* Rails 6 requires Ruby 2.4.1 or newer. +* Rails 6 requires Ruby 2.5.0 or newer. - *Jeremy Daer* + *Jeremy Daer*, *Kasper Timm Hansen* Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/MIT-LICENSE b/actionpack/MIT-LICENSE index 1cb3add0fc..ab7c27c209 100644 --- a/actionpack/MIT-LICENSE +++ b/actionpack/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2018 David Heinemeier Hansson +Copyright (c) 2004-2019 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/actionpack/README.rdoc b/actionpack/README.rdoc index f56230ffa0..fe85bc5b7a 100644 --- a/actionpack/README.rdoc +++ b/actionpack/README.rdoc @@ -23,6 +23,7 @@ by default and Action View rendering is implicitly triggered by Action Controller. However, these modules are designed to function on their own and can be used outside of Rails. +You can read more about Action Pack in the {Action Controller Overview}[https://guides.rubyonrails.org/action_controller_overview.html] guide. == Download and installation @@ -46,7 +47,7 @@ Action Pack is released under the MIT license: API documentation is at: -* http://api.rubyonrails.org +* https://api.rubyonrails.org Bug reports for the Ruby on Rails project can be filed here: diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index ec56de18f1..735eb734d0 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -9,13 +9,13 @@ Gem::Specification.new do |s| s.summary = "Web-flow and rendering framework putting the VC in MVC (part of Rails)." s.description = "Web apps on Rails. Simple, battle-tested conventions for building and testing MVC web applications. Works with any Rack-compatible server." - s.required_ruby_version = ">= 2.4.1" + s.required_ruby_version = ">= 2.5.0" s.license = "MIT" s.author = "David Heinemeier Hansson" s.email = "david@loudthinking.com" - s.homepage = "http://rubyonrails.org" + s.homepage = "https://rubyonrails.org" s.files = Dir["CHANGELOG.md", "README.rdoc", "MIT-LICENSE", "lib/**/*"] s.require_path = "lib" diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 0477e7f1c9..3a98931167 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -7,6 +7,7 @@ require "active_support/i18n" module AbstractController extend ActiveSupport::Autoload + autoload :ActionNotFound, "abstract_controller/base" autoload :Base autoload :Caching autoload :Callbacks diff --git a/actionpack/lib/abstract_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index 95078a2a28..18677ddd18 100644 --- a/actionpack/lib/abstract_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -28,7 +28,6 @@ module AbstractController self.fragment_cache_keys = [] if respond_to?(:helper_method) - helper_method :fragment_cache_key helper_method :combined_fragment_cache_key end end @@ -61,25 +60,6 @@ module AbstractController end # Given a key (as described in +expire_fragment+), returns - # a key suitable for use in reading, writing, or expiring a - # cached fragment. All keys begin with <tt>views/</tt>, - # followed by any controller-wide key prefix values, ending - # with the specified +key+ value. The key is expanded using - # ActiveSupport::Cache.expand_cache_key. - def fragment_cache_key(key) - ActiveSupport::Deprecation.warn(<<-MSG.squish) - Calling fragment_cache_key directly is deprecated and will be removed in Rails 6.0. - All fragment accessors now use the combined_fragment_cache_key method that retains the key as an array, - such that the caching stores can interrogate the parts for cache versions used in - recyclable cache keys. - MSG - - head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) } - tail = key.is_a?(Hash) ? url_for(key).split("://").last : key - ActiveSupport::Cache.expand_cache_key([*head, *tail], :views) - end - - # Given a key (as described in +expire_fragment+), returns # a key array suitable for use in reading, writing, or expiring a # cached fragment. All keys begin with <tt>:views</tt>, # followed by <tt>ENV["RAILS_CACHE_ID"]</tt> or <tt>ENV["RAILS_APP_VERSION"]</tt> if set, diff --git a/actionpack/lib/abstract_controller/translation.rb b/actionpack/lib/abstract_controller/translation.rb index 666e154e4c..4dad2a2b93 100644 --- a/actionpack/lib/abstract_controller/translation.rb +++ b/actionpack/lib/abstract_controller/translation.rb @@ -11,6 +11,7 @@ module AbstractController # to translate many keys within the same controller / action and gives you a # simple framework for scoping them consistently. def translate(key, options = {}) + options = options.dup if key.to_s.first == "." path = controller_path.tr("/", ".") defaults = [:"#{path}#{key}"] diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index afbd38e7fe..d8b04d8ddb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -56,7 +56,7 @@ module ActionController def unpermitted_parameters(event) debug do unpermitted_keys = event.payload[:keys] - "Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}" + color("Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}", RED) end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index f875aa5e6b..b9088e6d86 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -26,10 +26,10 @@ module ActionController end end - def build(action, app = Proc.new) + def build(action, app = nil, &block) action = action.to_s - middlewares.reverse.inject(app) do |a, middleware| + middlewares.reverse.inject(app || block) do |a, middleware| middleware.valid?(action) ? middleware.build(a) : a end end diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb index 2dc990f303..f9a758ff0e 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -6,7 +6,7 @@ module ActionController super.tap { default_render unless performed? } end - def default_render(*args) + def default_render head :no_content end end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index d6911ee2b5..29d1919ec5 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/hash/keys" - module ActionController module ConditionalGet extend ActiveSupport::Concern diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb index 640c75536e..2f1544c69c 100644 --- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb +++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb @@ -51,7 +51,7 @@ module ActionController end def lookup_and_digest_template(template) - ActionView::Digestor.digest name: template, finder: lookup_context + ActionView::Digestor.digest name: template, format: nil, finder: lookup_context end end end diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index 30034be018..e1e0c6f456 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -52,7 +52,7 @@ module ActionController end # Raised when a nested respond_to is triggered and the content types of each - # are incompatible. For exampe: + # are incompatible. For example: # # respond_to do |outer_type| # outer_type.js do diff --git a/actionpack/lib/action_controller/metal/flash.rb b/actionpack/lib/action_controller/metal/flash.rb index 380f2e9591..a4861dc2c0 100644 --- a/actionpack/lib/action_controller/metal/flash.rb +++ b/actionpack/lib/action_controller/metal/flash.rb @@ -44,18 +44,18 @@ module ActionController #:nodoc: end private - def redirect_to(options = {}, response_status_and_flash = {}) #:doc: + def redirect_to(options = {}, response_options_and_flash = {}) #:doc: self.class._flash_types.each do |flash_type| - if type = response_status_and_flash.delete(flash_type) + if type = response_options_and_flash.delete(flash_type) flash[flash_type] = type end end - if other_flashes = response_status_and_flash.delete(:flash) + if other_flashes = response_options_and_flash.delete(:flash) flash.update(other_flashes) end - super(options, response_status_and_flash) + super(options, response_options_and_flash) end end end diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 26e6f72b66..93fd57b640 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -40,7 +40,7 @@ module ActionController protocol: "https://", host: request.host, path: request.fullpath, - status: :moved_permanently + status: :moved_permanently, } if host_or_options.is_a?(Hash) diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 0faaac1ce4..193b488f6c 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -34,7 +34,7 @@ module ActionController # end # end # - # Then, in any view rendered by <tt>EventController</tt>, the <tt>format_time</tt> method can be called: + # Then, in any view rendered by <tt>EventsController</tt>, the <tt>format_time</tt> method can be called: # # <% @events.each do |event| -%> # <p> @@ -75,7 +75,7 @@ module ActionController # Provides a proxy to access helper methods from outside the view. def helpers @helper_proxy ||= begin - proxy = ActionView::Base.new + proxy = ActionView::Base.empty proxy.config = config.inheritable_copy proxy.extend(_helpers) end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 7036123d5d..6a274d35cb 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -69,21 +69,20 @@ module ActionController extend ActiveSupport::Concern module ClassMethods - def http_basic_authenticate_with(options = {}) - before_action(options.except(:name, :password, :realm)) do - authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password| - # This comparison uses & so that it doesn't short circuit and - # uses `secure_compare` so that length information - # isn't leaked. - ActiveSupport::SecurityUtils.secure_compare(name, options[:name]) & - ActiveSupport::SecurityUtils.secure_compare(password, options[:password]) - end - end + def http_basic_authenticate_with(name:, password:, realm: nil, **options) + before_action(options) { http_basic_authenticate_or_request_with name: name, password: password, realm: realm } + end + end + + def http_basic_authenticate_or_request_with(name:, password:, realm: nil, message: nil) + authenticate_or_request_with_http_basic(realm, message) do |given_name, given_password| + ActiveSupport::SecurityUtils.secure_compare(given_name, name) & + ActiveSupport::SecurityUtils.secure_compare(given_password, password) end end - def authenticate_or_request_with_http_basic(realm = "Application", message = nil, &login_procedure) - authenticate_with_http_basic(&login_procedure) || request_http_basic_authentication(realm, message) + def authenticate_or_request_with_http_basic(realm = nil, message = nil, &login_procedure) + authenticate_with_http_basic(&login_procedure) || request_http_basic_authentication(realm || "Application", message) end def authenticate_with_http_basic(&login_procedure) diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index d3bb58f48b..8365ddca57 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -30,9 +30,9 @@ module ActionController # :stopdoc: include BasicImplicitRender - def default_render(*args) + def default_render if template_exists?(action_name.to_s, _prefixes, variants: request.variant) - render(*args) + render elsif any_templates?(action_name.to_s, _prefixes) message = "#{self.class.name}\##{action_name} is missing a template " \ "for this request format and variant.\n" \ diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index be9449629f..51fac08749 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -30,13 +30,11 @@ module ActionController ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload| - begin - result = super + super.tap do payload[:status] = response.status - result - ensure - append_info_to_payload(payload) end + ensure + append_info_to_payload(payload) end end diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 083b762f5a..eb43ff9c63 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -146,7 +146,7 @@ module ActionController def write(string) unless @response.committed? - @response.set_header "Cache-Control", "no-cache" + @response.headers["Cache-Control"] ||= "no-cache" @response.delete_header "Content-Length" end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 118da11990..bf5e7a433f 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -124,6 +124,14 @@ module ActionController #:nodoc: # # render json: @people # + # +any+ can also be used with no arguments, in which case it will be used for any format requested by + # the user: + # + # respond_to do |format| + # format.html + # format.any { redirect_to support_path } + # end + # # Formats can have different variants. # # The request variant is a specialization of the request format, like <tt>:tablet</tt>, diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index a678377d4f..09716f7588 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -241,18 +241,7 @@ module ActionController # Performs parameters wrapping upon the request. Called automatically # by the metal call stack. def process_action(*args) - if _wrapper_enabled? - wrapped_hash = _wrap_parameters request.request_parameters - wrapped_keys = request.request_parameters.keys - wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys) - - # This will make the wrapped hash accessible from controller and view. - request.parameters.merge! wrapped_hash - request.request_parameters.merge! wrapped_hash - - # This will display the wrapped hash in the log file. - request.filtered_parameters.merge! wrapped_filtered_hash - end + _perform_parameter_wrapping if _wrapper_enabled? super end @@ -289,5 +278,20 @@ module ActionController ref = request.content_mime_type.ref _wrapper_formats.include?(ref) && _wrapper_key && !request.parameters.key?(_wrapper_key) end + + def _perform_parameter_wrapping + wrapped_hash = _wrap_parameters request.request_parameters + wrapped_keys = request.request_parameters.keys + wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys) + + # This will make the wrapped hash accessible from controller and view. + request.parameters.merge! wrapped_hash + request.request_parameters.merge! wrapped_hash + + # This will display the wrapped hash in the log file. + request.filtered_parameters.merge! wrapped_filtered_hash + rescue ActionDispatch::Http::Parameters::ParseError + # swallow parse error exception + end end end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 2804a06a58..67c198d150 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -55,11 +55,11 @@ module ActionController # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. # To terminate the execution of the function immediately after the +redirect_to+, use return. # redirect_to post_url(@post) and return - def redirect_to(options = {}, response_status = {}) + def redirect_to(options = {}, response_options = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body - self.status = _extract_redirect_to_status(options, response_status) + self.status = _extract_redirect_to_status(options, response_options) self.location = _compute_redirect_to_location(request, options) self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>" end @@ -114,11 +114,11 @@ module ActionController public :_compute_redirect_to_location private - def _extract_redirect_to_status(options, response_status) + def _extract_redirect_to_status(options, response_options) if options.is_a?(Hash) && options.key?(:status) Rack::Utils.status_code(options.delete(:status)) - elsif response_status.key?(:status) - Rack::Utils.status_code(response_status[:status]) + elsif response_options.key?(:status) + Rack::Utils.status_code(response_options[:status]) else 302 end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index cb109c6ad8..4bf8d90b69 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -431,7 +431,7 @@ module ActionController #:nodoc: The browser returned a 'null' origin for a request with origin-based forgery protection turned on. This usually means you have the 'no-referrer' Referrer-Policy header enabled, or that the request came from a site that refused to give its origin. This makes it impossible for Rails to verify the source of the requests. Likely the - best solution is to change your referrer policy to something less strict like same-origin or strict-same-origin. + best solution is to change your referrer policy to something less strict like same-origin or strict-origin. If you cannot change the referrer policy, you can disable origin checking with the Rails.application.config.action_controller.forgery_protection_origin_check setting. MSG diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 04922b0715..815f82a1f2 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -795,7 +795,7 @@ module ActionController @permitted = coder.map["ivars"][:@permitted] when "!ruby/object:ActionController::Parameters" # YAML's Object format. Only needed because of the format - # backwardscompability above, otherwise equivalent to YAML's initialization. + # backwards compatibility above, otherwise equivalent to YAML's initialization. @parameters, @permitted = coder.map["parameters"], coder.map["permitted"] end end diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index 2b4559c760..8c16308ce7 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/hash/keys" - module ActionController # ActionController::Renderer allows you to render arbitrary templates # without requirement of being in controller actions. @@ -76,7 +74,7 @@ module ActionController # * <tt>:partial</tt> - See <tt>ActionView::PartialRenderer</tt> for details. # * <tt>:file</tt> - Renders an explicit template file. Add <tt>:locals</tt> to pass in, if so desired. # It shouldn’t be used directly with unsanitized user input due to lack of validation. - # * <tt>:inline</tt> - Renders a ERB template string. + # * <tt>:inline</tt> - Renders an ERB template string. # * <tt>:plain</tt> - Renders provided text and sets the content type as <tt>text/plain</tt>. # * <tt>:html</tt> - Renders the provided HTML safe string, otherwise # performs HTML escape on the string first. Sets the content type as <tt>text/html</tt>. diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 5d784ceb31..57921f32b7 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -26,7 +26,7 @@ module ActionController end end - # ActionController::TestCase will be deprecated and moved to a gem in Rails 5.1. + # ActionController::TestCase will be deprecated and moved to a gem in the future. # Please use ActionDispatch::IntegrationTest going forward. class TestRequest < ActionDispatch::TestRequest #:nodoc: DEFAULT_ENV = ActionDispatch::TestRequest::DEFAULT_ENV.dup @@ -276,9 +276,6 @@ module ActionController # after calling +post+. If the various assert methods are not sufficient, then you # may use this object to inspect the HTTP response in detail. # - # (Earlier versions of \Rails required each functional test to subclass - # Test::Unit::TestCase and define @controller, @request, @response in +setup+.) - # # == Controller is automatically inferred # # ActionController::TestCase will automatically infer the controller under test @@ -457,7 +454,7 @@ module ActionController # respectively which will make tests more expressive. # # Note that the request method is not verified. - def process(action, method: "GET", params: {}, session: nil, body: nil, flash: {}, format: nil, xhr: false, as: nil) + def process(action, method: "GET", params: nil, session: nil, body: nil, flash: {}, format: nil, xhr: false, as: nil) check_required_ivars http_method = method.to_s.upcase @@ -485,7 +482,7 @@ module ActionController format ||= as end - parameters = params.symbolize_keys + parameters = (params || {}).symbolize_keys if format parameters[:format] = format diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 0822cdc0a6..8f39b88d56 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true #-- -# Copyright (c) 2004-2018 David Heinemeier Hansson +# Copyright (c) 2004-2019 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -49,11 +49,13 @@ module ActionDispatch end autoload_under "middleware" do + autoload :HostAuthorization autoload :RequestId autoload :Callbacks autoload :Cookies autoload :DebugExceptions autoload :DebugLocks + autoload :DebugView autoload :ExceptionWrapper autoload :Executor autoload :Flash diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index f67b13f657..8cc84ff36c 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -197,10 +197,12 @@ module ActionDispatch if control.empty? # Let middleware handle default behavior elsif control[:no_cache] - self._cache_control = NO_CACHE - if control[:extras] - self._cache_control = _cache_control + ", #{control[:extras].join(', ')}" - end + options = [] + options << PUBLIC if control[:public] + options << NO_CACHE + options.concat(control[:extras]) if control[:extras] + + self._cache_control = options.join(", ") else extras = control[:extras] max_age = control[:max_age] diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 9f8f178402..cbb772175c 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -41,6 +41,8 @@ module ActionDispatch # Returns a hash of parameters with all sensitive data replaced. def filtered_parameters @filtered_parameters ||= parameter_filter.filter(parameters) + rescue ActionDispatch::Http::Parameters::ParseError + @filtered_parameters = {} end # Returns a hash of request.env with all sensitive data replaced. diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index be129965d1..4e81ba12a5 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -7,6 +7,11 @@ module ActionDispatch module MimeNegotiation extend ActiveSupport::Concern + RESCUABLE_MIME_FORMAT_ERRORS = [ + ActionController::BadRequest, + ActionDispatch::Http::Parameters::ParseError, + ] + included do mattr_accessor :ignore_accept_header, default: false end @@ -59,7 +64,7 @@ module ActionDispatch fetch_header("action_dispatch.request.formats") do |k| params_readable = begin parameters[:format] - rescue ActionController::BadRequest + rescue *RESCUABLE_MIME_FORMAT_ERRORS false end @@ -74,6 +79,11 @@ module ActionDispatch else [Mime[:html]] end + + v = v.select do |format| + format.symbol || format.ref == "*/*" + end + set_header k, v end end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index c3e0ea3c89..88b3a93211 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -170,6 +170,7 @@ module Mime def parse(accept_header) if !accept_header.include?(",") accept_header = accept_header.split(PARAMETER_SEPARATOR_REGEXP).first + return [] unless accept_header parse_trailing_star(accept_header) || [Mime::Type.lookup(accept_header)].compact else list, index = [], 0 @@ -221,7 +222,18 @@ module Mime attr_reader :hash + MIME_NAME = "[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}" + MIME_PARAMETER_KEY = "[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}" + MIME_PARAMETER_VALUE = "#{Regexp.escape('"')}?[a-zA-Z0-9][a-zA-Z0-9#{Regexp.escape('!#$&-^_.+')}]{0,126}#{Regexp.escape('"')}?" + MIME_PARAMETER = "\s*\;\s+#{MIME_PARAMETER_KEY}(?:\=#{MIME_PARAMETER_VALUE})?" + MIME_REGEXP = /\A(?:\*\/\*|#{MIME_NAME}\/(?:\*|#{MIME_NAME})(?:\s*#{MIME_PARAMETER}\s*)*)\z/ + + class InvalidMimeType < StandardError; end + def initialize(string, symbol = nil, synonyms = []) + unless MIME_REGEXP.match?(string) + raise InvalidMimeType, "#{string.inspect} is not a valid MIME type" + end @symbol, @synonyms = symbol, synonyms @string = string @hash = [@string, @synonyms, @symbol].hash @@ -303,7 +315,7 @@ module Mime include Singleton def initialize - super "*/*", :all + super "*/*", nil end def all?; true; end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 8d7431fd6b..13d0963a33 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -111,13 +111,23 @@ module ActionDispatch begin strategy.call(raw_post) rescue # JSON or Ruby code block errors. - my_logger = logger || ActiveSupport::Logger.new($stderr) - my_logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{raw_post}" - + log_parse_error_once raise ParseError end end + def log_parse_error_once + @parse_error_logged ||= begin + parse_logger = logger || ActiveSupport::Logger.new($stderr) + parse_logger.debug <<~MSG.chomp + Error occurred while parsing request parameters. + Contents: + + #{raw_post} + MSG + end + end + def params_parsers ActionDispatch::Request.parameter_parsers end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7bc364d370..44f23940d3 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -383,9 +383,6 @@ module ActionDispatch end self.request_parameters = Request::Utils.normalize_encode_params(pr) end - rescue Http::Parameters::ParseError # one of the parse strategies blew up - self.request_parameters = Request::Utils.normalize_encode_params(super || {}) - raise rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}") end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 1d38942a31..69798f99e0 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -82,6 +82,7 @@ module ActionDispatch # :nodoc: SET_COOKIE = "Set-Cookie" LOCATION = "Location" NO_CONTENT_CODES = [100, 101, 102, 204, 205, 304] + CONTENT_TYPE_PARSER = /\A(?<type>[^;\s]+)?(?:.*;\s*charset=(?<quote>"?)(?<charset>[^;\s]+)\k<quote>)?/ # :nodoc: cattr_accessor :default_charset, default: "utf-8" cattr_accessor :default_headers @@ -409,10 +410,8 @@ module ActionDispatch # :nodoc: NullContentTypeHeader = ContentTypeHeader.new nil, nil def parse_content_type(content_type) - if content_type - type, charset = content_type.split(/;\s*charset=/) - type = nil if type && type.empty? - ContentTypeHeader.new(type, charset) + if content_type && match = CONTENT_TYPE_PARSER.match(content_type) + ContentTypeHeader.new(match[:type], match[:charset]) else NullContentTypeHeader end diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 827f022ca2..0da8f5c14e 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -20,7 +20,6 @@ module ActionDispatch # A +Tempfile+ object with the actual uploaded file. Note that some of # its interface is available directly. attr_accessor :tempfile - alias :to_io :tempfile # A string with the headers of the multipart request. attr_accessor :headers @@ -84,6 +83,10 @@ module ActionDispatch def eof? @tempfile.eof? end + + def to_io + @tempfile.to_io + end end end end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3af4c176a7..8227749986 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -79,108 +79,108 @@ module ActionDispatch private - def add_params(path, params) - params = { params: params } unless params.is_a?(Hash) - params.reject! { |_, v| v.to_param.nil? } - query = params.to_query - path << "?#{query}" unless query.empty? - end - - def add_anchor(path, anchor) - if anchor - path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + def add_params(path, params) + params = { params: params } unless params.is_a?(Hash) + params.reject! { |_, v| v.to_param.nil? } + query = params.to_query + path << "?#{query}" unless query.empty? end - end - def extract_domain_from(host, tld_length) - host.split(".").last(1 + tld_length).join(".") - end + def add_anchor(path, anchor) + if anchor + path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + end + end - def extract_subdomains_from(host, tld_length) - parts = host.split(".") - parts[0..-(tld_length + 2)] - end + def extract_domain_from(host, tld_length) + host.split(".").last(1 + tld_length).join(".") + end - def add_trailing_slash(path) - if path.include?("?") - path.sub!(/\?/, '/\&') - elsif !path.include?(".") - path.sub!(/[^\/]\z|\A\z/, '\&/') + def extract_subdomains_from(host, tld_length) + parts = host.split(".") + parts[0..-(tld_length + 2)] end - end - def build_host_url(host, port, protocol, options, path) - if match = host.match(HOST_REGEXP) - protocol ||= match[1] unless protocol == false - host = match[2] - port = match[3] unless options.key? :port + def add_trailing_slash(path) + if path.include?("?") + path.sub!(/\?/, '/\&') + elsif !path.include?(".") + path.sub!(/[^\/]\z|\A\z/, '\&/') + end end - protocol = normalize_protocol protocol - host = normalize_host(host, options) + def build_host_url(host, port, protocol, options, path) + if match = host.match(HOST_REGEXP) + protocol ||= match[1] unless protocol == false + host = match[2] + port = match[3] unless options.key? :port + end - result = protocol.dup + protocol = normalize_protocol protocol + host = normalize_host(host, options) - if options[:user] && options[:password] - result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" - end + result = protocol.dup - result << host - normalize_port(port, protocol) { |normalized_port| - result << ":#{normalized_port}" - } + if options[:user] && options[:password] + result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" + end - result.concat path - end + result << host + normalize_port(port, protocol) { |normalized_port| + result << ":#{normalized_port}" + } - def named_host?(host) - IP_HOST_REGEXP !~ host - end + result.concat path + end - def normalize_protocol(protocol) - case protocol - when nil - "http://" - when false, "//" - "//" - when PROTOCOL_REGEXP - "#{$1}://" - else - raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + def named_host?(host) + IP_HOST_REGEXP !~ host end - end - def normalize_host(_host, options) - return _host unless named_host?(_host) + def normalize_protocol(protocol) + case protocol + when nil + "http://" + when false, "//" + "//" + when PROTOCOL_REGEXP + "#{$1}://" + else + raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + end + end - tld_length = options[:tld_length] || @@tld_length - subdomain = options.fetch :subdomain, true - domain = options[:domain] + def normalize_host(_host, options) + return _host unless named_host?(_host) - host = +"" - if subdomain == true - return _host if domain.nil? + tld_length = options[:tld_length] || @@tld_length + subdomain = options.fetch :subdomain, true + domain = options[:domain] - host << extract_subdomains_from(_host, tld_length).join(".") - elsif subdomain - host << subdomain.to_param + host = +"" + if subdomain == true + return _host if domain.nil? + + host << extract_subdomains_from(_host, tld_length).join(".") + elsif subdomain + host << subdomain.to_param + end + host << "." unless host.empty? + host << (domain || extract_domain_from(_host, tld_length)) + host end - host << "." unless host.empty? - host << (domain || extract_domain_from(_host, tld_length)) - host - end - def normalize_port(port, protocol) - return unless port + def normalize_port(port, protocol) + return unless port - case protocol - when "//" then yield port - when "https://" - yield port unless port.to_i == 443 - else - yield port unless port.to_i == 80 + case protocol + when "//" then yield port + when "https://" + yield port unless port.to_i == 443 + else + yield port unless port.to_i == 80 + end end - end end def initialize diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 32f632800c..086d6a3e07 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -65,12 +65,12 @@ module ActionDispatch def literal?; false; end end - %w{ Symbol Slash Dot }.each do |t| - class_eval <<-eoruby, __FILE__, __LINE__ + 1 - class #{t} < Terminal; - def type; :#{t.upcase}; end - end - eoruby + class Slash < Terminal # :nodoc: + def type; :SLASH; end + end + + class Dot < Terminal # :nodoc: + def type; :DOT; end end class Symbol < Terminal # :nodoc: @@ -89,6 +89,7 @@ module ActionDispatch regexp == DEFAULT_EXP end + def type; :SYMBOL; end def symbol?; true; end end diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 537f479ee5..697f5b9d8b 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -119,7 +119,7 @@ module ActionDispatch class UnanchoredRegexp < AnchoredRegexp # :nodoc: def accept(node) - %r{\A#{visit node}} + %r{\A#{visit node}(?:\b|\Z)} end end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 30af3ff930..89a164f968 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -15,9 +15,6 @@ require "action_dispatch/journey/path/pattern" module ActionDispatch module Journey # :nodoc: class Router # :nodoc: - class RoutingError < ::StandardError # :nodoc: - end - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index 5b2ad36dd5..87fe19225b 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -24,10 +24,8 @@ module ActionDispatch def call(env) error = nil result = run_callbacks :call do - begin - @app.call(env) - rescue => error - end + @app.call(env) + rescue => error end raise error if error result diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 26d3fd936f..b69bcab05c 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -61,10 +61,6 @@ module ActionDispatch get_header Cookies::SIGNED_COOKIE_DIGEST end - def secret_token - get_header Cookies::SECRET_TOKEN - end - def secret_key_base get_header Cookies::SECRET_KEY_BASE end @@ -181,7 +177,6 @@ module ActionDispatch USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption" ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher" SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest" - SECRET_TOKEN = "action_dispatch.secret_token" SECRET_KEY_BASE = "action_dispatch.secret_key_base" COOKIES_SERIALIZER = "action_dispatch.cookies_serializer" COOKIES_DIGEST = "action_dispatch.cookies_digest" @@ -215,9 +210,6 @@ module ActionDispatch # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed # cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # - # If +secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, - # legacy cookies signed with the old key generator will be transparently upgraded. - # # This jar requires that you set a suitable secret for the verification on your app's +secret_key_base+. # # Example: @@ -233,9 +225,6 @@ module ActionDispatch # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read. # If the cookie was tampered with by the user (or a 3rd party), +nil+ will be returned. # - # If +secret_key_base+ and +secrets.secret_token+ (deprecated) are both set, - # legacy cookies signed with the old key generator will be transparently upgraded. - # # If +config.action_dispatch.encrypted_cookie_salt+ and +config.action_dispatch.encrypted_signed_cookie_salt+ # are both set, legacy cookies encrypted with HMAC AES-256-CBC will be transparently upgraded. # @@ -264,10 +253,6 @@ module ActionDispatch private - def upgrade_legacy_signed_cookies? - request.secret_token.present? && request.secret_key_base.present? - end - def upgrade_legacy_hmac_aes_cbc_cookies? request.secret_key_base.present? && request.encrypted_signed_cookie_salt.present? && @@ -353,7 +338,7 @@ module ActionDispatch def update_cookies_from_jar request_jar = @request.cookie_jar.instance_variable_get(:@cookies) - set_cookies = request_jar.reject { |k, _| @delete_cookies.key?(k) } + set_cookies = request_jar.reject { |k, _| @delete_cookies.key?(k) || @set_cookies.key?(k) } @cookies.update set_cookies if set_cookies end @@ -503,13 +488,8 @@ module ActionDispatch end def cookie_metadata(name, options) - if request.use_cookies_with_metadata - metadata = expiry_options(options) - metadata[:purpose] = "cookie.#{name}" - - metadata - else - {} + expiry_options(options).tap do |metadata| + metadata[:purpose] = "cookie.#{name}" if request.use_cookies_with_metadata end end @@ -592,10 +572,6 @@ module ActionDispatch request.cookies_rotations.signed.each do |*secrets, **options| @verifier.rotate(*secrets, serializer: SERIALIZER, **options) end - - if upgrade_legacy_signed_cookies? - @verifier.rotate request.secret_token, serializer: SERIALIZER - end end private @@ -640,10 +616,6 @@ module ActionDispatch @encryptor.rotate(secret, sign_secret, cipher: legacy_cipher, digest: digest, serializer: SERIALIZER) end - - if upgrade_legacy_signed_cookies? - @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, digest: digest, serializer: SERIALIZER) - end end private @@ -652,7 +624,7 @@ module ActionDispatch @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate, purpose: purpose) end rescue ActiveSupport::MessageEncryptor::InvalidMessage, ActiveSupport::MessageVerifier::InvalidSignature - parse_legacy_signed_message(name, encrypted_message) + nil end def commit(name, options) @@ -660,16 +632,6 @@ module ActionDispatch raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end - - def parse_legacy_signed_message(name, legacy_signed_message) - if defined?(@legacy_verifier) - deserialize(name) do |rotate| - rotate.call - - @legacy_verifier.verified(legacy_signed_message) - end - end - end end def initialize(app) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 5f5fdbc66a..bb49bc4dda 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -3,53 +3,14 @@ require "action_dispatch/http/request" require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" + require "action_view" require "action_view/base" -require "pp" - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. class DebugExceptions - RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) - - class DebugView < ActionView::Base - def debug_params(params) - clean_params = params.clone - clean_params.delete("action") - clean_params.delete("controller") - - if clean_params.empty? - "None" - else - PP.pp(clean_params, +"", 200) - end - end - - def debug_headers(headers) - if headers.present? - headers.inspect.gsub(",", ",\n") - else - "None" - end - end - - def debug_hash(object) - object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") - end - - def render(*) - logger = ActionView::Base.logger - - if logger && logger.respond_to?(:silence) - logger.silence { super } - else - super - end - end - end - cattr_reader :interceptors, instance_accessor: false, default: [] def self.register_interceptor(object = nil, &block) @@ -87,11 +48,9 @@ module ActionDispatch wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) @interceptors.each do |interceptor| - begin - interceptor.call(request, exception) - rescue Exception - log_error(request, wrapper) - end + interceptor.call(request, exception) + rescue Exception + log_error(request, wrapper) end end @@ -101,7 +60,11 @@ module ActionDispatch log_error(request, wrapper) if request.get_header("action_dispatch.show_detailed_exceptions") - content_type = request.formats.first + begin + content_type = request.formats.first + rescue Mime::Type::InvalidMimeType + render_for_api_request(Mime[:text], wrapper) + end if api_request?(content_type) render_for_api_request(content_type, wrapper) @@ -152,7 +115,7 @@ module ActionDispatch end def create_template(request, wrapper) - DebugView.new([RESCUES_TEMPLATE_PATH], + DebugView.new( request: request, exception_wrapper: wrapper, exception: wrapper.exception, @@ -180,11 +143,14 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - logger.fatal " " - logger.fatal "#{exception.class} (#{exception.message}):" - log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) - logger.fatal " " - log_array logger, trace + message = [] + message << " " + message << "#{exception.class} (#{exception.message}):" + message.concat(exception.annoted_source_code) if exception.respond_to?(:annoted_source_code) + message << " " + message.concat(trace) + + log_array(logger, message) end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb new file mode 100644 index 0000000000..43c0a84504 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require "pp" + +require "action_view" +require "action_view/base" + +module ActionDispatch + class DebugView < ActionView::Base # :nodoc: + RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) + + def initialize(assigns) + paths = [RESCUES_TEMPLATE_PATH] + lookup_context = ActionView::LookupContext.new(paths) + super(lookup_context, assigns) + end + + def compiled_method_container + self.class + end + + def debug_params(params) + clean_params = params.clone + clean_params.delete("action") + clean_params.delete("controller") + + if clean_params.empty? + "None" + else + PP.pp(clean_params, +"", 200) + end + end + + def debug_headers(headers) + if headers.present? + headers.inspect.gsub(",", ",\n") + else + "None" + end + end + + def debug_hash(object) + object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") + end + + def render(*) + logger = ActionView::Base.logger + + if logger && logger.respond_to?(:silence) + logger.silence { super } + else + super + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index fb2b2bd3b0..0cc56f5013 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -12,6 +12,7 @@ module ActionDispatch "ActionController::UnknownHttpMethod" => :method_not_allowed, "ActionController::NotImplemented" => :not_implemented, "ActionController::UnknownFormat" => :not_acceptable, + "Mime::Type::InvalidMimeType" => :not_acceptable, "ActionController::MissingExactTemplate" => :not_acceptable, "ActionController::InvalidAuthenticityToken" => :unprocessable_entity, "ActionController::InvalidCrossOriginRequest" => :unprocessable_entity, @@ -31,22 +32,34 @@ module ActionDispatch "ActionController::MissingExactTemplate" => "missing_exact_template", ) + cattr_accessor :wrapper_exceptions, default: [ + "ActionView::Template::Error" + ] + attr_reader :backtrace_cleaner, :exception, :wrapped_causes, :line_number, :file def initialize(backtrace_cleaner, exception) @backtrace_cleaner = backtrace_cleaner - @exception = original_exception(exception) + @exception = exception @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) + exception.cause + else + exception + end + end + def rescue_template @@rescue_templates[@exception.class.name] end def status_code - self.class.status_code_for_exception(@exception.class.name) + self.class.status_code_for_exception(unwrapped_exception.class.name) end def application_trace @@ -122,14 +135,6 @@ module ActionDispatch Array(@exception.backtrace) end - def original_exception(exception) - if @@rescue_responses.has_key?(exception.cause.class.name) - exception.cause - else - exception - end - end - def causes_for(exception) return enum_for(__method__, exception) unless block_given? diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb new file mode 100644 index 0000000000..b7dff1df41 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +require "action_dispatch/http/request" + +module ActionDispatch + # This middleware guards from DNS rebinding attacks by explicitly permitting + # the hosts a request can be sent to. + # + # When a request comes to an unauthorized host, the +response_app+ + # application will be executed and rendered. If no +response_app+ is given, a + # default one will run, which responds with +403 Forbidden+. + class HostAuthorization + class Permissions # :nodoc: + def initialize(hosts) + @hosts = sanitize_hosts(hosts) + end + + def empty? + @hosts.empty? + end + + def allows?(host) + @hosts.any? do |allowed| + allowed === host + rescue + # IPAddr#=== raises an error if you give it a hostname instead of + # IP. Treat similar errors as blocked access. + false + end + end + + private + + def sanitize_hosts(hosts) + Array(hosts).map do |host| + case host + when Regexp then sanitize_regexp(host) + when String then sanitize_string(host) + else host + end + end + end + + def sanitize_regexp(host) + /\A#{host}\z/ + end + + def sanitize_string(host) + if host.start_with?(".") + /\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/ + else + host + end + end + end + + DEFAULT_RESPONSE_APP = -> env do + request = Request.new(env) + + format = request.xhr? ? "text/plain" : "text/html" + template = DebugView.new(host: request.host) + body = template.render(template: "rescues/blocked_host", layout: "rescues/layout") + + [403, { + "Content-Type" => "#{format}; charset=#{Response.default_charset}", + "Content-Length" => body.bytesize.to_s, + }, [body]] + end + + def initialize(app, hosts, response_app = nil) + @app = app + @permissions = Permissions.new(hosts) + @response_app = response_app || DEFAULT_RESPONSE_APP + end + + def call(env) + return @app.call(env) if @permissions.empty? + + request = Request.new(env) + + if authorized?(request) + mark_as_authorized(request) + @app.call(env) + else + @response_app.call(env) + end + end + + private + + def authorized?(request) + origin_host = request.get_header("HTTP_HOST").to_s.sub(/:\d+\z/, "") + forwarded_host = request.x_forwarded_host.to_s.split(/,\s?/).last.to_s.sub(/:\d+\z/, "") + + @permissions.allows?(origin_host) && + (forwarded_host.blank? || @permissions.allows?(forwarded_host)) + end + + def mark_as_authorized(request) + request.set_header("action_dispatch.authorized_host", request.host) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb index 3feb3a19f3..a88ad40f21 100644 --- a/actionpack/lib/action_dispatch/middleware/public_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/public_exceptions.rb @@ -21,8 +21,12 @@ module ActionDispatch def call(env) request = ActionDispatch::Request.new(env) status = request.path_info[1..-1].to_i - content_type = request.formats.first - body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } + begin + content_type = request.formats.first + rescue Mime::Type::InvalidMimeType + content_type = Mime[:text] + end + body = { status: status, error: Rack::Utils::HTTP_STATUS_CODES.fetch(status, Rack::Utils::HTTP_STATUS_CODES[500]) } render(status, content_type, body) end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 35158f9062..a5667573f4 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -162,14 +162,12 @@ module ActionDispatch # Split the comma-separated list into an array of strings. ips = header.strip.split(/[,\s]+/) ips.select do |ip| - begin - # Only return IPs that are valid according to the IPAddr#new method. - range = IPAddr.new(ip).to_range - # We want to make sure nobody is sneaking a netmask in. - range.begin == range.end - rescue ArgumentError - nil - end + # Only return IPs that are valid according to the IPAddr#new method. + range = IPAddr.new(ip).to_range + # We want to make sure nobody is sneaking a netmask in. + range.begin == range.end + rescue ArgumentError + nil end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index df680c1c5f..7c43c781c7 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -16,11 +16,6 @@ module ActionDispatch # The cookie jar used for storage is automatically configured to be the # best possible option given your application's configuration. # - # If you only have secret_token set, your cookies will be signed, but - # not encrypted. This means a user cannot alter their +user_id+ without - # knowing your app's secret key, but can easily read their +user_id+. This - # was the default for Rails 3 apps. - # # Your cookies will be encrypted using your apps secret_key_base. This # goes a step further than signed cookies in that encrypted cookies cannot # be altered or read by users. This is the default starting in Rails 4. @@ -29,9 +24,10 @@ module ActionDispatch # # Rails.application.config.session_store :cookie_store, key: '_your_app_session' # - # By default, your secret key base is derived from your application name in - # the test and development environments. In all other environments, it is stored - # encrypted in the <tt>config/credentials.yml.enc</tt> file. + # In the development and test environments your application's secret key base is + # generated by Rails and stored in a temporary file in <tt>tmp/development_secret.txt</tt>. + # In all other environments, it is stored encrypted in the + # <tt>config/credentials.yml.enc</tt> file. # # If your application was not updated to Rails 5.2 defaults, the secret_key_base # will be found in the old <tt>config/secrets.yml</tt> file. diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 3c88afd4d3..767143a368 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -45,7 +45,7 @@ module ActionDispatch backtrace_cleaner = request.get_header "action_dispatch.backtrace_cleaner" wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) status = wrapper.status_code - request.set_header "action_dispatch.exception", wrapper.exception + request.set_header "action_dispatch.exception", wrapper.unwrapped_exception request.set_header "action_dispatch.original_path", request.path_info request.path_info = "/#{status}" response = @exceptions_app.call(request.env) diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index b82f8aa3a3..f0c869fba0 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -34,7 +34,28 @@ module ActionDispatch end def build(app) - klass.new(app, *args, &block) + InstrumentationProxy.new(klass.new(app, *args, &block), inspect) + end + end + + # This class is used to instrument the execution of a single middleware. + # It proxies the `call` method transparently and instruments the method + # call. + class InstrumentationProxy + EVENT_NAME = "process_middleware.action_dispatch" + + def initialize(middleware, class_name) + @middleware = middleware + + @payload = { + middleware: class_name, + } + end + + def call(env) + ActiveSupport::Notifications.instrument(EVENT_NAME, @payload) do + @middleware.call(env) + end end end @@ -97,8 +118,8 @@ module ActionDispatch middlewares.push(build_middleware(klass, args, block)) end - def build(app = Proc.new) - middlewares.freeze.reverse.inject(app) { |a, e| e.build(a) } + def build(app = nil, &block) + middlewares.freeze.reverse.inject(app || block) { |a, e| e.build(a) } end private diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb new file mode 100644 index 0000000000..1fbc107e28 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb @@ -0,0 +1,7 @@ +<header> + <h1>Blocked host: <%= @host %></h1> +</header> +<div id="container"> + <h2>To allow requests to <%= @host %>, add the following to your environment configuration:</h2> + <pre>config.hosts << "<%= @host %>"</pre> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb new file mode 100644 index 0000000000..a94dd982a7 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb @@ -0,0 +1,5 @@ +Blocked host: <%= @host %> + +To allow requests to <%= @host %>, add the following to your environment configuration: + + config.hosts << "<%= @host %>" diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb index e8454acfad..77cfdd20c8 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb @@ -10,9 +10,12 @@ <div id="container"> <h2> <%= h @exception.message %> - <% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> + <% if defined?(ActiveStorage) && @exception.message.match?(%r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}}) %> <br />To resolve this issue run: rails active_storage:install <% end %> + <% if defined?(ActionMailbox) && @exception.message.match?(%r{#{ActionMailbox::InboundEmail.table_name}}) %> + <br />To resolve this issue run: rails action_mailbox:install + <% end %> </h2> <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb index e5e3196710..16c3ecc331 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb @@ -4,8 +4,10 @@ <% end %> <%= @exception.message %> -<% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> +<% if defined?(ActiveStorage) && @exception.message.match?(%r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}}) %> To resolve this issue run: rails active_storage:install +<% if defined?(ActionMailbox) && @exception.message.match?(%r{#{ActionMailbox::InboundEmail.table_name}}) %> +To resolve this issue run: rails action_mailbox:install <% end %> <%= render template: "rescues/_source" %> diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 5cde677051..d78b1c4f71 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -74,8 +74,8 @@ module ActionDispatch # For routes that don't fit the <tt>resources</tt> mold, you can use the HTTP helper # methods <tt>get</tt>, <tt>post</tt>, <tt>patch</tt>, <tt>put</tt> and <tt>delete</tt>. # - # get 'post/:id' => 'posts#show' - # post 'post/:id' => 'posts#create_comment' + # get 'post/:id', to: 'posts#show' + # post 'post/:id', to: 'posts#create_comment' # # Now, if you POST to <tt>/posts/:id</tt>, it will route to the <tt>create_comment</tt> action. A GET on the same # URL will route to the <tt>show</tt> action. @@ -83,7 +83,7 @@ module ActionDispatch # If your route needs to respond to more than one HTTP method (or all methods) then using the # <tt>:via</tt> option on <tt>match</tt> is preferable. # - # match 'post/:id' => 'posts#show', via: [:get, :post] + # match 'post/:id', to: 'posts#show', via: [:get, :post] # # == Named routes # @@ -94,7 +94,7 @@ module ActionDispatch # Example: # # # In config/routes.rb - # get '/login' => 'accounts#login', as: 'login' + # get '/login', to: 'accounts#login', as: 'login' # # # With render, redirect_to, tests, etc. # redirect_to login_url @@ -120,9 +120,9 @@ module ActionDispatch # # # In config/routes.rb # controller :blog do - # get 'blog/show' => :list - # get 'blog/delete' => :delete - # get 'blog/edit' => :edit + # get 'blog/show', to: :list + # get 'blog/delete', to: :delete + # get 'blog/edit', to: :edit # end # # # provides named routes for show, delete, and edit @@ -132,7 +132,7 @@ module ActionDispatch # # Routes can generate pretty URLs. For example: # - # get '/articles/:year/:month/:day' => 'articles#find_by_id', constraints: { + # get '/articles/:year/:month/:day', to: 'articles#find_by_id', constraints: { # year: /\d{4}/, # month: /\d{1,2}/, # day: /\d{1,2}/ @@ -147,7 +147,7 @@ module ActionDispatch # You can specify a regular expression to define a format for a parameter. # # controller 'geocode' do - # get 'geocode/:postalcode' => :show, constraints: { + # get 'geocode/:postalcode', to: :show, constraints: { # postalcode: /\d{5}(-\d{4})?/ # } # end @@ -156,13 +156,13 @@ module ActionDispatch # expression modifiers: # # controller 'geocode' do - # get 'geocode/:postalcode' => :show, constraints: { + # get 'geocode/:postalcode', to: :show, constraints: { # postalcode: /hx\d\d\s\d[a-z]{2}/i # } # end # # controller 'geocode' do - # get 'geocode/:postalcode' => :show, constraints: { + # get 'geocode/:postalcode', to: :show, constraints: { # postalcode: /# Postalcode format # \d{5} #Prefix # (-\d{4})? #Suffix @@ -178,13 +178,13 @@ module ActionDispatch # # You can redirect any path to another path using the redirect helper in your router: # - # get "/stories" => redirect("/posts") + # get "/stories", to: redirect("/posts") # # == Unicode character routes # # You can specify unicode character routes in your router: # - # get "こんにちは" => "welcome#index" + # get "こんにちは", to: "welcome#index" # # == Routing to Rack Applications # @@ -192,7 +192,7 @@ module ActionDispatch # index action in the PostsController, you can specify any Rack application # as the endpoint for a matcher: # - # get "/application.js" => Sprockets + # get "/application.js", to: Sprockets # # == Reloading routes # @@ -210,8 +210,8 @@ module ActionDispatch # === +assert_routing+ # # def test_movie_route_properly_splits - # opts = {controller: "plugin", action: "checkout", id: "2"} - # assert_routing "plugin/checkout/2", opts + # opts = {controller: "plugin", action: "checkout", id: "2"} + # assert_routing "plugin/checkout/2", opts # end # # +assert_routing+ lets you test whether or not the route properly resolves into options. @@ -219,8 +219,8 @@ module ActionDispatch # === +assert_recognizes+ # # def test_route_has_options - # opts = {controller: "plugin", action: "show", id: "12"} - # assert_recognizes opts, "/plugins/show/12" + # opts = {controller: "plugin", action: "show", id: "12"} + # assert_recognizes opts, "/plugins/show/12" # end # # Note the subtle difference between the two: +assert_routing+ tests that diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 99f3b4c2cd..2d2073de9a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -160,17 +160,8 @@ module ActionDispatch end def make_route(name, precedence) - route = Journey::Route.new(name, - application, - path, - conditions, - required_defaults, - defaults, - request_method, - precedence, - @internal) - - route + Journey::Route.new(name, application, path, conditions, required_defaults, + defaults, request_method, precedence, @internal) end def application @@ -656,7 +647,7 @@ module ActionDispatch # Query if the following named route was already defined. def has_named_route?(name) - @set.named_routes.key? name + @set.named_routes.key?(name) end private @@ -1150,6 +1141,10 @@ module ActionDispatch attr_reader :controller, :path, :param def initialize(entities, api_only, shallow, options = {}) + if options[:param].to_s.include?(":") + raise ArgumentError, ":param option can't contain colons" + end + @name = entities.to_s @path = (options[:path] || @name).to_s @controller = (options[:controller] || @name).to_s @@ -1171,10 +1166,16 @@ module ActionDispatch end def actions + if @except + available_actions - Array(@except).map(&:to_sym) + else + available_actions + end + end + + def available_actions if @only Array(@only).map(&:to_sym) - elsif @except - default_actions - Array(@except).map(&:to_sym) else default_actions end @@ -1443,6 +1444,9 @@ module ActionDispatch # Allows you to specify the default value for optional +format+ # segment or disable it by supplying +false+. # + # [:param] + # Allows you to override the default param name of +:id+ in the URL. + # # === Examples # # # routes call <tt>Admin::PostsController</tt> @@ -1946,9 +1950,7 @@ module ActionDispatch end def match_root_route(options) - name = has_named_route?(name_for_action(:root, nil)) ? nil : :root - args = ["/", { as: name, via: :get }.merge!(options)] - + args = ["/", { as: :root, via: :get }.merge(options)] match(*args) end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2ae75b0da8..972953d4f3 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -90,11 +90,11 @@ module ActionDispatch def clear! @path_helpers.each do |helper| - @path_helpers_module.send :remove_method, helper + @path_helpers_module.remove_method helper end @url_helpers.each do |helper| - @url_helpers_module.send :remove_method, helper + @url_helpers_module.remove_method helper end @routes.clear @@ -108,8 +108,8 @@ module ActionDispatch url_name = :"#{name}_url" if routes.key? key - @path_helpers_module.send :undef_method, path_name - @url_helpers_module.send :undef_method, url_name + @path_helpers_module.undef_method path_name + @url_helpers_module.undef_method url_name end routes[key] = route define_url_helper @path_helpers_module, route, path_name, route.defaults, name, PATH @@ -820,6 +820,10 @@ module ActionDispatch path, params = generate(route_name, path_options, recall) + if options.key? :params + params.merge! options[:params] + end + options[:path] = path options[:script_name] = script_name options[:params] = params diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 1a31c7dbb8..fcb8ae296b 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -133,6 +133,7 @@ module ActionDispatch # <tt>ActionDispatch::Http::URL.tld_length</tt>, which in turn defaults to 1. # * <tt>:port</tt> - Optionally specify the port to connect to. # * <tt>:anchor</tt> - An anchor name to be appended to the path. + # * <tt>:params</tt> - The query parameters to be appended to the path. # * <tt>:trailing_slash</tt> - If true, adds a trailing slash, as in "/archive/2009/" # * <tt>:script_name</tt> - Specifies application path relative to domain root. If provided, prepends application path. # diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index c74c0ccced..066daa4a12 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -89,6 +89,24 @@ module ActionDispatch # { js_errors: true } # end # + # Some drivers require browser capabilities to be passed as a block instead + # of through the +options+ hash. + # + # As an example, if you want to add mobile emulation on chrome, you'll have to + # create an instance of selenium's +Chrome::Options+ object and add + # capabilities with a block. + # + # The block will be passed an instance of <tt><Driver>::Options</tt> where you can + # define the capabilities you want. Please refer to your driver documentation + # to learn about supported options. + # + # class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + # driven_by :selenium, using: :chrome, screen_size: [1024, 768] do |driver_option| + # driver_option.add_emulation(device_name: 'iPhone 6') + # driver_option.add_extension('path/to/chrome_extension.crx') + # end + # end + # # Because <tt>ActionDispatch::SystemTestCase</tt> is a shim between Capybara # and Rails, any driver that is supported by Capybara is supported by system # tests as long as you include the required gems and files. @@ -134,8 +152,10 @@ module ActionDispatch # driven_by :selenium, using: :firefox # # driven_by :selenium, using: :headless_firefox - def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}) - self.driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size, options: options) + def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400], options: {}, &capabilities) + driver_options = { using: using, screen_size: screen_size, options: options } + + self.driver = SystemTesting::Driver.new(driver, driver_options, &capabilities) end driven_by :selenium diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb index 1b0bce6b9e..c34907b6cb 100644 --- a/actionpack/lib/action_dispatch/system_testing/browser.rb +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -29,20 +29,28 @@ module ActionDispatch end end + def capabilities + @option ||= + case type + when :chrome + ::Selenium::WebDriver::Chrome::Options.new + when :firefox + ::Selenium::WebDriver::Firefox::Options.new + end + end + private def headless_chrome_browser_options - options = Selenium::WebDriver::Chrome::Options.new - options.args << "--headless" - options.args << "--disable-gpu" if Gem.win_platform? + capabilities.args << "--headless" + capabilities.args << "--disable-gpu" if Gem.win_platform? - options + capabilities end def headless_firefox_browser_options - options = Selenium::WebDriver::Firefox::Options.new - options.args << "-headless" + capabilities.args << "-headless" - options + capabilities end end end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 5252ff6746..25a09dd918 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -3,11 +3,12 @@ module ActionDispatch module SystemTesting class Driver # :nodoc: - def initialize(name, **options) + def initialize(name, **options, &capabilities) @name = name @browser = Browser.new(options[:using]) @screen_size = options[:screen_size] @options = options[:options] + @capabilities = capabilities end def use @@ -22,6 +23,8 @@ module ActionDispatch end def register + define_browser_capabilities(@browser.capabilities) + Capybara.register_driver @name do |app| case @name when :selenium then register_selenium(app) @@ -31,6 +34,10 @@ module ActionDispatch end end + def define_browser_capabilities(capabilities) + @capabilities.call(capabilities) if @capabilities + end + def browser_options @options.merge(options: @browser.options).compact end diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb index 884fb51d18..79359a0c8b 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -20,7 +20,7 @@ module ActionDispatch # * [+inline+] Display the screenshot in the terminal using the # iTerm image protocol (https://iterm2.com/documentation-images.html). # * [+artifact+] Display the screenshot in the terminal, using the terminal - # artifact format (https://buildkite.github.io/terminal/inline-images/). + # artifact format (https://buildkite.github.io/terminal-to-html/inline-images/). def take_screenshot save_image puts display_image 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 e47d5020f4..600e9c733b 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 @@ -17,8 +17,11 @@ module ActionDispatch end def after_teardown - take_failed_screenshot - Capybara.reset_sessions! + begin + take_failed_screenshot + ensure + Capybara.reset_sessions! + end ensure super end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index af41521c5c..28cde6704e 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -160,9 +160,16 @@ module ActionDispatch @controller.singleton_class.include(_routes.url_helpers) if @controller.respond_to? :view_context_class - @controller.view_context_class = Class.new(@controller.view_context_class) do + view_context_class = Class.new(@controller.view_context_class) do include _routes.url_helpers end + + custom_view_context = Module.new { + define_method(:view_context_class) do + view_context_class + end + } + @controller.extend(custom_view_context) end end yield @routes diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 45439a3bb1..bb8b43ad4d 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -194,7 +194,7 @@ module ActionDispatch # Adds request headers characteristic of XMLHttpRequest e.g. HTTP_X_REQUESTED_WITH. # The headers will be merged into the Rack env hash. # - +as+: Used for encoding the request with different content type. - # Supports `:json` by default and will set the approriate request headers. + # Supports `:json` by default and will set the appropriate request headers. # The headers will be merged into the Rack env hash. # # This method is rarely used directly. Use +#get+, +#post+, or other standard @@ -335,7 +335,7 @@ module ActionDispatch klass = APP_SESSIONS[app] ||= Class.new(Integration::Session) { # If the app is a Rails app, make url_helpers available on the session. # This makes app.url_for and app.foo_path available in the console. - if app.respond_to?(:routes) + if app.respond_to?(:routes) && app.routes.is_a?(ActionDispatch::Routing::RouteSet) include app.routes.url_helpers include app.routes.mounted_helpers end diff --git a/actionpack/lib/action_dispatch/testing/request_encoder.rb b/actionpack/lib/action_dispatch/testing/request_encoder.rb index 9889f61951..6c65bec62f 100644 --- a/actionpack/lib/action_dispatch/testing/request_encoder.rb +++ b/actionpack/lib/action_dispatch/testing/request_encoder.rb @@ -38,8 +38,8 @@ module ActionDispatch end def self.parser(content_type) - mime = Mime::Type.lookup(content_type) - encoder(mime ? mime.ref : nil).response_parser + type = Mime::Type.lookup(content_type).ref if content_type + encoder(type).response_parser end def self.encoder(name) diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 1e6b21f235..6f7c86fdcf 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -14,40 +14,12 @@ module ActionDispatch new response.status, response.headers, response.body end - def initialize(*) # :nodoc: - super - @response_parser = RequestEncoder.parser(content_type) - end - - # Was the response successful? - def success? - ActiveSupport::Deprecation.warn(<<-MSG.squish) - The success? predicate is deprecated and will be removed in Rails 6.0. - Please use successful? as provided by Rack::Response::Helpers. - MSG - successful? - end - - # Was the URL not found? - def missing? - ActiveSupport::Deprecation.warn(<<-MSG.squish) - The missing? predicate is deprecated and will be removed in Rails 6.0. - Please use not_found? as provided by Rack::Response::Helpers. - MSG - not_found? - end - - # Was there a server-side error? - def error? - ActiveSupport::Deprecation.warn(<<-MSG.squish) - The error? predicate is deprecated and will be removed in Rails 6.0. - Please use server_error? as provided by Rack::Response::Helpers. - MSG - server_error? + def parsed_body + @parsed_body ||= response_parser.call(body) end - def parsed_body - @parsed_body ||= @response_parser.call(body) + def response_parser + @response_parser ||= RequestEncoder.parser(content_type) end end end diff --git a/actionpack/lib/action_pack.rb b/actionpack/lib/action_pack.rb index 3f69109633..36ee77c693 100644 --- a/actionpack/lib/action_pack.rb +++ b/actionpack/lib/action_pack.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true #-- -# Copyright (c) 2004-2018 David Heinemeier Hansson +# Copyright (c) 2004-2019 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 37969fcb57..3bbb1734d9 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -10,7 +10,7 @@ module ActionPack MAJOR = 6 MINOR = 0 TINY = 0 - PRE = "alpha" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actionpack/test/abstract/collector_test.rb b/actionpack/test/abstract/collector_test.rb index a4770b66e1..6db045fcd7 100644 --- a/actionpack/test/abstract/collector_test.rb +++ b/actionpack/test/abstract/collector_test.rb @@ -30,7 +30,7 @@ module AbstractController end test "register mime types on method missing" do - AbstractController::Collector.send(:remove_method, :js) + AbstractController::Collector.remove_method :js begin collector = MyCollector.new assert_not_respond_to collector, :js diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 65dd28b3d7..f23151e518 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -13,13 +13,6 @@ silence_warnings do Encoding.default_external = Encoding::UTF_8 end -require "drb" -begin - require "drb/unix" -rescue LoadError - puts "'drb/unix' is not available" -end - if ENV["TRAVIS"] PROCESS_COUNT = 0 else @@ -80,7 +73,7 @@ end module ActiveSupport class TestCase if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0 - parallelize_me! + parallelize(workers: PROCESS_COUNT) end end end @@ -359,75 +352,6 @@ class ImagesController < ResourcesController; end require "active_support/testing/method_call_assertions" -class ForkingExecutor - class Server - include DRb::DRbUndumped - - def initialize - @queue = Queue.new - end - - def record(reporter, result) - reporter.record result - end - - def <<(o) - o[2] = DRbObject.new(o[2]) if o - @queue << o - end - def pop; @queue.pop; end - end - - def initialize(size) - @size = size - @queue = Server.new - @pool = nil - @url = DRb.start_service("drbunix:", @queue).uri - end - - def <<(work); @queue << work; end - - def shutdown - pool = @size.times.map { - fork { - DRb.stop_service - queue = DRbObject.new_with_uri @url - while job = queue.pop - klass = job[0] - method = job[1] - reporter = job[2] - result = Minitest.run_one_method klass, method - if result.error? - translate_exceptions result - end - queue.record reporter, result - end - } - } - @size.times { @queue << nil } - pool.each { |pid| Process.waitpid pid } - end - - private - def translate_exceptions(result) - result.failures.map! { |e| - begin - Marshal.dump e - e - rescue TypeError - ex = Exception.new e.message - ex.set_backtrace e.backtrace - Minitest::UnexpectedError.new ex - end - } - end -end - -if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0 - # Use N processes (N defaults to 4) - Minitest.parallel_executor = ForkingExecutor.new(PROCESS_COUNT) -end - class ActiveSupport::TestCase include ActiveSupport::Testing::MethodCallAssertions diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 763df3a776..ecb8c37e6b 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -276,16 +276,14 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_assert_redirect_failure_message_with_protocol_relative_url - begin - process :redirect_external_protocol_relative - assert_redirected_to "/foo" - rescue ActiveSupport::TestCase::Assertion => ex - assert_no_match( - /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, - ex.message, - "protocol relative url was incorrectly normalized" - ) - end + process :redirect_external_protocol_relative + assert_redirected_to "/foo" + rescue ActiveSupport::TestCase::Assertion => ex + assert_no_match( + /#{request.protocol}#{request.host}\/\/www.rubyonrails.org/, + ex.message, + "protocol relative url was incorrectly normalized" + ) end def test_template_objects_exist diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 558e710df9..d8cea10153 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -193,7 +193,7 @@ class UrlOptionsTest < ActionController::TestCase action: "home", controller: "pages", only_path: true, - token: "secret" + params: { "token" => "secret" } } assert_equal "/home?token=secret", rs.url_for(options) diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 6fe036dd15..f09e812147 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -60,14 +60,6 @@ class FragmentCachingTest < ActionController::TestCase @m2v2 = ModelWithKeyAndVersion.new("model/2", "2") end - def test_fragment_cache_key - assert_deprecated do - assert_equal "views/what a key", @controller.fragment_cache_key("what a key") - assert_equal "views/test.host/fragment_caching_test/some_action", - @controller.fragment_cache_key(controller: "fragment_caching_test", action: "some_action") - end - end - def test_combined_fragment_cache_key assert_equal [ :views, "what a key" ], @controller.combined_fragment_cache_key("what a key") assert_equal [ :views, "test.host/fragment_caching_test/some_action" ], @@ -220,7 +212,7 @@ CACHED assert_equal expected_body, @response.body assert_equal "This bit's fragment cached", - @store.read("views/functional_caching/fragment_cached:#{template_digest("functional_caching/fragment_cached")}/fragment") + @store.read("views/functional_caching/fragment_cached:#{template_digest("functional_caching/fragment_cached", "html")}/fragment") end def test_fragment_caching_in_partials @@ -229,7 +221,7 @@ CACHED assert_match(/Old fragment caching in a partial/, @response.body) assert_match("Old fragment caching in a partial", - @store.read("views/functional_caching/_partial:#{template_digest("functional_caching/_partial")}/test.host/functional_caching/html_fragment_cached_with_partial")) + @store.read("views/functional_caching/_partial:#{template_digest("functional_caching/_partial", "html")}/test.host/functional_caching/html_fragment_cached_with_partial")) end def test_skipping_fragment_cache_digesting @@ -259,7 +251,7 @@ CACHED assert_match(/Some inline content/, @response.body) assert_match(/Some cached content/, @response.body) assert_match("Some cached content", - @store.read("views/functional_caching/inline_fragment_cached:#{template_digest("functional_caching/inline_fragment_cached")}/test.host/functional_caching/inline_fragment_cached")) + @store.read("views/functional_caching/inline_fragment_cached:#{template_digest("functional_caching/inline_fragment_cached", "html")}/test.host/functional_caching/inline_fragment_cached")) end def test_fragment_cache_instrumentation @@ -279,36 +271,39 @@ CACHED end def test_html_formatted_fragment_caching - get :formatted_fragment_cached, format: "html" + format = "html" + get :formatted_fragment_cached, format: format assert_response :success expected_body = "<body>\n<p>ERB</p>\n</body>\n" assert_equal expected_body, @response.body assert_equal "<p>ERB</p>", - @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment") + @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached", format)}/fragment") end def test_xml_formatted_fragment_caching - get :formatted_fragment_cached, format: "xml" + format = "xml" + get :formatted_fragment_cached, format: format assert_response :success expected_body = "<body>\n <p>Builder</p>\n</body>\n" assert_equal expected_body, @response.body assert_equal " <p>Builder</p>\n", - @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached")}/fragment") + @store.read("views/functional_caching/formatted_fragment_cached:#{template_digest("functional_caching/formatted_fragment_cached", format)}/fragment") end def test_fragment_caching_with_variant - get :formatted_fragment_cached_with_variant, format: "html", params: { v: :phone } + format = "html" + get :formatted_fragment_cached_with_variant, format: format, params: { v: :phone } assert_response :success expected_body = "<body>\n<p>PHONE</p>\n</body>\n" assert_equal expected_body, @response.body assert_equal "<p>PHONE</p>", - @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant")}/fragment") + @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant", format)}/fragment") end def test_fragment_caching_with_html_partials_in_xml @@ -317,8 +312,8 @@ CACHED end private - def template_digest(name) - ActionView::Digestor.digest(name: name, finder: @controller.lookup_context) + def template_digest(name, format) + ActionView::Digestor.digest(name: name, format: format, finder: @controller.lookup_context) end end diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 425a6e25cc..fcee812ee4 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -457,6 +457,7 @@ class FilterTest < ActionController::TestCase prepend_before_action :before_all prepend_after_action :after_all before_action :between_before_all_and_after_all + after_action :between_before_all_and_after_all def before_all @ran_filter ||= [] @@ -472,6 +473,7 @@ class FilterTest < ActionController::TestCase @ran_filter ||= [] @ran_filter << "between_before_all_and_after_all" end + def show render plain: "hello" end @@ -765,7 +767,7 @@ class FilterTest < ActionController::TestCase def test_running_prepended_before_and_after_action test_process(PrependingBeforeAndAfterController) - assert_equal %w( before_all between_before_all_and_after_all after_all ), @controller.instance_variable_get(:@ran_filter) + assert_equal %w( before_all between_before_all_and_after_all between_before_all_and_after_all after_all ), @controller.instance_variable_get(:@ran_filter) end def test_skipping_and_limiting_controller @@ -886,7 +888,7 @@ class ControllerWithSymbolAsFilter < PostsController yield # Do stuff... - wtf += 1 + wtf + 1 end end @@ -998,16 +1000,12 @@ class YieldingAroundFiltersTest < ActionController::TestCase def test_nested_actions controller = ControllerWithNestedFilters assert_nothing_raised do - begin - test_process(controller, "raises_both") - rescue Before, After - end + test_process(controller, "raises_both") + rescue Before, After end assert_raise Before do - begin - test_process(controller, "raises_both") - rescue After - end + test_process(controller, "raises_both") + rescue After end end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 409a4ec2e6..bf95c633e5 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -242,8 +242,11 @@ end class FlashIntegrationTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" - Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") - Rotations = ActiveSupport::Messages::RotationConfiguration.new + Generator = ActiveSupport::CachingKeyGenerator.new( + ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 1000) + ) + Rotations = ActiveSupport::Messages::RotationConfiguration.new + SIGNED_COOKIE_SALT = "signed cookie" class TestController < ActionController::Base add_flash_types :bar @@ -365,6 +368,7 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest args[0][:env] ||= {} args[0][:env]["action_dispatch.key_generator"] ||= Generator args[0][:env]["action_dispatch.cookies_rotations"] = Rotations + args[0][:env]["action_dispatch.signed_cookie_salt"] = SIGNED_COOKIE_SALT super(path, *args) end diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index b133afb343..dd4ff85d11 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -44,7 +44,10 @@ class HttpDigestAuthenticationTest < ActionController::TestCase setup do # Used as secret in generating nonce to prevent tampering of timestamp @secret = "4fb45da9e4ab4ddeb7580d6a35503d99" - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new(@secret) + @request.env["action_dispatch.key_generator"] = ActiveSupport::CachingKeyGenerator.new( + ActiveSupport::KeyGenerator.new(@secret) + ) + @request.env["action_dispatch.http_auth_salt"] = "http authentication" end teardown do diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 39ede1442a..b5503a9c64 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -152,7 +152,7 @@ class IntegrationTestTest < ActiveSupport::TestCase assert_equal "pass", @test.foo ensure # leave other tests as unaffected as possible - mixin.__send__(:remove_method, :method_missing) + mixin.remove_method :method_missing end end end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 00e1d5f3b3..2f8f191828 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -125,7 +125,7 @@ class RespondToController < ActionController::Base def custom_type_handling respond_to do |type| type.html { render body: "HTML" } - type.custom("application/crazy-xml") { render body: "Crazy XML" } + type.custom("application/fancy-xml") { render body: "Fancy XML" } type.all { render body: "Nothing" } end end @@ -158,6 +158,12 @@ class RespondToController < ActionController::Base end end + def handle_any_with_template + respond_to do |type| + type.any { render "test/hello_world" } + end + end + def all_types_with_layout respond_to do |type| type.html @@ -314,12 +320,14 @@ class RespondToControllerTest < ActionController::TestCase @request.host = "www.example.com" Mime::Type.register_alias("text/html", :iphone) Mime::Type.register("text/x-mobile", :mobile) + Mime::Type.register("application/fancy-xml", :fancy_xml) end def teardown super Mime::Type.unregister(:iphone) Mime::Type.unregister(:mobile) + Mime::Type.unregister(:fancy_xml) end def test_html @@ -489,10 +497,10 @@ class RespondToControllerTest < ActionController::TestCase end def test_custom_types - @request.accept = "application/crazy-xml" + @request.accept = "application/fancy-xml" get :custom_type_handling - assert_equal "application/crazy-xml", @response.content_type - assert_equal "Crazy XML", @response.body + assert_equal "application/fancy-xml", @response.content_type + assert_equal "Fancy XML", @response.body @request.accept = "text/html" get :custom_type_handling @@ -570,6 +578,13 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "HTML", @response.body end + def test_handle_any_with_template + @request.accept = "*/*" + + get :handle_any_with_template + assert_equal "Hello world!", @response.body + end + def test_html_type_with_layout @request.accept = "text/html" get :all_types_with_layout diff --git a/actionpack/test/controller/new_base/content_negotiation_test.rb b/actionpack/test/controller/new_base/content_negotiation_test.rb index 7205e90176..548fa4300d 100644 --- a/actionpack/test/controller/new_base/content_negotiation_test.rb +++ b/actionpack/test/controller/new_base/content_negotiation_test.rb @@ -20,9 +20,19 @@ module ContentNegotiation assert_body "Hello world */*!" end - test "Not all mimes are converted to symbol" do + test "A js or */* Accept header will return HTML" do + get "/content_negotiation/basic/hello", headers: { "HTTP_ACCEPT" => "text/javascript, */*" } + assert_body "Hello world text/html!" + end + + test "A js or */* Accept header on xhr will return JavaScript" do + get "/content_negotiation/basic/hello", headers: { "HTTP_ACCEPT" => "text/javascript, */*" }, xhr: true + assert_body "Hello world text/javascript!" + end + + test "Unregistered mimes are ignored" do get "/content_negotiation/basic/all", headers: { "HTTP_ACCEPT" => "text/plain, mime/another" } - assert_body '[:text, "mime/another"]' + assert_body "[:text]" end end end diff --git a/actionpack/test/controller/new_base/render_context_test.rb b/actionpack/test/controller/new_base/render_context_test.rb deleted file mode 100644 index 5e570a1d79..0000000000 --- a/actionpack/test/controller/new_base/render_context_test.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "abstract_unit" - -# This is testing the decoupling of view renderer and view context -# by allowing the controller to be used as view context. This is -# similar to the way sinatra renders templates. -module RenderContext - class BasicController < ActionController::Base - self.view_paths = [ActionView::FixtureResolver.new( - "render_context/basic/hello_world.html.erb" => "<%= @value %> from <%= self.__controller_method__ %>", - "layouts/basic.html.erb" => "?<%= yield %>?" - )] - - # 1) Include ActionView::Context to bring the required dependencies - include ActionView::Context - - # 2) Call _prepare_context that will do the required initialization - before_action :_prepare_context - - def hello_world - @value = "Hello" - render action: "hello_world", layout: false - end - - def with_layout - @value = "Hello" - render action: "hello_world", layout: "basic" - end - - protected def __controller_method__ - "controller context!" - end - - private - # 3) Set view_context to self - def view_context - self - end - end - - class RenderContextTest < Rack::TestCase - test "rendering using the controller as context" do - get "/render_context/basic/hello_world" - assert_body "Hello from controller context!" - assert_status 200 - end - - test "rendering using the controller as context with layout" do - get "/render_context/basic/with_layout" - assert_body "?Hello from controller context!?" - assert_status 200 - end - end -end diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index d2fa0aa16e..fbfe24059b 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -365,17 +365,15 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "permitted takes a default value when Parameters.permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(person: { - age: "32", name: { first: "David", last: "Heinemeier Hansson" } - }) - - assert_predicate params.slice(:person), :permitted? - assert_predicate params[:person][:name], :permitted? - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(person: { + age: "32", name: { first: "David", last: "Heinemeier Hansson" } + }) + + assert_predicate params.slice(:person), :permitted? + assert_predicate params[:person][:name], :permitted? + ensure + ActionController::Parameters.permit_all_parameters = false end test "permitting parameters as an array" do @@ -396,16 +394,14 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_h returns converted hash when .permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") - - assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h - assert_not_kind_of ActionController::Parameters, params.to_h - assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") + + assert_instance_of ActiveSupport::HashWithIndifferentAccess, params.to_h + assert_not_kind_of ActionController::Parameters, params.to_h + assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) + ensure + ActionController::Parameters.permit_all_parameters = false end test "to_hash raises UnfilteredParameters on unfiltered params" do @@ -429,17 +425,15 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_hash returns converted hash when .permit_all_parameters is set" do - begin - ActionController::Parameters.permit_all_parameters = true - params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") - - assert_instance_of Hash, params.to_hash - assert_not_kind_of ActionController::Parameters, params.to_hash - assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash) - assert_equal({ "crab" => "Senjougahara Hitagi" }, params) - ensure - ActionController::Parameters.permit_all_parameters = false - end + ActionController::Parameters.permit_all_parameters = true + params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") + + assert_instance_of Hash, params.to_hash + assert_not_kind_of ActionController::Parameters, params.to_hash + assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_hash) + assert_equal({ "crab" => "Senjougahara Hitagi" }, params) + ensure + ActionController::Parameters.permit_all_parameters = false end test "to_unsafe_h returns unfiltered params" do diff --git a/actionpack/test/controller/params_parse_test.rb b/actionpack/test/controller/params_parse_test.rb new file mode 100644 index 0000000000..440ab06fd7 --- /dev/null +++ b/actionpack/test/controller/params_parse_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ParamsParseTest < ActionController::TestCase + class UsersController < ActionController::Base + def create + head :ok + end + end + + tests UsersController + + def test_parse_error_logged_once + log_output = capture_log_output do + post :create, body: "{", as: :json + end + assert_equal <<~LOG, log_output + Error occurred while parsing request parameters. + Contents: + + { + LOG + end + + private + + def capture_log_output + output = StringIO.new + request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output) + yield + output.string + end +end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 998498e1b2..7f1c41787a 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -68,10 +68,18 @@ class RedirectController < ActionController::Base redirect_back(fallback_location: "/things/stuff", status: 307) end + def redirect_back_with_status_and_fallback_location_to_another_host + redirect_back(fallback_location: "http://www.rubyonrails.org/", status: 307) + end + def safe_redirect_back_with_status redirect_back(fallback_location: "/things/stuff", status: 307, allow_other_host: false) end + def safe_redirect_back_with_status_and_fallback_location_to_another_host + redirect_back(fallback_location: "http://www.rubyonrails.org/", status: 307, allow_other_host: false) + end + def host_redirect redirect_to action: "other_host", only_path: false, host: "other.test.host" end @@ -280,6 +288,13 @@ class RedirectTest < ActionController::TestCase assert_equal "http://test.host/things/stuff", redirect_to_url end + def test_redirect_back_with_no_referer_redirects_to_another_host + get :redirect_back_with_status_and_fallback_location_to_another_host + + assert_response 307 + assert_equal "http://www.rubyonrails.org/", redirect_to_url + end + def test_safe_redirect_back_from_other_host @request.env["HTTP_REFERER"] = "http://another.host/coming/from" get :safe_redirect_back_with_status @@ -297,6 +312,20 @@ class RedirectTest < ActionController::TestCase assert_equal referer, redirect_to_url end + def test_safe_redirect_back_with_no_referer + get :safe_redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + + def test_safe_redirect_back_with_no_referer_redirects_to_another_host + get :safe_redirect_back_with_status_and_fallback_location_to_another_host + + assert_response 307 + assert_equal "http://www.rubyonrails.org/", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 306b245bd1..4750093c5c 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -183,6 +183,11 @@ class TestController < ActionController::Base render action: "hello_world" end + def conditional_hello_without_expires_and_public_header + response.headers["Cache-Control"] = "public, no-cache" + render action: "hello_world" + end + def conditional_hello_with_bangs render action: "hello_world" end @@ -418,6 +423,11 @@ class ExpiresInRenderTest < ActionController::TestCase assert_equal "no-cache", @response.headers["Cache-Control"] end + def test_no_expires_now_with_public + get :conditional_hello_without_expires_and_public_header + assert_equal "public, no-cache", @response.headers["Cache-Control"] + end + def test_date_header_when_expires_in time = Time.mktime(2011, 10, 30) Time.stub :now, time do diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 4ed79073e5..089b0b94d4 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -62,12 +62,8 @@ class RescueController < ActionController::Base render plain: exception.message end - rescue_from ActionView::TemplateError do - render plain: "action_view templater error" - end - - rescue_from IOError do - render plain: "io error" + rescue_from ActionDispatch::Http::Parameters::ParseError do + render plain: "parse error", status: :bad_request end before_action(only: :before_action_raises) { raise "umm nice" } @@ -75,19 +71,6 @@ class RescueController < ActionController::Base def before_action_raises end - def raises - render plain: "already rendered" - raise "don't panic!" - end - - def method_not_allowed - raise ActionController::MethodNotAllowed.new(:get, :head, :put) - end - - def not_implemented - raise ActionController::NotImplemented.new(:get, :put) - end - def not_authorized raise NotAuthorized end @@ -130,6 +113,11 @@ class RescueController < ActionController::Base raise ResourceUnavailableToRescueAsString end + def arbitrary_action + params + render plain: "arbitrary action" + end + def missing_template end @@ -306,6 +294,23 @@ class RescueControllerTest < ActionController::TestCase get :exception_with_no_handler_for_wrapper assert_response :unprocessable_entity end + + test "can rescue a ParseError" do + capture_log_output do + post :arbitrary_action, body: "{", as: :json + end + assert_response :bad_request + assert_equal "parse error", response.body + end + + private + + def capture_log_output + output = StringIO.new + request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output) + yield + output.string + end end class RescueTest < ActionDispatch::IntegrationTest @@ -325,10 +330,6 @@ class RescueTest < ActionDispatch::IntegrationTest raise RecordInvalid end - def b00m - raise "b00m" - end - private def show_errors(exception) render plain: exception.message @@ -356,7 +357,6 @@ class RescueTest < ActionDispatch::IntegrationTest set.draw do get "foo", to: ::RescueTest::TestController.action(:foo) get "invalid", to: ::RescueTest::TestController.action(:invalid) - get "b00m", to: ::RescueTest::TestController.action(:b00m) end yield end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index d336b96eff..d2146f12a5 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -853,6 +853,28 @@ class ResourcesTest < ActionController::TestCase end end + def test_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resources :products, only: [:show, :destroy], except: :destroy + end + + assert_resource_allowed_routes("products", {}, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + assert_resource_allowed_routes("products", { format: "xml" }, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + end + end + + def test_singleton_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resource :account, only: [:show, :destroy], except: :destroy + end + + assert_singleton_resource_allowed_routes("accounts", {}, :show, [:new, :create, :edit, :update, :destroy]) + assert_singleton_resource_allowed_routes("accounts", { format: "xml" }, :show, [:new, :create, :edit, :update, :destroy]) + end + end + def test_resource_has_only_create_action_and_named_route with_routing do |set| set.draw do diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 30f2a23b33..b378bb80b8 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -355,10 +355,10 @@ class LegacyRouteSetTests < ActiveSupport::TestCase rs.draw { ActiveSupport::Deprecation.silence { get "/:controller/:action", action: /auth[-|_].+/ } } assert_equal({ action: "auth_google", controller: "content" }, rs.recognize_path("/content/auth_google")) - assert_equal({ action: "auth-facebook", controller: "content" }, rs.recognize_path("/content/auth-facebook")) + assert_equal({ action: "auth-twitter", controller: "content" }, rs.recognize_path("/content/auth-twitter")) assert_equal "/content/auth_google", url_for(rs, controller: "content", action: "auth_google") - assert_equal "/content/auth-facebook", url_for(rs, controller: "content", action: "auth-facebook") + assert_equal "/content/auth-twitter", url_for(rs, controller: "content", action: "auth-twitter") end def test_route_with_regexp_for_controller diff --git a/actionpack/test/controller/show_exceptions_test.rb b/actionpack/test/controller/show_exceptions_test.rb index 2094aa1aed..8724f9bcdb 100644 --- a/actionpack/test/controller/show_exceptions_test.rb +++ b/actionpack/test/controller/show_exceptions_test.rb @@ -99,15 +99,16 @@ module ShowExceptions class ShowFailsafeExceptionsTest < ActionDispatch::IntegrationTest def test_render_failsafe_exception @app = ShowExceptionsOverriddenController.action(:boom) - @exceptions_app = @app.instance_variable_get(:@exceptions_app) - @app.instance_variable_set(:@exceptions_app, nil) + middleware = @app.instance_variable_get(:@middleware) + @exceptions_app = middleware.instance_variable_get(:@exceptions_app) + middleware.instance_variable_set(:@exceptions_app, nil) $stderr = StringIO.new get "/", headers: { "HTTP_ACCEPT" => "text/json" } assert_response :internal_server_error assert_equal "text/plain", response.content_type.to_s ensure - @app.instance_variable_set(:@exceptions_app, @exceptions_app) + middleware.instance_variable_set(:@exceptions_app, @exceptions_app) $stderr = STDERR end end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 6fc70d6248..d1cd190747 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -156,6 +156,10 @@ XML render html: '<body class="foo"></body>'.html_safe end + def render_json + render json: request.raw_post + end + def boom raise "boom!" end @@ -474,6 +478,18 @@ XML ) end + def test_nil_params + get :test_params, params: nil + parsed_params = JSON.parse(@response.body) + assert_equal( + { + "action" => "test_params", + "controller" => "test_case_test/test" + }, + parsed_params + ) + end + def test_query_param_named_action get :test_query_parameters, params: { action: "foobar" } parsed_params = JSON.parse(@response.body) @@ -965,6 +981,16 @@ XML assert_equal "q=test2", @response.body end + + def test_parsed_body_without_as_option + post :render_json, body: { foo: "heyo" } + assert_equal({ "foo" => "heyo" }, response.parsed_body) + end + + def test_parsed_body_with_as_option + post :render_json, body: { foo: "heyo" }.to_json, as: :json + assert_equal({ "foo" => "heyo" }, response.parsed_body) + end end class ResponseDefaultHeadersTest < ActionController::TestCase diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index e381abee36..9222250b9c 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -354,6 +354,14 @@ module AbstractController assert_equal({ p2: "Y2" }.to_query, params[1]) end + def test_params_option + url = W.new.url_for(only_path: true, controller: "c", action: "a", params: { domain: "foo", id: "1" }) + params = extract_params(url) + assert_equal("/c/a?domain=foo&id=1", url) + assert_equal({ domain: "foo" }.to_query, params[0]) + assert_equal({ id: "1" }.to_query, params[1]) + end + def test_hash_parameter url = W.new.url_for(only_path: true, controller: "c", action: "a", query: { name: "Bob", category: "prof" }) params = extract_params(url) diff --git a/actionpack/test/controller/webservice_test.rb b/actionpack/test/controller/webservice_test.rb index 4a10637b54..23a46df5cd 100644 --- a/actionpack/test/controller/webservice_test.rb +++ b/actionpack/test/controller/webservice_test.rb @@ -14,7 +14,7 @@ class WebServiceTest < ActionDispatch::IntegrationTest end def dump_params_keys(hash = params) - hash.keys.sort.inject("") do |s, k| + hash.keys.sort.each_with_object(+"") do |k, s| value = hash[k] if value.is_a?(Hash) || value.is_a?(ActionController::Parameters) @@ -23,8 +23,8 @@ class WebServiceTest < ActionDispatch::IntegrationTest value = "" end - s += ", " unless s.empty? - s += "#{k}#{value}" + s << ", " unless s.empty? + s << "#{k}#{value}" end end end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 6637c2cae9..2c67bb779f 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -123,6 +123,11 @@ class CookiesTest < ActionController::TestCase head :ok end + def set_cookie_if_not_present + cookies["user_name"] = "alice" unless cookies["user_name"].present? + head :ok + end + def logout cookies.delete("user_name") head :ok @@ -525,21 +530,6 @@ class CookiesTest < ActionController::TestCase assert_equal 45, verifier.verify(@response.cookies["user_id"]) end - def test_signed_cookie_with_legacy_secret_scheme - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - old_message = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", digest: "SHA1", serializer: Marshal).generate(45) - - @request.headers["Cookie"] = "user_id=#{old_message}" - get :get_signed_cookie - assert_equal 45, @controller.send(:cookies).signed[:user_id] - - key_generator = @request.env["action_dispatch.key_generator"] - secret = key_generator.generate_key("signed cookie") - verifier = ActiveSupport::MessageVerifier.new(secret, digest: "SHA1", serializer: Marshal) - assert_equal 45, verifier.verify(@response.cookies["user_id"]) - end - def test_tampered_with_signed_cookie key_generator = @request.env["action_dispatch.key_generator"] secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) @@ -759,175 +749,7 @@ class CookiesTest < ActionController::TestCase assert_equal ["user_name", "user_id"], @request.cookie_jar.instance_variable_get(:@cookies).keys end - def test_raises_argument_error_if_missing_secret - assert_raise(ArgumentError, nil.inspect) { - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new(nil) - get :set_signed_cookie - } - - assert_raise(ArgumentError, "".inspect) { - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("") - get :set_signed_cookie - } - end - - def test_raises_argument_error_if_secret_is_probably_insecure - assert_raise(ArgumentError, "password".inspect) { - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("password") - get :set_signed_cookie - } - - assert_raise(ArgumentError, "secret".inspect) { - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("secret") - get :set_signed_cookie - } - - assert_raise(ArgumentError, "12345678901234567890123456789".inspect) { - @request.env["action_dispatch.key_generator"] = ActiveSupport::LegacyKeyGenerator.new("12345678901234567890123456789") - get :set_signed_cookie - } - end - - def test_legacy_signed_cookie_is_read_and_transparently_upgraded_by_signed_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45) - - @request.headers["Cookie"] = "user_id=#{legacy_value}" - get :get_signed_cookie - - assert_equal 45, @controller.send(:cookies).signed[:user_id] - - key_generator = @request.env["action_dispatch.key_generator"] - secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) - verifier = ActiveSupport::MessageVerifier.new(secret) - assert_equal 45, verifier.verify(@response.cookies["user_id"]) - end - - def test_legacy_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate("bar") - - @request.headers["Cookie"] = "foo=#{legacy_value}" - get :get_encrypted_cookie - - assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - - secret = @request.env["action_dispatch.key_generator"].generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], 32) - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal) - assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) - end - - def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :json - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45) - - @request.headers["Cookie"] = "user_id=#{legacy_value}" - get :get_signed_cookie - - assert_equal 45, @controller.send(:cookies).signed[:user_id] - - key_generator = @request.env["action_dispatch.key_generator"] - secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) - verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON) - assert_equal 45, verifier.verify(@response.cookies["user_id"]) - end - - def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :json - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar") - - @request.headers["Cookie"] = "foo=#{legacy_value}" - get :get_encrypted_cookie - - assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - - cipher = "aes-256-gcm" - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON) - assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) - end - - def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :hybrid - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45) - - @request.headers["Cookie"] = "user_id=#{legacy_value}" - get :get_signed_cookie - - assert_equal 45, @controller.send(:cookies).signed[:user_id] - - key_generator = @request.env["action_dispatch.key_generator"] - secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) - verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON) - assert_equal 45, verifier.verify(@response.cookies["user_id"]) - end - - def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :hybrid - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar") - - @request.headers["Cookie"] = "foo=#{legacy_value}" - get :get_encrypted_cookie - - assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) - assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) - end - - def test_legacy_marshal_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :hybrid - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45) - - @request.headers["Cookie"] = "user_id=#{legacy_value}" - get :get_signed_cookie - - assert_equal 45, @controller.send(:cookies).signed[:user_id] - - key_generator = @request.env["action_dispatch.key_generator"] - secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) - verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON) - assert_equal 45, verifier.verify(@response.cookies["user_id"]) - end - - def test_legacy_marshal_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.cookies_serializer"] = :hybrid - - @request.env["action_dispatch.use_authenticated_cookie_encryption"] = true - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - - legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate("bar") - - @request.headers["Cookie"] = "foo=#{legacy_value}" - get :get_encrypted_cookie - - assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) - assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) - end - def test_legacy_signed_cookie_is_treated_as_nil_by_signed_cookie_jar_if_tampered - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.headers["Cookie"] = "user_id=45" get :get_signed_cookie @@ -936,8 +758,6 @@ class CookiesTest < ActionController::TestCase end def test_legacy_signed_cookie_is_treated_as_nil_by_encrypted_cookie_jar_if_tampered - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.headers["Cookie"] = "foo=baz" get :get_encrypted_cookie @@ -1313,6 +1133,14 @@ class CookiesTest < ActionController::TestCase assert_equal "bar", @controller.encrypted_cookie end + def test_cookie_override + get :set_cookie_if_not_present + assert_equal "alice", cookies["user_name"] + cookies["user_name"] = "bob" + get :set_cookie_if_not_present + assert_equal "bob", cookies["user_name"] + end + def test_signed_cookie_with_expires_set_relatively request.env["action_dispatch.use_cookies_with_metadata"] = true @@ -1378,11 +1206,7 @@ class CookiesTest < ActionController::TestCase get :encrypted_discount_and_user_id_cookie travel 2.hours - assert_equal 50, cookies.encrypted[:user_id] - - cookies[:discount_percentage] = cookies[:user_id] - assert_not_equal 10, cookies.encrypted[:discount_percentage] - assert_equal 50, cookies.encrypted[:discount_percentage] + assert_nil cookies.signed[:user_id] end def test_switch_off_metadata_for_signed_cookies_if_config_is_false @@ -1391,11 +1215,8 @@ class CookiesTest < ActionController::TestCase get :signed_discount_and_user_id_cookie travel 2.hours - assert_equal 50, cookies.signed[:user_id] - cookies[:discount_percentage] = cookies[:user_id] - assert_not_equal 10, cookies.signed[:discount_percentage] - assert_equal 50, cookies.signed[:discount_percentage] + assert_nil cookies.signed[:user_id] end def test_read_rails_5_2_stable_encrypted_cookies_if_config_is_false diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 37399cfd07..8b1b3c0277 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -8,7 +8,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest class Boomer attr_accessor :closed - def initialize(detailed = false) + def initialize(detailed = false) @detailed = detailed @closed = false end @@ -27,66 +27,70 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end def raise_nested_exceptions + raise "First error" + rescue begin - raise "First error" + raise "Second error" rescue - begin - raise "Second error" - rescue - raise "Third error" - end + raise "Third error" end end def call(env) env["action_dispatch.show_detailed_exceptions"] = @detailed req = ActionDispatch::Request.new(env) + template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, format: :html, locals: []) + case req.path - when %r{/pass} + when "/pass" [404, { "X-Cascade" => "pass" }, self] - when %r{/not_found} + when "/not_found" raise AbstractController::ActionNotFound - when %r{/runtime_error} + when "/runtime_error" raise RuntimeError - when %r{/method_not_allowed} + when "/method_not_allowed" raise ActionController::MethodNotAllowed - when %r{/intercepted_error} + when "/intercepted_error" raise InterceptedErrorInstance - when %r{/unknown_http_method} + when "/unknown_http_method" raise ActionController::UnknownHttpMethod - when %r{/not_implemented} + when "/not_implemented" raise ActionController::NotImplemented - when %r{/unprocessable_entity} + when "/unprocessable_entity" raise ActionController::InvalidAuthenticityToken - when %r{/not_found_original_exception} + when "/invalid_mimetype" + raise Mime::Type::InvalidMimeType + when "/not_found_original_exception" begin raise AbstractController::ActionNotFound.new rescue - raise ActionView::Template::Error.new("template") + raise ActionView::Template::Error.new(template) end - when %r{/missing_template} + when "/cause_mapped_to_rescue_responses" + begin + raise ActionController::ParameterMissing, :missing_param_key + rescue + raise NameError.new("uninitialized constant Userr") + end + when "/missing_template" raise ActionView::MissingTemplate.new(%w(foo), "foo/index", %w(foo), false, "mailer") - when %r{/bad_request} + when "/bad_request" raise ActionController::BadRequest - when %r{/missing_keys} + when "/missing_keys" raise ActionController::UrlGenerationError, "No route matches" - when %r{/parameter_missing} + when "/parameter_missing" raise ActionController::ParameterMissing, :missing_param_key - when %r{/original_syntax_error} + when "/original_syntax_error" eval "broke_syntax =" # `eval` need for raise native SyntaxError at runtime - when %r{/syntax_error_into_view} + when "/syntax_error_into_view" begin eval "broke_syntax =" rescue Exception - template = ActionView::Template.new(File.read(__FILE__), - __FILE__, - ActionView::Template::Handlers::Raw.new, - {}) raise ActionView::Template::Error.new(template) end - when %r{/framework_raises} + when "/framework_raises" method_that_raises - when %r{/nested_exceptions} + when "/nested_exceptions" raise_nested_exceptions else raise "puke!" @@ -176,6 +180,10 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true } assert_response 400 assert_match(/ActionController::ParameterMissing/, body) + + get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true } + assert_response 406 + assert_match(/Mime::Type::InvalidMimeType/, body) end test "rescue with text error for xhr request" do @@ -290,22 +298,20 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end test "rescue with JSON format as fallback if API request format is not supported" do - begin - Mime::Type.register "text/wibble", :wibble + Mime::Type.register "text/wibble", :wibble - ActionDispatch::IntegrationTest.register_encoder(:wibble, - param_encoder: -> params { params }) + ActionDispatch::IntegrationTest.register_encoder(:wibble, + param_encoder: -> params { params }) - @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) - get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble - assert_response 500 - assert_equal "application/json", response.content_type - assert_match(/RuntimeError: puke/, body) + get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble + assert_response 500 + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) - ensure - Mime::Type.unregister :wibble - end + ensure + Mime::Type.unregister :wibble end test "does not show filtered parameters" do @@ -317,12 +323,22 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(""foo"=>"[FILTERED]"", body) end - test "show registered original exception for wrapped exceptions" do + test "show registered original exception if the last exception is TemplateError" do @app = DevelopmentApp get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } assert_response 404 - assert_match(/AbstractController::ActionNotFound/, body) + assert_match %r{AbstractController::ActionNotFound}, body + assert_match %r{Showing <i>.*test/dispatch/debug_exceptions_test.rb</i>}, body + end + + test "show the last exception and cause even when the cause is mapped to resque_responses" do + @app = DevelopmentApp + + get "/cause_mapped_to_rescue_responses", headers: { "action_dispatch.show_exceptions" => true } + assert_response 500 + assert_match %r{ActionController::ParameterMissing}, body + assert_match %r{NameError}, body end test "named urls missing keys raise 500 level error" do @@ -484,6 +500,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_select "#Application-Trace-0" do assert_select "code", /syntax error, unexpected/ end + assert_match %r{Showing <i>.*test/dispatch/debug_exceptions_test.rb</i>}, body end test "debug exceptions app shows user code that caused the error in source view" do diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb new file mode 100644 index 0000000000..5263dd2597 --- /dev/null +++ b/actionpack/test/dispatch/host_authorization_test.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "ipaddr" + +class HostAuthorizationTest < ActionDispatch::IntegrationTest + App = -> env { [200, {}, %w(Success)] } + + test "blocks requests to unallowed host" do + @app = ActionDispatch::HostAuthorization.new(App, %w(only.com)) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "allows all requests if hosts is empty" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "hosts can be a single element array" do + @app = ActionDispatch::HostAuthorization.new(App, %w(www.example.com)) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "hosts can be a string" do + @app = ActionDispatch::HostAuthorization.new(App, "www.example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "passes requests to allowed hosts with domain name notation" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "does not allow domain name notation in the HOST header itself" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/", env: { + "HOST" => ".example.com", + } + + assert_response :forbidden + assert_match "Blocked host: .example.com", response.body + end + + test "checks for requests with #=== to support wider range of host checks" do + @app = ActionDispatch::HostAuthorization.new(App, [-> input { input == "www.example.com" }]) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "mark the host when authorized" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_equal "www.example.com", request.get_header("action_dispatch.authorized_host") + end + + test "sanitizes regular expressions to prevent accidental matches" do + @app = ActionDispatch::HostAuthorization.new(App, [/w.example.co/]) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "blocks requests to unallowed host supporting custom responses" do + @app = ActionDispatch::HostAuthorization.new(App, ["w.example.co"], -> env do + [401, {}, %w(Custom)] + end) + + get "/" + + assert_response :unauthorized + assert_equal "Custom", body + end + + test "blocks requests with spoofed X-FORWARDED-HOST" do + @app = ActionDispatch::HostAuthorization.new(App, [IPAddr.new("127.0.0.1")]) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: 127.0.0.1", response.body + end + + test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :ok + assert_equal "Success", body + end + + test "detects localhost domain spoofing" do + @app = ActionDispatch::HostAuthorization.new(App, "localhost") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "localhost", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: localhost", response.body + end + + test "forwarded hosts should be permitted" do + @app = ActionDispatch::HostAuthorization.new(App, "domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :forbidden + assert_match "Blocked host: sub.domain.com", response.body + end + + test "forwarded hosts are allowed when permitted" do + @app = ActionDispatch::HostAuthorization.new(App, ".domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :ok + assert_equal "Success", body + end +end diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index a9a56f205f..f2459112b2 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -51,18 +51,24 @@ module ActionController assert_equal ["omg"], @response.body_parts end - def test_cache_control_is_set + def test_cache_control_is_set_by_default @response.stream.write "omg" assert_equal "no-cache", @response.headers["Cache-Control"] end + def test_cache_control_is_set_manually + @response.set_header("Cache-Control", "public") + @response.stream.write "omg" + assert_equal "public", @response.headers["Cache-Control"] + end + def test_content_length_is_removed @response.headers["Content-Length"] = "1234" @response.stream.write "omg" assert_nil @response.headers["Content-Length"] end - def test_headers_cannot_be_written_after_webserver_reads + def test_headers_cannot_be_written_after_web_server_reads @response.stream.write "omg" latch = Concurrent::CountDownLatch.new diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index 5f43e5a3c5..90f2eccd19 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -3,13 +3,24 @@ require "abstract_unit" class MiddlewareStackTest < ActiveSupport::TestCase - class FooMiddleware; end - class BarMiddleware; end - class BazMiddleware; end - class HiyaMiddleware; end - class BlockMiddleware + class Base + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + end + end + + class FooMiddleware < Base; end + class BarMiddleware < Base; end + class BazMiddleware < Base; end + class HiyaMiddleware < Base; end + class BlockMiddleware < Base attr_reader :block - def initialize(&block) + def initialize(app, &block) + super(app) @block = block end end @@ -109,6 +120,24 @@ class MiddlewareStackTest < ActiveSupport::TestCase assert_equal @stack.last, @stack.last end + test "instruments the execution of middlewares" do + app = @stack.build(proc { |env| [200, {}, []] }) + env = {} + + events = [] + + subscriber = proc do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + ActiveSupport::Notifications.subscribed(subscriber, "process_middleware.action_dispatch") do + app.call(env) + end + + assert_equal 2, events.count + assert_equal ["MiddlewareStackTest::BarMiddleware", "MiddlewareStackTest::FooMiddleware"], events.map { |e| e.payload[:middleware] } + end + test "includes a middleware" do assert_equal true, @stack.include?(ActionDispatch::MiddlewareStack::Middleware.new(BarMiddleware, nil, nil)) end diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index fa264417e1..50f6c06fee 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -96,57 +96,47 @@ class MimeTypeTest < ActiveSupport::TestCase end test "custom type" do - begin - type = Mime::Type.register("image/foo", :foo) - assert_equal type, Mime[:foo] - ensure - Mime::Type.unregister(:foo) - end + type = Mime::Type.register("image/foo", :foo) + assert_equal type, Mime[:foo] + ensure + Mime::Type.unregister(:foo) end test "custom type with type aliases" do - begin - Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] - %w[text/foobar text/foo text/bar].each do |type| - assert_equal Mime[:foobar], type - end - ensure - Mime::Type.unregister(:foobar) + Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"] + %w[text/foobar text/foo text/bar].each do |type| + assert_equal Mime[:foobar], type end + ensure + Mime::Type.unregister(:foobar) end test "register callbacks" do - begin - registered_mimes = [] - Mime::Type.register_callback do |mime| - registered_mimes << mime - end - - mime = Mime::Type.register("text/foo", :foo) - assert_equal [mime], registered_mimes - ensure - Mime::Type.unregister(:foo) + registered_mimes = [] + Mime::Type.register_callback do |mime| + registered_mimes << mime end + + mime = Mime::Type.register("text/foo", :foo) + assert_equal [mime], registered_mimes + ensure + Mime::Type.unregister(:foo) end test "custom type with extension aliases" do - begin - Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] - %w[foobar foo bar].each do |extension| - assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension] - end - ensure - Mime::Type.unregister(:foobar) + Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"] + %w[foobar foo bar].each do |extension| + assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension] end + ensure + Mime::Type.unregister(:foobar) end test "register alias" do - begin - Mime::Type.register_alias "application/xhtml+xml", :foobar - assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"] - ensure - Mime::Type.unregister(:foobar) - end + Mime::Type.register_alias "application/xhtml+xml", :foobar + assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"] + ensure + Mime::Type.unregister(:foobar) end test "type should be equal to symbol" do @@ -184,4 +174,51 @@ class MimeTypeTest < ActiveSupport::TestCase assert_not (Mime[:js] !~ "application/javascript") assert Mime[:html] =~ "application/xhtml+xml" end + + test "can be initialized with wildcards" do + assert_equal "*/*", Mime::Type.new("*/*").to_s + assert_equal "text/*", Mime::Type.new("text/*").to_s + assert_equal "video/*", Mime::Type.new("video/*").to_s + end + + test "can be initialized with parameters" do + assert_equal "text/html; parameter", Mime::Type.new("text/html; parameter").to_s + assert_equal "text/html; parameter=abc", Mime::Type.new("text/html; parameter=abc").to_s + assert_equal 'text/html; parameter="abc"', Mime::Type.new('text/html; parameter="abc"').to_s + assert_equal 'text/html; parameter=abc; parameter2="xyz"', Mime::Type.new('text/html; parameter=abc; parameter2="xyz"').to_s + end + + test "invalid mime types raise error" do + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("too/many/slash") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("missingslash") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("improper/semicolon;") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new('improper/semicolon; parameter=abc; parameter2="xyz";') + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("text/html, text/plain") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("*/html") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new("") + end + + assert_raises Mime::Type::InvalidMimeType do + Mime::Type.new(nil) + end + end end diff --git a/actionpack/test/dispatch/mount_test.rb b/actionpack/test/dispatch/mount_test.rb index f6cf653980..e42ea89f6f 100644 --- a/actionpack/test/dispatch/mount_test.rb +++ b/actionpack/test/dispatch/mount_test.rb @@ -80,6 +80,12 @@ class TestRoutingMount < ActionDispatch::IntegrationTest assert_equal "/shorthand -- /omg", response.body end + def test_mounting_does_not_match_similar_paths + get "/shorthandomg" + assert_not_equal "/shorthand -- /omg", response.body + assert_equal " -- /shorthandomg", response.body + end + def test_mounting_works_with_via get "/getfake" assert_equal "OK", response.body diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index beab8e78b5..2a48a12497 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -74,17 +74,15 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test "occurring a parse error if parsing unsuccessful" do with_test_routing do - begin - $stderr = StringIO.new # suppress the log - json = "[\"person]\": {\"name\": \"David\"}}" - exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do - post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false } - end - assert_equal JSON::ParserError, exception.cause.class - assert_equal exception.cause.message, exception.message - ensure - $stderr = STDERR + $stderr = StringIO.new # suppress the log + json = "[\"person]\": {\"name\": \"David\"}}" + exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do + post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false } end + assert_equal JSON::ParserError, exception.cause.class + assert_equal exception.cause.message, exception.message + ensure + $stderr = STDERR end end @@ -157,31 +155,27 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest end test "parses json params after custom json mime type registered" do - begin - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) - assert_parses( - { "user" => { "username" => "meinac" }, "username" => "meinac" }, - "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json" - ) - ensure - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) - end + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) + assert_parses( + { "user" => { "username" => "meinac" }, "username" => "meinac" }, + "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json" + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) end test "parses json params after custom json mime type registered with synonym" do - begin - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) - assert_parses( - { "user" => { "username" => "meinac" }, "username" => "meinac" }, - "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json" - ) - ensure - Mime::Type.unregister :json - Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) - end + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w(application/vnd.rails+json) + assert_parses( + { "user" => { "username" => "meinac" }, "username" => "meinac" }, + "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json" + ) + ensure + Mime::Type.unregister :json + Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) end private diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 9d1246b3a4..2a4d59affe 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -763,7 +763,6 @@ class RequestMethod < BaseRequestTest test "post uneffected by local inflections" do existing_acronyms = ActiveSupport::Inflector.inflections.acronyms.dup - assert_deprecated { ActiveSupport::Inflector.inflections.acronym_regex.dup } begin ActiveSupport::Inflector.inflections do |inflect| inflect.acronym "POS" diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 0f37d074af..7758b0406a 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -42,7 +42,7 @@ class ResponseTest < ActiveSupport::TestCase def test_each_isnt_called_if_str_body_is_written # Controller writes and reads response body each_counter = 0 - @response.body = Object.new.tap { |o| o.singleton_class.send(:define_method, :each) { |&block| each_counter += 1; block.call "foo" } } + @response.body = Object.new.tap { |o| o.singleton_class.define_method(:each) { |&block| each_counter += 1; block.call "foo" } } @response["X-Foo"] = @response.body assert_equal 1, each_counter, "#each was not called once" @@ -539,4 +539,38 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('"202cb962ac59075b964b07152d234b70"', @response.headers["ETag"]) assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) end + + test "response Content-Type with optional parameters" do + @app = lambda { |env| + [ + 200, + { "Content-Type" => "text/csv; charset=utf-16; header=present" }, + ["Hello"] + ] + } + + get "/" + assert_response :success + + assert_equal("text/csv; charset=utf-16; header=present", @response.headers["Content-Type"]) + assert_equal("text/csv", @response.content_type) + assert_equal("utf-16", @response.charset) + end + + test "response Content-Type with quoted-string" do + @app = lambda { |env| + [ + 200, + { "Content-Type" => 'text/csv; header=present; charset="utf-16"' }, + ["Hello"] + ] + } + + get "/" + assert_response :success + + assert_equal('text/csv; header=present; charset="utf-16"', @response.headers["Content-Type"]) + assert_equal("text/csv", @response.content_type) + assert_equal("utf-16", @response.charset) + end end diff --git a/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb new file mode 100644 index 0000000000..676a8c38d4 --- /dev/null +++ b/actionpack/test/dispatch/routing/non_dispatch_routed_app_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "abstract_unit" + +module ActionDispatch + module Routing + class NonDispatchRoutedAppTest < ActionDispatch::IntegrationTest + # For example, Grape::API + class SimpleApp + def self.call(env) + [ 200, { "Content-Type" => "text/plain" }, [] ] + end + + def self.routes + [] + end + end + + setup { @app = SimpleApp } + + test "does not except" do + get "/foo" + assert_response :success + end + end + end +end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index affc2d8497..7b763ec2bd 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3338,6 +3338,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal "0c0c0b68-d24b-11e1-a861-001ff3fffe6f", @request.params[:download] end + def test_colon_containing_custom_param + ex = assert_raises(ArgumentError) { + draw do + resources :profiles, param: "username/:is_admin" + end + } + + assert_match(/:param option can't contain colon/, ex.message) + end + def test_action_from_path_is_not_frozen draw do get "search" => "search" @@ -3698,15 +3708,25 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_multiple_roots + def test_multiple_roots_raises_error + ex = assert_raises(ArgumentError) { + draw do + root "pages#index", constraints: { host: "www.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" } + end + } + assert_match(/Invalid route name, already in use: 'root'/, ex.message) + end + + def test_multiple_named_roots draw do namespace :foo do root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end get "http://www.example.com/foo" @@ -4981,8 +5001,12 @@ end class FlashRedirectTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" - Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") - Rotations = ActiveSupport::Messages::RotationConfiguration.new + Generator = ActiveSupport::CachingKeyGenerator.new( + ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 1000) + ) + Rotations = ActiveSupport::Messages::RotationConfiguration.new + SIGNED_COOKIE_SALT = "signed cookie" + ENCRYPTED_SIGNED_COOKIE_SALT = "sigend encrypted cookie" class KeyGeneratorMiddleware def initialize(app) @@ -4992,6 +5016,8 @@ class FlashRedirectTest < ActionDispatch::IntegrationTest def call(env) env["action_dispatch.key_generator"] ||= Generator env["action_dispatch.cookies_rotations"] ||= Rotations + env["action_dispatch.signed_cookie_salt"] = SIGNED_COOKIE_SALT + env["action_dispatch.encrypted_signed_cookie_salt"] = ENCRYPTED_SIGNED_COOKIE_SALT @app.call(env) end diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb index 9b51ee1cad..ac685a7dca 100644 --- a/actionpack/test/dispatch/session/mem_cache_store_test.rb +++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb @@ -38,8 +38,9 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest begin require "dalli" - ss = Dalli::Client.new("localhost:11211").stats - raise Dalli::DalliError unless ss["localhost:11211"] + servers = ENV["MEMCACHE_SERVERS"] || "localhost:11211" + ss = Dalli::Client.new(servers).stats + raise Dalli::DalliError unless ss[servers] def test_setting_and_getting_session_value with_test_route_set do @@ -195,7 +196,9 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest end @app = self.class.build_app(set) do |middleware| - middleware.use ActionDispatch::Session::MemCacheStore, key: "_session_id", namespace: "mem_cache_store_test:#{SecureRandom.hex(10)}" + middleware.use ActionDispatch::Session::MemCacheStore, + key: "_session_id", namespace: "mem_cache_store_test:#{SecureRandom.hex(10)}", + memcache_server: ENV["MEMCACHE_SERVERS"] || "localhost:11211" middleware.delete ActionDispatch::ShowExceptions end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index b69071b44b..6fafa4e426 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -9,6 +9,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest case req.path when "/not_found" raise AbstractController::ActionNotFound + when "/invalid_mimetype" + raise Mime::Type::InvalidMimeType when "/bad_params", "/bad_params.json" begin raise StandardError.new @@ -36,32 +38,36 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "skip exceptions app if not showing exceptions" do @app = ProductionApp assert_raise RuntimeError do - get "/", headers: { "action_dispatch.show_exceptions" => false } + get "/", env: { "action_dispatch.show_exceptions" => false } end end test "rescue with error page" do @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_response 500 assert_equal "500 error fixture\n", body - get "/bad_params", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params", env: { "action_dispatch.show_exceptions" => true } assert_response 400 assert_equal "400 error fixture\n", body - get "/not_found", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "404 error fixture\n", body - get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true } + get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body - get "/unknown_http_method", headers: { "action_dispatch.show_exceptions" => true } + get "/unknown_http_method", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body + + get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => true } + assert_response 406 + assert_equal "", body end test "localize rescue error page" do @@ -70,11 +76,11 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest begin @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_response 500 assert_equal "500 localized error fixture\n", body - get "/not_found", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "404 error fixture\n", body ensure @@ -85,14 +91,14 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "sets the HTTP charset parameter" do @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end test "show registered original exception for wrapped exceptions" do @app = ProductionApp - get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_match(/404 error/, body) end @@ -106,7 +112,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "YOU FAILED", body end @@ -117,7 +123,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true } + get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body end @@ -125,12 +131,12 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "bad params exception is returned in the correct format" do @app = ProductionApp - get "/bad_params", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params", env: { "action_dispatch.show_exceptions" => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] assert_response 400 assert_match(/400 error/, body) - get "/bad_params.json", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params.json", env: { "action_dispatch.show_exceptions" => true } assert_equal "application/json; charset=utf-8", response.headers["Content-Type"] assert_response 400 assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body) diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb index a824ee0c84..0d08f17af3 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -2,6 +2,7 @@ require "abstract_unit" require "action_dispatch/system_testing/driver" +require "selenium/webdriver" class DriverTest < ActiveSupport::TestCase test "initializing the driver" do @@ -22,6 +23,7 @@ class DriverTest < ActiveSupport::TestCase driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :headless_chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) assert_equal :headless_chrome, driver.instance_variable_get(:@browser).name + assert_instance_of Selenium::WebDriver::Chrome::Options, driver.instance_variable_get(:@browser).options assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end @@ -30,6 +32,7 @@ class DriverTest < ActiveSupport::TestCase driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :headless_firefox, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) assert_equal :headless_firefox, driver.instance_variable_get(:@browser).name + assert_instance_of Selenium::WebDriver::Firefox::Options, driver.instance_variable_get(:@browser).options assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end @@ -51,4 +54,70 @@ class DriverTest < ActiveSupport::TestCase test "registerable? returns false if driver is rack_test" do assert_not ActionDispatch::SystemTesting::Driver.new(:rack_test).send(:registerable?) end + + test "define extra capabilities using chrome" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome) do |option| + option.add_argument("start-maximized") + option.add_emulation(device_name: "iphone 6") + option.add_preference(:detach, true) + + driver_option = option + end + driver.use + + expected = { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using headless_chrome" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_chrome) do |option| + option.add_argument("start-maximized") + option.add_emulation(device_name: "iphone 6") + option.add_preference(:detach, true) + + driver_option = option + end + driver.use + + expected = { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using firefox" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :firefox) do |option| + option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/") + option.add_argument("--host=127.0.0.1") + + driver_option = option + end + driver.use + + expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } } + assert_equal expected, driver_option.as_json + end + + test "define extra capabilities using headless_firefox" do + driver_option = nil + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_firefox) do |option| + option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/") + option.add_argument("--host=127.0.0.1") + + driver_option = option + end + driver.use + + expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } } + assert_equal expected, driver_option.as_json + end + + test "does not define extra capabilities" do + driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :firefox) + + assert_nothing_raised do + driver.use + end + end end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index de79c05657..b756b91379 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -3,6 +3,7 @@ require "abstract_unit" require "action_dispatch/system_testing/test_helpers/screenshot_helper" require "capybara/dsl" +require "selenium/webdriver" class ScreenshotHelperTest < ActiveSupport::TestCase test "image path is saved in tmp directory" do @@ -41,22 +42,20 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do - begin - original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] - ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact" + original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact" - new_test = DrivenBySeleniumWithChrome.new("x") + new_test = DrivenBySeleniumWithChrome.new("x") - assert_equal "artifact", new_test.send(:output_type) + assert_equal "artifact", new_test.send(:output_type) - Rails.stub :root, Pathname.getwd do - new_test.stub :passed?, false do - assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image) - end + Rails.stub :root, Pathname.getwd do + new_test.stub :passed?, false do + assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image) end - ensure - ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type end + ensure + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type end test "image path returns the absolute path from root" do diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb index b078a5abc5..847b09dcfe 100644 --- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "abstract_unit" +require "selenium/webdriver" class SetDriverToRackTestTest < DrivenByRackTest test "uses rack_test" do diff --git a/actionpack/test/dispatch/test_response_test.rb b/actionpack/test/dispatch/test_response_test.rb index f0b8f7785d..2629a61057 100644 --- a/actionpack/test/dispatch/test_response_test.rb +++ b/actionpack/test/dispatch/test_response_test.rb @@ -27,11 +27,4 @@ class TestResponseTest < ActiveSupport::TestCase response = ActionDispatch::TestResponse.create(200, { "Content-Type" => "application/json" }, '{ "foo": "fighters" }') assert_equal({ "foo" => "fighters" }, response.parsed_body) end - - test "response status aliases deprecated" do - response = ActionDispatch::TestResponse.create - assert_deprecated { response.success? } - assert_deprecated { response.missing? } - assert_deprecated { response.error? } - end end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 21169fcb5c..03e5274541 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require "abstract_unit" +require "tempfile" +require "stringio" module ActionDispatch class UploadedFileTest < ActiveSupport::TestCase @@ -11,109 +13,118 @@ module ActionDispatch end def test_original_filename - uf = Http::UploadedFile.new(filename: "foo", tempfile: Object.new) + uf = Http::UploadedFile.new(filename: "foo", tempfile: Tempfile.new) assert_equal "foo", uf.original_filename end def test_filename_is_different_object file_str = "foo" - uf = Http::UploadedFile.new(filename: file_str, tempfile: Object.new) + uf = Http::UploadedFile.new(filename: file_str, tempfile: Tempfile.new) assert_not_equal file_str.object_id, uf.original_filename.object_id end def test_filename_should_be_in_utf_8 - uf = Http::UploadedFile.new(filename: "foo", tempfile: Object.new) + uf = Http::UploadedFile.new(filename: "foo", tempfile: Tempfile.new) assert_equal "UTF-8", uf.original_filename.encoding.to_s end def test_filename_should_always_be_in_utf_8 uf = Http::UploadedFile.new(filename: "foo".encode(Encoding::SHIFT_JIS), - tempfile: Object.new) + tempfile: Tempfile.new) assert_equal "UTF-8", uf.original_filename.encoding.to_s end def test_content_type - uf = Http::UploadedFile.new(type: "foo", tempfile: Object.new) + uf = Http::UploadedFile.new(type: "foo", tempfile: Tempfile.new) assert_equal "foo", uf.content_type end def test_headers - uf = Http::UploadedFile.new(head: "foo", tempfile: Object.new) + uf = Http::UploadedFile.new(head: "foo", tempfile: Tempfile.new) assert_equal "foo", uf.headers end def test_tempfile - uf = Http::UploadedFile.new(tempfile: "foo") - assert_equal "foo", uf.tempfile + tf = Tempfile.new + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal tf, uf.tempfile end - def test_to_io_returns_the_tempfile - tf = Object.new + def test_to_io_returns_file + tf = Tempfile.new uf = Http::UploadedFile.new(tempfile: tf) - assert_equal tf, uf.to_io + assert_equal tf.to_io, uf.to_io end def test_delegates_path_to_tempfile - tf = Class.new { def path; "thunderhorse" end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal "thunderhorse", uf.path + tf = Tempfile.new + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal tf.path, uf.path end def test_delegates_open_to_tempfile - tf = Class.new { def open; "thunderhorse" end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal "thunderhorse", uf.open + tf = Tempfile.new + tf.close + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal tf, uf.open + assert_not tf.closed? end def test_delegates_close_to_tempfile - tf = Class.new { def close(unlink_now = false); "thunderhorse" end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal "thunderhorse", uf.close + tf = Tempfile.new + uf = Http::UploadedFile.new(tempfile: tf) + uf.close + assert tf.closed? end def test_close_accepts_parameter - tf = Class.new { def close(unlink_now = false); "thunderhorse: #{unlink_now}" end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal "thunderhorse: true", uf.close(true) + tf = Tempfile.new + uf = Http::UploadedFile.new(tempfile: tf) + uf.close(true) + assert tf.closed? + assert_nil tf.path end def test_delegates_read_to_tempfile - tf = Class.new { def read(length = nil, buffer = nil); "thunderhorse" end } - uf = Http::UploadedFile.new(tempfile: tf.new) + tf = Tempfile.new + tf << "thunderhorse" + tf.rewind + uf = Http::UploadedFile.new(tempfile: tf) assert_equal "thunderhorse", uf.read end def test_delegates_read_to_tempfile_with_params - tf = Class.new { def read(length = nil, buffer = nil); [length, buffer] end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal %w{ thunder horse }, uf.read(*%w{ thunder horse }) - end - - def test_delegate_respects_respond_to? - tf = Class.new { def read; yield end; private :read } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_raises(NoMethodError) do - uf.read - end + tf = Tempfile.new + tf << "thunderhorse" + tf.rewind + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal "thunder", uf.read(7) + assert_equal "horse", uf.read(5, String.new) end def test_delegate_eof_to_tempfile - tf = Class.new { def eof?; true end; } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_predicate uf, :eof? + tf = Tempfile.new + tf << "thunderhorse" + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal true, uf.eof? + tf.rewind + assert_equal false, uf.eof? end def test_delegate_to_path_to_tempfile - tf = Class.new { def to_path; "/any/file/path" end; } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_equal "/any/file/path", uf.to_path + tf = Tempfile.new + uf = Http::UploadedFile.new(tempfile: tf) + assert_equal tf.to_path, uf.to_path end - def test_respond_to? - tf = Class.new { def read; yield end } - uf = Http::UploadedFile.new(tempfile: tf.new) - assert_respond_to uf, :headers - assert_respond_to uf, :read + def test_io_copy_stream + tf = Tempfile.new + tf << "thunderhorse" + tf.rewind + uf = Http::UploadedFile.new(tempfile: tf) + result = StringIO.new + IO.copy_stream(uf, result) + assert_equal "thunderhorse", result.string end end end diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index 3e7aea57f1..fcfaba96b0 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -34,17 +34,17 @@ module ActionDispatch end { - "/:controller(/:action)" => %r{\A/(#{x})(?:/([^/.?]+))?}, - "/:controller/foo" => %r{\A/(#{x})/foo}, - "/:controller/:action" => %r{\A/(#{x})/([^/.?]+)}, - "/:controller" => %r{\A/(#{x})}, - "/:controller(/:action(/:id))" => %r{\A/(#{x})(?:/([^/.?]+)(?:/([^/.?]+))?)?}, - "/:controller/:action.xml" => %r{\A/(#{x})/([^/.?]+)\.xml}, - "/:controller.:format" => %r{\A/(#{x})\.([^/.?]+)}, - "/:controller(.:format)" => %r{\A/(#{x})(?:\.([^/.?]+))?}, - "/:controller/*foo" => %r{\A/(#{x})/(.+)}, - "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar}, - "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))}, + "/:controller(/:action)" => %r{\A/(#{x})(?:/([^/.?]+))?(?:\b|\Z)}, + "/:controller/foo" => %r{\A/(#{x})/foo(?:\b|\Z)}, + "/:controller/:action" => %r{\A/(#{x})/([^/.?]+)(?:\b|\Z)}, + "/:controller" => %r{\A/(#{x})(?:\b|\Z)}, + "/:controller(/:action(/:id))" => %r{\A/(#{x})(?:/([^/.?]+)(?:/([^/.?]+))?)?(?:\b|\Z)}, + "/:controller/:action.xml" => %r{\A/(#{x})/([^/.?]+)\.xml(?:\b|\Z)}, + "/:controller.:format" => %r{\A/(#{x})\.([^/.?]+)(?:\b|\Z)}, + "/:controller(.:format)" => %r{\A/(#{x})(?:\.([^/.?]+))?(?:\b|\Z)}, + "/:controller/*foo" => %r{\A/(#{x})/(.+)(?:\b|\Z)}, + "/:controller/*foo/bar" => %r{\A/(#{x})/(.+)/bar(?:\b|\Z)}, + "/:foo|*bar" => %r{\A/(?:([^/.?]+)|(.+))(?:\b|\Z)}, }.each do |path, expected| define_method(:"test_to_non_anchored_regexp_#{Regexp.escape(path)}") do path = Pattern.build( diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 1f4e14aef6..f8d89def6a 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -284,7 +284,7 @@ module ActionDispatch def test_generate_missing_keys_no_matches_different_format_keys get "/:controller/:action/:name", to: "foo#bar" - primarty_parameters = { + primary_parameters = { id: 1, controller: "tasks", action: "show", @@ -297,9 +297,9 @@ module ActionDispatch missing_parameters = { missing_key => "task_1" } - request_parameters = primarty_parameters.merge(redirection_parameters).merge(missing_parameters) + request_parameters = primary_parameters.merge(redirection_parameters).merge(missing_parameters) - message = "No route matches #{Hash[request_parameters.sort_by { |k, v|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}" + message = "No route matches #{Hash[request_parameters.sort_by { |k, _|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}" error = assert_raises(ActionController::UrlGenerationError) do @formatter.generate( |