diff options
Diffstat (limited to 'actionpack')
153 files changed, 2943 insertions, 1052 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index e01f88e902..a30f178190 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,139 +1,95 @@ -* Make `assert_recognizes` to traverse mounted engines +* Purpose metadata for signed/encrypted cookies. - *Yuichiro Kaneko* + Rails can now thwart attacks that attempt to copy signed/encrypted value + of a cookie and use it as the value of another cookie. -* Remove deprecated `ActionController::ParamsParser::ParseError`. + It does so by stashing the cookie-name in the purpose field which is + then signed/encrypted along with the cookie value. Then, on a server-side + read, we verify the cookie-names and discard any attacked cookies. - *Rafael Mendonça França* + Enable `action_dispatch.use_cookies_with_metadata` to use this feature, which + writes cookies with the new purpose and expiry metadata embedded. -* Add `:allow_other_host` option to `redirect_back` method. - When `allow_other_host` is set to `false`, the `redirect_back` - will not allow a redirecting from a different host. - `allow_other_host` is `true` by default. - - *Tim Masliuchenko* - -* Add headless chrome support to System Tests. - - *Yuji Yaginuma* - -* Add ability to enable Early Hints for HTTP/2 - - If supported by the server, and enabled in Puma this allows H2 Early Hints to be used. - - The `javascript_include_tag` and the `stylesheet_link_tag` automatically add Early Hints if requested. - - *Eileen M. Uchitelle*, *Aaron Patterson* - -* Simplify cookies middleware with key rotation support - - Use the `rotate` method for both `MessageEncryptor` and - `MessageVerifier` to add key rotation support for encrypted and - signed cookies. This also helps simplify support for legacy cookie - security. - - *Michael J Coyne* - -* Use Capybara registered `:puma` server config. - - The Capybara registered `:puma` server ensures the puma server is run in process so - connection sharing and open request detection work correctly by default. - - *Thomas Walpole* - -* Cookies `:expires` option supports `ActiveSupport::Duration` object. - - cookies[:user_name] = { value: "assain", expires: 1.hour } - cookies[:key] = { value: "a yummy cookie", expires: 6.months } - - Pull Request: #30121 + Pull Request: #32937 *Assain Jaleel* -* Enforce signed/encrypted cookie expiry server side. - - Rails can thwart attacks by malicious clients that don't honor a cookie's expiry. - - It does so by stashing the expiry within the written cookie and relying on the - signing/encrypting to vouch that it hasn't been tampered with. Then on a - server-side read, the expiry is verified and any expired cookie is discarded. +* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. - Pull Request: #30121 - - *Assain Jaleel* + `respond_to` can match multiple types and lead to undefined behavior when + multiple invocations are made and the types do not match: -* Make `take_failed_screenshot` work within engine. + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end - Fixes #30405. + *Patrick Toomey* - *Yuji Yaginuma* +* `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile. -* Deprecate `ActionDispatch::TestResponse` response aliases + This allows uploaded file objects to be passed directly to `File.read` + without raising a `TypeError`: - `#success?`, `#missing?` & `#error?` are not supported by the actual - `ActionDispatch::Response` object and can produce false-positives. Instead, - use the response helpers provided by `Rack::Response`. + uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: tmp_file) + File.read(uploaded_file) - *Trevor Wistaff* + *Aaron Kromer* -* Protect from forgery by default +* Pass along arguments to underlying `get` method in `follow_redirect!` - Rather than protecting from forgery in the generated `ApplicationController`, - add it to `ActionController::Base` depending on - `config.action_controller.default_protect_from_forgery`. This configuration - defaults to false to support older versions which have removed it from their - `ApplicationController`, but is set to true for Rails 5.2. + Now all arguments passed to `follow_redirect!` are passed to the underlying + `get` method. This for example allows to set custom headers for the + redirection request to the server. - *Lisa Ugray* + follow_redirect!(params: { foo: :bar }) -* Fallback `ActionController::Parameters#to_s` to `Hash#to_s`. + *Remo Fritzsche* - *Kir Shatrov* +* Introduce a new error page to when the implicit render page is accessed in the browser. -* `driven_by` now registers poltergeist and capybara-webkit. + Now instead of showing an error page that with exception and backtraces we now show only + one informative page. - If poltergeist or capybara-webkit are set as drivers is set for System Tests, - `driven_by` will register the driver and set additional options passed via - the `:options` parameter. + *Vinicius Stock* - Refer to the respective driver's documentation to see what options can be passed. +* Introduce ActionDispatch::DebugExceptions.register_interceptor - *Mario Chavez* + Exception aware plugin authors can use the newly introduced + `.register_interceptor` method to get the processed exception, instead of + monkey patching DebugExceptions. -* AEAD encrypted cookies and sessions with GCM. + ActionDispatch::DebugExceptions.register_interceptor do |request, exception| + HypoteticalPlugin.capture_exception(request, exception) + end - Encrypted cookies now use AES-GCM which couples authentication and - encryption in one faster step and produces shorter ciphertexts. Cookies - encrypted using AES in CBC HMAC mode will be seamlessly upgraded when - this new mode is enabled via the - `action_dispatch.use_authenticated_cookie_encryption` configuration value. + *Genadi Samokovarov* - *Michael J Coyne* +* Output only one Content-Security-Policy nonce header value per request. -* Change the cache key format for fragments to make it easier to debug key churn. The new format is: + Fixes #32597. - views/template/action.html.erb:7a1156131a6928cb0026877f8b749ac9/projects/123 - ^template path ^template tree digest ^class ^id + *Andrey Novikov*, *Andrew White* - *DHH* +* Move default headers configuration into their own module that can be included in controllers. -* Add support for recyclable cache keys with fragment caching. This uses the new versioned entries in the - `ActiveSupport::Cache` stores and relies on the fact that Active Record has split `#cache_key` and `#cache_version` - to support it. + *Kevin Deisz* - *DHH* +* Add method `dig` to `session`. -* Add `action_controller_api` and `action_controller_base` load hooks to be called in `ActiveSupport.on_load` + *claudiob*, *Takumi Shotoku* - `ActionController::Base` and `ActionController::API` have differing implementations. This means that - the one umbrella hook `action_controller` is not able to address certain situations where a method - may not exist in a certain implementation. +* Controller level `force_ssl` has been deprecated in favor of + `config.force_ssl`. - This is fixed by adding two new hooks so you can target `ActionController::Base` vs `ActionController::API` + *Derek Prior* - Fixes #27013. +* Rails 6 requires Ruby 2.4.1 or newer. - *Julian Nadeau* + *Jeremy Daer* -Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actionpack/CHANGELOG.md) for previous changes. +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 ac810e86d0..1cb3add0fc 100644 --- a/actionpack/MIT-LICENSE +++ b/actionpack/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2017 David Heinemeier Hansson +Copyright (c) 2004-2018 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 93b2a0932a..f56230ffa0 100644 --- a/actionpack/README.rdoc +++ b/actionpack/README.rdoc @@ -30,7 +30,7 @@ The latest version of Action Pack can be installed with RubyGems: $ gem install actionpack -Source code can be downloaded as part of the Rails project on GitHub +Source code can be downloaded as part of the Rails project on GitHub: * https://github.com/rails/rails/tree/master/actionpack @@ -44,11 +44,11 @@ Action Pack is released under the MIT license: == Support -API documentation is at +API documentation is at: * http://api.rubyonrails.org -Bug reports can be filed for the Ruby on Rails project here: +Bug reports for the Ruby on Rails project can be filed here: * https://github.com/rails/rails/issues diff --git a/actionpack/Rakefile b/actionpack/Rakefile index 4dd7c59ce8..e99eb1723a 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -28,7 +28,7 @@ namespace :test do end task :lines do - load File.expand_path("..", __dir__) + "/tools/line_statistics" + load File.expand_path("../tools/line_statistics", __dir__) files = FileList["lib/**/*.rb"] CodeTools::LineStatistics.new(files).print_loc end diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 33d42e69d8..1dc8abf746 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -9,7 +9,7 @@ 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.2.2" + s.required_ruby_version = ">= 2.4.1" s.license = "MIT" diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 146d17cf40..42bab411d2 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -103,6 +103,10 @@ module AbstractController # :call-seq: before_action(names, block) # # Append a callback before actions. See _insert_callbacks for parameter details. + # + # If the callback renders or redirects, the action will not run. If there + # are additional callbacks scheduled to run after that callback, they are + # also cancelled. ## # :method: prepend_before_action @@ -110,6 +114,10 @@ module AbstractController # :call-seq: prepend_before_action(names, block) # # Prepend a callback before actions. See _insert_callbacks for parameter details. + # + # If the callback renders or redirects, the action will not run. If there + # are additional callbacks scheduled to run after that callback, they are + # also cancelled. ## # :method: skip_before_action @@ -124,6 +132,10 @@ module AbstractController # :call-seq: append_before_action(names, block) # # Append a callback before actions. See _insert_callbacks for parameter details. + # + # If the callback renders or redirects, the action will not run. If there + # are additional callbacks scheduled to run after that callback, they are + # also cancelled. ## # :method: after_action diff --git a/actionpack/lib/abstract_controller/collector.rb b/actionpack/lib/abstract_controller/collector.rb index 297ec5ca40..d4a078ab32 100644 --- a/actionpack/lib/abstract_controller/collector.rb +++ b/actionpack/lib/abstract_controller/collector.rb @@ -26,7 +26,7 @@ module AbstractController def method_missing(symbol, &block) unless mime_constant = Mime[symbol] raise NoMethodError, "To respond to a custom format, register it as a MIME type first: " \ - "http://guides.rubyonrails.org/action_controller_overview.html#restful-downloads. " \ + "https://guides.rubyonrails.org/action_controller_overview.html#restful-downloads. " \ "If you meant to respond to a variant like :tablet or :phone, not a custom format, " \ "be sure to nest your variant response within a format response: " \ "format.html { |html| html.tablet { ... } }" diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index 35b462bc92..3191584770 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -17,7 +17,7 @@ module AbstractController @path = "helpers/#{path}.rb" set_backtrace error.backtrace - if error.path =~ /^#{path}(\.rb)?$/ + if /^#{path}(\.rb)?$/.match?(error.path) super("Missing helper file helpers/%s.rb" % path) else raise error diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index bd19b8cd5d..29d61c3ceb 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -22,8 +22,10 @@ module ActionController autoload_under "metal" do autoload :ConditionalGet + autoload :ContentSecurityPolicy autoload :Cookies autoload :DataStreaming + autoload :DefaultHeaders autoload :EtagWithTemplateDigest autoload :EtagWithFlash autoload :Flash diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index b192e496de..93ffff1bd6 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -122,6 +122,7 @@ module ActionController ForceSSL, DataStreaming, + DefaultHeaders, # Before callbacks should also be executed as early as possible, so # also include them at the bottom. diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index b73269871b..2e565d5d44 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -78,7 +78,7 @@ module ActionController # # You can retrieve it again through the same hash: # - # Hello #{session[:person]} + # "Hello #{session[:person]}" # # For removing objects from the session, you can either assign a single key to +nil+: # @@ -225,12 +225,14 @@ module ActionController Flash, FormBuilder, RequestForgeryProtection, + ContentSecurityPolicy, ForceSSL, Streaming, DataStreaming, HttpAuthentication::Basic::ControllerMethods, HttpAuthentication::Digest::ControllerMethods, HttpAuthentication::Token::ControllerMethods, + DefaultHeaders, # Before callbacks should also be executed as early as possible, so # also include them at the bottom. @@ -263,12 +265,6 @@ module ActionController PROTECTED_IVARS end - def self.make_response!(request) - ActionDispatch::Response.create.tap do |res| - res.request = request - end - end - ActiveSupport.run_load_hooks(:action_controller_base, self) ActiveSupport.run_load_hooks(:action_controller, self) end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 457884ea08..f875aa5e6b 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -230,18 +230,16 @@ module ActionController # Returns a Rack endpoint for the given action name. def self.action(name) + app = lambda { |env| + req = ActionDispatch::Request.new(env) + res = make_response! req + new.dispatch(name, req, res) + } + if middleware_stack.any? - middleware_stack.build(name) do |env| - req = ActionDispatch::Request.new(env) - res = make_response! req - new.dispatch(name, req, res) - end + middleware_stack.build(name, app) else - lambda { |env| - req = ActionDispatch::Request.new(env) - res = make_response! req - new.dispatch(name, req, res) - } + app end end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 06b6a95ff8..4be4557e2c 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -235,7 +235,9 @@ module ActionController response.cache_control.merge!( max_age: seconds, public: options.delete(:public), - must_revalidate: options.delete(:must_revalidate) + must_revalidate: options.delete(:must_revalidate), + stale_while_revalidate: options.delete(:stale_while_revalidate), + stale_if_error: options.delete(:stale_if_error), ) options.delete(:private) diff --git a/actionpack/lib/action_controller/metal/content_security_policy.rb b/actionpack/lib/action_controller/metal/content_security_policy.rb new file mode 100644 index 0000000000..b8fab4ebe3 --- /dev/null +++ b/actionpack/lib/action_controller/metal/content_security_policy.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module ActionController #:nodoc: + module ContentSecurityPolicy + # TODO: Documentation + extend ActiveSupport::Concern + + include AbstractController::Helpers + include AbstractController::Callbacks + + included do + helper_method :content_security_policy? + helper_method :content_security_policy_nonce + end + + module ClassMethods + def content_security_policy(enabled = true, **options, &block) + before_action(options) do + if block_given? + policy = current_content_security_policy + yield policy + request.content_security_policy = policy + end + + unless enabled + request.content_security_policy = nil + end + end + end + + def content_security_policy_report_only(report_only = true, **options) + before_action(options) do + request.content_security_policy_report_only = report_only + end + end + end + + private + + def content_security_policy? + request.content_security_policy + end + + def content_security_policy_nonce + request.content_security_policy_nonce + end + + def current_content_security_policy + request.content_security_policy.try(:clone) || ActionDispatch::ContentSecurityPolicy.new + end + end +end diff --git a/actionpack/lib/action_controller/metal/default_headers.rb b/actionpack/lib/action_controller/metal/default_headers.rb new file mode 100644 index 0000000000..eef0602fcd --- /dev/null +++ b/actionpack/lib/action_controller/metal/default_headers.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ActionController + # Allows configuring default headers that will be automatically merged into + # each response. + module DefaultHeaders + extend ActiveSupport::Concern + + module ClassMethods + def make_response!(request) + ActionDispatch::Response.create.tap do |res| + res.request = request + end + end + end + end +end diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index a65857d6ef..30034be018 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -22,7 +22,7 @@ module ActionController end end - class ActionController::UrlGenerationError < ActionControllerError #:nodoc: + class UrlGenerationError < ActionControllerError #:nodoc: end class MethodNotAllowed < ActionControllerError #:nodoc: @@ -50,4 +50,25 @@ module ActionController class UnknownFormat < ActionControllerError #:nodoc: end + + # Raised when a nested respond_to is triggered and the content types of each + # are incompatible. For exampe: + # + # respond_to do |outer_type| + # outer_type.js do + # respond_to do |inner_type| + # inner_type.html { render body: "HTML" } + # end + # end + # end + class RespondToMismatchError < ActionControllerError + DEFAULT_MESSAGE = "respond_to was called multiple times and matched with conflicting formats in this action. Please note that you may only call respond_to and match on a single format per action." + + def initialize(message = nil) + super(message || DEFAULT_MESSAGE) + end + end + + class MissingExactTemplate < UnknownFormat #:nodoc: + end end diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 0ba1f9f783..8d53a30e93 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -4,18 +4,10 @@ require "active_support/core_ext/hash/except" require "active_support/core_ext/hash/slice" module ActionController - # This module provides a method which will redirect the browser to use the secured HTTPS - # protocol. This will ensure that users' sensitive information will be - # transferred safely over the internet. You _should_ always force the browser - # to use HTTPS when you're transferring sensitive information such as - # user authentication, account information, or credit card information. - # - # Note that if you are really concerned about your application security, - # you might consider using +config.force_ssl+ in your config file instead. - # That will ensure all the data is transferred via HTTPS, and will - # prevent the user from getting their session hijacked when accessing the - # site over unsecured HTTP protocol. - module ForceSSL + # This module is deprecated in favor of +config.force_ssl+ in your environment + # config file. This will ensure all communication to non-whitelisted endpoints + # served by your application occurs over HTTPS. + module ForceSSL # :nodoc: extend ActiveSupport::Concern include AbstractController::Callbacks @@ -23,45 +15,17 @@ module ActionController URL_OPTIONS = [:protocol, :host, :domain, :subdomain, :port, :path] REDIRECT_OPTIONS = [:status, :flash, :alert, :notice] - module ClassMethods - # Force the request to this particular controller or specified actions to be - # through the HTTPS protocol. - # - # If you need to disable this for any reason (e.g. development) then you can use - # an +:if+ or +:unless+ condition. - # - # class AccountsController < ApplicationController - # force_ssl if: :ssl_configured? - # - # def ssl_configured? - # !Rails.env.development? - # end - # end - # - # ==== URL Options - # You can pass any of the following options to affect the redirect url - # * <tt>host</tt> - Redirect to a different host name - # * <tt>subdomain</tt> - Redirect to a different subdomain - # * <tt>domain</tt> - Redirect to a different domain - # * <tt>port</tt> - Redirect to a non-standard port - # * <tt>path</tt> - Redirect to a different path - # - # ==== Redirect Options - # You can pass any of the following options to affect the redirect status and response - # * <tt>status</tt> - Redirect with a custom status (default is 301 Moved Permanently) - # * <tt>flash</tt> - Set a flash message when redirecting - # * <tt>alert</tt> - Set an alert message when redirecting - # * <tt>notice</tt> - Set a notice message when redirecting - # - # ==== Action Options - # You can pass any of the following options to affect the before_action callback - # * <tt>only</tt> - The callback should be run only for this action - # * <tt>except</tt> - The callback should be run for all actions except this action - # * <tt>if</tt> - A symbol naming an instance method or a proc; the - # callback will be called only when it returns a true value. - # * <tt>unless</tt> - A symbol naming an instance method or a proc; the - # callback will be called only when it returns a false value. + module ClassMethods # :nodoc: def force_ssl(options = {}) + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + Controller-level `force_ssl` is deprecated and will be removed from + Rails 6.1. Please enable `config.force_ssl` in your environment + configuration to enable the ActionDispatch::SSL middleware to more + fully enforce that your application communicate over HTTPS. If needed, + you can use `config.ssl_options` to exempt matching endpoints from + being redirected to HTTPS. + MESSAGE + action_options = options.slice(*ACTION_OPTIONS) redirect_options = options.except(*ACTION_OPTIONS) before_action(action_options) do @@ -70,11 +34,6 @@ module ActionController end end - # Redirect the existing request to use the HTTPS protocol. - # - # ==== Parameters - # * <tt>host_or_options</tt> - Either a host name or any of the url and - # redirect options available to the <tt>force_ssl</tt> method. def force_ssl_redirect(host_or_options = nil) unless request.ssl? options = { diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index bac9bc5e5f..3c84bebb85 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -38,7 +38,7 @@ module ActionController self.response_body = "" if include_content?(response_code) - self.content_type = content_type || (Mime[formats.first] if formats) + self.content_type = content_type || (Mime[formats.first] if formats) || Mime[:html] response.charset = false end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0c8132684a..a871ccd533 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -56,8 +56,9 @@ module ActionController # In your integration tests, you can do something like this: # # def test_access_granted_from_xml - # @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(users(:dhh).name, users(:dhh).password) - # get "/notes/1.xml" + # authorization = ActionController::HttpAuthentication::Basic.encode_credentials(users(:dhh).name, users(:dhh).password) + # + # get "/notes/1.xml", headers: { 'HTTP_AUTHORIZATION' => authorization } # # assert_equal 200, status # end @@ -72,10 +73,10 @@ module ActionController 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 `variable_size_secure_compare` so that length information + # uses `secure_compare` so that length information # isn't leaked. - ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) & - ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password]) + ActiveSupport::SecurityUtils.secure_compare(name, options[:name]) & + ActiveSupport::SecurityUtils.secure_compare(password, options[:password]) end end end @@ -350,10 +351,7 @@ module ActionController # authenticate_or_request_with_http_token do |token, options| # # Compare the tokens in a time-constant manner, to mitigate # # timing attacks. - # ActiveSupport::SecurityUtils.secure_compare( - # ::Digest::SHA256.hexdigest(token), - # ::Digest::SHA256.hexdigest(TOKEN) - # ) + # ActiveSupport::SecurityUtils.secure_compare(token, TOKEN) # end # end # end @@ -392,10 +390,9 @@ module ActionController # In your integration tests, you can do something like this: # # def test_access_granted_from_xml - # get( - # "/notes/1.xml", nil, - # 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Token.encode_credentials(users(:dhh).token) - # ) + # authorization = ActionController::HttpAuthentication::Token.encode_credentials(users(:dhh).token) + # + # get "/notes/1.xml", headers: { 'HTTP_AUTHORIZATION' => authorization } # # assert_equal 200, status # end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index ac0c127cdc..d3bb58f48b 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -41,18 +41,8 @@ module ActionController raise ActionController::UnknownFormat, message elsif interactive_browser_request? - message = "#{self.class.name}\##{action_name} is missing a template " \ - "for this request format and variant.\n\n" \ - "request.formats: #{request.formats.map(&:to_s).inspect}\n" \ - "request.variant: #{request.variant.inspect}\n\n" \ - "NOTE! For XHR/Ajax or API requests, this action would normally " \ - "respond with 204 No Content: an empty white screen. Since you're " \ - "loading it in a web browser, we assume that you expected to " \ - "actually render a template, not nothing, so we're showing an " \ - "error to be extra-clear. If you expect 204 No Content, carry on. " \ - "That's what you'll get from an XHR or API request. Give it a shot." - - raise ActionController::UnknownFormat, message + message = "#{self.class.name}\##{action_name} is missing a template for request formats: #{request.formats.map(&:to_s).join(',')}" + raise ActionController::MissingExactTemplate, message else logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger super diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 2233b93406..2b55b9347c 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -197,6 +197,9 @@ module ActionController #:nodoc: yield collector if block_given? if format = collector.negotiate_format(request) + if content_type && content_type != format + raise ActionController::RespondToMismatchError + end _process_format(format) _set_rendered_content_type format response = collector.response diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 8de57f9199..4c2b5120eb 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -68,7 +68,7 @@ module ActionController # if possible, otherwise redirects to the provided default fallback # location. # - # The referrer information is pulled from the HTTP `Referer` (sic) header on + # The referrer information is pulled from the HTTP +Referer+ (sic) header on # the request. This is an optional header and its presence on the request is # subject to browser security settings and user preferences. If the request # is missing this header, the <tt>fallback_location</tt> will be used. @@ -82,8 +82,8 @@ module ActionController # redirect_back fallback_location: '/', allow_other_host: false # # ==== Options - # * <tt>:fallback_location</tt> - The default fallback location that will be used on missing `Referer` header. - # * <tt>:allow_other_host</tt> - Allows or disallow redirection to the host that is different to the current host + # * <tt>:fallback_location</tt> - The default fallback location that will be used on missing +Referer+ header. + # * <tt>:allow_other_host</tt> - Allow or disallow redirection to the host that is different to the current host, defaults to true. # # All other options that can be passed to <tt>redirect_to</tt> are accepted as # options and the behavior is identical. diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index bd133f24a1..7ed7b9d546 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -17,7 +17,7 @@ module ActionController #:nodoc: # access. When a request reaches your application, \Rails verifies the received # token with the token in the session. All requests are checked except GET requests # as these should be idempotent. Keep in mind that all session-oriented requests - # should be CSRF protected, including JavaScript and HTML requests. + # are CSRF protected by default, including JavaScript and HTML requests. # # Since HTML and JavaScript requests are typically made from the browser, we # need to ensure to verify request authenticity for the web browser. We can @@ -30,16 +30,23 @@ module ActionController #:nodoc: # URL on your site. When your JavaScript response loads on their site, it executes. # With carefully crafted JavaScript on their end, sensitive data in your JavaScript # response may be extracted. To prevent this, only XmlHttpRequest (known as XHR or - # Ajax) requests are allowed to make GET requests for JavaScript responses. + # Ajax) requests are allowed to make requests for JavaScript responses. # - # It's important to remember that XML or JSON requests are also affected and if - # you're building an API you should change forgery protection method in + # It's important to remember that XML or JSON requests are also checked by default. If + # you're building an API or an SPA you could change forgery protection method in # <tt>ApplicationController</tt> (by default: <tt>:exception</tt>): # # class ApplicationController < ActionController::Base # protect_from_forgery unless: -> { request.format.json? } # end # + # It is generally safe to exclude XHR requests from CSRF protection + # (like the code snippet above does), because XHR requests can only be made from + # the same origin. Note however that any cross-origin third party domain + # allowed via {CORS}[https://en.wikipedia.org/wiki/Cross-origin_resource_sharing] + # will also be able to create XHR requests. Be sure to check your + # CORS whitelist before disabling forgery protection for XHR. + # # CSRF protection is turned on with the <tt>protect_from_forgery</tt> method. # By default <tt>protect_from_forgery</tt> protects your session with # <tt>:null_session</tt> method, which provides an empty session @@ -54,7 +61,7 @@ module ActionController #:nodoc: # <tt>csrf_meta_tags</tt> in the HTML +head+. # # Learn more about CSRF attacks and securing your application in the - # {Ruby on Rails Security Guide}[http://guides.rubyonrails.org/security.html]. + # {Ruby on Rails Security Guide}[https://guides.rubyonrails.org/security.html]. module RequestForgeryProtection extend ActiveSupport::Concern @@ -216,7 +223,7 @@ module ActionController #:nodoc: # The actual before_action that is used to verify the CSRF token. # Don't override this directly. Provide your own forgery protection # strategy instead. If you override, you'll disable same-origin - # `<script>` verification. + # <tt><script></tt> verification. # # Lean on the protect_from_forgery declaration to mark which actions are # due for same-origin request verification. If protect_from_forgery is @@ -250,7 +257,7 @@ module ActionController #:nodoc: private_constant :CROSS_ORIGIN_JAVASCRIPT_WARNING # :startdoc: - # If `verify_authenticity_token` was run (indicating that we have + # If +verify_authenticity_token+ was run (indicating that we have # forgery protection enabled for this request) then also verify that # we aren't serving an unauthorized cross-origin response. def verify_same_origin_request # :doc: @@ -267,7 +274,7 @@ module ActionController #:nodoc: @marked_for_same_origin_verification = request.get? end - # If the `verify_authenticity_token` before_action ran, verify that + # If the +verify_authenticity_token+ before_action ran, verify that # JavaScript responses are only served to same-origin GET requests. def marked_for_same_origin_verification? # :doc: @marked_for_same_origin_verification ||= false @@ -275,7 +282,7 @@ module ActionController #:nodoc: # Check for cross-origin JavaScript responses. def non_xhr_javascript_response? # :doc: - content_type =~ %r(\Atext/javascript) && !request.xhr? + content_type =~ %r(\A(?:text|application)/javascript) && !request.xhr? end AUTHENTICITY_TOKEN_LENGTH = 32 @@ -369,7 +376,7 @@ module ActionController #:nodoc: end def compare_with_real_token(token, session) # :doc: - ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session)) end def valid_per_form_csrf_token?(token, session) # :doc: @@ -380,7 +387,7 @@ module ActionController #:nodoc: request.request_method ) - ActiveSupport::SecurityUtils.secure_compare(token, correct_token) + ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, correct_token) else false end @@ -400,9 +407,14 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) # :doc: - s2_bytes = s2.bytes - s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } - s2_bytes.pack("C*") + s2 = s2.dup + size = s1.bytesize + i = 0 + while i < size + s2.setbyte(i, s1.getbyte(i) ^ s2.getbyte(i)) + i += 1 + end + s2 end # The form's authenticity parameter. Override to provide your own. @@ -415,11 +427,21 @@ module ActionController #:nodoc: allow_forgery_protection end + NULL_ORIGIN_MESSAGE = <<~MSG + 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. + 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 + # Checks if the request originated from the same origin by looking at the # Origin header. def valid_request_origin? # :doc: if forgery_protection_origin_check # We accept blank origin headers because some user agents don't send it. + raise InvalidAuthenticityToken, NULL_ORIGIN_MESSAGE if request.origin == "null" request.origin.nil? || request.origin == request.base_url else true diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 0b1598bf1b..8dc01a5eb9 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -183,7 +183,7 @@ module ActionController #:nodoc: # unicorn_rails --config-file unicorn.config.rb # # You may also want to configure other parameters like <tt>:tcp_nodelay</tt>. - # Please check its documentation for more information: http://unicorn.bogomips.org/Unicorn/Configurator.html#method-i-listen + # Please check its documentation for more information: https://bogomips.org/unicorn/Unicorn/Configurator.html#method-i-listen # # If you are using Unicorn with NGINX, you may need to tweak NGINX. # Streaming should work out of the box on Rainbows. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index ef7c4c4c16..7af29f8dca 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/hash/indifferent_access" -require "active_support/core_ext/hash/transform_values" require "active_support/core_ext/array/wrap" require "active_support/core_ext/string/filters" require "active_support/core_ext/object/to_query" @@ -335,7 +334,7 @@ module ActionController # the same way as <tt>Hash#each_pair</tt>. def each_pair(&block) @parameters.each_pair do |key, value| - yield key, convert_hashes_to_parameters(key, value) + yield [key, convert_hashes_to_parameters(key, value)] end end alias_method :each, :each_pair @@ -375,7 +374,7 @@ module ActionController # Person.new(params) # => #<Person id: nil, name: "Francesco"> def permit! each_pair do |key, value| - Array.wrap(value).each do |v| + Array.wrap(value).flatten.each do |v| v.permit! if v.respond_to? :permit! end end @@ -561,12 +560,14 @@ module ActionController # Returns a parameter for the given +key+. If the +key+ # can't be found, there are several options: With no other arguments, # it will raise an <tt>ActionController::ParameterMissing</tt> error; - # if more arguments are given, then that will be returned; if a block + # if a second argument is given, then that is returned (converted to an + # instance of ActionController::Parameters if possible); if a block # is given, then that will be run and its result returned. # # params = ActionController::Parameters.new(person: { name: "Francesco" }) # params.fetch(:person) # => <ActionController::Parameters {"name"=>"Francesco"} permitted: false> # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none + # params.fetch(:none, {}) # => <ActionController::Parameters {} permitted: false> # params.fetch(:none, "Francesco") # => "Francesco" # params.fetch(:none) { "Francesco" } # => "Francesco" def fetch(key, *args) @@ -581,19 +582,18 @@ module ActionController ) end - if Hash.method_defined?(:dig) - # Extracts the nested parameter from the given +keys+ by calling +dig+ - # at each step. Returns +nil+ if any intermediate step is +nil+. - # - # params = ActionController::Parameters.new(foo: { bar: { baz: 1 } }) - # params.dig(:foo, :bar, :baz) # => 1 - # params.dig(:foo, :zot, :xyz) # => nil - # - # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) - # params2.dig(:foo, 1) # => 11 - def dig(*keys) - convert_value_to_parameters(@parameters.dig(*keys)) - end + # Extracts the nested parameter from the given +keys+ by calling +dig+ + # at each step. Returns +nil+ if any intermediate step is +nil+. + # + # params = ActionController::Parameters.new(foo: { bar: { baz: 1 } }) + # params.dig(:foo, :bar, :baz) # => 1 + # params.dig(:foo, :zot, :xyz) # => nil + # + # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) + # params2.dig(:foo, 1) # => 11 + def dig(*keys) + convert_hashes_to_parameters(keys.first, @parameters[keys.first]) + @parameters.dig(*keys) end # Returns a new <tt>ActionController::Parameters</tt> instance that @@ -639,20 +639,18 @@ module ActionController # params = ActionController::Parameters.new(a: 1, b: 2, c: 3) # params.transform_values { |x| x * 2 } # # => <ActionController::Parameters {"a"=>2, "b"=>4, "c"=>6} permitted: false> - def transform_values(&block) - if block - new_instance_with_inherited_permitted_status( - @parameters.transform_values(&block) - ) - else - @parameters.transform_values - end + def transform_values + return to_enum(:transform_values) unless block_given? + new_instance_with_inherited_permitted_status( + @parameters.transform_values { |v| yield convert_value_to_parameters(v) } + ) end # Performs values transformation and returns the altered # <tt>ActionController::Parameters</tt> instance. - def transform_values!(&block) - @parameters.transform_values!(&block) + def transform_values! + return to_enum(:transform_values!) unless block_given? + @parameters.transform_values! { |v| yield convert_value_to_parameters(v) } self end @@ -795,9 +793,7 @@ module ActionController protected attr_reader :parameters - def permitted=(new_permitted) - @permitted = new_permitted - end + attr_writer :permitted def fields_for_style? @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index 49c5b782f0..2b4559c760 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -71,6 +71,21 @@ module ActionController end # Render templates with any options from ActionController::Base#render_to_string. + # + # The primary options are: + # * <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>: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>. + # * <tt>:json</tt> - Renders the provided hash or object in JSON. You don't + # need to call <tt>.to_json</tt> on the object you want to render. + # * <tt>:body</tt> - Renders provided text and sets content type of <tt>text/plain</tt>. + # + # If no <tt>options</tt> hash is passed or if <tt>:update</tt> is specified, the default is + # to render a partial and use the second parameter as the locals hash. def render(*args) raise "missing controller" unless controller diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 4b408750a4..5d784ceb31 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -256,7 +256,7 @@ module ActionController # # def test_create # json = {book: { title: "Love Hina" }}.to_json - # post :create, json + # post :create, body: json # end # # == Special instance variables @@ -460,10 +460,6 @@ module ActionController def process(action, method: "GET", params: {}, session: nil, body: nil, flash: {}, format: nil, xhr: false, as: nil) check_required_ivars - if body - @request.set_header "RAW_POST_DATA", body - end - http_method = method.to_s.upcase @html_document = nil @@ -478,6 +474,10 @@ module ActionController @response.request = @request @controller.recycle! + if body + @request.set_header "RAW_POST_DATA", body + end + @request.set_header "REQUEST_METHOD", http_method if as @@ -604,6 +604,8 @@ module ActionController env.delete "action_dispatch.request.query_parameters" env.delete "action_dispatch.request.request_parameters" env["rack.input"] = StringIO.new + env.delete "CONTENT_LENGTH" + env.delete "RAW_POST_DATA" env end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 34937f3229..0822cdc0a6 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true #-- -# Copyright (c) 2004-2017 David Heinemeier Hansson +# Copyright (c) 2004-2018 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -42,6 +42,7 @@ module ActionDispatch eager_autoload do autoload_under "http" do + autoload :ContentSecurityPolicy autoload :Request autoload :Response end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 3328ce17a0..a7c7cfc1e5 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -133,7 +133,7 @@ module ActionDispatch end def generate_strong_etag(validators) - %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(validators))}") + %("#{ActiveSupport::Digest.hexdigest(ActiveSupport::Cache.expand_cache_key(validators))}") end def cache_control_segments @@ -202,13 +202,17 @@ module ActionDispatch self._cache_control = _cache_control + ", #{control[:extras].join(', ')}" end else - extras = control[:extras] + extras = control[:extras] max_age = control[:max_age] + stale_while_revalidate = control[:stale_while_revalidate] + stale_if_error = control[:stale_if_error] options = [] options << "max-age=#{max_age.to_i}" if max_age options << (control[:public] ? PUBLIC : PRIVATE) options << MUST_REVALIDATE if control[:must_revalidate] + options << "stale-while-revalidate=#{stale_while_revalidate.to_i}" if stale_while_revalidate + options << "stale-if-error=#{stale_if_error.to_i}" if stale_if_error options.concat(extras) if extras self._cache_control = options.join(", ") diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb new file mode 100644 index 0000000000..35041fd072 --- /dev/null +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -0,0 +1,271 @@ +# frozen_string_literal: true + +require "active_support/core_ext/object/deep_dup" + +module ActionDispatch #:nodoc: + class ContentSecurityPolicy + class Middleware + CONTENT_TYPE = "Content-Type".freeze + POLICY = "Content-Security-Policy".freeze + POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only".freeze + + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new env + _, headers, _ = response = @app.call(env) + + return response unless html_response?(headers) + return response if policy_present?(headers) + + if policy = request.content_security_policy + nonce = request.content_security_policy_nonce + headers[header_name(request)] = policy.build(request.controller_instance, nonce) + end + + response + end + + private + + def html_response?(headers) + if content_type = headers[CONTENT_TYPE] + content_type =~ /html/ + end + end + + def header_name(request) + if request.content_security_policy_report_only + POLICY_REPORT_ONLY + else + POLICY + end + end + + def policy_present?(headers) + headers[POLICY] || headers[POLICY_REPORT_ONLY] + end + end + + module Request + POLICY = "action_dispatch.content_security_policy".freeze + POLICY_REPORT_ONLY = "action_dispatch.content_security_policy_report_only".freeze + NONCE_GENERATOR = "action_dispatch.content_security_policy_nonce_generator".freeze + NONCE = "action_dispatch.content_security_policy_nonce".freeze + + def content_security_policy + get_header(POLICY) + end + + def content_security_policy=(policy) + set_header(POLICY, policy) + end + + def content_security_policy_report_only + get_header(POLICY_REPORT_ONLY) + end + + def content_security_policy_report_only=(value) + set_header(POLICY_REPORT_ONLY, value) + end + + def content_security_policy_nonce_generator + get_header(NONCE_GENERATOR) + end + + def content_security_policy_nonce_generator=(generator) + set_header(NONCE_GENERATOR, generator) + end + + def content_security_policy_nonce + if content_security_policy_nonce_generator + if nonce = get_header(NONCE) + nonce + else + set_header(NONCE, generate_content_security_policy_nonce) + end + end + end + + private + + def generate_content_security_policy_nonce + content_security_policy_nonce_generator.call(self) + end + end + + MAPPINGS = { + self: "'self'", + unsafe_eval: "'unsafe-eval'", + unsafe_inline: "'unsafe-inline'", + none: "'none'", + http: "http:", + https: "https:", + data: "data:", + mediastream: "mediastream:", + blob: "blob:", + filesystem: "filesystem:", + report_sample: "'report-sample'", + strict_dynamic: "'strict-dynamic'", + ws: "ws:", + wss: "wss:" + }.freeze + + DIRECTIVES = { + base_uri: "base-uri", + child_src: "child-src", + connect_src: "connect-src", + default_src: "default-src", + font_src: "font-src", + form_action: "form-action", + frame_ancestors: "frame-ancestors", + frame_src: "frame-src", + img_src: "img-src", + manifest_src: "manifest-src", + media_src: "media-src", + object_src: "object-src", + prefetch_src: "prefetch-src", + script_src: "script-src", + style_src: "style-src", + worker_src: "worker-src" + }.freeze + + NONCE_DIRECTIVES = %w[script-src].freeze + + private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES + + attr_reader :directives + + def initialize + @directives = {} + yield self if block_given? + end + + def initialize_copy(other) + @directives = other.directives.deep_dup + end + + DIRECTIVES.each do |name, directive| + define_method(name) do |*sources| + if sources.first + @directives[directive] = apply_mappings(sources) + else + @directives.delete(directive) + end + end + end + + def block_all_mixed_content(enabled = true) + if enabled + @directives["block-all-mixed-content"] = true + else + @directives.delete("block-all-mixed-content") + end + end + + def plugin_types(*types) + if types.first + @directives["plugin-types"] = types + else + @directives.delete("plugin-types") + end + end + + def report_uri(uri) + @directives["report-uri"] = [uri] + end + + def require_sri_for(*types) + if types.first + @directives["require-sri-for"] = types + else + @directives.delete("require-sri-for") + end + end + + def sandbox(*values) + if values.empty? + @directives["sandbox"] = true + elsif values.first + @directives["sandbox"] = values + else + @directives.delete("sandbox") + end + end + + def upgrade_insecure_requests(enabled = true) + if enabled + @directives["upgrade-insecure-requests"] = true + else + @directives.delete("upgrade-insecure-requests") + end + end + + def build(context = nil, nonce = nil) + build_directives(context, nonce).compact.join("; ") + end + + private + def apply_mappings(sources) + sources.map do |source| + case source + when Symbol + apply_mapping(source) + when String, Proc + source + else + raise ArgumentError, "Invalid content security policy source: #{source.inspect}" + end + end + end + + def apply_mapping(source) + MAPPINGS.fetch(source) do + raise ArgumentError, "Unknown content security policy source mapping: #{source.inspect}" + end + end + + def build_directives(context, nonce) + @directives.map do |directive, sources| + if sources.is_a?(Array) + if nonce && nonce_directive?(directive) + "#{directive} #{build_directive(sources, context).join(' ')} 'nonce-#{nonce}'" + else + "#{directive} #{build_directive(sources, context).join(' ')}" + end + elsif sources + directive + else + nil + end + end + end + + def build_directive(sources, context) + sources.map { |source| resolve_source(source, context) } + end + + def resolve_source(source, context) + case source + when String + source + when Symbol + source.to_s + when Proc + if context.nil? + raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}" + else + context.instance_exec(&source) + end + else + raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" + end + end + + def nonce_directive?(directive) + NONCE_DIRECTIVES.include?(directive) + end + end +end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index ec86b8bc47..ec012ad02d 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -9,8 +9,8 @@ module ActionDispatch # sub-hashes of the params hash to filter. Filtering only certain sub-keys # from a hash is possible by using the dot notation: 'credit_card.number'. # If a block is given, each key and value of the params hash and all - # sub-hashes is passed to it, where the value or the key can be replaced using - # String#replace or similar method. + # sub-hashes are passed to it, where the value or the key can be replaced using + # String#replace or similar methods. # # env["action_dispatch.parameter_filter"] = [:password] # => replaces the value to all keys matching /password/i with "[FILTERED]" diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index c3c2a9d8c5..6c7d24d2d0 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -121,7 +121,7 @@ module ActionDispatch # not contained within the headers hash. def env_name(key) key = key.to_s - if key =~ HTTP_HEADER + if HTTP_HEADER.match?(key) key = key.upcase.tr("-", "_") key = "HTTP_" + key unless CGI_VARIABLES.include?(key) end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index d2b2106845..295539281f 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -279,8 +279,6 @@ module Mime def all?; false; end - # TODO Change this to private once we've dropped Ruby 2.2 support. - # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :string, :synonyms diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index f8e6fca36d..342e6de312 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -10,6 +10,7 @@ Mime::Type.register "text/css", :css Mime::Type.register "text/calendar", :ics Mime::Type.register "text/csv", :csv Mime::Type.register "text/vcard", :vcf +Mime::Type.register "text/vtt", :vtt, %w(vtt) Mime::Type.register "image/png", :png, [], %w(png) Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg) @@ -20,6 +21,18 @@ Mime::Type.register "image/svg+xml", :svg Mime::Type.register "video/mpeg", :mpeg, [], %w(mpg mpeg mpe) +Mime::Type.register "audio/mpeg", :mp3, [], %w(mp1 mp2 mp3) +Mime::Type.register "audio/ogg", :ogg, [], %w(oga ogg spx opus) +Mime::Type.register "audio/aac", :m4a, %w( audio/mp4 ), %w(m4a mpg4 aac) + +Mime::Type.register "video/webm", :webm, [], %w(webm) +Mime::Type.register "video/mp4", :mp4, [], %w(mp4 m4v) + +Mime::Type.register "font/otf", :otf, [], %w(otf) +Mime::Type.register "font/ttf", :ttf, [], %w(ttf) +Mime::Type.register "font/woff", :woff, [], %w(woff) +Mime::Type.register "font/woff2", :woff2, [], %w(woff2) + Mime::Type.register "application/xml", :xml, %w( text/xml application/x-xml ) Mime::Type.register "application/rss+xml", :rss Mime::Type.register "application/atom+xml", :atom diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index 1d58964862..de11939fa8 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/core_ext/object/duplicable" +require "active_support/core_ext/array/extract" module ActionDispatch module Http @@ -38,8 +39,8 @@ module ActionDispatch end end - deep_regexps, regexps = regexps.partition { |r| r.to_s.include?("\\.".freeze) } - deep_strings, strings = strings.partition { |s| s.include?("\\.".freeze) } + deep_regexps = regexps.extract! { |r| r.to_s.include?("\\.".freeze) } + deep_strings = strings.extract! { |s| s.include?("\\.".freeze) } regexps << Regexp.new(strings.join("|".freeze), true) unless strings.empty? deep_regexps << Regexp.new(deep_strings.join("|".freeze), true) unless deep_strings.empty? diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index d631281e4b..3838b84a7a 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -22,6 +22,7 @@ module ActionDispatch include ActionDispatch::Http::Parameters include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::URL + include ActionDispatch::ContentSecurityPolicy::Request include Rack::Request::Env autoload :Session, "action_dispatch/request/session" diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 0b162dc7f1..827f022ca2 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -65,6 +65,11 @@ module ActionDispatch @tempfile.path end + # Shortcut for +tempfile.to_path+. + def to_path + @tempfile.to_path + end + # Shortcut for +tempfile.rewind+. def rewind @tempfile.rewind diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index f0344fd927..35ba44005a 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -274,7 +274,7 @@ module ActionDispatch def standard_port case protocol when "https://" then 443 - else 80 + else 80 end end diff --git a/actionpack/lib/action_dispatch/journey/nfa/dot.rb b/actionpack/lib/action_dispatch/journey/nfa/dot.rb index bdb78d8d48..56e9e3c83d 100644 --- a/actionpack/lib/action_dispatch/journey/nfa/dot.rb +++ b/actionpack/lib/action_dispatch/journey/nfa/dot.rb @@ -9,16 +9,16 @@ module ActionDispatch " #{from} -> #{to} [label=\"#{sym || 'ε'}\"];" } - #memo_nodes = memos.values.flatten.map { |n| - # label = n - # if Journey::Route === n - # label = "#{n.verb.source} #{n.path.spec}" - # end - # " #{n.object_id} [label=\"#{label}\", shape=box];" - #} - #memo_edges = memos.flat_map { |k, memos| - # (memos || []).map { |v| " #{k} -> #{v.object_id};" } - #}.uniq + # memo_nodes = memos.values.flatten.map { |n| + # label = n + # if Journey::Route === n + # label = "#{n.verb.source} #{n.path.spec}" + # end + # " #{n.object_id} [label=\"#{label}\", shape=box];" + # } + # memo_edges = memos.flat_map { |k, memos| + # (memos || []).map { |v| " #{k} -> #{v.object_id};" } + # }.uniq <<-eodot digraph nfa { diff --git a/actionpack/lib/action_dispatch/journey/nfa/simulator.rb b/actionpack/lib/action_dispatch/journey/nfa/simulator.rb index 8efe48d91c..002f6feb97 100644 --- a/actionpack/lib/action_dispatch/journey/nfa/simulator.rb +++ b/actionpack/lib/action_dispatch/journey/nfa/simulator.rb @@ -25,8 +25,6 @@ module ActionDispatch state = tt.eclosure(0) until input.eos? sym = input.scan(%r([/.?]|[^/.?]+)) - - # FIXME: tt.eclosure is not needed for the GTG state = tt.eclosure(tt.move(state, sym)) end diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 08b931a3cd..32f632800c 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -32,7 +32,7 @@ module ActionDispatch end def name - left.tr "*:".freeze, "".freeze + -left.tr("*:", "") end def type @@ -82,7 +82,7 @@ module ActionDispatch def initialize(left) super @regexp = DEFAULT_EXP - @name = left.tr "*:".freeze, "".freeze + @name = -left.tr("*:", "") end def default_regexp? diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 2d85a89a56..537f479ee5 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -90,7 +90,7 @@ module ActionDispatch return @separator_re unless @matchers.key?(node) re = @matchers[node] - "(#{re})" + "(#{Regexp.union(re)})" end def visit_GROUP(node) @@ -183,7 +183,7 @@ module ActionDispatch node = node.to_sym if @requirements.key?(node) - re = /#{@requirements[node]}|/ + re = /#{Regexp.union(@requirements[node])}|/ @offsets.push((re.match("").length - 1) + @offsets.last) else @offsets << @offsets.last diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb index 639c063495..c0377459d5 100644 --- a/actionpack/lib/action_dispatch/journey/routes.rb +++ b/actionpack/lib/action_dispatch/journey/routes.rb @@ -51,11 +51,12 @@ module ActionDispatch def ast @ast ||= begin asts = anchored_routes.map(&:ast) - Nodes::Or.new(asts) unless asts.empty? + Nodes::Or.new(asts) end end def simulator + return if ast.nil? @simulator ||= begin gtg = GTG::Builder.new(ast).transition_table GTG::Simulator.new(gtg) diff --git a/actionpack/lib/action_dispatch/journey/scanner.rb b/actionpack/lib/action_dispatch/journey/scanner.rb index 4ae77903fa..2a075862e9 100644 --- a/actionpack/lib/action_dispatch/journey/scanner.rb +++ b/actionpack/lib/action_dispatch/journey/scanner.rb @@ -34,6 +34,13 @@ module ActionDispatch private + # takes advantage of String @- deduping capabilities in Ruby 2.5 upwards + # see: https://bugs.ruby-lang.org/issues/13077 + def dedup_scan(regex) + r = @ss.scan(regex) + r ? -r : nil + end + def scan case # / @@ -47,15 +54,15 @@ module ActionDispatch [:OR, "|"] when @ss.skip(/\./) [:DOT, "."] - when text = @ss.scan(/:\w+/) + when text = dedup_scan(/:\w+/) [:SYMBOL, text] - when text = @ss.scan(/\*\w+/) + when text = dedup_scan(/\*\w+/) [:STAR, text] when text = @ss.scan(/(?:[\w%\-~!$&'*+,;=@]|\\[:()])+/) text.tr! "\\", "" - [:LITERAL, text] + [:LITERAL, -text] # any char - when text = @ss.scan(/./) + when text = dedup_scan(/./) [:LITERAL, text] end end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 86a070c6ad..34331b7e4b 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -81,6 +81,10 @@ module ActionDispatch get_header Cookies::COOKIES_ROTATIONS end + def use_cookies_with_metadata + get_header Cookies::USE_COOKIES_WITH_METADATA + end + # :startdoc: end @@ -161,7 +165,7 @@ module ActionDispatch # # * <tt>:tld_length</tt> - When using <tt>:domain => :all</tt>, this option can be used to explicitly # set the TLD length when using a short (<= 3 character) domain that is being interpreted as part of a TLD. - # For example, to share cookies between user1.lvh.me and user2.lvh.me, set <tt>:tld_length</tt> to 1. + # For example, to share cookies between user1.lvh.me and user2.lvh.me, set <tt>:tld_length</tt> to 2. # * <tt>:expires</tt> - The time at which this cookie expires, as a \Time or ActiveSupport::Duration object. # * <tt>:secure</tt> - Whether this cookie is only transmitted to HTTPS servers. # Default is +false+. @@ -182,6 +186,7 @@ module ActionDispatch COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze COOKIES_ROTATIONS = "action_dispatch.cookies_rotations".freeze + USE_COOKIES_WITH_METADATA = "action_dispatch.use_cookies_with_metadata".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -338,6 +343,9 @@ module ActionDispatch end alias :has_key? :key? + # Returns the cookies as Hash. + alias :to_hash :to_h + def update(other_hash) @cookies.update other_hash.stringify_keys self @@ -467,7 +475,7 @@ module ActionDispatch def [](name) if data = @parent_jar[name.to_s] - parse name, data + parse(name, data, purpose: "cookie.#{name}") || parse(name, data) end end @@ -478,7 +486,7 @@ module ActionDispatch options = { value: options } end - commit(options) + commit(name, options) @parent_jar[name] = options end @@ -494,13 +502,24 @@ module ActionDispatch end end - def parse(name, data); data; end - def commit(options); end + def cookie_metadata(name, options) + if request.use_cookies_with_metadata + metadata = expiry_options(options) + metadata[:purpose] = "cookie.#{name}" + + metadata + else + {} + end + end + + def parse(name, data, purpose: nil); data; end + def commit(name, options); end end class PermanentCookieJar < AbstractCookieJar # :nodoc: private - def commit(options) + def commit(name, options) options[:expires] = 20.years.from_now end end @@ -580,14 +599,14 @@ module ActionDispatch end private - def parse(name, signed_message) + def parse(name, signed_message, purpose: nil) deserialize(name) do |rotate| - @verifier.verified(signed_message, on_rotation: rotate) + @verifier.verified(signed_message, on_rotation: rotate, purpose: purpose) end end - def commit(options) - options[:value] = @verifier.generate(serialize(options[:value]), expiry_options(options)) + def commit(name, options) + options[:value] = @verifier.generate(serialize(options[:value]), cookie_metadata(name, options)) raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end @@ -628,16 +647,16 @@ module ActionDispatch end private - def parse(name, encrypted_message) + def parse(name, encrypted_message, purpose: nil) deserialize(name) do |rotate| - @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate) + @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) end - def commit(options) - options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), expiry_options(options)) + def commit(name, options) + options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value]), cookie_metadata(name, options)) raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 511306eb0e..077a83b112 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -50,10 +50,18 @@ module ActionDispatch end end - def initialize(app, routes_app = nil, response_format = :default) + cattr_reader :interceptors, instance_accessor: false, default: [] + + def self.register_interceptor(object = nil, &block) + interceptor = object || block + interceptors << interceptor + end + + def initialize(app, routes_app = nil, response_format = :default, interceptors = self.class.interceptors) @app = app @routes_app = routes_app @response_format = response_format + @interceptors = interceptors end def call(env) @@ -67,12 +75,26 @@ module ActionDispatch response rescue Exception => exception + invoke_interceptors(request, exception) raise exception unless request.show_exceptions? render_exception(request, exception) end private + def invoke_interceptors(request, exception) + backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner") + wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) + + @interceptors.each do |interceptor| + begin + interceptor.call(request, exception) + rescue Exception + log_error(request, wrapper) + end + end + end + def render_exception(request, exception) backtrace_cleaner = request.get_header("action_dispatch.backtrace_cleaner") wrapper = ExceptionWrapper.new(backtrace_cleaner, exception) @@ -130,23 +152,13 @@ module ActionDispatch end def create_template(request, wrapper) - traces = wrapper.traces - - trace_to_show = "Application Trace" - if traces[trace_to_show].empty? && wrapper.rescue_template != "routing_error" - trace_to_show = "Full Trace" - end - - if source_to_show = traces[trace_to_show].first - source_to_show_id = source_to_show[:id] - end - DebugView.new([RESCUES_TEMPLATE_PATH], request: request, + exception_wrapper: wrapper, exception: wrapper.exception, - traces: traces, - show_source_idx: source_to_show_id, - trace_to_show: trace_to_show, + traces: wrapper.traces, + show_source_idx: wrapper.source_to_show_id, + trace_to_show: wrapper.trace_to_show, routes_inspector: routes_inspector(wrapper.exception), source_extracts: wrapper.source_extracts, line_number: wrapper.line_number, diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 4f69abfa6f..fb2b2bd3b0 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, + "ActionController::MissingExactTemplate" => :not_acceptable, "ActionController::InvalidAuthenticityToken" => :unprocessable_entity, "ActionController::InvalidCrossOriginRequest" => :unprocessable_entity, "ActionDispatch::Http::Parameters::ParseError" => :bad_request, @@ -22,17 +23,20 @@ module ActionDispatch ) cattr_accessor :rescue_templates, default: Hash.new("diagnostics").merge!( - "ActionView::MissingTemplate" => "missing_template", - "ActionController::RoutingError" => "routing_error", - "AbstractController::ActionNotFound" => "unknown_action", - "ActionView::Template::Error" => "template_error" + "ActionView::MissingTemplate" => "missing_template", + "ActionController::RoutingError" => "routing_error", + "AbstractController::ActionNotFound" => "unknown_action", + "ActiveRecord::StatementInvalid" => "invalid_statement", + "ActionView::Template::Error" => "template_error", + "ActionController::MissingExactTemplate" => "missing_exact_template", ) - attr_reader :backtrace_cleaner, :exception, :line_number, :file + attr_reader :backtrace_cleaner, :exception, :wrapped_causes, :line_number, :file def initialize(backtrace_cleaner, exception) @backtrace_cleaner = backtrace_cleaner @exception = original_exception(exception) + @wrapped_causes = wrapped_causes_for(exception, backtrace_cleaner) expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError) end @@ -63,7 +67,11 @@ module ActionDispatch full_trace_with_ids = [] full_trace.each_with_index do |trace, idx| - trace_with_id = { id: idx, trace: trace } + trace_with_id = { + exception_object_id: @exception.object_id, + id: idx, + trace: trace + } if application_trace.include?(trace) application_trace_with_ids << trace_with_id @@ -96,6 +104,18 @@ module ActionDispatch end end + def trace_to_show + if traces["Application Trace"].empty? && rescue_template != "routing_error" + "Full Trace" + else + "Application Trace" + end + end + + def source_to_show_id + (traces[trace_to_show].first || {})[:id] + end + private def backtrace @@ -110,6 +130,16 @@ module ActionDispatch end end + def causes_for(exception) + return enum_for(__method__, exception) unless block_given? + + yield exception while exception = exception.cause + end + + def wrapped_causes_for(exception, backtrace_cleaner) + causes_for(exception).map { |cause| self.class.new(backtrace_cleaner, cause) } + end + def clean_backtrace(*args) if backtrace_cleaner backtrace_cleaner.clean(backtrace, *args) diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 3e11846778..fd05eec172 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -73,7 +73,7 @@ module ActionDispatch end end - def reset_session # :nodoc + def reset_session # :nodoc: super self.flash = nil end diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index 805d3f2148..da2871b551 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -30,7 +30,7 @@ module ActionDispatch private def make_request_id(request_id) if request_id.presence - request_id.gsub(/[^\w\-]/, "".freeze).first(255) + request_id.gsub(/[^\w\-@]/, "".freeze).first(255) else internal_request_id end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 4ea96196d3..df680c1c5f 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -25,7 +25,7 @@ module ActionDispatch # 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. # - # Configure your session store in <tt>config/initializers/session_store.rb</tt>: + # Configure your session store in an initializer: # # Rails.application.config.session_store :cookie_store, key: '_your_app_session' # diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index ef633aadc6..190e54223e 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -15,6 +15,8 @@ module ActionDispatch # # config.ssl_options = { redirect: { exclude: -> request { request.path =~ /healthcheck/ } } } # + # Cookies will not be flagged as secure for excluded requests. + # # 2. <b>Secure cookies</b>: Sets the +secure+ flag on cookies to tell browsers they # must not be sent along with +http://+ requests. Enabled by default. Set # +config.ssl_options+ with <tt>secure_cookies: false</tt> to disable this feature. @@ -26,8 +28,8 @@ module ActionDispatch # Set +config.ssl_options+ with <tt>hsts: { ... }</tt> to configure HSTS: # # * +expires+: How long, in seconds, these settings will stick. The minimum - # required to qualify for browser preload lists is 18 weeks. Defaults to - # 180 days (recommended). + # required to qualify for browser preload lists is 1 year. Defaults to + # 1 year (recommended). # # * +subdomains+: Set to +true+ to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a @@ -47,9 +49,8 @@ module ActionDispatch class SSL # :stopdoc: - # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/ - # and greater than the 18-week requirement for browser preload lists. - HSTS_EXPIRES_IN = 15552000 + # Default to 1 year, the minimum for browser preload lists. + HSTS_EXPIRES_IN = 31536000 def self.default_hsts_options { expires: HSTS_EXPIRES_IN, subdomains: true, preload: false } @@ -72,7 +73,7 @@ module ActionDispatch if request.ssl? @app.call(env).tap do |status, headers, body| set_hsts_header! headers - flag_cookies_as_secure! headers if @secure_cookies + flag_cookies_as_secure! headers if @secure_cookies && !@exclude.call(request) end else return redirect_to_https request unless @exclude.call(request) @@ -112,7 +113,7 @@ module ActionDispatch cookies = cookies.split("\n".freeze) headers["Set-Cookie".freeze] = cookies.map { |cookie| - if cookie !~ /;\s*secure\s*(;|$)/i + if !/;\s*secure\s*(;|$)/i.match?(cookie) "#{cookie}; secure" else cookie diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 23492e14eb..277074f216 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -16,7 +16,7 @@ module ActionDispatch # does not exist, a 404 "File not Found" response will be returned. class FileHandler def initialize(root, index: "index", headers: {}) - @root = root.chomp("/") + @root = root.chomp("/").b @file_server = ::Rack::File.new(@root, headers) @index = index end @@ -35,15 +35,14 @@ module ActionDispatch paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] if match = paths.detect { |p| - path = File.join(@root, p.dup.force_encoding(Encoding::UTF_8)) + path = File.join(@root, p.b) begin File.file?(path) && File.readable?(path) rescue SystemCallError false end - } - return ::Rack::Utils.escape_path(match) + return ::Rack::Utils.escape_path(match).b end end @@ -69,7 +68,7 @@ module ActionDispatch headers["Vary"] = "Accept-Encoding" if gzip_path - return [status, headers, body] + [status, headers, body] ensure request.path_info = path end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb index e7b913bbe4..88a8e6ad83 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb @@ -1,6 +1,8 @@ -<% @source_extracts.each_with_index do |source_extract, index| %> +<% error_index = local_assigns[:error_index] || 0 %> + +<% source_extracts.each_with_index do |source_extract, index| %> <% if source_extract[:code] %> - <div class="source <%="hidden" if @show_source_idx != index%>" id="frame-source-<%=index%>"> + <div class="source <%= "hidden" if show_source_idx != index %>" id="frame-source-<%= error_index %>-<%= index %>"> <div class="info"> Extracted source (around line <strong>#<%= source_extract[:line_number] %></strong>): </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb index ab57b11c7d..835ca8d260 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb @@ -1,52 +1,62 @@ -<% names = @traces.keys %> +<% names = traces.keys %> +<% error_index = local_assigns[:error_index] || 0 %> <p><code>Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %></code></p> -<div id="traces"> +<div id="traces-<%= error_index %>"> <% names.each do |name| %> <% - show = "show('#{name.gsub(/\s/, '-')}');" - hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"} + show = "show('#{name.gsub(/\s/, '-')}-#{error_index}');" + hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}-#{error_index}');"} %> <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %> <% end %> - <% @traces.each do |name, trace| %> - <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == @trace_to_show) ? 'block' : 'none' %>;"> - <pre><code><% trace.each do |frame| %><a class="trace-frames" data-frame-id="<%= frame[:id] %>" href="#"><%= frame[:trace] %></a><br><% end %></code></pre> + <% traces.each do |name, trace| %> + <div id="<%= "#{name.gsub(/\s/, '-')}-#{error_index}" %>" style="display: <%= (name == trace_to_show) ? 'block' : 'none' %>;"> + <code style="font-size: 11px;"> + <% trace.each do |frame| %> + <a class="trace-frames trace-frames-<%= error_index %>" data-exception-object-id="<%= frame[:exception_object_id] %>" data-frame-id="<%= frame[:id] %>" href="#"> + <%= frame[:trace] %> + </a> + <br> + <% end %> + </code> </div> <% end %> <script type="text/javascript"> - var traceFrames = document.getElementsByClassName('trace-frames'); - var selectedFrame, currentSource = document.getElementById('frame-source-0'); - - // Add click listeners for all stack frames - for (var i = 0; i < traceFrames.length; i++) { - traceFrames[i].addEventListener('click', function(e) { - e.preventDefault(); - var target = e.target; - var frame_id = target.dataset.frameId; - - if (selectedFrame) { - selectedFrame.className = selectedFrame.className.replace("selected", ""); - } - - target.className += " selected"; - selectedFrame = target; - - // Change the extracted source code - changeSourceExtract(frame_id); - }); - - function changeSourceExtract(frame_id) { - var el = document.getElementById('frame-source-' + frame_id); - if (currentSource && el) { - currentSource.className += " hidden"; - el.className = el.className.replace(" hidden", ""); - currentSource = el; + (function() { + var traceFrames = document.getElementsByClassName('trace-frames-<%= error_index %>'); + var selectedFrame, currentSource = document.getElementById('frame-source-<%= error_index %>-0'); + + // Add click listeners for all stack frames + for (var i = 0; i < traceFrames.length; i++) { + traceFrames[i].addEventListener('click', function(e) { + e.preventDefault(); + var target = e.target; + var frame_id = target.dataset.frameId; + + if (selectedFrame) { + selectedFrame.className = selectedFrame.className.replace("selected", ""); + } + + target.className += " selected"; + selectedFrame = target; + + // Change the extracted source code + changeSourceExtract(frame_id); + }); + + function changeSourceExtract(frame_id) { + var el = document.getElementById('frame-source-<%= error_index %>-' + frame_id); + if (currentSource && el) { + currentSource.className += " hidden"; + el.className = el.className.replace(" hidden", ""); + currentSource = el; + } } } - } + })(); </script> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb index f154021ae6..bde26f46c2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb @@ -10,7 +10,25 @@ <div id="container"> <h2><%= h @exception.message %></h2> - <%= render template: "rescues/_source" %> - <%= render template: "rescues/_trace" %> + <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %> + <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %> + + <% if @exception.cause %> + <h2>Exception Causes</h2> + <% end %> + + <% @exception_wrapper.wrapped_causes.each.with_index(1) do |wrapper, index| %> + <div class="details"> + <a class="summary" href="#" style="color: #F0F0F0; text-decoration: none; background: #C52F24; border-bottom: none;" onclick="return toggle(<%= wrapper.exception.object_id %>)"> + <%= wrapper.exception.class.name %>: <%= h wrapper.exception.message %> + </a> + </div> + + <div id="<%= wrapper.exception.object_id %>" style="display: none;"> + <%= render "rescues/source", source_extracts: wrapper.source_extracts, show_source_idx: wrapper.source_to_show_id, error_index: index %> + <%= render "rescues/trace", traces: wrapper.traces, trace_to_show: wrapper.trace_to_show, error_index: index %> + </div> + <% end %> + <%= render template: "rescues/_request_and_response" %> </div> 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 new file mode 100644 index 0000000000..e8454acfad --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb @@ -0,0 +1,21 @@ +<header> + <h1> + <%= @exception.class.to_s %> + <% if @request.parameters['controller'] %> + in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %> + <% end %> + </h1> +</header> + +<div id="container"> + <h2> + <%= h @exception.message %> + <% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> + <br />To resolve this issue run: rails active_storage:install + <% end %> + </h2> + + <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %> + <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %> + <%= render template: "rescues/_request_and_response" %> +</div> 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 new file mode 100644 index 0000000000..e5e3196710 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb @@ -0,0 +1,13 @@ +<%= @exception.class.to_s %><% + if @request.parameters['controller'] +%> in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %> +<% end %> + +<%= @exception.message %> +<% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> +To resolve this issue run: rails active_storage:install +<% end %> + +<%= render template: "rescues/_source" %> +<%= render template: "rescues/_trace" %> +<%= render template: "rescues/_request_and_response" %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb new file mode 100644 index 0000000000..76ab1691b5 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.html.erb @@ -0,0 +1,19 @@ +<header> + <h1>No template for interactive request</h1> +</header> + +<div id="container"> + <h2><%= h @exception.message %></h2> + + <p class="summary"> + <strong>NOTE!</strong><br> + Unless told otherwise, Rails expects an action to render a template with the same name,<br> + contained in a folder named after its controller. + + If this controller is an API responding with 204 (No Content), <br> + which does not require a template, + then this error will occur when trying to access it via browser,<br> + since we expect an HTML template + to be rendered for such requests. If that's the case, carry on. + </p> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb new file mode 100644 index 0000000000..fcdbe6069d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_exact_template.text.erb @@ -0,0 +1,3 @@ +Missing exact template + +<%= @exception.message %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb index 2a65fd06ad..22eb6e9b4e 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb @@ -5,7 +5,7 @@ <div id="container"> <h2><%= h @exception.message %></h2> - <%= render template: "rescues/_source" %> - <%= render template: "rescues/_trace" %> + <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %> + <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %> <%= render template: "rescues/_request_and_response" %> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb index 55dd5ddc7b..2b8f3f2a5e 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb @@ -14,7 +14,7 @@ </p> <% end %> - <%= render template: "rescues/_trace" %> + <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %> <% if @routes_inspector %> <h2> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb index 5060da9369..324ef1567a 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb @@ -11,10 +11,10 @@ </p> <pre><code><%= h @exception.message %></code></pre> - <%= render template: "rescues/_source" %> + <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %> <p><%= @exception.sub_template_message %></p> - <%= render template: "rescues/_trace" %> + <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %> <%= render template: "rescues/_request_and_response" %> </div> diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 855f2ffa47..efc3988bc3 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -21,12 +21,16 @@ module ActionDispatch config.action_dispatch.encrypted_signed_cookie_salt = "signed encrypted cookie" config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie" config.action_dispatch.use_authenticated_cookie_encryption = false + config.action_dispatch.use_cookies_with_metadata = false config.action_dispatch.perform_deep_munge = true config.action_dispatch.default_headers = { "X-Frame-Options" => "SAMEORIGIN", "X-XSS-Protection" => "1; mode=block", - "X-Content-Type-Options" => "nosniff" + "X-Content-Type-Options" => "nosniff", + "X-Download-Options" => "noopen", + "X-Permitted-Cross-Domain-Policies" => "none", + "Referrer-Policy" => "strict-origin-when-cross-origin" } config.action_dispatch.cookies_rotations = ActiveSupport::Messages::RotationConfiguration.new diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index d86d0b10c2..bc5e0670e0 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -93,6 +93,14 @@ module ActionDispatch @delegate[key.to_s] end + # Returns the nested value specified by the sequence of keys, returning + # +nil+ if any intermediate step is +nil+. + def dig(*keys) + load_for_read! + keys = keys.map.with_index { |key, i| i.zero? ? key.to_s : key } + @delegate.dig(*keys) + end + # Returns true if the session has the given key or false. def has_key?(key) load_for_read! @@ -130,6 +138,7 @@ module ActionDispatch load_for_read! @delegate.dup.delete_if { |_, v| v.nil? } end + alias :to_h :to_hash # Updates the session with given Hash. # diff --git a/actionpack/lib/action_dispatch/request/utils.rb b/actionpack/lib/action_dispatch/request/utils.rb index 0ae464082d..fb0efb9a58 100644 --- a/actionpack/lib/action_dispatch/request/utils.rb +++ b/actionpack/lib/action_dispatch/request/utils.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/hash/indifferent_access" + module ActionDispatch class Request class Utils # :nodoc: diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 72f7407c6e..5cde677051 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -243,8 +243,9 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with <tt>-c</tt> option. - # + # Target a specific controller with <tt>-c</tt>, or grep routes + # using <tt>-g</tt>. Useful in conjunction with <tt>--expanded</tt> + # which displays routes vertically. module Routing extend ActiveSupport::Autoload diff --git a/actionpack/lib/action_dispatch/routing/endpoint.rb b/actionpack/lib/action_dispatch/routing/endpoint.rb index 24dced1efd..28bb20d688 100644 --- a/actionpack/lib/action_dispatch/routing/endpoint.rb +++ b/actionpack/lib/action_dispatch/routing/endpoint.rb @@ -3,12 +3,15 @@ module ActionDispatch module Routing class Endpoint # :nodoc: - def dispatcher?; false; end - def redirect?; false; end - def engine?; rack_app.respond_to?(:routes); end - def matches?(req); true; end - def app; self; end - def rack_app; app; end + def dispatcher?; false; end + def redirect?; false; end + def matches?(req); true; end + def app; self; end + def rack_app; app; end + + def engine? + rack_app.is_a?(Class) && rack_app < Rails::Engine + end end end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index a2205569b4..cba49d1a0b 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "delegate" -require "active_support/core_ext/string/strip" +require "io/console/size" module ActionDispatch module Routing @@ -61,11 +61,11 @@ module ActionDispatch @routes = routes end - def format(formatter, filter = nil) + def format(formatter, filter = {}) routes_to_display = filter_routes(normalize_filter(filter)) routes = collect_routes(routes_to_display) if routes.none? - formatter.no_routes(collect_routes(@routes)) + formatter.no_routes(collect_routes(@routes), filter) return formatter.result end @@ -81,12 +81,12 @@ module ActionDispatch end private - def normalize_filter(filter) - if filter.is_a?(Hash) && filter[:controller] + if filter[:controller] { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } - elsif filter - { controller: /#{filter}/, action: /#{filter}/, verb: /#{filter}/, name: /#{filter}/, path: /#{filter}/ } + elsif filter[:grep] + { controller: /#{filter[:grep]}/, action: /#{filter[:grep]}/, + verb: /#{filter[:grep]}/, name: /#{filter[:grep]}/, path: /#{filter[:grep]}/ } end end @@ -126,62 +126,111 @@ module ActionDispatch end end - class ConsoleFormatter - def initialize - @buffer = [] - end + module ConsoleFormatter + class Base + def initialize + @buffer = [] + end - def result - @buffer.join("\n") - end + def result + @buffer.join("\n") + end - def section_title(title) - @buffer << "\n#{title}:" - end + def section_title(title) + end - def section(routes) - @buffer << draw_section(routes) - end + def section(routes) + end - def header(routes) - @buffer << draw_header(routes) - end + def header(routes) + end - def no_routes(routes) - @buffer << - if routes.none? - <<-MESSAGE.strip_heredoc - You don't have any routes defined! + def no_routes(routes, filter) + @buffer << + if routes.none? + <<~MESSAGE + You don't have any routes defined! + + Please add some routes in config/routes.rb. + MESSAGE + elsif filter.key?(:controller) + "No routes were found for this controller." + elsif filter.key?(:grep) + "No routes were found for this grep pattern." + end - Please add some routes in config/routes.rb. - MESSAGE - else - "No routes were found for this controller" + @buffer << "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." end - @buffer << "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." end - private - def draw_section(routes) - header_lengths = ["Prefix", "Verb", "URI Pattern"].map(&:length) - name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) + class Sheet < Base + def section_title(title) + @buffer << "\n#{title}:" + end - routes.map do |r| - "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" - end + def section(routes) + @buffer << draw_section(routes) + end + + def header(routes) + @buffer << draw_header(routes) end - def draw_header(routes) - name_width, verb_width, path_width = widths(routes) + private + + def draw_section(routes) + header_lengths = ["Prefix", "Verb", "URI Pattern"].map(&:length) + name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) + + routes.map do |r| + "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + end + end + + def draw_header(routes) + name_width, verb_width, path_width = widths(routes) + + "#{"Prefix".rjust(name_width)} #{"Verb".ljust(verb_width)} #{"URI Pattern".ljust(path_width)} Controller#Action" + end + + def widths(routes) + [routes.map { |r| r[:name].length }.max || 0, + routes.map { |r| r[:verb].length }.max || 0, + routes.map { |r| r[:path].length }.max || 0] + end + end - "#{"Prefix".rjust(name_width)} #{"Verb".ljust(verb_width)} #{"URI Pattern".ljust(path_width)} Controller#Action" + class Expanded < Base + def section_title(title) + @buffer << "\n#{"[ #{title} ]"}" end - def widths(routes) - [routes.map { |r| r[:name].length }.max || 0, - routes.map { |r| r[:verb].length }.max || 0, - routes.map { |r| r[:path].length }.max || 0] + def section(routes) + @buffer << draw_expanded_section(routes) end + + private + + def draw_expanded_section(routes) + routes.map.each_with_index do |r, i| + <<~MESSAGE.chomp + #{route_header(index: i + 1)} + Prefix | #{r[:name]} + Verb | #{r[:verb]} + URI | #{r[:path]} + Controller#Action | #{r[:reqs]} + MESSAGE + end + end + + def route_header(index:) + console_width = IO.console_size.second + header_prefix = "--[ Route #{index} ]" + dash_remainder = [console_width - header_prefix.size, 0].max + + "#{header_prefix}#{'-' * dash_remainder}" + end + end end class HtmlTableFormatter @@ -203,16 +252,16 @@ module ActionDispatch end def no_routes(*) - @buffer << <<-MESSAGE.strip_heredoc + @buffer << <<~MESSAGE <p>You don't have any routes defined!</p> <ul> <li>Please add some routes in <tt>config/routes.rb</tt>.</li> <li> For more information about routes, please see the Rails guide - <a href="http://guides.rubyonrails.org/routing.html">Rails Routing from the Outside In</a>. + <a href="https://guides.rubyonrails.org/routing.html">Rails Routing from the Outside In</a>. </li> </ul> - MESSAGE + MESSAGE end def result diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index ded42adee9..ff325afc54 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -279,7 +279,7 @@ module ActionDispatch def verify_regexp_requirements(requirements) requirements.each do |requirement| - if requirement.source =~ ANCHOR_CHARACTERS_REGEX + if ANCHOR_CHARACTERS_REGEX.match?(requirement.source) raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end @@ -309,7 +309,7 @@ module ActionDispatch hash = check_part(:controller, controller, path_params, {}) do |part| translate_controller(part) { message = "'#{part}' is not a supported controller name. This can lead to potential routing problems.".dup - message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" + message << " See https://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" raise ArgumentError, message } @@ -333,7 +333,7 @@ module ActionDispatch end def split_to(to) - if to =~ /#/ + if /#/.match?(to) to.split("#") else [] @@ -342,7 +342,7 @@ module ActionDispatch def add_controller_module(controller, modyoule) if modyoule && !controller.is_a?(Regexp) - if controller =~ %r{\A/} + if %r{\A/}.match?(controller) controller[1..-1] else [modyoule, controller].compact.join("/") @@ -611,7 +611,7 @@ module ActionDispatch end raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call) - raise ArgumentError, <<-MSG.strip_heredoc unless path + raise ArgumentError, <<~MSG unless path Must be called with mount point mount SomeRackApp, at: "some_route" @@ -664,6 +664,7 @@ module ActionDispatch def define_generate_prefix(app, name) _route = @set.named_routes.get name _routes = @set + _url_helpers = @set.url_helpers script_namer = ->(options) do prefix_options = options.slice(*_route.segment_keys) @@ -675,7 +676,7 @@ module ActionDispatch # We must actually delete prefix segment keys to avoid passing them to next url_for. _route.segment_keys.each { |k| options.delete(k) } - _routes.url_helpers.send("#{name}_path", prefix_options) + _url_helpers.send("#{name}_path", prefix_options) end app.routes.define_mounted_helper(name, script_namer) @@ -1573,7 +1574,7 @@ module ActionDispatch # Matches a URL pattern to one or more routes. # For more information, see match[rdoc-ref:Base#match]. # - # match 'path' => 'controller#action', via: patch + # match 'path' => 'controller#action', via: :patch # match 'path', to: 'controller#action', via: :post # match 'path', 'otherpath', on: :member, via: :get def match(path, *rest, &block) @@ -1587,7 +1588,7 @@ module ActionDispatch when Symbol options[:action] = to when String - if to =~ /#/ + if /#/.match?(to) options[:to] = to else options[:controller] = to @@ -1913,7 +1914,7 @@ module ActionDispatch default_action = options.delete(:action) || @scope[:action] - if action =~ /^[\w\-\/]+$/ + if /^[\w\-\/]+$/.match?(action) default_action ||= action.tr("-", "_") unless action.include?("/") else action = nil @@ -2046,7 +2047,7 @@ module ActionDispatch end module CustomUrls - # Define custom url helpers that will be added to the application's + # Define custom URL helpers that will be added to the application's # routes. This allows you to override and/or replace the default behavior # of routing helpers, e.g: # @@ -2066,11 +2067,11 @@ module ActionDispatch # arguments for +url_for+ which will actually build the URL string. This can # be one of the following: # - # * A string, which is treated as a generated URL - # * A hash, e.g. { controller: "pages", action: "index" } - # * An array, which is passed to `polymorphic_url` - # * An Active Model instance - # * An Active Model class + # * A string, which is treated as a generated URL + # * A hash, e.g. <tt>{ controller: "pages", action: "index" }</tt> + # * An array, which is passed to +polymorphic_url+ + # * An Active Model instance + # * An Active Model class # # NOTE: Other URL helpers can be called in the block but be careful not to invoke # your custom URL helper again otherwise it will result in a stack overflow error. @@ -2082,9 +2083,9 @@ module ActionDispatch # [ :products, options.merge(params.permit(:page, :size).to_h.symbolize_keys) ] # end # - # In this instance the +params+ object comes from the context in which the the + # In this instance the +params+ object comes from the context in which the # block is executed, e.g. generating a URL inside a controller action or a view. - # If the block is executed where there isn't a params object such as this: + # If the block is executed where there isn't a +params+ object such as this: # # Rails.application.routes.url_helpers.browse_path # diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index 6da869c0c2..e17ccaf986 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -120,8 +120,7 @@ module ActionDispatch opts end - # Returns the path component of a URL for the given record. It uses - # <tt>polymorphic_url</tt> with <tt>routing_type: :path</tt>. + # Returns the path component of a URL for the given record. def polymorphic_path(record_or_hash_or_array, options = {}) if Hash === record_or_hash_or_array options = record_or_hash_or_array.merge(options) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 987e709f6f..07d3a41173 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -2,7 +2,6 @@ require "action_dispatch/journey" require "active_support/core_ext/object/to_query" -require "active_support/core_ext/hash/slice" require "active_support/core_ext/module/redefine_method" require "active_support/core_ext/module/remove_method" require "active_support/core_ext/array/extract_options" @@ -36,7 +35,7 @@ module ActionDispatch if @raise_on_name_error raise else - return [404, { "X-Cascade" => "pass" }, []] + [404, { "X-Cascade" => "pass" }, []] end end @@ -154,13 +153,13 @@ module ActionDispatch url_name = :"#{name}_url" @path_helpers_module.module_eval do - define_method(path_name) do |*args| + redefine_method(path_name) do |*args| helper.call(self, args, true) end end @url_helpers_module.module_eval do - define_method(url_name) do |*args| + redefine_method(url_name) do |*args| helper.call(self, args, false) end end @@ -199,6 +198,16 @@ module ActionDispatch if args.size == arg_size && !inner_options && optimize_routes_generation?(t) options = t.url_options.merge @options options[:path] = optimized_helper(args) + + original_script_name = options.delete(:original_script_name) + script_name = t._routes.find_script_name(options) + + if original_script_name + script_name = original_script_name + script_name + end + + options[:script_name] = script_name + url_strategy.call options else super @@ -575,7 +584,7 @@ module ActionDispatch "You may have defined two routes with the same name using the `:as` option, or " \ "you may be overriding a route already defined by a resource with the same naming. " \ "For the latter, you can restrict the routes created with `resources` as explained here: \n" \ - "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created" + "https://guides.rubyonrails.org/routing.html#restricting-the-routes-created" end route = @set.add_route(name, mapping) @@ -845,7 +854,7 @@ module ActionDispatch recognize_path_with_request(req, path, extras) end - def recognize_path_with_request(req, path, extras) + def recognize_path_with_request(req, path, extras, raise_on_missing: true) @router.recognize(req) do |route, params| params.merge!(extras) params.each do |key, value| @@ -865,12 +874,14 @@ module ActionDispatch return req.path_parameters elsif app.matches?(req) && app.engine? - path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras) - return path_parameters + path_parameters = app.rack_app.routes.recognize_path_with_request(req, path, extras, raise_on_missing: false) + return path_parameters if path_parameters end end - raise ActionController::RoutingError, "No route matches #{path.inspect}" + if raise_on_missing + raise ActionController::RoutingError, "No route matches #{path.inspect}" + end end end # :startdoc: diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 3ae533dd37..1a31c7dbb8 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -155,7 +155,7 @@ module ActionDispatch # Missing routes keys may be filled in from the current request's parameters # (e.g. +:controller+, +:action+, +:id+ and any other parameters that are # placed in the path). Given that the current action has been reached - # through `GET /users/1`: + # through <tt>GET /users/1</tt>: # # url_for(only_path: true) # => '/users/1' # url_for(only_path: true, action: 'edit') # => '/users/1/edit' @@ -191,7 +191,25 @@ module ActionDispatch end end - def route_for(name, *args) # :nodoc: + # Allows calling direct or regular named route. + # + # resources :buckets + # + # direct :recordable do |recording| + # route_for(:bucket, recording.bucket) + # end + # + # direct :threadable do |threadable| + # route_for(:recordable, threadable.parent) + # end + # + # This maintains the context of the original caller on + # whether to return a path or full URL, e.g: + # + # threadable_path(threadable) # => "/buckets/1" + # threadable_url(threadable) # => "http://example.com/buckets/1" + # + def route_for(name, *args) public_send(:"#{name}_url", *args) end diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb index 7246e01cff..c74c0ccced 100644 --- a/actionpack/lib/action_dispatch/system_test_case.rb +++ b/actionpack/lib/action_dispatch/system_test_case.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true -gem "capybara", "~> 2.15" +gem "capybara", ">= 2.15" require "capybara/dsl" require "capybara/minitest" require "action_controller" require "action_dispatch/system_testing/driver" +require "action_dispatch/system_testing/browser" require "action_dispatch/system_testing/server" require "action_dispatch/system_testing/test_helpers/screenshot_helper" require "action_dispatch/system_testing/test_helpers/setup_and_teardown" @@ -69,6 +70,9 @@ module ActionDispatch # size of the browser screen. These two options are not applicable for # headless drivers and will be silently ignored if passed. # + # Headless browsers such as headless Chrome and headless Firefox are also supported. + # You can use these browsers by setting the +:using+ argument to +:headless_chrome+ or +:headless_firefox+. + # # To use a headless driver, like Poltergeist, update your Gemfile to use # Poltergeist instead of Selenium and then declare the driver name in the # +application_system_test_case.rb+ file. In this case, you would leave out @@ -121,11 +125,15 @@ module ActionDispatch # # driven_by :poltergeist # - # driven_by :selenium, using: :firefox + # driven_by :selenium, screen_size: [800, 800] + # + # driven_by :selenium, using: :chrome # # driven_by :selenium, using: :headless_chrome # - # driven_by :selenium, screen_size: [800, 800] + # 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) end diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb new file mode 100644 index 0000000000..1b0bce6b9e --- /dev/null +++ b/actionpack/lib/action_dispatch/system_testing/browser.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ActionDispatch + module SystemTesting + class Browser # :nodoc: + attr_reader :name + + def initialize(name) + @name = name + end + + def type + case name + when :headless_chrome + :chrome + when :headless_firefox + :firefox + else + name + end + end + + def options + case name + when :headless_chrome + headless_chrome_browser_options + when :headless_firefox + headless_firefox_browser_options + 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? + + options + end + + def headless_firefox_browser_options + options = Selenium::WebDriver::Firefox::Options.new + options.args << "-headless" + + options + end + end + end +end diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb index 2687772b4b..5252ff6746 100644 --- a/actionpack/lib/action_dispatch/system_testing/driver.rb +++ b/actionpack/lib/action_dispatch/system_testing/driver.rb @@ -5,7 +5,7 @@ module ActionDispatch class Driver # :nodoc: def initialize(name, **options) @name = name - @browser = options[:using] + @browser = Browser.new(options[:using]) @screen_size = options[:screen_size] @options = options[:options] end @@ -32,23 +32,11 @@ module ActionDispatch end def browser_options - if @browser == :headless_chrome - browser_options = Selenium::WebDriver::Chrome::Options.new - browser_options.args << "--headless" - browser_options.args << "--disable-gpu" - - @options.merge(options: browser_options) - else - @options - end - end - - def browser - @browser == :headless_chrome ? :chrome : @browser + @options.merge(options: @browser.options).compact end def register_selenium(app) - Capybara::Selenium::Driver.new(app, { browser: browser }.merge(browser_options)).tap do |driver| + Capybara::Selenium::Driver.new(app, { browser: @browser.type }.merge(browser_options)).tap do |driver| driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size) end end diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb index 8f1b6725b1..4fc1f33767 100644 --- a/actionpack/lib/action_dispatch/system_testing/server.rb +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -20,7 +20,7 @@ module ActionDispatch end def set_server - Capybara.server = :puma, { Silent: self.class.silence_puma } + Capybara.server = :puma, { Silent: self.class.silence_puma } if Capybara.server == Capybara.servers[:default] end def set_port 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 6c337cdc31..d2685e0452 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 @@ -15,12 +15,11 @@ module ActionDispatch # # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT+ environment variable to # control the output. Possible values are: - # * [+inline+ (default)] display the screenshot in the terminal using the + # * [+simple+ (default)] Only displays the screenshot path. + # This is the default value. + # * [+inline+] Display the screenshot in the terminal using the # iTerm image protocol (https://iterm2.com/documentation-images.html). - # * [+simple+] only display the screenshot path. - # This is the default value if the +CI+ environment variables - # is defined. - # * [+artifact+] display the screenshot in the terminal, using the terminal + # * [+artifact+] Display the screenshot in the terminal, using the terminal # artifact format (https://buildkite.github.io/terminal/inline-images/). def take_screenshot save_image @@ -44,7 +43,7 @@ module ActionDispatch end def image_path - @image_path ||= absolute_image_path.relative_path_from(Pathname.pwd).to_s + @image_path ||= absolute_image_path.to_s end def absolute_image_path @@ -59,11 +58,8 @@ module ActionDispatch # Environment variables have priority output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] || ENV["CAPYBARA_INLINE_SCREENSHOT"] - # If running in a CI environment, default to simple - output_type ||= "simple" if ENV["CI"] - - # Default - output_type ||= "inline" + # Default to outputting a path to the screenshot + output_type ||= "simple" output_type end @@ -84,7 +80,7 @@ module ActionDispatch end def inline_base64(path) - Base64.encode64(path).gsub("\n", "") + Base64.strict_encode64(path) end def failed? 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 ffa85f4e14..e47d5020f4 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 @@ -19,6 +19,7 @@ module ActionDispatch def after_teardown take_failed_screenshot Capybara.reset_sessions! + ensure super end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 5390581139..77cb311630 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -78,7 +78,7 @@ module ActionDispatch # # Asserts that the generated route gives us our custom route # assert_generates "changesets/12", { controller: 'scm', action: 'show_diff', revision: "12" } def assert_generates(expected_path, options, defaults = {}, extras = {}, message = nil) - if expected_path =~ %r{://} + if %r{://}.match?(expected_path) fail_on(URI::InvalidURIError, message) do uri = URI.parse(expected_path) expected_path = uri.path.to_s.empty? ? "/" : uri.path @@ -189,7 +189,7 @@ module ActionDispatch request = ActionController::TestRequest.create @controller.class - if path =~ %r{://} + if %r{://}.match?(path) fail_on(URI::InvalidURIError, msg) do uri = URI.parse(path) request.env["rack.url_scheme"] = uri.scheme || "http" diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 7171b6942c..45439a3bb1 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -50,10 +50,11 @@ module ActionDispatch # Follow a single redirect response. If the last response was not a # redirect, an exception will be raised. Otherwise, the redirect is - # performed on the location header. - def follow_redirect! + # performed on the location header. Any arguments are passed to the + # underlying call to `get`. + def follow_redirect!(**args) raise "not a redirect! #{status} #{status_message}" unless redirect? - get(response.location) + get(response.location, **args) status end end @@ -189,6 +190,12 @@ module ActionDispatch # merged into the Rack env hash. # - +env+: Additional env to pass, as a Hash. The headers will be # merged into the Rack env hash. + # - +xhr+: Set to `true` if you want to make and Ajax request. + # 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. + # The headers will be merged into the Rack env hash. # # This method is rarely used directly. Use +#get+, +#post+, or other standard # HTTP methods in integration tests. +#process+ is only required when using a @@ -210,7 +217,7 @@ module ActionDispatch method = :post end - if path =~ %r{://} + if %r{://}.match?(path) path = build_expanded_path(path) do |location| https! URI::HTTPS === location if location.scheme diff --git a/actionpack/lib/action_dispatch/testing/request_encoder.rb b/actionpack/lib/action_dispatch/testing/request_encoder.rb index 01246b7a2e..9889f61951 100644 --- a/actionpack/lib/action_dispatch/testing/request_encoder.rb +++ b/actionpack/lib/action_dispatch/testing/request_encoder.rb @@ -34,7 +34,7 @@ module ActionDispatch end def encode_params(params) - @param_encoder.call(params) + @param_encoder.call(params) if params end def self.parser(content_type) diff --git a/actionpack/lib/action_pack.rb b/actionpack/lib/action_pack.rb index 95fdd3affb..3f69109633 100644 --- a/actionpack/lib/action_pack.rb +++ b/actionpack/lib/action_pack.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true #-- -# Copyright (c) 2004-2017 David Heinemeier Hansson +# Copyright (c) 2004-2018 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 28bc153f4d..37969fcb57 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -7,8 +7,8 @@ module ActionPack end module VERSION - MAJOR = 5 - MINOR = 2 + MAJOR = 6 + MINOR = 0 TINY = 0 PRE = "alpha" diff --git a/actionpack/test/abstract/callbacks_test.rb b/actionpack/test/abstract/callbacks_test.rb index fdc09bd951..4512ea27b3 100644 --- a/actionpack/test/abstract/callbacks_test.rb +++ b/actionpack/test/abstract/callbacks_test.rb @@ -154,7 +154,7 @@ module AbstractController test "when :except is specified, an after action is not triggered on that action" do @controller.process(:index) - assert !@controller.instance_variable_defined?("@authenticated") + assert_not @controller.instance_variable_defined?("@authenticated") end end @@ -198,7 +198,7 @@ module AbstractController test "when :except is specified with an array, an after action is not triggered on that action" do @controller.process(:index) - assert !@controller.instance_variable_defined?("@authenticated") + assert_not @controller.instance_variable_defined?("@authenticated") end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 5262e85a28..f4787ed27a 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -380,10 +380,8 @@ class ForkingExecutor def initialize(size) @size = size @queue = Server.new - file = File.join Dir.tmpdir, Dir::Tmpname.make_tmpname("rails-tests", "fd") - @url = "drbunix://#{file}" @pool = nil - DRb.start_service @url, @queue + @url = DRb.start_service("drbunix:", @queue).uri end def <<(work); @queue << work; end @@ -453,3 +451,7 @@ end class DrivenBySeleniumWithHeadlessChrome < ActionDispatch::SystemTestCase driven_by :selenium, using: :headless_chrome end + +class DrivenBySeleniumWithHeadlessFirefox < ActionDispatch::SystemTestCase + driven_by :selenium, using: :headless_firefox +end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index f9a037e3cc..763df3a776 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -290,29 +290,29 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_template_objects_exist process :assign_this - assert !@controller.instance_variable_defined?(:"@hi") + assert_not @controller.instance_variable_defined?(:"@hi") assert @controller.instance_variable_get(:"@howdy") end def test_template_objects_missing process :nothing - assert !@controller.instance_variable_defined?(:@howdy) + assert_not @controller.instance_variable_defined?(:@howdy) end def test_empty_flash process :flash_me_naked - assert flash.empty? + assert_empty flash end def test_flash_exist process :flash_me - assert flash.any? - assert flash["hello"].present? + assert_predicate flash, :any? + assert_predicate flash["hello"], :present? end def test_flash_does_not_exist process :nothing - assert flash.empty? + assert_empty flash end def test_session_exist @@ -322,7 +322,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def session_does_not_exist process :nothing - assert session.empty? + assert_empty session end def test_redirection_location @@ -343,46 +343,46 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase def test_server_error_response_code process :response500 - assert @response.server_error? + assert_predicate @response, :server_error? process :response599 - assert @response.server_error? + assert_predicate @response, :server_error? process :response404 - assert !@response.server_error? + assert_not_predicate @response, :server_error? end def test_missing_response_code process :response404 - assert @response.not_found? + assert_predicate @response, :not_found? end def test_client_error_response_code process :response404 - assert @response.client_error? + assert_predicate @response, :client_error? end def test_redirect_url_match process :redirect_external - assert @response.redirect? + assert_predicate @response, :redirect? assert_match(/rubyonrails/, @response.redirect_url) - assert !/perloffrails/.match(@response.redirect_url) + assert_no_match(/perloffrails/, @response.redirect_url) end def test_redirection process :redirect_internal - assert @response.redirect? + assert_predicate @response, :redirect? process :redirect_external - assert @response.redirect? + assert_predicate @response, :redirect? process :nothing - assert !@response.redirect? + assert_not_predicate @response, :redirect? end def test_successful_response_code process :nothing - assert @response.successful? + assert_predicate @response, :successful? end def test_response_object diff --git a/actionpack/test/controller/api/conditional_get_test.rb b/actionpack/test/controller/api/conditional_get_test.rb index fd1997f26c..e366ce9532 100644 --- a/actionpack/test/controller/api/conditional_get_test.rb +++ b/actionpack/test/controller/api/conditional_get_test.rb @@ -53,7 +53,7 @@ class ConditionalGetApiTest < ActionController::TestCase @request.if_modified_since = @last_modified get :one assert_equal 304, @response.status.to_i - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal @last_modified, @response.headers["Last-Modified"] end end diff --git a/actionpack/test/controller/api/force_ssl_test.rb b/actionpack/test/controller/api/force_ssl_test.rb index 07459c3753..8191578eb0 100644 --- a/actionpack/test/controller/api/force_ssl_test.rb +++ b/actionpack/test/controller/api/force_ssl_test.rb @@ -3,7 +3,9 @@ require "abstract_unit" class ForceSSLApiController < ActionController::API - force_ssl + ActiveSupport::Deprecation.silence do + force_ssl + end def one; end def two diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 9ac82c0d65..a672ede1a9 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -107,9 +107,9 @@ class ControllerInstanceTests < ActiveSupport::TestCase end def test_performed? - assert !@empty.performed? + assert_not_predicate @empty, :performed? @empty.response_body = ["sweet"] - assert @empty.performed? + assert_predicate @empty, :performed? end def test_action_methods diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 3557f9f888..6fe036dd15 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -94,14 +94,14 @@ class FragmentCachingTest < ActionController::TestCase def test_fragment_exist_with_caching_enabled @store.write("views/name", "value") assert @controller.fragment_exist?("name") - assert !@controller.fragment_exist?("other_name") + assert_not @controller.fragment_exist?("other_name") end def test_fragment_exist_with_caching_disabled @controller.perform_caching = false @store.write("views/name", "value") - assert !@controller.fragment_exist?("name") - assert !@controller.fragment_exist?("other_name") + assert_not @controller.fragment_exist?("name") + assert_not @controller.fragment_exist?("other_name") end def test_write_fragment_with_caching_enabled @@ -144,7 +144,7 @@ class FragmentCachingTest < ActionController::TestCase buffer = "generated till now -> ".html_safe buffer << view_context.send(:fragment_for, "expensive") { fragment_computed = true } - assert !fragment_computed + assert_not fragment_computed assert_equal "generated till now -> fragment content", buffer end @@ -159,7 +159,7 @@ class FragmentCachingTest < ActionController::TestCase html_safe = @controller.read_fragment("name") assert_equal content, html_safe - assert html_safe.html_safe? + assert_predicate html_safe, :html_safe? end end @@ -173,6 +173,9 @@ class FunctionalCachingController < CachingController end end + def xml_fragment_cached_with_html_partial + end + def formatted_fragment_cached respond_to do |format| format.html @@ -308,6 +311,11 @@ CACHED @store.read("views/functional_caching/formatted_fragment_cached_with_variant:#{template_digest("functional_caching/formatted_fragment_cached_with_variant")}/fragment") end + def test_fragment_caching_with_html_partials_in_xml + get :xml_fragment_cached_with_html_partial, format: "*/*" + assert_response :success + end + private def template_digest(name) ActionView::Digestor.digest(name: name, finder: @controller.lookup_context) @@ -382,7 +390,7 @@ class ViewCacheDependencyTest < ActionController::TestCase end def test_view_cache_dependencies_are_empty_by_default - assert NoDependenciesController.new.view_cache_dependencies.empty? + assert_empty NoDependenciesController.new.view_cache_dependencies end def test_view_cache_dependencies_are_listed_in_declaration_order diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 9f0a9dec7a..425a6e25cc 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -787,7 +787,7 @@ class FilterTest < ActionController::TestCase assert_equal %w( ensure_login find_user ), @controller.instance_variable_get(:@ran_filter) test_process(ConditionalSkippingController, "login") - assert !@controller.instance_variable_defined?("@ran_after_action") + assert_not @controller.instance_variable_defined?("@ran_after_action") test_process(ConditionalSkippingController, "change_password") assert_equal %w( clean_up ), @controller.instance_variable_get("@ran_after_action") end @@ -819,7 +819,7 @@ class FilterTest < ActionController::TestCase response = test_process(RescuedController) end - assert response.successful? + assert_predicate response, :successful? assert_equal("I rescued this: #<FilterTest::ErrorToRescue: Something made the bad noise.>", response.body) end diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index f31a4d9329..e3ec5bb7fc 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -44,7 +44,7 @@ module ActionDispatch @hash["foo"] = "bar" @hash.delete "foo" - assert !@hash.key?("foo") + assert_not @hash.key?("foo") assert_nil @hash["foo"] end @@ -53,7 +53,7 @@ module ActionDispatch assert_equal({ "foo" => "bar" }, @hash.to_hash) @hash.to_hash["zomg"] = "aaron" - assert !@hash.key?("zomg") + assert_not @hash.key?("zomg") assert_equal({ "foo" => "bar" }, @hash.to_hash) end @@ -92,11 +92,11 @@ module ActionDispatch end def test_empty? - assert @hash.empty? + assert_empty @hash @hash["zomg"] = "bears" - assert !@hash.empty? + assert_not_empty @hash @hash.clear - assert @hash.empty? + assert_empty @hash end def test_each diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb index 84ac1fda3c..7f59f6acaf 100644 --- a/actionpack/test/controller/force_ssl_test.rb +++ b/actionpack/test/controller/force_ssl_test.rb @@ -13,19 +13,23 @@ class ForceSSLController < ActionController::Base end class ForceSSLControllerLevel < ForceSSLController - force_ssl + ActiveSupport::Deprecation.silence do + force_ssl + end end class ForceSSLCustomOptions < ForceSSLController - force_ssl host: "secure.example.com", only: :redirect_host - force_ssl port: 8443, only: :redirect_port - force_ssl subdomain: "secure", only: :redirect_subdomain - force_ssl domain: "secure.com", only: :redirect_domain - force_ssl path: "/foo", only: :redirect_path - force_ssl status: :found, only: :redirect_status - force_ssl flash: { message: "Foo, Bar!" }, only: :redirect_flash - force_ssl alert: "Foo, Bar!", only: :redirect_alert - force_ssl notice: "Foo, Bar!", only: :redirect_notice + ActiveSupport::Deprecation.silence do + force_ssl host: "secure.example.com", only: :redirect_host + force_ssl port: 8443, only: :redirect_port + force_ssl subdomain: "secure", only: :redirect_subdomain + force_ssl domain: "secure.com", only: :redirect_domain + force_ssl path: "/foo", only: :redirect_path + force_ssl status: :found, only: :redirect_status + force_ssl flash: { message: "Foo, Bar!" }, only: :redirect_flash + force_ssl alert: "Foo, Bar!", only: :redirect_alert + force_ssl notice: "Foo, Bar!", only: :redirect_notice + end def force_ssl_action render plain: action_name @@ -55,15 +59,21 @@ class ForceSSLCustomOptions < ForceSSLController end class ForceSSLOnlyAction < ForceSSLController - force_ssl only: :cheeseburger + ActiveSupport::Deprecation.silence do + force_ssl only: :cheeseburger + end end class ForceSSLExceptAction < ForceSSLController - force_ssl except: :banana + ActiveSupport::Deprecation.silence do + force_ssl except: :banana + end end class ForceSSLIfCondition < ForceSSLController - force_ssl if: :use_force_ssl? + ActiveSupport::Deprecation.silence do + force_ssl if: :use_force_ssl? + end def use_force_ssl? action_name == "cheeseburger" @@ -71,7 +81,9 @@ class ForceSSLIfCondition < ForceSSLController end class ForceSSLFlash < ForceSSLController - force_ssl except: [:banana, :set_flash, :use_flash] + ActiveSupport::Deprecation.silence do + force_ssl except: [:banana, :set_flash, :use_flash] + end def set_flash flash["that"] = "hello" diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 76ff784926..b133afb343 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -9,7 +9,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase before_action :authenticate_with_request, only: :display USERS = { "lifo" => "world", "pretty" => "please", - "dhh" => ::Digest::MD5::hexdigest(["dhh", "SuperSecret", "secret"].join(":")) } + "dhh" => ::Digest::MD5.hexdigest(["dhh", "SuperSecret", "secret"].join(":")) } def index render plain: "Hello Secret" @@ -181,9 +181,10 @@ class HttpDigestAuthenticationTest < ActionController::TestCase end test "authentication request with password stored as ha1 digest hash" do - @request.env["HTTP_AUTHORIZATION"] = encode_credentials(username: "dhh", - password: ::Digest::MD5::hexdigest(["dhh", "SuperSecret", "secret"].join(":")), - password_is_ha1: true) + @request.env["HTTP_AUTHORIZATION"] = encode_credentials( + username: "dhh", + password: ::Digest::MD5.hexdigest(["dhh", "SuperSecret", "secret"].join(":")), + password_is_ha1: true) get :display assert_response :success @@ -201,7 +202,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase test "validate_digest_response should fail with nil returning password_procedure" do @request.env["HTTP_AUTHORIZATION"] = encode_credentials(username: nil, password: nil) - assert !ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret") { nil } + assert_not ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret") { nil } end test "authentication request with request-uri ending in '/'" do @@ -271,7 +272,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase credentials.merge!(options) path_info = @request.env["PATH_INFO"].to_s uri = options[:uri] || path_info - credentials.merge!(uri: uri) + credentials[:uri] = uri @request.env["ORIGINAL_FULLPATH"] = path_info ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1]) end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index fd1c5e693f..39ede1442a 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -14,11 +14,11 @@ class SessionTest < ActiveSupport::TestCase end def test_https_bang_works_and_sets_truth_by_default - assert !@session.https? + assert_not_predicate @session, :https? @session.https! - assert @session.https? + assert_predicate @session, :https? @session.https! false - assert !@session.https? + assert_not_predicate @session, :https? end def test_host! @@ -135,7 +135,7 @@ class IntegrationTestTest < ActiveSupport::TestCase session1 = @test.open_session { |sess| } session2 = @test.open_session # implicit session - assert !session1.equal?(session2) + assert_not session1.equal?(session2) end # RSpec mixes Matchers (which has a #method_missing) into @@ -345,7 +345,17 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :ok - refute_same previous_html_document, html_document + assert_not_same previous_html_document, html_document + end + end + + def test_redirect_with_arguments + with_test_route_set do + get "/redirect" + follow_redirect! params: { foo: :bar } + + assert_response :ok + assert_equal "bar", request.parameters["foo"] end end @@ -375,7 +385,7 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest a = open_session b = open_session - refute_same(a.integration_session, b.integration_session) + assert_not_same(a.integration_session, b.integration_session) end def test_get_with_query_string @@ -412,11 +422,11 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest get "/get_with_params", params: { foo: "bar" } - assert request.env["rack.input"].string.empty? + assert_empty request.env["rack.input"].string assert_equal "foo=bar", request.env["QUERY_STRING"] assert_equal "foo=bar", request.query_string assert_equal "bar", request.parameters["foo"] - assert request.parameters["leaks"].nil? + assert_predicate request.parameters["leaks"], :nil? end end @@ -1069,6 +1079,20 @@ class IntegrationRequestEncodersTest < ActionDispatch::IntegrationTest end end + def test_get_request_with_json_excludes_null_query_string + with_routing do |routes| + routes.draw do + ActiveSupport::Deprecation.silence do + get ":action" => FooController + end + end + + get "/foos_json", as: :json + + assert_equal "http://www.example.com/foos_json", request.url + end + end + private def post_to_foos(as:) with_routing do |routes| diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 8cfb43a6bc..431fe90b23 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -464,7 +464,7 @@ module ActionController end def test_stale_with_etag - @request.if_none_match = %(W/"#{Digest::MD5.hexdigest('123')}") + @request.if_none_match = %(W/"#{ActiveSupport::Digest.hexdigest('123')}") get :with_stale assert_equal 304, response.status.to_i end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index be455642de..0562c16284 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -82,9 +82,7 @@ module Another @last_payload = payload end - def last_payload - @last_payload - end + attr_reader :last_payload end end diff --git a/actionpack/test/controller/metal_test.rb b/actionpack/test/controller/metal_test.rb index c235c9df86..248ef36b7c 100644 --- a/actionpack/test/controller/metal_test.rb +++ b/actionpack/test/controller/metal_test.rb @@ -9,7 +9,7 @@ class MetalControllerInstanceTests < ActiveSupport::TestCase end end - def test_response_has_default_headers + def test_response_does_not_have_default_headers original_default_headers = ActionDispatch::Response.default_headers ActionDispatch::Response.default_headers = { @@ -23,9 +23,9 @@ class MetalControllerInstanceTests < ActiveSupport::TestCase "rack.input" => -> {} )[1] - refute response_headers.key?("X-Frame-Options") - refute response_headers.key?("X-Content-Type-Options") - refute response_headers.key?("X-XSS-Protection") + assert_not response_headers.key?("X-Frame-Options") + assert_not response_headers.key?("X-Content-Type-Options") + assert_not response_headers.key?("X-XSS-Protection") ensure ActionDispatch::Response.default_headers = original_default_headers end diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index f9ffd5f54c..1163775d3c 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -102,6 +102,26 @@ class RespondToController < ActionController::Base end end + def using_conflicting_nested_js_then_html + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + end + + def using_non_conflicting_nested_js_then_js + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.js { render body: "JS" } + end + end + end + end + def custom_type_handling respond_to do |type| type.html { render body: "HTML" } @@ -430,6 +450,20 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "<p>Hello world!</p>\n", @response.body end + def test_using_conflicting_nested_js_then_html + @request.accept = "*/*" + assert_raises(ActionController::RespondToMismatchError) do + get :using_conflicting_nested_js_then_html + end + end + + def test_using_non_conflicting_nested_js_then_js + @request.accept = "*/*" + get :using_non_conflicting_nested_js_then_js + assert_equal "text/javascript", @response.content_type + assert_equal "JS", @response.body + end + def test_with_atom_content_type @request.accept = "" @request.env["CONTENT_TYPE"] = "application/atom+xml" @@ -658,13 +692,13 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_without_implicit_rendering_from_browser - assert_raises(ActionController::UnknownFormat) do + assert_raises(ActionController::MissingExactTemplate) do get :variant_without_implicit_template_rendering, params: { v: :does_not_matter } end end def test_variant_variant_not_set_and_without_implicit_rendering_from_browser - assert_raises(ActionController::UnknownFormat) do + assert_raises(ActionController::MissingExactTemplate) do get :variant_without_implicit_template_rendering end end diff --git a/actionpack/test/controller/output_escaping_test.rb b/actionpack/test/controller/output_escaping_test.rb index e33a99068f..d683bc73e6 100644 --- a/actionpack/test/controller/output_escaping_test.rb +++ b/actionpack/test/controller/output_escaping_test.rb @@ -4,7 +4,7 @@ require "abstract_unit" class OutputEscapingTest < ActiveSupport::TestCase test "escape_html shouldn't die when passed nil" do - assert ERB::Util.h(nil).blank? + assert_predicate ERB::Util.h(nil), :blank? end test "escapeHTML should escape strings" do diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 43cabae7d2..68c7f2d9ea 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -2,7 +2,6 @@ require "abstract_unit" require "action_controller/metal/strong_parameters" -require "active_support/core_ext/hash/transform_values" class ParametersAccessorsTest < ActiveSupport::TestCase setup do @@ -22,13 +21,13 @@ class ParametersAccessorsTest < ActiveSupport::TestCase test "[] retains permitted status" do @params.permit! - assert @params[:person].permitted? - assert @params[:person][:name].permitted? + assert_predicate @params[:person], :permitted? + assert_predicate @params[:person][:name], :permitted? end test "[] retains unpermitted status" do - assert_not @params[:person].permitted? - assert_not @params[:person][:name].permitted? + assert_not_predicate @params[:person], :permitted? + assert_not_predicate @params[:person][:name], :permitted? end test "as_json returns the JSON representation of the parameters hash" do @@ -51,6 +50,14 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each returns key,value array for block with arity 1" do + @params.each do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "each_pair carries permitted status" do @params.permit! @params.each_pair { |key, value| assert(value.permitted?) if key == "person" } @@ -60,35 +67,43 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each_pair returns key,value array for block with arity 1" do + @params.each_pair do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "empty? returns true when params contains no key/value pairs" do params = ActionController::Parameters.new - assert params.empty? + assert_empty params end test "empty? returns false when any params are present" do - refute @params.empty? + assert_not_empty @params end test "except retains permitted status" do @params.permit! - assert @params.except(:person).permitted? - assert @params[:person].except(:name).permitted? + assert_predicate @params.except(:person), :permitted? + assert_predicate @params[:person].except(:name), :permitted? end test "except retains unpermitted status" do - assert_not @params.except(:person).permitted? - assert_not @params[:person].except(:name).permitted? + assert_not_predicate @params.except(:person), :permitted? + assert_not_predicate @params[:person].except(:name), :permitted? end test "fetch retains permitted status" do @params.permit! - assert @params.fetch(:person).permitted? - assert @params[:person].fetch(:name).permitted? + assert_predicate @params.fetch(:person), :permitted? + assert_predicate @params[:person].fetch(:name), :permitted? end test "fetch retains unpermitted status" do - assert_not @params.fetch(:person).permitted? - assert_not @params[:person].fetch(:name).permitted? + assert_not_predicate @params.fetch(:person), :permitted? + assert_not_predicate @params[:person].fetch(:name), :permitted? end test "has_key? returns true if the given key is present in the params" do @@ -96,7 +111,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase end test "has_key? returns false if the given key is not present in the params" do - refute @params.has_key?(:address) + assert_not @params.has_key?(:address) end test "has_value? returns true if the given value is present in the params" do @@ -106,7 +121,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase test "has_value? returns false if the given value is not present in the params" do params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") - refute params.has_value?("New York") + assert_not params.has_value?("New York") end test "include? returns true if the given key is present in the params" do @@ -114,7 +129,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase end test "include? returns false if the given key is not present in the params" do - refute @params.include?(:address) + assert_not @params.include?(:address) end test "key? returns true if the given key is present in the params" do @@ -122,7 +137,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase end test "key? returns false if the given key is not present in the params" do - refute @params.key?(:address) + assert_not @params.key?(:address) end test "keys returns an array of the keys of the params" do @@ -131,48 +146,69 @@ class ParametersAccessorsTest < ActiveSupport::TestCase end test "reject retains permitted status" do - assert_not @params.reject { |k| k == "person" }.permitted? + assert_not_predicate @params.reject { |k| k == "person" }, :permitted? end test "reject retains unpermitted status" do @params.permit! - assert @params.reject { |k| k == "person" }.permitted? + assert_predicate @params.reject { |k| k == "person" }, :permitted? end test "select retains permitted status" do @params.permit! - assert @params.select { |k| k == "person" }.permitted? + assert_predicate @params.select { |k| k == "person" }, :permitted? end test "select retains unpermitted status" do - assert_not @params.select { |k| k == "person" }.permitted? + assert_not_predicate @params.select { |k| k == "person" }, :permitted? end test "slice retains permitted status" do @params.permit! - assert @params.slice(:person).permitted? + assert_predicate @params.slice(:person), :permitted? end test "slice retains unpermitted status" do - assert_not @params.slice(:person).permitted? + assert_not_predicate @params.slice(:person), :permitted? end test "transform_keys retains permitted status" do @params.permit! - assert @params.transform_keys { |k| k }.permitted? + assert_predicate @params.transform_keys { |k| k }, :permitted? end test "transform_keys retains unpermitted status" do - assert_not @params.transform_keys { |k| k }.permitted? + assert_not_predicate @params.transform_keys { |k| k }, :permitted? end test "transform_values retains permitted status" do @params.permit! - assert @params.transform_values { |v| v }.permitted? + assert_predicate @params.transform_values { |v| v }, :permitted? end test "transform_values retains unpermitted status" do - assert_not @params.transform_values { |v| v }.permitted? + assert_not_predicate @params.transform_values { |v| v }, :permitted? + end + + test "transform_values converts hashes to parameters" do + @params.transform_values do |value| + assert_kind_of ActionController::Parameters, value + value + end + end + + test "transform_values without block yieds an enumerator" do + assert_kind_of Enumerator, @params.transform_values + end + + test "transform_values! converts hashes to parameters" do + @params.transform_values! do |value| + assert_kind_of ActionController::Parameters, value + end + end + + test "transform_values! without block yields an enumerator" do + assert_kind_of Enumerator, @params.transform_values! end test "value? returns true if the given value is present in the params" do @@ -182,7 +218,7 @@ class ParametersAccessorsTest < ActiveSupport::TestCase test "value? returns false if the given value is not present in the params" do params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") - refute params.value?("New York") + assert_not params.value?("New York") end test "values returns an array of the values of the params" do @@ -192,13 +228,13 @@ class ParametersAccessorsTest < ActiveSupport::TestCase test "values_at retains permitted status" do @params.permit! - assert @params.values_at(:person).first.permitted? - assert @params[:person].values_at(:name).first.permitted? + assert_predicate @params.values_at(:person).first, :permitted? + assert_predicate @params[:person].values_at(:name).first, :permitted? end test "values_at retains unpermitted status" do - assert_not @params.values_at(:person).first.permitted? - assert_not @params[:person].values_at(:name).first.permitted? + assert_not_predicate @params.values_at(:person).first, :permitted? + assert_not_predicate @params[:person].values_at(:name).first, :permitted? end test "is equal to Parameters instance with same params" do @@ -257,23 +293,24 @@ class ParametersAccessorsTest < ActiveSupport::TestCase assert_match(/permitted: true/, @params.inspect) end - if Hash.method_defined?(:dig) - test "#dig delegates the dig method to its values" do - assert_equal "David", @params.dig(:person, :name, :first) - assert_equal "Chicago", @params.dig(:person, :addresses, 0, :city) - end + test "#dig delegates the dig method to its values" do + assert_equal "David", @params.dig(:person, :name, :first) + assert_equal "Chicago", @params.dig(:person, :addresses, 0, :city) + end - test "#dig converts hashes to parameters" do - assert_kind_of ActionController::Parameters, @params.dig(:person) - assert_kind_of ActionController::Parameters, @params.dig(:person, :addresses, 0) - assert @params.dig(:person, :addresses).all? do |value| - value.is_a?(ActionController::Parameters) - end - end - else - test "ActionController::Parameters does not respond to #dig on Ruby 2.2" do - assert_not ActionController::Parameters.method_defined?(:dig) - assert_not @params.respond_to?(:dig) + test "#dig converts hashes to parameters" do + assert_kind_of ActionController::Parameters, @params.dig(:person) + assert_kind_of ActionController::Parameters, @params.dig(:person, :addresses, 0) + assert @params.dig(:person, :addresses).all? do |value| + value.is_a?(ActionController::Parameters) end end + + test "mutating #dig return value mutates underlying parameters" do + @params.dig(:person, :name)[:first] = "Bill" + assert_equal "Bill", @params.dig(:person, :name, :first) + + @params.dig(:person, :addresses)[0] = { city: "Boston", state: "Massachusetts" } + assert_equal "Boston", @params.dig(:person, :addresses, 0, :city) + end end diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index 1e8b71d789..fe0e5e368d 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -25,6 +25,6 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase book: { pages: 65 }, format: "json") permitted = params.permit book: [:pages] - assert permitted.permitted? + assert_predicate permitted, :permitted? end end diff --git a/actionpack/test/controller/parameters/dup_test.rb b/actionpack/test/controller/parameters/dup_test.rb index f5833aff46..5403fc6d93 100644 --- a/actionpack/test/controller/parameters/dup_test.rb +++ b/actionpack/test/controller/parameters/dup_test.rb @@ -23,7 +23,7 @@ class ParametersDupTest < ActiveSupport::TestCase test "a duplicate maintains the original's permitted status" do @params.permit! dupped_params = @params.dup - assert dupped_params.permitted? + assert_predicate dupped_params, :permitted? end test "a duplicate maintains the original's parameters" do @@ -57,11 +57,11 @@ class ParametersDupTest < ActiveSupport::TestCase dupped_params = @params.deep_dup dupped_params.permit! - assert_not @params.permitted? + assert_not_predicate @params, :permitted? end test "deep_dup @permitted is being copied" do @params.permit! - assert @params.deep_dup.permitted? + assert_predicate @params.deep_dup, :permitted? end end diff --git a/actionpack/test/controller/parameters/multi_parameter_attributes_test.rb b/actionpack/test/controller/parameters/multi_parameter_attributes_test.rb index dcf848a620..c890839727 100644 --- a/actionpack/test/controller/parameters/multi_parameter_attributes_test.rb +++ b/actionpack/test/controller/parameters/multi_parameter_attributes_test.rb @@ -21,7 +21,7 @@ class MultiParameterAttributesTest < ActiveSupport::TestCase permitted = params.permit book: [ :shipped_at, :price ] - assert permitted.permitted? + assert_predicate permitted, :permitted? assert_equal "2012", permitted[:book]["shipped_at(1i)"] assert_equal "3", permitted[:book]["shipped_at(2i)"] diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index 49dede03c2..312b1e5b27 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -2,7 +2,6 @@ require "abstract_unit" require "action_controller/metal/strong_parameters" -require "active_support/core_ext/hash/transform_values" class ParametersMutatorsTest < ActiveSupport::TestCase setup do @@ -20,11 +19,11 @@ class ParametersMutatorsTest < ActiveSupport::TestCase test "delete retains permitted status" do @params.permit! - assert @params.delete(:person).permitted? + assert_predicate @params.delete(:person), :permitted? end test "delete retains unpermitted status" do - assert_not @params.delete(:person).permitted? + assert_not_predicate @params.delete(:person), :permitted? end test "delete returns the value when the key is present" do @@ -50,73 +49,73 @@ class ParametersMutatorsTest < ActiveSupport::TestCase test "delete_if retains permitted status" do @params.permit! - assert @params.delete_if { |k| k == "person" }.permitted? + assert_predicate @params.delete_if { |k| k == "person" }, :permitted? end test "delete_if retains unpermitted status" do - assert_not @params.delete_if { |k| k == "person" }.permitted? + assert_not_predicate @params.delete_if { |k| k == "person" }, :permitted? end test "extract! retains permitted status" do @params.permit! - assert @params.extract!(:person).permitted? + assert_predicate @params.extract!(:person), :permitted? end test "extract! retains unpermitted status" do - assert_not @params.extract!(:person).permitted? + assert_not_predicate @params.extract!(:person), :permitted? end test "keep_if retains permitted status" do @params.permit! - assert @params.keep_if { |k, v| k == "person" }.permitted? + assert_predicate @params.keep_if { |k, v| k == "person" }, :permitted? end test "keep_if retains unpermitted status" do - assert_not @params.keep_if { |k, v| k == "person" }.permitted? + assert_not_predicate @params.keep_if { |k, v| k == "person" }, :permitted? end test "reject! retains permitted status" do @params.permit! - assert @params.reject! { |k| k == "person" }.permitted? + assert_predicate @params.reject! { |k| k == "person" }, :permitted? end test "reject! retains unpermitted status" do - assert_not @params.reject! { |k| k == "person" }.permitted? + assert_not_predicate @params.reject! { |k| k == "person" }, :permitted? end test "select! retains permitted status" do @params.permit! - assert @params.select! { |k| k != "person" }.permitted? + assert_predicate @params.select! { |k| k != "person" }, :permitted? end test "select! retains unpermitted status" do - assert_not @params.select! { |k| k != "person" }.permitted? + assert_not_predicate @params.select! { |k| k != "person" }, :permitted? end test "slice! retains permitted status" do @params.permit! - assert @params.slice!(:person).permitted? + assert_predicate @params.slice!(:person), :permitted? end test "slice! retains unpermitted status" do - assert_not @params.slice!(:person).permitted? + assert_not_predicate @params.slice!(:person), :permitted? end test "transform_keys! retains permitted status" do @params.permit! - assert @params.transform_keys! { |k| k }.permitted? + assert_predicate @params.transform_keys! { |k| k }, :permitted? end test "transform_keys! retains unpermitted status" do - assert_not @params.transform_keys! { |k| k }.permitted? + assert_not_predicate @params.transform_keys! { |k| k }, :permitted? end test "transform_values! retains permitted status" do @params.permit! - assert @params.transform_values! { |v| v }.permitted? + assert_predicate @params.transform_values! { |v| v }, :permitted? end test "transform_values! retains unpermitted status" do - assert_not @params.transform_values! { |v| v }.permitted? + assert_not_predicate @params.transform_values! { |v| v }, :permitted? end end diff --git a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb index c9fcc483ee..1403e224c0 100644 --- a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb @@ -5,7 +5,7 @@ require "action_controller/metal/strong_parameters" class NestedParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) - assert !params.has_key?(key), "key #{key.inspect} has not been filtered out" + assert_not params.has_key?(key), "key #{key.inspect} has not been filtered out" end test "permitted nested parameters" do @@ -32,7 +32,7 @@ class NestedParametersPermitTest < ActiveSupport::TestCase permitted = params.permit book: [ :title, { authors: [ :name ] }, { details: :pages }, :id ] - assert permitted.permitted? + assert_predicate permitted, :permitted? assert_equal "Romeo and Juliet", permitted[:book][:title] assert_equal "William Shakespeare", permitted[:book][:authors][0][:name] assert_equal "Christopher Marlowe", permitted[:book][:authors][1][:name] diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index ebdaca0162..d2fa0aa16e 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -6,7 +6,7 @@ require "action_controller/metal/strong_parameters" class ParametersPermitTest < ActiveSupport::TestCase def assert_filtered_out(params, key) - assert !params.has_key?(key), "key #{key.inspect} has not been filtered out" + assert_not params.has_key?(key), "key #{key.inspect} has not been filtered out" end setup do @@ -53,13 +53,13 @@ class ParametersPermitTest < ActiveSupport::TestCase test "if nothing is permitted, the hash becomes empty" do params = ActionController::Parameters.new(id: "1234") permitted = params.permit - assert permitted.permitted? - assert permitted.empty? + assert_predicate permitted, :permitted? + assert_empty permitted end test "key: permitted scalar values" do values = ["a", :a, nil] - values += [0, 1.0, 2**128, BigDecimal.new(1)] + values += [0, 1.0, 2**128, BigDecimal(1)] values += [true, false] values += [Date.today, Time.now, DateTime.now] values += [STDOUT, StringIO.new, ActionDispatch::Http::UploadedFile.new(tempfile: __FILE__), @@ -136,7 +136,7 @@ class ParametersPermitTest < ActiveSupport::TestCase test "key: it is not assigned if not present in params" do params = ActionController::Parameters.new(name: "Joe") permitted = params.permit(:id) - assert !permitted.has_key?(:id) + assert_not permitted.has_key?(:id) end test "key to empty array: empty arrays pass" do @@ -227,7 +227,7 @@ class ParametersPermitTest < ActiveSupport::TestCase test "hashes in array values get wrapped" do params = ActionController::Parameters.new(foo: [{}, {}]) params[:foo].each do |hash| - assert !hash.permitted? + assert_not_predicate hash, :permitted? end end @@ -250,7 +250,7 @@ class ParametersPermitTest < ActiveSupport::TestCase permitted = params.permit(users: [:id]) permitted[:users] << { injected: 1 } - assert_not permitted[:users].last.permitted? + assert_not_predicate permitted[:users].last, :permitted? end test "fetch doesnt raise ParameterMissing exception if there is a default" do @@ -272,12 +272,12 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "not permitted is sticky beyond merges" do - assert !@params.merge(a: "b").permitted? + assert_not_predicate @params.merge(a: "b"), :permitted? end test "permitted is sticky beyond merges" do @params.permit! - assert @params.merge(a: "b").permitted? + assert_predicate @params.merge(a: "b"), :permitted? end test "merge with parameters" do @@ -288,12 +288,12 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "not permitted is sticky beyond merge!" do - assert_not @params.merge!(a: "b").permitted? + assert_not_predicate @params.merge!(a: "b"), :permitted? end test "permitted is sticky beyond merge!" do @params.permit! - assert @params.merge!(a: "b").permitted? + assert_predicate @params.merge!(a: "b"), :permitted? end test "merge! with parameters" do @@ -309,7 +309,7 @@ class ParametersPermitTest < ActiveSupport::TestCase merged_params = @params.reverse_merge(default_params) assert_equal "1234", merged_params[:id] - refute_predicate merged_params[:person], :empty? + assert_not_predicate merged_params[:person], :empty? end test "#with_defaults is an alias of reverse_merge" do @@ -317,11 +317,11 @@ class ParametersPermitTest < ActiveSupport::TestCase merged_params = @params.with_defaults(default_params) assert_equal "1234", merged_params[:id] - refute_predicate merged_params[:person], :empty? + assert_not_predicate merged_params[:person], :empty? end test "not permitted is sticky beyond reverse_merge" do - refute_predicate @params.reverse_merge(a: "b"), :permitted? + assert_not_predicate @params.reverse_merge(a: "b"), :permitted? end test "permitted is sticky beyond reverse_merge" do @@ -334,7 +334,7 @@ class ParametersPermitTest < ActiveSupport::TestCase @params.reverse_merge!(default_params) assert_equal "1234", @params[:id] - refute_predicate @params[:person], :empty? + assert_not_predicate @params[:person], :empty? end test "#with_defaults! is an alias of reverse_merge!" do @@ -342,7 +342,7 @@ class ParametersPermitTest < ActiveSupport::TestCase @params.with_defaults!(default_params) assert_equal "1234", @params[:id] - refute_predicate @params[:person], :empty? + assert_not_predicate @params[:person], :empty? end test "modifying the parameters" do @@ -353,12 +353,15 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "Jonas", @params[:person][:family][:brother] end - test "permit is recursive" do + test "permit! is recursive" do + @params[:nested_array] = [[{ x: 2, y: 3 }, { x: 21, y: 42 }]] @params.permit! - assert @params.permitted? - assert @params[:person].permitted? - assert @params[:person][:name].permitted? - assert @params[:person][:addresses][0].permitted? + assert_predicate @params, :permitted? + assert_predicate @params[:person], :permitted? + assert_predicate @params[:person][:name], :permitted? + assert_predicate @params[:person][:addresses][0], :permitted? + assert_predicate @params[:nested_array][0][0], :permitted? + assert_predicate @params[:nested_array][0][1], :permitted? end test "permitted takes a default value when Parameters.permit_all_parameters is set" do @@ -368,8 +371,8 @@ class ParametersPermitTest < ActiveSupport::TestCase age: "32", name: { first: "David", last: "Heinemeier Hansson" } }) - assert params.slice(:person).permitted? - assert params[:person][:name].permitted? + assert_predicate params.slice(:person), :permitted? + assert_predicate params[:person][:name], :permitted? ensure ActionController::Parameters.permit_all_parameters = false end @@ -500,9 +503,9 @@ class ParametersPermitTest < ActiveSupport::TestCase params = ActionController::Parameters.new(foo: "bar") assert params.permit(:foo).has_key?(:foo) - refute params.permit(foo: []).has_key?(:foo) - refute params.permit(foo: [:bar]).has_key?(:foo) - refute params.permit(foo: :bar).has_key?(:foo) + assert_not params.permit(foo: []).has_key?(:foo) + assert_not params.permit(foo: [:bar]).has_key?(:foo) + assert_not params.permit(foo: :bar).has_key?(:foo) end test "#permitted? is false by default" do diff --git a/actionpack/test/controller/parameters/serialization_test.rb b/actionpack/test/controller/parameters/serialization_test.rb index 823f01d82a..7708c8e4fe 100644 --- a/actionpack/test/controller/parameters/serialization_test.rb +++ b/actionpack/test/controller/parameters/serialization_test.rb @@ -2,7 +2,6 @@ require "abstract_unit" require "action_controller/metal/strong_parameters" -require "active_support/core_ext/string/strip" class ParametersSerializationTest < ActiveSupport::TestCase setup do @@ -27,21 +26,21 @@ class ParametersSerializationTest < ActiveSupport::TestCase roundtripped = YAML.load(YAML.dump(params)) assert_equal params, roundtripped - assert_not roundtripped.permitted? + assert_not_predicate roundtripped, :permitted? end test "yaml backwardscompatible with psych 2.0.8 format" do - params = YAML.load <<-end_of_yaml.strip_heredoc + params = YAML.load <<~end_of_yaml --- !ruby/hash:ActionController::Parameters key: :value end_of_yaml assert_equal :value, params[:key] - assert_not params.permitted? + assert_not_predicate params, :permitted? end test "yaml backwardscompatible with psych 2.0.9+ format" do - params = YAML.load(<<-end_of_yaml.strip_heredoc) + params = YAML.load(<<~end_of_yaml) --- !ruby/hash-with-ivars:ActionController::Parameters elements: key: :value @@ -50,6 +49,6 @@ class ParametersSerializationTest < ActiveSupport::TestCase end_of_yaml assert_equal :value, params[:key] - assert_not params.permitted? + assert_not_predicate params, :permitted? end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 37a62edc15..306b245bd1 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -141,6 +141,16 @@ class TestController < ActionController::Base render action: "hello_world" end + def conditional_hello_with_expires_in_with_stale_while_revalidate + expires_in 1.minute, public: true, stale_while_revalidate: 5.minutes + render action: "hello_world" + end + + def conditional_hello_with_expires_in_with_stale_if_error + expires_in 1.minute, public: true, stale_if_error: 5.minutes + render action: "hello_world" + end + def conditional_hello_with_expires_in_with_public_with_more_keys expires_in 1.minute, :public => true, "s-maxage" => 5.hours render action: "hello_world" @@ -240,6 +250,15 @@ class TestController < ActionController::Base head 204 end + def head_default_content_type + # simulating path like "/1.foobar" + request.formats = [] + + respond_to do |format| + format.any { head 200 } + end + end + private def set_variable_for_layout @@ -358,6 +377,16 @@ class ExpiresInRenderTest < ActionController::TestCase assert_equal "max-age=60, public, must-revalidate", @response.headers["Cache-Control"] end + def test_expires_in_header_with_stale_while_revalidate + get :conditional_hello_with_expires_in_with_stale_while_revalidate + assert_equal "max-age=60, public, stale-while-revalidate=300", @response.headers["Cache-Control"] + end + + def test_expires_in_header_with_stale_if_error + get :conditional_hello_with_expires_in_with_stale_if_error + assert_equal "max-age=60, public, stale-if-error=300", @response.headers["Cache-Control"] + end + def test_expires_in_header_with_additional_headers get :conditional_hello_with_expires_in_with_public_with_more_keys assert_equal "max-age=60, public, s-maxage=18000", @response.headers["Cache-Control"] @@ -415,7 +444,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = @last_modified get :conditional_hello assert_equal 304, @response.status.to_i - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -430,7 +459,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = "Thu, 16 Jul 2008 00:00:00 GMT" get :conditional_hello assert_equal 200, @response.status.to_i - assert @response.body.present? + assert_predicate @response.body, :present? assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -443,7 +472,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = @last_modified get :conditional_hello_with_record assert_equal 304, @response.status.to_i - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_not_nil @response.etag assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -459,7 +488,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = "Thu, 16 Jul 2008 00:00:00 GMT" get :conditional_hello_with_record assert_equal 200, @response.status.to_i - assert @response.body.present? + assert_predicate @response.body, :present? assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -472,7 +501,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = @last_modified get :conditional_hello_with_collection_of_records assert_equal 304, @response.status.to_i - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -487,7 +516,7 @@ class LastModifiedRenderTest < ActionController::TestCase @request.if_modified_since = "Thu, 16 Jul 2008 00:00:00 GMT" get :conditional_hello_with_collection_of_records assert_equal 200, @response.status.to_i - assert @response.body.present? + assert_predicate @response.body, :present? assert_equal @last_modified, @response.headers["Last-Modified"] end @@ -592,7 +621,7 @@ class EtagRenderTest < ActionController::TestCase end def strong_etag(record) - %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") + %("#{ActiveSupport::Digest.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") end end @@ -650,7 +679,7 @@ class ImplicitRenderTest < ActionController::TestCase tests ImplicitRenderTestController def test_implicit_no_content_response_as_browser - assert_raises(ActionController::UnknownFormat) do + assert_raises(ActionController::MissingExactTemplate) do get :empty_action end end @@ -682,27 +711,27 @@ class HeadRenderTest < ActionController::TestCase def test_head_created post :head_created - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_response :created end def test_head_created_with_application_json_content_type post :head_created_with_application_json_content_type - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "application/json", @response.header["Content-Type"] assert_response :created end def test_head_ok_with_image_png_content_type post :head_ok_with_image_png_content_type - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "image/png", @response.header["Content-Type"] assert_response :ok end def test_head_with_location_header get :head_with_location_header - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "/foo", @response.headers["Location"] assert_response :ok end @@ -718,7 +747,7 @@ class HeadRenderTest < ActionController::TestCase end get :head_with_location_object - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "http://www.nextangle.com/customers/1", @response.headers["Location"] assert_response :ok end @@ -726,14 +755,14 @@ class HeadRenderTest < ActionController::TestCase def test_head_with_custom_header get :head_with_custom_header - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "something", @response.headers["X-Custom-Header"] assert_response :ok end def test_head_with_www_authenticate_header get :head_with_www_authenticate_header - assert @response.body.blank? + assert_predicate @response.body, :blank? assert_equal "something", @response.headers["WWW-Authenticate"] assert_response :ok end @@ -794,6 +823,11 @@ class HeadRenderTest < ActionController::TestCase get :head_and_return end end + + def test_head_default_content_type + post :head_default_content_type + assert_equal "text/html", @response.header["Content-Type"] + end end class HttpCacheForeverTest < ActionController::TestCase @@ -812,7 +846,7 @@ class HttpCacheForeverTest < ActionController::TestCase assert_response :ok assert_equal "max-age=#{100.years}, public", @response.headers["Cache-Control"] assert_not_nil @response.etag - assert @response.weak_etag? + assert_predicate @response, :weak_etag? end def test_cache_with_private @@ -820,7 +854,7 @@ class HttpCacheForeverTest < ActionController::TestCase assert_response :ok assert_equal "max-age=#{100.years}, private", @response.headers["Cache-Control"] assert_not_nil @response.etag - assert @response.weak_etag? + assert_predicate @response, :weak_etag? end def test_cache_response_code_with_if_modified_since diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index eb3d2f34a8..ea94a3e048 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -446,6 +446,19 @@ module RequestForgeryProtectionTests end end + def test_should_raise_for_post_with_null_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + exception = assert_raises(ActionController::InvalidAuthenticityToken) do + @request.set_header "HTTP_ORIGIN", "null" + post :index, params: { custom_authenticity_token: @token } + end + assert_match "The browser returned a 'null' origin for a request", exception.message + end + end + end + def test_should_block_post_with_origin_checking_and_wrong_origin old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new @@ -508,6 +521,11 @@ module RequestForgeryProtectionTests get :negotiate_same_origin end + assert_cross_origin_blocked do + @request.accept = "application/javascript" + get :negotiate_same_origin + end + assert_cross_origin_not_blocked { get :same_origin_js, xhr: true } assert_cross_origin_not_blocked { get :same_origin_js, xhr: true, format: "js" } assert_cross_origin_not_blocked do @@ -733,7 +751,7 @@ class FreeCookieControllerTest < ActionController::TestCase test "should not emit a csrf-token meta tag" do SecureRandom.stub :base64, @token do get :meta - assert @response.body.blank? + assert_predicate @response.body, :blank? end end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 3d98237003..d336b96eff 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -66,7 +66,6 @@ class ResourcesTest < ActionController::TestCase member_methods.each_key do |action| assert_named_route "/messages/1/#{path_names[action] || action}", "#{action}_message_path", action: action, id: "1" end - end end end @@ -307,7 +306,7 @@ class ResourcesTest < ActionController::TestCase set.draw do resources :messages do member do - match :mark , via: method + match :mark, via: method match :unmark, via: method end end @@ -1323,7 +1322,7 @@ class ResourcesTest < ActionController::TestCase def assert_resource_allowed_routes(controller, options, shallow_options, allowed, not_allowed, path = controller) shallow_path = "#{path}/#{shallow_options[:id]}" format = options[:format] && ".#{options[:format]}" - options.merge!(controller: controller) + options[:controller] = controller shallow_options.merge!(options) assert_whether_allowed(allowed, not_allowed, options, "index", "#{path}#{format}", :get) @@ -1337,7 +1336,7 @@ class ResourcesTest < ActionController::TestCase def assert_singleton_resource_allowed_routes(controller, options, allowed, not_allowed, path = controller.singularize) format = options[:format] && ".#{options[:format]}" - options.merge!(controller: controller) + options[:controller] = controller assert_whether_allowed(allowed, not_allowed, options, "new", "#{path}/new#{format}", :get) assert_whether_allowed(allowed, not_allowed, options, "create", "#{path}#{format}", :post) diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f09051b306..a7033b2d30 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -23,7 +23,7 @@ class UriReservedCharactersRoutingTest < ActiveSupport::TestCase end safe, unsafe = %w(: @ & = + $ , ;), %w(^ ? # [ ]) - hex = unsafe.map { |char| "%" + char.unpack("H2").first.upcase } + hex = unsafe.map { |char| "%" + char.unpack1("H2").upcase } @segment = "#{safe.join}#{unsafe.join}".freeze @escaped = "#{safe.join}#{hex.join}".freeze @@ -213,7 +213,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal expected, ActiveSupport::JSON.decode(get(u)) end - def test_regexp_precidence + def test_regexp_precedence rs.draw do get "/whois/:domain", constraints: { domain: /\w+\.[\w\.]+/ }, @@ -676,7 +676,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase token = "\321\202\320\265\320\272\321\201\321\202".dup # 'text' in Russian token.force_encoding(Encoding::BINARY) - escaped_token = CGI::escape(token) + escaped_token = CGI.escape(token) assert_equal "/page/" + escaped_token, url_for(rs, controller: "content", action: "show_page", id: token) assert_equal({ controller: "content", action: "show_page", id: token }, rs.recognize_path("/page/#{escaped_token}")) @@ -937,7 +937,6 @@ class RouteSetTest < ActiveSupport::TestCase @default_route_set ||= begin set = ActionDispatch::Routing::RouteSet.new set.draw do - ActiveSupport::Deprecation.silence do get "/:controller(/:action(/:id))" end @@ -1288,14 +1287,14 @@ class RouteSetTest < ActiveSupport::TestCase end def test_routing_traversal_does_not_load_extra_classes - assert !Object.const_defined?("Profiler__"), "Profiler should not be loaded" + assert_not Object.const_defined?("Profiler__"), "Profiler should not be loaded" set.draw do get "/profile" => "profile#index" end request_path_params("/profile") rescue nil - assert !Object.const_defined?("Profiler__"), "Profiler should not be loaded" + assert_not Object.const_defined?("Profiler__"), "Profiler should not be loaded" end def test_recognize_with_conditions_and_format @@ -1342,11 +1341,9 @@ class RouteSetTest < ActiveSupport::TestCase def test_namespace set.draw do - namespace "api" do get "inventory" => "products#inventory" end - end params = request_path_params("/api/inventory", method: :get) @@ -1687,7 +1684,7 @@ class RouteSetTest < ActiveSupport::TestCase def test_routes_with_symbols set.draw do get "unnamed", controller: :pages, action: :show, name: :as_symbol - get "named" , controller: :pages, action: :show, name: :as_symbol, as: :named + get "named", controller: :pages, action: :show, name: :as_symbol, as: :named end assert_equal({ controller: "pages", action: "show", name: :as_symbol }, set.recognize_path("/unnamed")) assert_equal({ controller: "pages", action: "show", name: :as_symbol }, set.recognize_path("/named")) @@ -1893,7 +1890,7 @@ class RouteSetTest < ActiveSupport::TestCase assert_equal({ controller: "blog", action: "show_date", year: "2006", month: "07", day: "28" }, controller.request.path_parameters) assert_equal("/blog/2006/07/25", controller.url_for(day: 25, only_path: true)) assert_equal("/blog/2005", controller.url_for(year: 2005, only_path: true)) - assert_equal("/blog/show/123", controller.url_for(action: "show" , id: 123, only_path: true)) + assert_equal("/blog/show/123", controller.url_for(action: "show", id: 123, only_path: true)) assert_equal("/blog/2006", controller.url_for(year: 2006, only_path: true)) assert_equal("/blog/2006", controller.url_for(year: 2006, month: nil, only_path: true)) end diff --git a/actionpack/test/controller/runner_test.rb b/actionpack/test/controller/runner_test.rb index a96c9c519b..1709ab5f6d 100644 --- a/actionpack/test/controller/runner_test.rb +++ b/actionpack/test/controller/runner_test.rb @@ -17,8 +17,8 @@ module ActionDispatch def test_respond_to? runner = MyRunner.new(Class.new { def x; end }.new) - assert runner.respond_to?(:hi) - assert runner.respond_to?(:x) + assert_respond_to runner, :hi + assert_respond_to runner, :x end end end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index fd2399e433..7b1a52b277 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -178,7 +178,7 @@ class SendFileTest < ActionController::TestCase "image.jpg" => "image/jpeg", "image.tif" => "image/tiff", "image.gif" => "image/gif", - "movie.mpg" => "video/mpeg", + "movie.mp4" => "video/mp4", "file.zip" => "application/zip", "file.unk" => "application/octet-stream", "zip" => "application/octet-stream" diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 536c5ed97a..dda2686a9b 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -223,6 +223,27 @@ XML assert_equal params.to_query, @response.body end + def test_params_round_trip + params = { "foo" => { "contents" => [{ "name" => "gorby", "id" => "123" }, { "name" => "puff", "d" => "true" }] } } + post :test_params, params: params.dup + + controller_info = { "controller" => "test_case_test/test", "action" => "test_params" } + assert_equal params.merge(controller_info), JSON.parse(@response.body) + end + + def test_handle_to_params + klass = Class.new do + def to_param + "bar" + end + end + + post :test_params, params: { foo: klass.new } + + assert_equal JSON.parse(@response.body)["foo"], "bar" + end + + def test_body_stream params = Hash[:page, { name: "page name" }, "some key", 123] @@ -380,7 +401,13 @@ XML process :test_xml_output, params: { response_as: "text/html" } # <area> auto-closes, so the <p> becomes a sibling - assert_select "root > area + p" + if defined?(JRUBY_VERSION) + # https://github.com/sparklemotion/nokogiri/issues/1653 + # HTML parser "fixes" "broken" markup in slightly different ways + assert_select "root > map > area + p" + else + assert_select "root > area + p" + end end def test_should_not_impose_childless_html_tags_in_xml @@ -670,7 +697,7 @@ XML assert_equal "bar", @request.params[:foo] post :no_op - assert @request.params[:foo].blank? + assert_predicate @request.params[:foo], :blank? end def test_filtered_parameters_reset_between_requests @@ -681,6 +708,22 @@ XML assert_equal "baz", @request.filtered_parameters[:foo] end + def test_raw_post_reset_between_post_requests + post :no_op, params: { foo: "bar" } + assert_equal "foo=bar", @request.raw_post + + post :no_op, params: { foo: "baz" } + assert_equal "foo=baz", @request.raw_post + end + + def test_content_length_reset_after_post_request + post :no_op, params: { foo: "bar" } + assert_not_equal 0, @request.content_length + + get :no_op + assert_equal 0, @request.content_length + end + def test_path_is_kept_after_the_request get :test_params, params: { id: "foo" } assert_equal "/test_case_test/test/test_params/foo", @request.path @@ -740,6 +783,14 @@ XML assert_equal "application/json", @response.body end + def test_request_format_kwarg_doesnt_mutate_params + params = { foo: "bar" }.freeze + + assert_nothing_raised do + get :test_format, format: "json", params: params + end + end + def test_should_have_knowledge_of_client_side_cookie_state_even_if_they_are_not_set cookies["foo"] = "bar" get :no_op @@ -838,7 +889,7 @@ XML def test_fixture_file_upload_should_be_able_access_to_tempfile file = fixture_file_upload(FILES_DIR + "/ruby_on_rails.jpg", "image/jpg") - assert file.respond_to?(:tempfile), "expected tempfile should respond on fixture file object, got nothing" + assert_respond_to file, :tempfile end def test_fixture_file_upload diff --git a/actionpack/test/controller/url_for_integration_test.rb b/actionpack/test/controller/url_for_integration_test.rb index a7c7356921..a1521da702 100644 --- a/actionpack/test/controller/url_for_integration_test.rb +++ b/actionpack/test/controller/url_for_integration_test.rb @@ -35,7 +35,6 @@ module ActionPack as: "blog" resources :people - #match 'legacy/people' => "people#index", :legacy => "true" get "symbols", controller: :symbols, action: :show, name: :as_symbol get "id_default(/:id)" => "foo#id_default", :id => 1 diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index cf11227897..e381abee36 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -288,7 +288,7 @@ module AbstractController kls = Class.new { include set.url_helpers } controller = kls.new - assert controller.respond_to?(:home_url) + assert_respond_to controller, :home_url assert_equal "http://www.basecamphq.com/home/sweet/home/again", controller.send(:home_url, host: "www.basecamphq.com", user: "again") diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index 0f79c83b6d..ca83b850d5 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -19,7 +19,7 @@ class UrlRewriterTests < ActionController::TestCase def setup @params = {} - @rewriter = Rewriter.new(@request) #.new(@request, @params) + @rewriter = Rewriter.new(@request) @routes = ActionDispatch::Routing::RouteSet.new.tap do |r| r.draw do ActiveSupport::Deprecation.silence do diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb new file mode 100644 index 0000000000..4f9a4ff2bd --- /dev/null +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -0,0 +1,525 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ContentSecurityPolicyTest < ActiveSupport::TestCase + def setup + @policy = ActionDispatch::ContentSecurityPolicy.new + end + + def test_build + assert_equal "", @policy.build + + @policy.script_src :self + assert_equal "script-src 'self'", @policy.build + end + + def test_dup + @policy.img_src :self + @policy.block_all_mixed_content + @policy.upgrade_insecure_requests + @policy.sandbox + copied = @policy.dup + assert_equal copied.build, @policy.build + end + + def test_mappings + @policy.script_src :data + assert_equal "script-src data:", @policy.build + + @policy.script_src :mediastream + assert_equal "script-src mediastream:", @policy.build + + @policy.script_src :blob + assert_equal "script-src blob:", @policy.build + + @policy.script_src :filesystem + assert_equal "script-src filesystem:", @policy.build + + @policy.script_src :self + assert_equal "script-src 'self'", @policy.build + + @policy.script_src :unsafe_inline + assert_equal "script-src 'unsafe-inline'", @policy.build + + @policy.script_src :unsafe_eval + assert_equal "script-src 'unsafe-eval'", @policy.build + + @policy.script_src :none + assert_equal "script-src 'none'", @policy.build + + @policy.script_src :strict_dynamic + assert_equal "script-src 'strict-dynamic'", @policy.build + + @policy.script_src :ws + assert_equal "script-src ws:", @policy.build + + @policy.script_src :wss + assert_equal "script-src wss:", @policy.build + + @policy.script_src :none, :report_sample + assert_equal "script-src 'none' 'report-sample'", @policy.build + end + + def test_fetch_directives + @policy.child_src :self + assert_match %r{child-src 'self'}, @policy.build + + @policy.child_src false + assert_no_match %r{child-src}, @policy.build + + @policy.connect_src :self + assert_match %r{connect-src 'self'}, @policy.build + + @policy.connect_src false + assert_no_match %r{connect-src}, @policy.build + + @policy.default_src :self + assert_match %r{default-src 'self'}, @policy.build + + @policy.default_src false + assert_no_match %r{default-src}, @policy.build + + @policy.font_src :self + assert_match %r{font-src 'self'}, @policy.build + + @policy.font_src false + assert_no_match %r{font-src}, @policy.build + + @policy.frame_src :self + assert_match %r{frame-src 'self'}, @policy.build + + @policy.frame_src false + assert_no_match %r{frame-src}, @policy.build + + @policy.img_src :self + assert_match %r{img-src 'self'}, @policy.build + + @policy.img_src false + assert_no_match %r{img-src}, @policy.build + + @policy.manifest_src :self + assert_match %r{manifest-src 'self'}, @policy.build + + @policy.manifest_src false + assert_no_match %r{manifest-src}, @policy.build + + @policy.media_src :self + assert_match %r{media-src 'self'}, @policy.build + + @policy.media_src false + assert_no_match %r{media-src}, @policy.build + + @policy.object_src :self + assert_match %r{object-src 'self'}, @policy.build + + @policy.object_src false + assert_no_match %r{object-src}, @policy.build + + @policy.prefetch_src :self + assert_match %r{prefetch-src 'self'}, @policy.build + + @policy.prefetch_src false + assert_no_match %r{prefetch-src}, @policy.build + + @policy.script_src :self + assert_match %r{script-src 'self'}, @policy.build + + @policy.script_src false + assert_no_match %r{script-src}, @policy.build + + @policy.style_src :self + assert_match %r{style-src 'self'}, @policy.build + + @policy.style_src false + assert_no_match %r{style-src}, @policy.build + + @policy.worker_src :self + assert_match %r{worker-src 'self'}, @policy.build + + @policy.worker_src false + assert_no_match %r{worker-src}, @policy.build + end + + def test_document_directives + @policy.base_uri "https://example.com" + assert_match %r{base-uri https://example\.com}, @policy.build + + @policy.plugin_types "application/x-shockwave-flash" + assert_match %r{plugin-types application/x-shockwave-flash}, @policy.build + + @policy.sandbox + assert_match %r{sandbox}, @policy.build + + @policy.sandbox "allow-scripts", "allow-modals" + assert_match %r{sandbox allow-scripts allow-modals}, @policy.build + + @policy.sandbox false + assert_no_match %r{sandbox}, @policy.build + end + + def test_navigation_directives + @policy.form_action :self + assert_match %r{form-action 'self'}, @policy.build + + @policy.frame_ancestors :self + assert_match %r{frame-ancestors 'self'}, @policy.build + end + + def test_reporting_directives + @policy.report_uri "/violations" + assert_match %r{report-uri /violations}, @policy.build + end + + def test_other_directives + @policy.block_all_mixed_content + assert_match %r{block-all-mixed-content}, @policy.build + + @policy.block_all_mixed_content false + assert_no_match %r{block-all-mixed-content}, @policy.build + + @policy.require_sri_for :script, :style + assert_match %r{require-sri-for script style}, @policy.build + + @policy.require_sri_for "script", "style" + assert_match %r{require-sri-for script style}, @policy.build + + @policy.require_sri_for + assert_no_match %r{require-sri-for}, @policy.build + + @policy.upgrade_insecure_requests + assert_match %r{upgrade-insecure-requests}, @policy.build + + @policy.upgrade_insecure_requests false + assert_no_match %r{upgrade-insecure-requests}, @policy.build + end + + def test_multiple_sources + @policy.script_src :self, :https + assert_equal "script-src 'self' https:", @policy.build + end + + def test_multiple_directives + @policy.script_src :self, :https + @policy.style_src :self, :https + assert_equal "script-src 'self' https:; style-src 'self' https:", @policy.build + end + + def test_dynamic_directives + request = ActionDispatch::Request.new("HTTP_HOST" => "www.example.com") + controller = Struct.new(:request).new(request) + + @policy.script_src -> { request.host } + assert_equal "script-src www.example.com", @policy.build(controller) + end + + def test_mixed_static_and_dynamic_directives + @policy.script_src :self, -> { "foo.com" }, "bar.com" + request = ActionDispatch::Request.new({}) + controller = Struct.new(:request).new(request) + assert_equal "script-src 'self' foo.com bar.com", @policy.build(controller) + end + + def test_invalid_directive_source + exception = assert_raises(ArgumentError) do + @policy.script_src [:self] + end + + assert_equal "Invalid content security policy source: [:self]", exception.message + end + + def test_missing_context_for_dynamic_source + @policy.script_src -> { request.host } + + exception = assert_raises(RuntimeError) do + @policy.build + end + + assert_match %r{\AMissing context for the dynamic content security policy source:}, exception.message + end + + def test_raises_runtime_error_when_unexpected_source + @policy.plugin_types [:flash] + + exception = assert_raises(RuntimeError) do + @policy.build + end + + assert_match %r{\AUnexpected content security policy source:}, exception.message + end +end + +class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + def index + head :ok + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "default_content_security_policy_integration_test" do + get "/", to: "policy#index" + end + end + + POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + p.script_src :https + end + + class PolicyConfigMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.content_security_policy"] = POLICY + env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" } + env["action_dispatch.content_security_policy_report_only"] = false + env["action_dispatch.show_exceptions"] = false + + @app.call(env) + end + end + + APP = build_app(ROUTES) do |middleware| + middleware.use PolicyConfigMiddleware + middleware.use ActionDispatch::ContentSecurityPolicy::Middleware + end + + def app + APP + end + + def test_adds_nonce_to_script_src_content_security_policy_only_once + get "/" + get "/" + assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" + end + + private + + def assert_policy(expected, report_only: false) + assert_response :success + + if report_only + expected_header = "Content-Security-Policy-Report-Only" + unexpected_header = "Content-Security-Policy" + else + expected_header = "Content-Security-Policy" + unexpected_header = "Content-Security-Policy-Report-Only" + end + + assert_nil response.headers[unexpected_header] + assert_equal expected, response.headers[expected_header] + end +end + +class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + content_security_policy only: :inline do |p| + p.default_src "https://example.com" + end + + content_security_policy only: :conditional, if: :condition? do |p| + p.default_src "https://true.example.com" + end + + content_security_policy only: :conditional, unless: :condition? do |p| + p.default_src "https://false.example.com" + end + + content_security_policy only: :report_only do |p| + p.report_uri "/violations" + end + + content_security_policy only: :script_src do |p| + p.default_src false + p.script_src :self + end + + content_security_policy(false, only: :no_policy) + + content_security_policy_report_only only: :report_only + + def index + head :ok + end + + def inline + head :ok + end + + def conditional + head :ok + end + + def report_only + head :ok + end + + def script_src + head :ok + end + + def no_policy + head :ok + end + + private + def condition? + params[:condition] == "true" + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "content_security_policy_integration_test" do + get "/", to: "policy#index" + get "/inline", to: "policy#inline" + get "/conditional", to: "policy#conditional" + get "/report-only", to: "policy#report_only" + get "/script-src", to: "policy#script_src" + get "/no-policy", to: "policy#no_policy" + end + end + + POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src :self + end + + class PolicyConfigMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.content_security_policy"] = POLICY + env["action_dispatch.content_security_policy_nonce_generator"] = proc { "iyhD0Yc0W+c=" } + env["action_dispatch.content_security_policy_report_only"] = false + env["action_dispatch.show_exceptions"] = false + + @app.call(env) + end + end + + APP = build_app(ROUTES) do |middleware| + middleware.use PolicyConfigMiddleware + middleware.use ActionDispatch::ContentSecurityPolicy::Middleware + end + + def app + APP + end + + def test_generates_content_security_policy_header + get "/" + assert_policy "default-src 'self'" + end + + def test_generates_inline_content_security_policy + get "/inline" + assert_policy "default-src https://example.com" + end + + def test_generates_conditional_content_security_policy + get "/conditional", params: { condition: "true" } + assert_policy "default-src https://true.example.com" + + get "/conditional", params: { condition: "false" } + assert_policy "default-src https://false.example.com" + end + + def test_generates_report_only_content_security_policy + get "/report-only" + assert_policy "default-src 'self'; report-uri /violations", report_only: true + end + + def test_adds_nonce_to_script_src_content_security_policy + get "/script-src" + assert_policy "script-src 'self' 'nonce-iyhD0Yc0W+c='" + end + + def test_generates_no_content_security_policy + get "/no-policy" + + assert_nil response.headers["Content-Security-Policy"] + assert_nil response.headers["Content-Security-Policy-Report-Only"] + end + + private + + def assert_policy(expected, report_only: false) + assert_response :success + + if report_only + expected_header = "Content-Security-Policy-Report-Only" + unexpected_header = "Content-Security-Policy" + else + expected_header = "Content-Security-Policy" + unexpected_header = "Content-Security-Policy-Report-Only" + end + + assert_nil response.headers[unexpected_header] + assert_equal expected, response.headers[expected_header] + end +end + +class DisabledContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest + class PolicyController < ActionController::Base + content_security_policy only: :inline do |p| + p.default_src "https://example.com" + end + + def index + head :ok + end + + def inline + head :ok + end + end + + ROUTES = ActionDispatch::Routing::RouteSet.new + ROUTES.draw do + scope module: "disabled_content_security_policy_integration_test" do + get "/", to: "policy#index" + get "/inline", to: "policy#inline" + end + end + + class PolicyConfigMiddleware + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.content_security_policy"] = nil + env["action_dispatch.content_security_policy_nonce_generator"] = nil + env["action_dispatch.content_security_policy_report_only"] = false + env["action_dispatch.show_exceptions"] = false + + @app.call(env) + end + end + + APP = build_app(ROUTES) do |middleware| + middleware.use PolicyConfigMiddleware + middleware.use ActionDispatch::ContentSecurityPolicy::Middleware + end + + def app + APP + end + + def test_generates_no_content_security_policy_by_default + get "/" + assert_nil response.headers["Content-Security-Policy"] + end + + def test_generates_content_security_policy_header_when_globally_disabled + get "/inline" + assert_equal "default-src https://example.com", response.headers["Content-Security-Policy"] + end +end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 40cbad3b0d..6637c2cae9 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -36,6 +36,12 @@ class CookieJarTest < ActiveSupport::TestCase assert_equal "bar", request.cookie_jar.fetch(:foo) end + def test_to_hash + request.cookie_jar["foo"] = "bar" + assert_equal({ "foo" => "bar" }, request.cookie_jar.to_hash) + assert_equal({ "foo" => "bar" }, request.cookie_jar.to_h) + end + def test_fetch_type_error assert_raises(KeyError) do request.cookie_jar.fetch(:omglolwut) @@ -59,8 +65,8 @@ class CookieJarTest < ActiveSupport::TestCase end def test_key_methods - assert !request.cookie_jar.key?(:foo) - assert !request.cookie_jar.has_key?("foo") + assert_not request.cookie_jar.key?(:foo) + assert_not request.cookie_jar.has_key?("foo") request.cookie_jar[:foo] = :bar assert request.cookie_jar.key?(:foo) @@ -283,6 +289,46 @@ class CookiesTest < ActionController::TestCase cookies[:user_name] = { value: "assain", expires: 2.hours } head :ok end + + def encrypted_discount_and_user_id_cookie + cookies.encrypted[:user_id] = { value: 50, expires: 1.hour } + cookies.encrypted[:discount_percentage] = 10 + + head :ok + end + + def signed_discount_and_user_id_cookie + cookies.signed[:user_id] = { value: 50, expires: 1.hour } + cookies.signed[:discount_percentage] = 10 + + head :ok + end + + def rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + # cookies.encrypted[:favorite] = { value: "5-2-Stable Chocolate Cookies", expires: 1000.years } + cookies[:favorite] = "KvH5lIHvX5vPQkLIK63r/NuIMwzWky8M0Zwk8SZ6DwUv8+srf36geR4nWq5KmhsZIYXA8NRdCZYIfxMKJsOFlz77Gf+Fq8vBBCWJTp95rx39A28TCUTJEyMhCNJO5eie7Skef76Qt5Jo/SCnIADAhzyGQkGBopKRcA==--qXZZFWGbCy6N8AGy--WswoH+xHrNh9MzSXDpB2fA==" + + head :ok + end + + def rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + cookies[:favorite] = "Wmg4amgvcVVvWGcwK3c4WjJEbTdRQUgrWXhBdDliUTR0cVNidXpmVTMrc2RjcitwUzVsWWEwZGtuVGtFUjJwNi0tcVhVMTFMOTQ1d0hIVE1FK0pJc05SQT09--8b2a55c375049a50f7a959b9d42b31ef0b2bb594" + + head :ok + end + + def rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + # cookies.signed[:favorite] = { value: "5-2-Stable Choco Chip Cookie", expires: 1000.years } + cookies[:favorite] = "eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaEpJaUUxTFRJdFUzUmhZbXhsSUVOb2IyTnZJRU5vYVhBZ1EyOXZhMmxsQmpvR1JWUT0iLCJleHAiOiIzMDE4LTA3LTExVDE2OjExOjI2Ljc1M1oiLCJwdXIiOm51bGx9fQ==--7df5d885b78b70a501d6e82140ae91b24060ac00" + + head :ok + end + + def rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + cookies[:favorite] = "BAhJIiE1LTItU3RhYmxlIENob2NvIENoaXAgQ29va2llBjoGRVQ=--50bbdbf8d64f5a3ec3e54878f54d4f55b6cb3aff" + + head :ok + end end tests TestController @@ -319,7 +365,7 @@ class CookiesTest < ActionController::TestCase def test_setting_the_same_value_to_cookie request.cookies[:user_name] = "david" get :authenticate - assert_predicate response.cookies, :empty? + assert_empty response.cookies end def test_setting_the_same_value_to_permanent_cookie @@ -401,7 +447,7 @@ class CookiesTest < ActionController::TestCase def test_delete_unexisting_cookie request.cookies.clear get :delete_cookie - assert_predicate @response.cookies, :empty? + assert_empty @response.cookies end def test_deleted_cookie_predicate @@ -1268,6 +1314,8 @@ class CookiesTest < ActionController::TestCase end def test_signed_cookie_with_expires_set_relatively + request.env["action_dispatch.use_cookies_with_metadata"] = true + cookies.signed[:user_name] = { value: "assain", expires: 2.hours } travel 1.hour @@ -1278,6 +1326,8 @@ class CookiesTest < ActionController::TestCase end def test_encrypted_cookie_with_expires_set_relatively + request.env["action_dispatch.use_cookies_with_metadata"] = true + cookies.encrypted[:user_name] = { value: "assain", expires: 2.hours } travel 1.hour @@ -1294,6 +1344,124 @@ class CookiesTest < ActionController::TestCase end end + def test_purpose_metadata_for_encrypted_cookies + get :encrypted_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_equal 50, cookies.encrypted[:discount_percentage] + + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :encrypted_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_nil cookies.encrypted[:discount_percentage] + end + + def test_purpose_metadata_for_signed_cookies + get :signed_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_equal 50, cookies.signed[:discount_percentage] + + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :signed_discount_and_user_id_cookie + + cookies[:discount_percentage] = cookies[:user_id] + assert_nil cookies.signed[:discount_percentage] + end + + def test_switch_off_metadata_for_encrypted_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + 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] + end + + def test_switch_off_metadata_for_signed_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + 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] + end + + def test_read_rails_5_2_stable_encrypted_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + + travel 1001.years do + assert_nil cookies.encrypted[:favorite] + end + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + end + + def test_read_rails_5_2_stable_signed_cookies_if_config_is_false + request.env["action_dispatch.use_cookies_with_metadata"] = false + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + + travel 1001.years do + assert_nil cookies.signed[:favorite] + end + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + end + + def test_read_rails_5_2_stable_encrypted_cookies_if_use_metadata_config_is_true + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + + travel 1001.years do + assert_nil cookies.encrypted[:favorite] + end + + get :rails_5_2_stable_encrypted_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Chocolate Cookies", cookies.encrypted[:favorite] + end + + def test_read_rails_5_2_stable_signed_cookies_if_use_metadata_config_is_true + request.env["action_dispatch.use_cookies_with_metadata"] = true + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_on + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + + travel 1001.years do + assert_nil cookies.signed[:favorite] + end + + get :rails_5_2_stable_signed_cookie_with_authenticated_encryption_flag_off + + assert_equal "5-2-Stable Choco Chip Cookie", cookies.signed[:favorite] + end + private def assert_cookie_header(expected) header = @response.headers["Set-Cookie"] diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 60acba0616..44b79c0e5d 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -3,6 +3,8 @@ require "abstract_unit" class DebugExceptionsTest < ActionDispatch::IntegrationTest + InterceptedErrorInstance = StandardError.new + class Boomer attr_accessor :closed @@ -24,6 +26,18 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest raise StandardError.new "error in framework" end + def raise_nested_exceptions + begin + raise "First error" + rescue + begin + raise "Second error" + rescue + raise "Third error" + end + end + end + def call(env) env["action_dispatch.show_detailed_exceptions"] = @detailed req = ActionDispatch::Request.new(env) @@ -36,6 +50,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest raise RuntimeError when %r{/method_not_allowed} raise ActionController::MethodNotAllowed + when %r{/intercepted_error} + raise InterceptedErrorInstance when %r{/unknown_http_method} raise ActionController::UnknownHttpMethod when %r{/not_implemented} @@ -70,15 +86,21 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end when %r{/framework_raises} method_that_raises + when %r{/nested_exceptions} + raise_nested_exceptions else raise "puke!" end end end + Interceptor = proc { |request, exception| request.set_header("int", exception) } + BadInterceptor = proc { |request, exception| raise "bad" } RoutesApp = Struct.new(:routes).new(SharedTestRoutes) ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) + InterceptedApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :default, [Interceptor]) + BadInterceptedApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :default, [BadInterceptor]) test "skip diagnosis if not showing detailed exceptions" do @app = ProductionApp @@ -432,8 +454,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/original_syntax_error", headers: { "action_dispatch.backtrace_cleaner" => ActiveSupport::BacktraceCleaner.new } assert_response 500 - assert_select "#Application-Trace" do - assert_select "pre code", /syntax error, unexpected/ + assert_select "#Application-Trace-0" do + assert_select "code", /syntax error, unexpected/ end end @@ -446,9 +468,9 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_select "#container h2", /^Missing template/ - assert_select "#Application-Trace" - assert_select "#Framework-Trace" - assert_select "#Full-Trace" + assert_select "#Application-Trace-0" + assert_select "#Framework-Trace-0" + assert_select "#Full-Trace-0" assert_select "h2", /Request/ end @@ -459,8 +481,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest get "/syntax_error_into_view", headers: { "action_dispatch.backtrace_cleaner" => ActiveSupport::BacktraceCleaner.new } assert_response 500 - assert_select "#Application-Trace" do - assert_select "pre code", /syntax error, unexpected/ + assert_select "#Application-Trace-0" do + assert_select "code", /syntax error, unexpected/ end end @@ -489,13 +511,64 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end # assert application trace refers to line that calls method_that_raises is first - assert_select "#Application-Trace" do - assert_select "pre code a:first", %r{test/dispatch/debug_exceptions_test\.rb:\d+:in `call} + assert_select "#Application-Trace-0" do + assert_select "code a:first", %r{test/dispatch/debug_exceptions_test\.rb:\d+:in `call} end # assert framework trace that threw the error is first - assert_select "#Framework-Trace" do - assert_select "pre code a:first", /method_that_raises/ + assert_select "#Framework-Trace-0" do + assert_select "code a:first", /method_that_raises/ + end + end + end + + test "invoke interceptors before rendering" do + @app = InterceptedApp + get "/intercepted_error", headers: { "action_dispatch.show_exceptions" => true } + + assert_equal InterceptedErrorInstance, request.get_header("int") + end + + test "bad interceptors doesn't debug exceptions" do + @app = BadInterceptedApp + + get "/puke", headers: { "action_dispatch.show_exceptions" => true } + + assert_response 500 + assert_match(/puke/, body) + end + + test "debug exceptions app shows all the nested exceptions in source view" do + @app = DevelopmentApp + Rails.stub :root, Pathname.new(".") do + cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| + bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + end + + get "/nested_exceptions", headers: { "action_dispatch.backtrace_cleaner" => cleaner } + + # Assert correct error + assert_response 500 + assert_select "h2", /Third error/ + + # assert source view line shows the last error + assert_select "div.source:not(.hidden)" do + assert_select "pre .line.active", /raise "Third error"/ + end + + # assert application trace refers to line that raises the last exception + assert_select "#Application-Trace-0" do + assert_select "code a:first", %r{in `rescue in rescue in raise_nested_exceptions'} + end + + # assert the second application trace refers to the line that raises the second exception + assert_select "#Application-Trace-1" do + assert_select "code a:first", %r{in `rescue in raise_nested_exceptions'} + end + + # assert the third application trace refers to the line that raises the first exception + assert_select "#Application-Trace-2" do + assert_select "code a:first", %r{in `raise_nested_exceptions'} end end end diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index f6e70382a8..668469a01d 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -20,6 +20,7 @@ module ActionDispatch setup do @cleaner = ActiveSupport::BacktraceCleaner.new + @cleaner.remove_filters! @cleaner.add_silencer { |line| line !~ /^lib/ } end @@ -108,11 +109,27 @@ module ActionDispatch wrapper = ExceptionWrapper.new(@cleaner, exception) assert_equal({ - "Application Trace" => [ id: 0, trace: "lib/file.rb:42:in `index'" ], - "Framework Trace" => [ id: 1, trace: "/gems/rack.rb:43:in `index'" ], + "Application Trace" => [ + exception_object_id: exception.object_id, + id: 0, + trace: "lib/file.rb:42:in `index'" + ], + "Framework Trace" => [ + exception_object_id: exception.object_id, + id: 1, + trace: "/gems/rack.rb:43:in `index'" + ], "Full Trace" => [ - { id: 0, trace: "lib/file.rb:42:in `index'" }, - { id: 1, trace: "/gems/rack.rb:43:in `index'" } + { + exception_object_id: exception.object_id, + id: 0, + trace: "lib/file.rb:42:in `index'" + }, + { + exception_object_id: exception.object_id, + id: 1, + trace: "/gems/rack.rb:43:in `index'" + } ] }, wrapper.traces) end diff --git a/actionpack/test/dispatch/executor_test.rb b/actionpack/test/dispatch/executor_test.rb index 8eb6450385..5b8be39b6d 100644 --- a/actionpack/test/dispatch/executor_test.rb +++ b/actionpack/test/dispatch/executor_test.rb @@ -81,7 +81,7 @@ class ExecutorTest < ActiveSupport::TestCase running = false body.close - assert !running + assert_not running end def test_complete_callbacks_are_called_on_close @@ -89,7 +89,7 @@ class ExecutorTest < ActiveSupport::TestCase executor.to_complete { completed = true } body = call_and_return_body - assert !completed + assert_not completed body.close assert completed @@ -116,7 +116,7 @@ class ExecutorTest < ActiveSupport::TestCase call_and_return_body.close assert result - assert !defined?(@in_shared_context) # it's not in the test itself + assert_not defined?(@in_shared_context) # it's not in the test itself end private diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 3a265a056b..bd2a5b35fb 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -156,7 +156,7 @@ class HeaderTest < ActiveSupport::TestCase env = { "HTTP_REFERER" => "/" } headers = make_headers(env) headers["Referer"] = "http://example.com/" - headers.merge! "CONTENT_TYPE" => "text/plain" + headers["CONTENT_TYPE"] = "text/plain" assert_equal({ "HTTP_REFERER" => "http://example.com/", "CONTENT_TYPE" => "text/plain" }, env) end diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index 2901148a9e..a9a56f205f 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -73,7 +73,7 @@ module ActionController } latch.wait - assert @response.headers.frozen? + assert_predicate @response.headers, :frozen? e = assert_raises(ActionDispatch::IllegalStateError) do @response.headers["Content-Length"] = "zomg" end diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 90e95e972d..fa264417e1 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -30,21 +30,21 @@ class MimeTypeTest < ActiveSupport::TestCase test "parse text with trailing star at the beginning" do accept = "text/*, text/html, application/json, multipart/form-data" - expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml], Mime[:json], Mime[:multipart_form]] + expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:vtt], Mime[:xml], Mime[:yaml], Mime[:json], Mime[:multipart_form]] parsed = Mime::Type.parse(accept) - assert_equal expect, parsed + assert_equal expect.map(&:to_s), parsed.map(&:to_s) end test "parse text with trailing star in the end" do accept = "text/html, application/json, multipart/form-data, text/*" - expect = [Mime[:html], Mime[:json], Mime[:multipart_form], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml]] + expect = [Mime[:html], Mime[:json], Mime[:multipart_form], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:vtt], Mime[:xml], Mime[:yaml]] parsed = Mime::Type.parse(accept) - assert_equal expect, parsed + assert_equal expect.map(&:to_s), parsed.map(&:to_s) end test "parse text with trailing star" do accept = "text/*" - expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:xml], Mime[:yaml], Mime[:json]] + expect = [Mime[:html], Mime[:text], Mime[:js], Mime[:css], Mime[:ics], Mime[:csv], Mime[:vcf], Mime[:vtt], Mime[:xml], Mime[:yaml], Mime[:json]] parsed = Mime::Type.parse(accept) assert_equal expect.map(&:to_s).sort!, parsed.map(&:to_s).sort! end @@ -159,7 +159,7 @@ class MimeTypeTest < ActiveSupport::TestCase types.each do |type| mime = Mime[type] - assert mime.respond_to?("#{type}?"), "#{mime.inspect} does not respond to #{type}?" + assert_respond_to mime, "#{type}?" assert_equal type, mime.symbol, "#{mime.inspect} is not #{type}?" invalid_types = types - [type] invalid_types.delete(:html) @@ -180,8 +180,8 @@ class MimeTypeTest < ActiveSupport::TestCase assert Mime[:js] =~ "text/javascript" assert Mime[:js] =~ "application/javascript" assert Mime[:js] !~ "text/html" - assert !(Mime[:js] !~ "text/javascript") - assert !(Mime[:js] !~ "application/javascript") + assert_not (Mime[:js] !~ "text/javascript") + assert_not (Mime[:js] !~ "application/javascript") assert Mime[:html] =~ "application/xhtml+xml" end end diff --git a/actionpack/test/dispatch/reloader_test.rb b/actionpack/test/dispatch/reloader_test.rb index e529229fae..edc4cd62a3 100644 --- a/actionpack/test/dispatch/reloader_test.rb +++ b/actionpack/test/dispatch/reloader_test.rb @@ -115,7 +115,7 @@ class ReloaderTest < ActiveSupport::TestCase reloader.to_complete { completed = true } body = call_and_return_body - assert !completed + assert_not completed body.close assert completed @@ -129,7 +129,7 @@ class ReloaderTest < ActiveSupport::TestCase prepared = false body.close - assert !prepared + assert_not prepared end def test_complete_callbacks_are_called_on_exceptions diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index 7b6ce31f29..74da2fe7d3 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -22,6 +22,7 @@ module ActionDispatch s["foo"] = "bar" assert_equal "bar", s["foo"] assert_equal({ "foo" => "bar" }, s.to_hash) + assert_equal({ "foo" => "bar" }, s.to_h) end def test_create_merges_old @@ -117,6 +118,18 @@ module ActionDispatch end end + def test_dig + session = Session.create(store, req, {}) + session["one"] = { "two" => "3" } + + assert_equal "3", session.dig("one", "two") + assert_equal "3", session.dig(:one, "two") + + assert_nil session.dig("three", "two") + assert_nil session.dig("one", "three") + assert_nil session.dig("one", :two) + end + private def store Class.new { diff --git a/actionpack/test/dispatch/request_id_test.rb b/actionpack/test/dispatch/request_id_test.rb index aa3175c986..9df4712dab 100644 --- a/actionpack/test/dispatch/request_id_test.rb +++ b/actionpack/test/dispatch/request_id_test.rb @@ -11,6 +11,11 @@ class RequestIdTest < ActiveSupport::TestCase assert_equal "X-Hacked-HeaderStuff", stub_request("HTTP_X_REQUEST_ID" => "; X-Hacked-Header: Stuff").request_id end + test "accept Apache mod_unique_id format" do + mod_unique_id = "abcxyz@ABCXYZ-0123456789" + assert_equal mod_unique_id, stub_request("HTTP_X_REQUEST_ID" => mod_unique_id).request_id + end + test "ensure that 255 char limit on the request id is being enforced" do assert_equal "X" * 255, stub_request("HTTP_X_REQUEST_ID" => "X" * 500).request_id end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 2a18395aac..84a2d1f69e 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -329,20 +329,20 @@ class RequestPort < BaseRequestTest test "standard_port?" do request = stub_request - assert !request.ssl? - assert request.standard_port? + assert_not_predicate request, :ssl? + assert_predicate request, :standard_port? request = stub_request "HTTPS" => "on" - assert request.ssl? - assert request.standard_port? + assert_predicate request, :ssl? + assert_predicate request, :standard_port? request = stub_request "HTTP_HOST" => "www.example.org:8080" - assert !request.ssl? - assert !request.standard_port? + assert_not_predicate request, :ssl? + assert_not_predicate request, :standard_port? request = stub_request "HTTP_HOST" => "www.example.org:8443", "HTTPS" => "on" - assert request.ssl? - assert !request.standard_port? + assert_predicate request, :ssl? + assert_not_predicate request, :standard_port? end test "optional port" do @@ -571,7 +571,7 @@ end class LocalhostTest < BaseRequestTest test "IPs that match localhost" do request = stub_request("REMOTE_IP" => "127.1.1.1", "REMOTE_ADDR" => "127.1.1.1") - assert request.local? + assert_predicate request, :local? end end @@ -643,37 +643,37 @@ class RequestProtocol < BaseRequestTest test "xml http request" do request = stub_request - assert !request.xml_http_request? - assert !request.xhr? + assert_not_predicate request, :xml_http_request? + assert_not_predicate request, :xhr? request = stub_request "HTTP_X_REQUESTED_WITH" => "DefinitelyNotAjax1.0" - assert !request.xml_http_request? - assert !request.xhr? + assert_not_predicate request, :xml_http_request? + assert_not_predicate request, :xhr? request = stub_request "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" - assert request.xml_http_request? - assert request.xhr? + assert_predicate request, :xml_http_request? + assert_predicate request, :xhr? end test "reports ssl" do - assert !stub_request.ssl? - assert stub_request("HTTPS" => "on").ssl? + assert_not_predicate stub_request, :ssl? + assert_predicate stub_request("HTTPS" => "on"), :ssl? end test "reports ssl when proxied via lighttpd" do - assert stub_request("HTTP_X_FORWARDED_PROTO" => "https").ssl? + assert_predicate stub_request("HTTP_X_FORWARDED_PROTO" => "https"), :ssl? end test "scheme returns https when proxied" do request = stub_request "rack.url_scheme" => "http" - assert !request.ssl? + assert_not_predicate request, :ssl? assert_equal "http", request.scheme request = stub_request( "rack.url_scheme" => "http", "HTTP_X_FORWARDED_PROTO" => "https" ) - assert request.ssl? + assert_predicate request, :ssl? assert_equal "https", request.scheme end end @@ -700,7 +700,7 @@ class RequestMethod < BaseRequestTest assert_equal "GET", request.request_method assert_equal "GET", request.env["REQUEST_METHOD"] - assert request.get? + assert_predicate request, :get? end test "invalid http method raises exception" do @@ -748,7 +748,7 @@ class RequestMethod < BaseRequestTest assert_equal "POST", request.method assert_equal "PATCH", request.request_method - assert request.patch? + assert_predicate request, :patch? end test "post masquerading as put" do @@ -758,12 +758,12 @@ class RequestMethod < BaseRequestTest ) assert_equal "POST", request.method assert_equal "PUT", request.request_method - assert request.put? + assert_predicate request, :put? end test "post uneffected by local inflections" do existing_acronyms = ActiveSupport::Inflector.inflections.acronyms.dup - existing_acronym_regex = ActiveSupport::Inflector.inflections.acronym_regex.dup + assert_deprecated { ActiveSupport::Inflector.inflections.acronym_regex.dup } begin ActiveSupport::Inflector.inflections do |inflect| inflect.acronym "POS" @@ -772,12 +772,12 @@ class RequestMethod < BaseRequestTest request = stub_request "REQUEST_METHOD" => "POST" assert_equal :post, ActionDispatch::Request::HTTP_METHOD_LOOKUP["POST"] assert_equal :post, request.method_symbol - assert request.post? + assert_predicate request, :post? ensure # Reset original acronym set ActiveSupport::Inflector.inflections do |inflect| inflect.send(:instance_variable_set, "@acronyms", existing_acronyms) - inflect.send(:instance_variable_set, "@acronym_regex", existing_acronym_regex) + inflect.send(:define_acronym_regex_patterns) end end end @@ -785,50 +785,44 @@ end class RequestFormat < BaseRequestTest test "xml format" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :xml }) do - assert_equal Mime[:xml], request.format - end + request = stub_request "QUERY_STRING" => "format=xml" + + assert_equal Mime[:xml], request.format end test "xhtml format" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :xhtml }) do - assert_equal Mime[:html], request.format - end + request = stub_request "QUERY_STRING" => "format=xhtml" + + assert_equal Mime[:html], request.format end test "txt format" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :txt }) do - assert_equal Mime[:text], request.format - end + request = stub_request "QUERY_STRING" => "format=txt" + + assert_equal Mime[:text], request.format end test "XMLHttpRequest" do request = stub_request( "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", - "HTTP_ACCEPT" => [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(",") + "HTTP_ACCEPT" => [Mime[:js], Mime[:html], Mime[:xml], "text/xml", "*/*"].join(","), + "QUERY_STRING" => "" ) - assert_called(request, :parameters, times: 1, returns: {}) do - assert request.xhr? - assert_equal Mime[:js], request.format - end + assert_predicate request, :xhr? + assert_equal Mime[:js], request.format end test "can override format with parameter negative" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :txt }) do - assert !request.format.xml? - end + request = stub_request("QUERY_STRING" => "format=txt") + + assert_not_predicate request.format, :xml? end test "can override format with parameter positive" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :xml }) do - assert request.format.xml? - end + request = stub_request("QUERY_STRING" => "format=xml") + + assert_predicate request.format, :xml? end test "formats text/html with accept header" do @@ -853,40 +847,37 @@ class RequestFormat < BaseRequestTest end test "formats format:text with accept header" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :txt }) do - assert_equal [Mime[:text]], request.formats - end + request = stub_request("QUERY_STRING" => "format=txt") + + assert_equal [Mime[:text]], request.formats end test "formats format:unknown with accept header" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :unknown }) do - assert_instance_of Mime::NullType, request.format - end + request = stub_request("QUERY_STRING" => "format=unknown") + + assert_instance_of Mime::NullType, request.format end test "format is not nil with unknown format" do - request = stub_request - assert_called(request, :parameters, times: 2, returns: { format: :hello }) do - assert request.format.nil? - assert_not request.format.html? - assert_not request.format.xml? - assert_not request.format.json? - end + request = stub_request("QUERY_STRING" => "format=hello") + + assert_nil request.format + assert_not_predicate request.format, :html? + assert_not_predicate request.format, :xml? + assert_not_predicate request.format, :json? end test "format does not throw exceptions when malformed parameters" do request = stub_request("QUERY_STRING" => "x[y]=1&x[y][][w]=2") assert request.formats - assert request.format.html? + assert_predicate request.format, :html? end test "formats with xhr request" do - request = stub_request "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [Mime[:js]], request.formats - end + request = stub_request "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", + "QUERY_STRING" => "" + + assert_equal [Mime[:js]], request.formats end test "ignore_accept_header" do @@ -894,62 +885,58 @@ class RequestFormat < BaseRequestTest ActionDispatch::Request.ignore_accept_header = true begin - request = stub_request "HTTP_ACCEPT" => "application/xml" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime[:html] ], request.formats - end + request = stub_request "HTTP_ACCEPT" => "application/xml", + "QUERY_STRING" => "" - request = stub_request "HTTP_ACCEPT" => "koz-asked/something-crazy" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime[:html] ], request.formats - end + assert_equal [ Mime[:html] ], request.formats - request = stub_request "HTTP_ACCEPT" => "*/*;q=0.1" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime[:html] ], request.formats - end + request = stub_request "HTTP_ACCEPT" => "koz-asked/something-crazy", + "QUERY_STRING" => "" - request = stub_request "HTTP_ACCEPT" => "application/jxw" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime[:html] ], request.formats - end + assert_equal [ Mime[:html] ], request.formats + + request = stub_request "HTTP_ACCEPT" => "*/*;q=0.1", + "QUERY_STRING" => "" + + assert_equal [ Mime[:html] ], request.formats + + request = stub_request "HTTP_ACCEPT" => "application/jxw", + "QUERY_STRING" => "" + + assert_equal [ Mime[:html] ], request.formats request = stub_request "HTTP_ACCEPT" => "application/xml", - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" + "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", + "QUERY_STRING" => "" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [ Mime[:js] ], request.formats - end + assert_equal [ Mime[:js] ], request.formats request = stub_request "HTTP_ACCEPT" => "application/xml", - "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest" - assert_called(request, :parameters, times: 2, returns: { format: :json }) do - assert_equal [ Mime[:json] ], request.formats - end + "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", + "QUERY_STRING" => "format=json" + + assert_equal [ Mime[:json] ], request.formats ensure ActionDispatch::Request.ignore_accept_header = old_ignore_accept_header end end test "format taken from the path extension" do - request = stub_request "PATH_INFO" => "/foo.xml" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [Mime[:xml]], request.formats - end + request = stub_request "PATH_INFO" => "/foo.xml", "QUERY_STRING" => "" - request = stub_request "PATH_INFO" => "/foo.123" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [Mime[:html]], request.formats - end + assert_equal [Mime[:xml]], request.formats + + request = stub_request "PATH_INFO" => "/foo.123", "QUERY_STRING" => "" + + assert_equal [Mime[:html]], request.formats end test "formats from accept headers have higher precedence than path extension" do request = stub_request "HTTP_ACCEPT" => "application/json", - "PATH_INFO" => "/foo.xml" + "PATH_INFO" => "/foo.xml", + "QUERY_STRING" => "" - assert_called(request, :parameters, times: 1, returns: {}) do - assert_equal [Mime[:json]], request.formats - end + assert_equal [Mime[:json]], request.formats end end @@ -997,15 +984,14 @@ end class RequestParameters < BaseRequestTest test "parameters" do - request = stub_request + request = stub_request "CONTENT_TYPE" => "application/json", + "CONTENT_LENGTH" => 9, + "RAW_POST_DATA" => '{"foo":1}', + "QUERY_STRING" => "bar=2" - assert_called(request, :request_parameters, times: 2, returns: { "foo" => 1 }) do - assert_called(request, :query_parameters, times: 2, returns: { "bar" => 2 }) do - assert_equal({ "foo" => 1, "bar" => 2 }, request.parameters) - assert_equal({ "foo" => 1 }, request.request_parameters) - assert_equal({ "bar" => 2 }, request.query_parameters) - end - end + assert_equal({ "foo" => 1, "bar" => "2" }, request.parameters) + assert_equal({ "foo" => 1 }, request.request_parameters) + assert_equal({ "bar" => "2" }, request.query_parameters) end test "parameters not accessible after rack parse error" do @@ -1248,8 +1234,8 @@ class RequestVariant < BaseRequestTest test "setting variant to a symbol" do @request.variant = :phone - assert @request.variant.phone? - assert_not @request.variant.tablet? + assert_predicate @request.variant, :phone? + assert_not_predicate @request.variant, :tablet? assert @request.variant.any?(:phone, :tablet) assert_not @request.variant.any?(:tablet, :desktop) end @@ -1257,9 +1243,9 @@ class RequestVariant < BaseRequestTest test "setting variant to an array of symbols" do @request.variant = [:phone, :tablet] - assert @request.variant.phone? - assert @request.variant.tablet? - assert_not @request.variant.desktop? + assert_predicate @request.variant, :phone? + assert_predicate @request.variant, :tablet? + assert_not_predicate @request.variant, :desktop? assert @request.variant.any?(:tablet, :desktop) assert_not @request.variant.any?(:desktop, :watch) end @@ -1267,8 +1253,8 @@ class RequestVariant < BaseRequestTest test "clearing variant" do @request.variant = nil - assert @request.variant.empty? - assert_not @request.variant.phone? + assert_empty @request.variant + assert_not_predicate @request.variant, :phone? assert_not @request.variant.any?(:phone, :tablet) end @@ -1287,13 +1273,13 @@ end class RequestFormData < BaseRequestTest test "media_type is from the FORM_DATA_MEDIA_TYPES array" do - assert stub_request("CONTENT_TYPE" => "application/x-www-form-urlencoded").form_data? - assert stub_request("CONTENT_TYPE" => "multipart/form-data").form_data? + assert_predicate stub_request("CONTENT_TYPE" => "application/x-www-form-urlencoded"), :form_data? + assert_predicate stub_request("CONTENT_TYPE" => "multipart/form-data"), :form_data? end test "media_type is not from the FORM_DATA_MEDIA_TYPES array" do - assert !stub_request("CONTENT_TYPE" => "application/xml").form_data? - assert !stub_request("CONTENT_TYPE" => "multipart/related").form_data? + assert_not_predicate stub_request("CONTENT_TYPE" => "application/xml"), :form_data? + assert_not_predicate stub_request("CONTENT_TYPE" => "multipart/related"), :form_data? end test "no Content-Type header is provided and the request_method is POST" do @@ -1301,7 +1287,7 @@ class RequestFormData < BaseRequestTest assert_equal "", request.media_type assert_equal "POST", request.request_method - assert !request.form_data? + assert_not_predicate request, :form_data? end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index c4ee3add2a..0f37d074af 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -15,13 +15,13 @@ class ResponseTest < ActiveSupport::TestCase @response.await_commit } @response.commit! - assert @response.committed? + assert_predicate @response, :committed? assert t.join(0.5) end def test_stream_close @response.stream.close - assert @response.stream.closed? + assert_predicate @response.stream, :closed? end def test_stream_write @@ -158,7 +158,7 @@ class ResponseTest < ActiveSupport::TestCase @response.status = c.to_s @response.set_header "Content-Length", "0" _, headers, _ = @response.to_a - assert !headers.has_key?("Content-Length"), "#{c} must not have a Content-Length header field" + assert_not headers.has_key?("Content-Length"), "#{c} must not have a Content-Length header field" end end @@ -177,7 +177,7 @@ class ResponseTest < ActiveSupport::TestCase @response = ActionDispatch::Response.new @response.status = c.to_s _, headers, _ = @response.to_a - assert !headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" + assert_not headers.has_key?("Content-Type"), "#{c} should not have Content-Type header" end [200, 302, 404, 500].each do |c| @@ -191,7 +191,7 @@ class ResponseTest < ActiveSupport::TestCase test "does not include Status header" do @response.status = "200 OK" _, headers, _ = @response.to_a - assert !headers.has_key?("Status") + assert_not headers.has_key?("Status") end test "response code" do @@ -257,9 +257,9 @@ class ResponseTest < ActiveSupport::TestCase } resp.to_a - assert resp.etag? - assert resp.weak_etag? - assert_not resp.strong_etag? + assert_predicate resp, :etag? + assert_predicate resp, :weak_etag? + assert_not_predicate resp, :strong_etag? assert_equal('W/"202cb962ac59075b964b07152d234b70"', resp.etag) assert_equal({ public: true }, resp.cache_control) @@ -275,9 +275,9 @@ class ResponseTest < ActiveSupport::TestCase } resp.to_a - assert resp.etag? - assert_not resp.weak_etag? - assert resp.strong_etag? + assert_predicate resp, :etag? + assert_not_predicate resp, :weak_etag? + assert_predicate resp, :strong_etag? assert_equal('"202cb962ac59075b964b07152d234b70"', resp.etag) end @@ -311,13 +311,16 @@ class ResponseTest < ActiveSupport::TestCase end end - test "read x_frame_options, x_content_type_options and x_xss_protection" do + test "read x_frame_options, x_content_type_options, x_xss_protection, x_download_options and x_permitted_cross_domain_policies, referrer_policy" do original_default_headers = ActionDispatch::Response.default_headers begin ActionDispatch::Response.default_headers = { "X-Frame-Options" => "DENY", "X-Content-Type-Options" => "nosniff", - "X-XSS-Protection" => "1;" + "X-XSS-Protection" => "1;", + "X-Download-Options" => "noopen", + "X-Permitted-Cross-Domain-Policies" => "none", + "Referrer-Policy" => "strict-origin-when-cross-origin" } resp = ActionDispatch::Response.create.tap { |response| response.body = "Hello" @@ -327,6 +330,9 @@ class ResponseTest < ActiveSupport::TestCase assert_equal("DENY", resp.headers["X-Frame-Options"]) assert_equal("nosniff", resp.headers["X-Content-Type-Options"]) assert_equal("1;", resp.headers["X-XSS-Protection"]) + assert_equal("noopen", resp.headers["X-Download-Options"]) + assert_equal("none", resp.headers["X-Permitted-Cross-Domain-Policies"]) + assert_equal("strict-origin-when-cross-origin", resp.headers["Referrer-Policy"]) ensure ActionDispatch::Response.default_headers = original_default_headers end @@ -350,7 +356,7 @@ class ResponseTest < ActiveSupport::TestCase end test "respond_to? accepts include_private" do - assert_not @response.respond_to?(:method_missing) + assert_not_respond_to @response, :method_missing assert @response.respond_to?(:method_missing, true) end diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 438a918567..f1f6547889 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -3,6 +3,7 @@ require "abstract_unit" require "rails/engine" require "action_dispatch/routing/inspector" +require "io/console/size" class MountedRackApp def self.call(env) @@ -15,16 +16,10 @@ end module ActionDispatch module Routing class RoutesInspectorTest < ActiveSupport::TestCase - def setup + setup do @set = ActionDispatch::Routing::RouteSet.new end - def draw(options = nil, &block) - @set.draw(&block) - inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) - inspector.format(ActionDispatch::Routing::ConsoleFormatter.new, options).split("\n") - end - def test_displaying_routes_for_engines engine = Class.new(Rails::Engine) do def self.inspect @@ -305,7 +300,7 @@ module ActionDispatch end def test_routes_can_be_filtered - output = draw("posts") do + output = draw(grep: "posts") do resources :articles resources :posts end @@ -321,8 +316,76 @@ module ActionDispatch " DELETE /posts/:id(.:format) posts#destroy"], output end + def test_routes_when_expanded + previous_console_winsize = IO.console.winsize + IO.console.winsize = [0, 23] + + engine = Class.new(Rails::Engine) do + def self.inspect + "Blog::Engine" + end + end + engine.routes.draw do + get "/cart", to: "cart#show" + end + + output = draw(formatter: ActionDispatch::Routing::ConsoleFormatter::Expanded.new) do + get "/custom/assets", to: "custom_assets#show" + get "/custom/furnitures", to: "custom_furnitures#show" + mount engine => "/blog", :as => "blog" + end + + assert_equal ["--[ Route 1 ]----------", + "Prefix | custom_assets", + "Verb | GET", + "URI | /custom/assets(.:format)", + "Controller#Action | custom_assets#show", + "--[ Route 2 ]----------", + "Prefix | custom_furnitures", + "Verb | GET", + "URI | /custom/furnitures(.:format)", + "Controller#Action | custom_furnitures#show", + "--[ Route 3 ]----------", + "Prefix | blog", + "Verb | ", + "URI | /blog", + "Controller#Action | Blog::Engine", + "", + "[ Routes for Blog::Engine ]", + "--[ Route 1 ]----------", + "Prefix | cart", + "Verb | GET", + "URI | /cart(.:format)", + "Controller#Action | cart#show"], output + ensure + IO.console.winsize = previous_console_winsize + end + + def test_no_routes_matched_filter_when_expanded + output = draw(grep: "rails/dummy", formatter: ActionDispatch::Routing::ConsoleFormatter::Expanded.new) do + get "photos/:id" => "photos#show", :id => /[A-Z]\d{5}/ + end + + assert_equal [ + "No routes were found for this grep pattern.", + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." + ], output + end + + def test_not_routes_when_expanded + output = draw(grep: "rails/dummy", formatter: ActionDispatch::Routing::ConsoleFormatter::Expanded.new) {} + + assert_equal [ + "You don't have any routes defined!", + "", + "Please add some routes in config/routes.rb.", + "", + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." + ], output + end + def test_routes_can_be_filtered_with_namespaced_controllers - output = draw("admin/posts") do + output = draw(grep: "admin/posts") do resources :articles namespace :admin do resources :posts @@ -370,31 +433,31 @@ module ActionDispatch end assert_equal [ - "No routes were found for this controller", - "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + "No routes were found for this controller.", + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." ], output end def test_no_routes_matched_filter - output = draw("rails/dummy") do + output = draw(grep: "rails/dummy") do get "photos/:id" => "photos#show", :id => /[A-Z]\d{5}/ end assert_equal [ - "No routes were found for this controller", - "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + "No routes were found for this grep pattern.", + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." ], output end def test_no_routes_were_defined - output = draw("Rails::DummyController") {} + output = draw(grep: "Rails::DummyController") {} assert_equal [ "You don't have any routes defined!", "", "Please add some routes in config/routes.rb.", "", - "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." ], output end @@ -420,6 +483,13 @@ module ActionDispatch "custom_assets GET /custom/assets(.:format) custom_assets#show", ], output end + + private + def draw(formatter: ActionDispatch::Routing::ConsoleFormatter::Sheet.new, **options, &block) + @set.draw(&block) + inspector = ActionDispatch::Routing::RoutesInspector.new(@set.routes) + inspector.format(formatter, options).split("\n") + end end end end diff --git a/actionpack/test/dispatch/routing_assertions_test.rb b/actionpack/test/dispatch/routing_assertions_test.rb index a5198f2f13..009b6d9bc3 100644 --- a/actionpack/test/dispatch/routing_assertions_test.rb +++ b/actionpack/test/dispatch/routing_assertions_test.rb @@ -52,6 +52,8 @@ class RoutingAssertionsTest < ActionController::TestCase end mount engine => "/shelf" + + get "/shelf/foo", controller: "query_articles", action: "index" end end @@ -154,6 +156,10 @@ class RoutingAssertionsTest < ActionController::TestCase assert_match err.message, "This is a really bad msg" end + def test_assert_recognizes_continue_to_recoginize_after_it_tried_engines + assert_recognizes({ controller: "query_articles", action: "index" }, "/shelf/foo") + end + def test_assert_routing assert_routing("/articles", controller: "articles", action: "index") end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 44f902c163..5efbe5b553 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3153,7 +3153,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest after = has_named_route?(:hello) end - assert !before, "expected to not have named route :hello before route definition" + assert_not before, "expected to not have named route :hello before route definition" assert after, "expected to have named route :hello after route definition" end @@ -3166,7 +3166,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - assert !respond_to?(:routes_no_collision_path) + assert_not respond_to?(:routes_no_collision_path) end def test_controller_name_with_leading_slash_raise_error @@ -3313,7 +3313,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end get "/search" - assert !@request.params[:action].frozen? + assert_not_predicate @request.params[:action], :frozen? end def test_multiple_positional_args_with_the_same_name @@ -4225,7 +4225,7 @@ class TestGlobRoutingMapper < ActionDispatch::IntegrationTest end end - #include Routes.url_helpers + # include Routes.url_helpers APP = build_app Routes def app; APP end @@ -4267,7 +4267,7 @@ class TestOptimizedNamedRoutes < ActionDispatch::IntegrationTest def app; APP end test "enabled when not mounted and default_url_options is empty" do - assert Routes.url_helpers.optimize_routes_generation? + assert_predicate Routes.url_helpers, :optimize_routes_generation? end test "named route called as singleton method" do @@ -4500,7 +4500,7 @@ class TestPortConstraints < ActionDispatch::IntegrationTest get "/integer", to: ok, constraints: { port: 8080 } get "/string", to: ok, constraints: { port: "8080" } - get "/array", to: ok, constraints: { port: [8080] } + get "/array/:idx", to: ok, constraints: { port: [8080], idx: %w[first last] } get "/regexp", to: ok, constraints: { port: /8080/ } end end @@ -4529,7 +4529,10 @@ class TestPortConstraints < ActionDispatch::IntegrationTest get "http://www.example.com/array" assert_response :not_found - get "http://www.example.com:8080/array" + get "http://www.example.com:8080/array/middle" + assert_response :not_found + + get "http://www.example.com:8080/array/first" assert_response :success end @@ -5057,3 +5060,40 @@ class TestRecognizePath < ActionDispatch::IntegrationTest Routes.recognize_path(*args) end end + +class TestRelativeUrlRootGeneration < ActionDispatch::IntegrationTest + config = ActionDispatch::Routing::RouteSet::Config.new("/blog", false) + + stub_controllers(config) do |routes| + Routes = routes + + routes.draw do + get "/", to: "posts#index", as: :posts + get "/:id", to: "posts#show", as: :post + end + end + + include Routes.url_helpers + + APP = build_app Routes + + def app + APP + end + + def test_url_helpers + assert_equal "/blog/", posts_path({}) + assert_equal "/blog/", Routes.url_helpers.posts_path({}) + + assert_equal "/blog/1", post_path(id: "1") + assert_equal "/blog/1", Routes.url_helpers.post_path(id: "1") + end + + def test_optimized_url_helpers + assert_equal "/blog/", posts_path + assert_equal "/blog/", Routes.url_helpers.posts_path + + assert_equal "/blog/1", post_path("1") + assert_equal "/blog/1", Routes.url_helpers.post_path("1") + end +end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index cf51c47068..e34426a471 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -8,11 +8,14 @@ require "active_support/messages/rotation_configuration" class CookieStoreTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" SessionSecret = "b3c631c314c0bbca50c1b2843150fe33" - Generator = ActiveSupport::LegacyKeyGenerator.new(SessionSecret) + SessionSalt = "authenticated encrypted cookie" + + Generator = ActiveSupport::KeyGenerator.new(SessionSecret, iterations: 1000) Rotations = ActiveSupport::Messages::RotationConfiguration.new - Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, digest: "SHA1") - SignedBar = Verifier.generate(foo: "bar", session_id: SecureRandom.hex(16)) + Encryptor = ActiveSupport::MessageEncryptor.new( + Generator.generate_key(SessionSalt, 32), cipher: "aes-256-gcm", serializer: Marshal + ) class TestController < ActionController::Base def no_session_access @@ -25,12 +28,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def set_session_value session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) - end - - def set_session_value_expires_in_five_hours - session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash, expires_in: 5.hours)) + render body: nil end def get_session_value @@ -72,19 +70,35 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end end + def parse_cookie_from_header + cookie_matches = headers["Set-Cookie"].match(/#{SessionKey}=([^;]+)/) + cookie_matches && cookie_matches[1] + end + + def assert_session_cookie(cookie_string, contents) + assert_includes headers["Set-Cookie"], cookie_string + + session_value = parse_cookie_from_header + session_data = Encryptor.decrypt_and_verify(Rack::Utils.unescape(session_value)) rescue nil + + assert_not_nil session_data, "session failed to decrypt" + assert_equal session_data.slice(*contents.keys), contents + end + def test_setting_session_value with_test_route_set do get "/set_session_value" + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" end end def test_getting_session_value with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/get_session_value" + assert_response :success assert_equal 'foo: "bar"', response.body end @@ -92,8 +106,9 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_getting_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" + assert_response :success assert_equal 32, response.body.size session_id = response.body @@ -106,8 +121,12 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_disregards_tampered_sessions with_test_route_set do - cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--123456780" + encryptor = ActiveSupport::MessageEncryptor.new("A" * 32, cipher: "aes-256-gcm", serializer: Marshal) + + cookies[SessionKey] = encryptor.encrypt_and_sign("foo" => "bar", "session_id" => "abc") + get "/get_session_value" + assert_response :success assert_equal "foo: nil", response.body end @@ -135,19 +154,19 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_does_set_secure_cookies_over_https with_test_route_set(secure: true) do get "/set_session_value", headers: { "HTTPS" => "on" } + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; secure; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; secure; HttpOnly", "foo" => "bar" end end # {:foo=>#<SessionAutoloadTest::Foo bar:"baz">, :session_id=>"ce8b0752a6ab7c7af3cdb8a80e6b9e46"} - SignedSerializedCookie = "BAh7BzoIZm9vbzodU2Vzc2lvbkF1dG9sb2FkVGVzdDo6Rm9vBjoJQGJhciIIYmF6Og9zZXNzaW9uX2lkIiVjZThiMDc1MmE2YWI3YzdhZjNjZGI4YTgwZTZiOWU0Ng==--2bf3af1ae8bd4e52b9ac2099258ace0c380e601c" + EncryptedSerializedCookie = "9RZ2Fij0qLveUwM4s+CCjGqhpjyUC8jiBIf/AiBr9M3TB8xh2vQZtvSOMfN3uf6oYbbpIDHAcOFIEl69FcW1ozQYeSrCLonYCazoh34ZdYskIQfGwCiSYleVXG1OD9Z4jFqeVArw4Ewm0paOOPLbN1rc6A==--I359v/KWdZ1ok0ey--JFFhuPOY7WUo6tB/eP05Aw==" def test_deserializes_unloaded_classes_on_get_id with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_id" assert_response :success assert_equal "id: ce8b0752a6ab7c7af3cdb8a80e6b9e46", response.body, "should auto-load unloaded class" @@ -158,7 +177,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_deserializes_unloaded_classes_on_get_value with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_value" assert_response :success assert_equal 'foo: #<SessionAutoloadTest::Foo bar:"baz">', response.body, "should auto-load unloaded class" @@ -197,8 +216,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest get "/set_session_value" assert_response :success session_payload = response.body - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_reset_session" assert_response :success @@ -216,8 +234,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/get_class_after_reset_session" assert_response :success @@ -239,8 +256,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_session_clear" assert_response :success @@ -253,7 +269,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_persistent_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" assert_response :success assert_equal 32, response.body.size @@ -268,8 +284,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_setting_session_id_to_nil_is_respected with_test_route_set do - cookies[SessionKey] = SignedBar - + get "/set_session_value" get "/get_session_id" sid = response.body assert_equal 36, sid.size @@ -283,31 +298,53 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set(expire_after: 5.hours) do # First request accesses the session time = Time.local(2008, 4, 24) - cookie_body = nil Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - cookies[SessionKey] = SignedBar + get "/set_session_value" - get "/set_session_value_expires_in_five_hours" assert_response :success - - cookie_body = response.body - assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" end # Second request does not access the session - time = Time.local(2008, 4, 25) + time = time + 3.hours Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") get "/no_session_access" + + assert_response :success + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + end + end + + def test_session_store_with_expire_after_does_not_accept_expired_session + with_test_route_set(expire_after: 5.hours) do + # First request accesses the session + time = Time.local(2017, 11, 12) + + Time.stub :now, time do + expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") + + get "/set_session_value" + get "/get_session_value" + assert_response :success + assert_equal 'foo: "bar"', response.body + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + + # Second request is beyond the expiry time and the session is invalidated + time += 5.hours + 1.minute - assert_equal "_myapp_session=#{cookies[SessionKey]}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + Time.stub :now, time do + get "/get_session_value" + + assert_response :success + assert_equal "foo: nil", response.body end end end @@ -347,8 +384,14 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def get(path, *args) args[0] ||= {} args[0][:headers] ||= {} - args[0][:headers]["action_dispatch.key_generator"] ||= Generator - args[0][:headers]["action_dispatch.cookies_rotations"] ||= Rotations + args[0][:headers].tap do |config| + config["action_dispatch.secret_key_base"] = SessionSecret + config["action_dispatch.authenticated_encrypted_cookie_salt"] = SessionSalt + config["action_dispatch.use_authenticated_cookie_encryption"] = true + + config["action_dispatch.key_generator"] ||= Generator + config["action_dispatch.cookies_rotations"] ||= Rotations + end super(path, *args) end diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 8ac9502af9..baf46e7c7e 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -98,8 +98,8 @@ class RedirectSSLTest < SSLTest end class StrictTransportSecurityTest < SSLTest - EXPECTED = "max-age=15552000" - EXPECTED_WITH_SUBDOMAINS = "max-age=15552000; includeSubDomains" + EXPECTED = "max-age=31536000" + EXPECTED_WITH_SUBDOMAINS = "max-age=31536000; includeSubDomains" def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }, headers: {}) self.app = build_app ssl_options: { hsts: hsts }, headers: headers @@ -208,6 +208,14 @@ class SecureCookiesTest < SSLTest assert_cookies(*DEFAULT.split("\n")) end + def test_cookies_as_not_secure_with_exclude + excluding = { exclude: -> request { request.domain =~ /example/ } } + get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { redirect: excluding } + + assert_cookies(*DEFAULT.split("\n")) + assert_response :ok + end + def test_no_cookies get assert_nil response.headers["Set-Cookie"] diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 0bdff68692..6b69cd9999 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -71,7 +71,16 @@ module StaticTests end def test_served_static_file_with_non_english_filename - assert_html "means hello in Japanese\n", get("/foo/#{Rack::Utils.escape("こんにちは.html")}") + assert_html "means hello in Japanese\n", get("/foo/%E3%81%93%E3%82%93%E3%81%AB%E3%81%A1%E3%81%AF.html") + end + + def test_served_gzipped_static_file_with_non_english_filename + response = get("/foo/%E3%81%95%E3%82%88%E3%81%86%E3%81%AA%E3%82%89.html", "HTTP_ACCEPT_ENCODING" => "gzip") + + assert_gzip "/foo/さようなら.html", response + assert_equal "text/html", response.headers["Content-Type"] + assert_equal "Accept-Encoding", response.headers["Vary"] + assert_equal "gzip", response.headers["Content-Encoding"] end def test_serves_static_file_with_exclamation_mark_in_filename diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb index 75feae6fe0..a824ee0c84 100644 --- a/actionpack/test/dispatch/system_testing/driver_test.rb +++ b/actionpack/test/dispatch/system_testing/driver_test.rb @@ -12,7 +12,8 @@ class DriverTest < ActiveSupport::TestCase test "initializing the driver with a browser" do driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" }) assert_equal :selenium, driver.instance_variable_get(:@name) - assert_equal :chrome, driver.instance_variable_get(:@browser) + assert_equal :chrome, driver.instance_variable_get(:@browser).name + assert_nil 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 @@ -20,7 +21,15 @@ class DriverTest < ActiveSupport::TestCase test "initializing the driver with a headless chrome" do 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) + assert_equal :headless_chrome, driver.instance_variable_get(:@browser).name + assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size) + assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) + end + + test "initializing the driver with a headless firefox" do + 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_equal [1400, 1400], driver.instance_variable_get(:@screen_size) assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options) end diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index 2afda31cf5..de79c05657 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -9,7 +9,7 @@ class ScreenshotHelperTest < ActiveSupport::TestCase new_test = DrivenBySeleniumWithChrome.new("x") Rails.stub :root, Pathname.getwd do - assert_equal "tmp/screenshots/x.png", new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) end end @@ -18,7 +18,7 @@ class ScreenshotHelperTest < ActiveSupport::TestCase Rails.stub :root, Pathname.getwd do new_test.stub :passed?, false do - assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/failures_x.png").to_s, new_test.send(:image_path) end end end @@ -29,12 +29,17 @@ class ScreenshotHelperTest < ActiveSupport::TestCase Rails.stub :root, Pathname.getwd do new_test.stub :passed?, false do new_test.stub :skipped?, true do - assert_equal "tmp/screenshots/x.png", new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) end end end end + test "defaults to simple output for the screenshot" do + new_test = DrivenBySeleniumWithChrome.new("x") + assert_equal "simple", new_test.send(:output_type) + end + test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do begin original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] @@ -42,6 +47,8 @@ class ScreenshotHelperTest < ActiveSupport::TestCase new_test = DrivenBySeleniumWithChrome.new("x") + 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) @@ -52,11 +59,11 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end end - test "image path returns the relative path from current directory" do + test "image path returns the absolute path from root" do new_test = DrivenBySeleniumWithChrome.new("x") Rails.stub :root, Pathname.getwd.join("..") do - assert_equal "../tmp/screenshots/x.png", new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) end end end diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb index 1866225fc1..740e90a4da 100644 --- a/actionpack/test/dispatch/system_testing/server_test.rb +++ b/actionpack/test/dispatch/system_testing/server_test.rb @@ -6,10 +6,27 @@ require "action_dispatch/system_testing/server" class ServerTest < ActiveSupport::TestCase setup do - ActionDispatch::SystemTesting::Server.new.run + @old_capybara_server = Capybara.server end test "port is always included" do + ActionDispatch::SystemTesting::Server.new.run assert Capybara.always_include_port, "expected Capybara.always_include_port to be true" end + + test "server is changed from `default` to `puma`" do + Capybara.server = :default + ActionDispatch::SystemTesting::Server.new.run + assert_not_equal Capybara.server, Capybara.servers[:default] + end + + test "server is not changed to `puma` when is different than default" do + Capybara.server = :webrick + ActionDispatch::SystemTesting::Server.new.run + assert_equal Capybara.server, Capybara.servers[:webrick] + end + + teardown do + Capybara.server = @old_capybara_server + end end 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 c6a6aef92b..b078a5abc5 100644 --- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb +++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb @@ -28,6 +28,12 @@ class SetDriverToSeleniumHeadlessChromeTest < DrivenBySeleniumWithHeadlessChrome end end +class SetDriverToSeleniumHeadlessFirefoxTest < DrivenBySeleniumWithHeadlessFirefox + test "uses selenium headless firefox" do + assert_equal :selenium, Capybara.current_driver + end +end + class SetHostTest < DrivenByRackTest test "sets default host" do assert_equal "http://127.0.0.1", Capybara.app_host diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 4673d7cc11..21169fcb5c 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -18,7 +18,7 @@ module ActionDispatch def test_filename_is_different_object file_str = "foo" uf = Http::UploadedFile.new(filename: file_str, tempfile: Object.new) - assert_not_equal file_str.object_id , uf.original_filename.object_id + assert_not_equal file_str.object_id, uf.original_filename.object_id end def test_filename_should_be_in_utf_8 @@ -100,14 +100,20 @@ module ActionDispatch def test_delegate_eof_to_tempfile tf = Class.new { def eof?; true end; } uf = Http::UploadedFile.new(tempfile: tf.new) - assert uf.eof? + assert_predicate 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 end def test_respond_to? tf = Class.new { def read; yield end } uf = Http::UploadedFile.new(tempfile: tf.new) - assert uf.respond_to?(:headers), "responds to headers" - assert uf.respond_to?(:read), "responds to read" + assert_respond_to uf, :headers + assert_respond_to uf, :read end end end diff --git a/actionpack/test/fixtures/functional_caching/_formatted_partial.html.erb b/actionpack/test/fixtures/functional_caching/_formatted_partial.html.erb new file mode 100644 index 0000000000..aad73c0d6b --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/_formatted_partial.html.erb @@ -0,0 +1 @@ +<p>Hello!</p> diff --git a/actionpack/test/fixtures/functional_caching/xml_fragment_cached_with_html_partial.xml.builder b/actionpack/test/fixtures/functional_caching/xml_fragment_cached_with_html_partial.xml.builder new file mode 100644 index 0000000000..2bdda3af18 --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/xml_fragment_cached_with_html_partial.xml.builder @@ -0,0 +1,5 @@ +cache do + xml.title "Hello!" +end + +xml.body cdata_section(render("formatted_partial")) diff --git a/actionpack/test/fixtures/public/foo/さようなら.html b/actionpack/test/fixtures/public/foo/さようなら.html new file mode 100644 index 0000000000..627bb2469f --- /dev/null +++ b/actionpack/test/fixtures/public/foo/さようなら.html @@ -0,0 +1 @@ +means goodbye in Japanese diff --git a/actionpack/test/fixtures/public/foo/さようなら.html.gz b/actionpack/test/fixtures/public/foo/さようなら.html.gz Binary files differnew file mode 100644 index 0000000000..4f484cfe86 --- /dev/null +++ b/actionpack/test/fixtures/public/foo/さようなら.html.gz diff --git a/actionpack/test/fixtures/公共/foo/さようなら.html b/actionpack/test/fixtures/公共/foo/さようなら.html new file mode 100644 index 0000000000..627bb2469f --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/さようなら.html @@ -0,0 +1 @@ +means goodbye in Japanese diff --git a/actionpack/test/fixtures/公共/foo/さようなら.html.gz b/actionpack/test/fixtures/公共/foo/さようなら.html.gz Binary files differnew file mode 100644 index 0000000000..4f484cfe86 --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/さようなら.html.gz diff --git a/actionpack/test/journey/nodes/symbol_test.rb b/actionpack/test/journey/nodes/symbol_test.rb index 1e687acef2..b0622ac71a 100644 --- a/actionpack/test/journey/nodes/symbol_test.rb +++ b/actionpack/test/journey/nodes/symbol_test.rb @@ -8,10 +8,10 @@ module ActionDispatch class TestSymbol < ActiveSupport::TestCase def test_default_regexp? sym = Symbol.new "foo" - assert sym.default_regexp? + assert_predicate sym, :default_regexp? sym.regexp = nil - assert_not sym.default_regexp? + assert_not_predicate sym, :default_regexp? end end end diff --git a/actionpack/test/journey/route/definition/scanner_test.rb b/actionpack/test/journey/route/definition/scanner_test.rb index 070886c7df..092177d315 100644 --- a/actionpack/test/journey/route/definition/scanner_test.rb +++ b/actionpack/test/journey/route/definition/scanner_test.rb @@ -10,61 +10,70 @@ module ActionDispatch @scanner = Scanner.new end - # /page/:id(/:action)(.:format) - def test_tokens - [ - ["/", [[:SLASH, "/"]]], - ["*omg", [[:STAR, "*omg"]]], - ["/page", [[:SLASH, "/"], [:LITERAL, "page"]]], - ["/page!", [[:SLASH, "/"], [:LITERAL, "page!"]]], - ["/page$", [[:SLASH, "/"], [:LITERAL, "page$"]]], - ["/page&", [[:SLASH, "/"], [:LITERAL, "page&"]]], - ["/page'", [[:SLASH, "/"], [:LITERAL, "page'"]]], - ["/page*", [[:SLASH, "/"], [:LITERAL, "page*"]]], - ["/page+", [[:SLASH, "/"], [:LITERAL, "page+"]]], - ["/page,", [[:SLASH, "/"], [:LITERAL, "page,"]]], - ["/page;", [[:SLASH, "/"], [:LITERAL, "page;"]]], - ["/page=", [[:SLASH, "/"], [:LITERAL, "page="]]], - ["/page@", [[:SLASH, "/"], [:LITERAL, "page@"]]], - ['/page\:', [[:SLASH, "/"], [:LITERAL, "page:"]]], - ['/page\(', [[:SLASH, "/"], [:LITERAL, "page("]]], - ['/page\)', [[:SLASH, "/"], [:LITERAL, "page)"]]], - ["/~page", [[:SLASH, "/"], [:LITERAL, "~page"]]], - ["/pa-ge", [[:SLASH, "/"], [:LITERAL, "pa-ge"]]], - ["/:page", [[:SLASH, "/"], [:SYMBOL, ":page"]]], - ["/(:page)", [ + CASES = [ + ["/", [[:SLASH, "/"]]], + ["*omg", [[:STAR, "*omg"]]], + ["/page", [[:SLASH, "/"], [:LITERAL, "page"]]], + ["/page!", [[:SLASH, "/"], [:LITERAL, "page!"]]], + ["/page$", [[:SLASH, "/"], [:LITERAL, "page$"]]], + ["/page&", [[:SLASH, "/"], [:LITERAL, "page&"]]], + ["/page'", [[:SLASH, "/"], [:LITERAL, "page'"]]], + ["/page*", [[:SLASH, "/"], [:LITERAL, "page*"]]], + ["/page+", [[:SLASH, "/"], [:LITERAL, "page+"]]], + ["/page,", [[:SLASH, "/"], [:LITERAL, "page,"]]], + ["/page;", [[:SLASH, "/"], [:LITERAL, "page;"]]], + ["/page=", [[:SLASH, "/"], [:LITERAL, "page="]]], + ["/page@", [[:SLASH, "/"], [:LITERAL, "page@"]]], + ['/page\:', [[:SLASH, "/"], [:LITERAL, "page:"]]], + ['/page\(', [[:SLASH, "/"], [:LITERAL, "page("]]], + ['/page\)', [[:SLASH, "/"], [:LITERAL, "page)"]]], + ["/~page", [[:SLASH, "/"], [:LITERAL, "~page"]]], + ["/pa-ge", [[:SLASH, "/"], [:LITERAL, "pa-ge"]]], + ["/:page", [[:SLASH, "/"], [:SYMBOL, ":page"]]], + ["/:page|*foo", [ + [:SLASH, "/"], + [:SYMBOL, ":page"], + [:OR, "|"], + [:STAR, "*foo"] + ]], + ["/(:page)", [ + [:SLASH, "/"], + [:LPAREN, "("], + [:SYMBOL, ":page"], + [:RPAREN, ")"], + ]], + ["(/:action)", [ + [:LPAREN, "("], [:SLASH, "/"], + [:SYMBOL, ":action"], + [:RPAREN, ")"], + ]], + ["(())", [[:LPAREN, "("], + [:LPAREN, "("], [:RPAREN, ")"], [:RPAREN, ")"]]], + ["(.:format)", [ [:LPAREN, "("], - [:SYMBOL, ":page"], + [:DOT, "."], + [:SYMBOL, ":format"], [:RPAREN, ")"], ]], - ["(/:action)", [ - [:LPAREN, "("], - [:SLASH, "/"], - [:SYMBOL, ":action"], - [:RPAREN, ")"], - ]], - ["(())", [[:LPAREN, "("], - [:LPAREN, "("], [:RPAREN, ")"], [:RPAREN, ")"]]], - ["(.:format)", [ - [:LPAREN, "("], - [:DOT, "."], - [:SYMBOL, ":format"], - [:RPAREN, ")"], - ]], - ].each do |str, expected| - @scanner.scan_setup str - assert_tokens expected, @scanner + ] + + CASES.each do |pattern, expected_tokens| + test "Scanning `#{pattern}`" do + @scanner.scan_setup pattern + assert_tokens expected_tokens, @scanner, pattern end end - def assert_tokens(tokens, scanner) - toks = [] - while tok = scanner.next_token - toks << tok + private + + def assert_tokens(expected_tokens, scanner, pattern) + actual_tokens = [] + while token = scanner.next_token + actual_tokens << token + end + assert_equal expected_tokens, actual_tokens, "Wrong tokens for `#{pattern}`" end - assert_equal tokens, toks - end end end end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 29cc74471d..1f4e14aef6 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -30,7 +30,7 @@ module ActionDispatch def test_unicode get "/ほげ", to: "foo#bar" - #match the escaped version of /ほげ + # match the escaped version of /ほげ env = rails_env "PATH_INFO" => "/%E3%81%BB%E3%81%92" called = false router.recognize(env) do |r, params| @@ -493,6 +493,15 @@ module ActionDispatch assert_not called end + def test_eager_load_with_routes + get "/foo-bar", to: "foo#bar" + assert_nil router.eager_load! + end + + def test_eager_load_without_routes + assert_nil router.eager_load! + end + private def get(*args) diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb index 81ce07526f..d5c81a8421 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -17,11 +17,11 @@ module ActionDispatch def test_clear mapper.get "/foo(/:id)", to: "foo#bar", as: "aaron" - assert_not_predicate routes, :empty? + assert_not_empty routes assert_equal 1, routes.length routes.clear - assert routes.empty? + assert_empty routes assert_equal 0, routes.length end @@ -43,7 +43,7 @@ module ActionDispatch mapper.get "/foo(/:id)", to: "foo#bar", as: "aaron" assert_equal 1, @routes.anchored_routes.length - assert_predicate @routes.custom_routes, :empty? + assert_empty @routes.custom_routes mapper.get "/hello/:who", to: "foo#bar", as: "bar", who: /\d/ diff --git a/actionpack/test/tmp/.gitignore b/actionpack/test/tmp/.gitignore deleted file mode 100644 index e69de29bb2..0000000000 --- a/actionpack/test/tmp/.gitignore +++ /dev/null |