diff options
Diffstat (limited to 'actionpack')
125 files changed, 1567 insertions, 746 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a13d9e1078..740c6db06f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,11 +1,140 @@ -* Introduce a new error page to when the implict render page is accessed in the browser. +* Raise an error on root route naming conflicts. + + Raises an ArgumentError when multiple root routes are defined in the + same context instead of assigning nil names to subsequent roots. + + *Gannon McGibbon* + +* Allow rescue from parameter parse errors: + + ``` + rescue_from ActionDispatch::Http::Parameters::ParseError do + head :unauthorized + end + ``` + + *Gannon McGibbon*, *Josh Cheek* + +* Reset Capybara sessions if failed system test screenshot raising an exception. + + Reset Capybara sessions if `take_failed_screenshot` raise exception + in system test `after_teardown`. + + *Maxim Perepelitsa* + +* Use request object for context if there's no controller + + There is no controller instance when using a redirect route or a + mounted rack application so pass the request object as the context + when resolving dynamic CSP sources in this scenario. + + Fixes #34200. + + *Andrew White* + +* Apply mapping to symbols returned from dynamic CSP sources + + Previously if a dynamic source returned a symbol such as :self it + would be converted to a string implicity, e.g: + + policy.default_src -> { :self } + + would generate the header: + + Content-Security-Policy: default-src self + + and now it generates: + + Content-Security-Policy: default-src 'self' + + *Andrew White* + +* Add `ActionController::Parameters#each_value`. + + *Lukáš Zapletal* + +* Deprecate `ActionDispatch::Http::ParameterFilter` in favor of `ActiveSupport::ParameterFilter`. + + *Yoshiyuki Kinjo* + +* Remove undocumented `params` option from `url_for` helper. + + *Ilkka Oksanen* + +* Encode Content-Disposition filenames on `send_data` and `send_file`. + Previously, `send_data 'data', filename: "\u{3042}.txt"` sends + `"filename=\"\u{3042}.txt\""` as Content-Disposition and it can be + garbled. + Now it follows [RFC 2231](https://tools.ietf.org/html/rfc2231) and + [RFC 5987](https://tools.ietf.org/html/rfc5987) and sends + `"filename=\"%3F.txt\"; filename*=UTF-8''%E3%81%82.txt"`. + Most browsers can find filename correctly and old browsers fallback to ASCII + converted name. + + *Fumiaki Matsushima* + +* Expose `ActionController::Parameters#each_key` which allows iterating over + keys without allocating an array. + + *Richard Schneeman* + +* Purpose metadata for signed/encrypted cookies. + + Rails can now thwart attacks that attempt to copy signed/encrypted value + of a cookie and use it as the value of another cookie. + + 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. + + Enable `action_dispatch.use_cookies_with_metadata` to use this feature, which + writes cookies with the new purpose and expiry metadata embedded. + + *Assain Jaleel* + +* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. + + `respond_to` can match multiple types and lead to undefined behavior when + multiple invocations are made and the types do not match: + + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + + *Patrick Toomey* + +* `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile. + + This allows uploaded file objects to be passed directly to `File.read` + without raising a `TypeError`: + + uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: tmp_file) + File.read(uploaded_file) + + *Aaron Kromer* + +* Pass along arguments to underlying `get` method in `follow_redirect!`. + + Now all arguments passed to `follow_redirect!` are passed to the underlying + `get` method. This for example allows to set custom headers for the + redirection request to the server. + + follow_redirect!(params: { foo: :bar }) + + *Remo Fritzsche* + +* Introduce a new error page to when the implicit render page is accessed in the browser. Now instead of showing an error page that with exception and backtraces we now show only one informative page. *Vinicius Stock* -* Introduce ActionDispatch::DebugExceptions.register_interceptor +* Introduce `ActionDispatch::DebugExceptions.register_interceptor`. Exception aware plugin authors can use the newly introduced `.register_interceptor` method to get the processed exception, instead of diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 1dc8abf746..ec56de18f1 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -26,6 +26,9 @@ Gem::Specification.new do |s| "changelog_uri" => "https://github.com/rails/rails/blob/v#{version}/actionpack/CHANGELOG.md" } + # NOTE: Please read our dependency guidelines before updating versions: + # https://edgeguides.rubyonrails.org/security.html#dependency-management-and-cves + s.add_dependency "activesupport", version s.add_dependency "rack", "~> 2.0" diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 0477e7f1c9..3a98931167 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -7,6 +7,7 @@ require "active_support/i18n" module AbstractController extend ActiveSupport::Autoload + autoload :ActionNotFound, "abstract_controller/base" autoload :Base autoload :Caching autoload :Callbacks diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index a312af6715..bb42f2e119 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -78,7 +78,9 @@ module AbstractController # Except for public instance methods of Base and its ancestors internal_methods + # Be sure to include shadowed public instance methods of this class - public_instance_methods(false)).uniq.map(&:to_s) + public_instance_methods(false)) + + methods.map!(&:to_s) methods.to_set end @@ -102,7 +104,7 @@ module AbstractController # ==== Returns # * <tt>String</tt> def controller_path - @controller_path ||= name.sub(/Controller$/, "".freeze).underscore unless anonymous? + @controller_path ||= name.sub(/Controller$/, "").underscore unless anonymous? end # Refresh the cached action_methods when a new action_method is added. diff --git a/actionpack/lib/abstract_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index f99b0830b2..95078a2a28 100644 --- a/actionpack/lib/abstract_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -82,13 +82,17 @@ module AbstractController # Given a key (as described in +expire_fragment+), returns # a key array suitable for use in reading, writing, or expiring a # cached fragment. All keys begin with <tt>:views</tt>, - # followed by ENV["RAILS_CACHE_ID"] or ENV["RAILS_APP_VERSION"] if set, + # followed by <tt>ENV["RAILS_CACHE_ID"]</tt> or <tt>ENV["RAILS_APP_VERSION"]</tt> if set, # followed by any controller-wide key prefix values, ending # with the specified +key+ value. def combined_fragment_cache_key(key) head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) } tail = key.is_a?(Hash) ? url_for(key).split("://").last : key - [ :views, (ENV["RAILS_CACHE_ID"] || ENV["RAILS_APP_VERSION"]), *head, *tail ].compact + + cache_key = [:views, ENV["RAILS_CACHE_ID"] || ENV["RAILS_APP_VERSION"], head, tail] + cache_key.flatten!(1) + cache_key.compact! + cache_key end # Writes +content+ to the location signified by 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..3913259ecc 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 @@ -181,7 +181,7 @@ module AbstractController end def default_helper_module! - module_name = name.sub(/Controller$/, "".freeze) + module_name = name.sub(/Controller$/, "") module_path = module_name.underscore helper module_path rescue LoadError => e diff --git a/actionpack/lib/abstract_controller/railties/routes_helpers.rb b/actionpack/lib/abstract_controller/railties/routes_helpers.rb index b6e5631a4e..fbd93705ed 100644 --- a/actionpack/lib/abstract_controller/railties/routes_helpers.rb +++ b/actionpack/lib/abstract_controller/railties/routes_helpers.rb @@ -7,7 +7,7 @@ module AbstractController Module.new do define_method(:inherited) do |klass| super(klass) - if namespace = klass.parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) } + if namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_routes_url_helpers) } klass.include(namespace.railtie_routes_url_helpers(include_path_helpers)) else klass.include(routes.url_helpers(include_path_helpers)) diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index 93ffff1bd6..c276ee57c0 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -12,7 +12,7 @@ module ActionController # # An API Controller is different from a normal controller in the sense that # by default it doesn't include a number of features that are usually required - # by browser access only: layouts and templates rendering, cookies, sessions, + # by browser access only: layouts and templates rendering, # flash, assets, and so on. This makes the entire controller stack thinner, # suitable for API applications. It doesn't mean you won't have such # features if you need them: they're all available for you to include in diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 97775d1dc8..bf3b00a7b7 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -40,7 +40,7 @@ module ActionController end def instrument_name - "action_controller".freeze + "action_controller" end end end diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 14f41eb55f..d8b04d8ddb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -18,16 +18,19 @@ module ActionController def process_action(event) info do - payload = event.payload + payload = event.payload additions = ActionController::Base.log_process_action(payload) - status = payload[:status] + if status.nil? && payload[:exception].present? exception_class_name = payload[:exception].first status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end - message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms".dup - message << " (#{additions.join(" | ".freeze)})" unless additions.empty? + + additions << "Allocations: #{event.allocations}" + + message = +"Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" + message << " (#{additions.join(" | ")})" unless additions.empty? message << "\n\n" if defined?(Rails.env) && Rails.env.development? message @@ -53,7 +56,7 @@ module ActionController def unpermitted_parameters(event) debug do unpermitted_keys = event.payload[:keys] - "Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}" + color("Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}", RED) end end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 06b6a95ff8..d6911ee2b5 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -230,12 +230,20 @@ module ActionController # This method will overwrite an existing Cache-Control header. # See https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities. # + # HTTP Cache-Control Extensions for Stale Content. See https://tools.ietf.org/html/rfc5861 + # It helps to cache an asset and serve it while is being revalidated and/or returning with an error. + # + # expires_in 3.hours, public: true, stale_while_revalidate: 60.seconds + # expires_in 3.hours, public: true, stale_while_revalidate: 60.seconds, stale_if_error: 5.minutes + # # The method will also ensure an HTTP Date header for client compatibility. def expires_in(seconds, options = {}) 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/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 5a82ccf668..9ef4f50df1 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "action_controller/metal/exceptions" +require "action_dispatch/http/content_disposition" module ActionController #:nodoc: # Methods for sending arbitrary data and for streaming files to the browser, @@ -10,8 +11,8 @@ module ActionController #:nodoc: include ActionController::Rendering - DEFAULT_SEND_FILE_TYPE = "application/octet-stream".freeze #:nodoc: - DEFAULT_SEND_FILE_DISPOSITION = "attachment".freeze #:nodoc: + DEFAULT_SEND_FILE_TYPE = "application/octet-stream" #:nodoc: + DEFAULT_SEND_FILE_DISPOSITION = "attachment" #:nodoc: private # Sends the file. This uses a server-appropriate method (such as X-Sendfile) @@ -132,10 +133,8 @@ module ActionController #:nodoc: end disposition = options.fetch(:disposition, DEFAULT_SEND_FILE_DISPOSITION) - unless disposition.nil? - disposition = disposition.to_s - disposition += %(; filename="#{options[:filename]}") if options[:filename] - headers["Content-Disposition"] = disposition + if disposition + headers["Content-Disposition"] = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: options[:filename]) end headers["Content-Transfer-Encoding"] = "binary" diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index ce9eb209fe..30034be018 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -51,6 +51,24 @@ 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/flash.rb b/actionpack/lib/action_controller/metal/flash.rb index 5115c2fadf..380f2e9591 100644 --- a/actionpack/lib/action_controller/metal/flash.rb +++ b/actionpack/lib/action_controller/metal/flash.rb @@ -36,7 +36,7 @@ module ActionController #:nodoc: define_method(type) do request.flash[type] end - helper_method type + helper_method(type) if respond_to?(:helper_method) self._flash_types += [type] end diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index 8d53a30e93..26e6f72b66 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -5,8 +5,8 @@ require "active_support/core_ext/hash/slice" module ActionController # 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. + # config file. This will ensure all endpoints not explicitly marked otherwise + # will have all communication served over HTTPS. module ForceSSL # :nodoc: extend ActiveSupport::Concern include AbstractController::Callbacks 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/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 22c84e440b..0faaac1ce4 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -100,8 +100,7 @@ module ActionController # # => ["application", "chart", "rubygems"] def all_helpers_from_path(path) helpers = Array(path).flat_map do |_path| - extract = /^#{Regexp.quote(_path.to_s)}\/?(.*)_helper.rb$/ - names = Dir["#{_path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1'.freeze) } + names = Dir["#{_path}/**/*_helper.rb"].map { |file| file[_path.to_s.size + 1..-"_helper.rb".size - 1] } names.sort! end helpers.uniq! diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 01676f3237..7036123d5d 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 @@ -126,7 +127,7 @@ module ActionController def authentication_request(controller, realm, message) message ||= "HTTP Basic: Access denied.\n" - controller.headers["WWW-Authenticate"] = %(Basic realm="#{realm.tr('"'.freeze, "".freeze)}") + controller.headers["WWW-Authenticate"] = %(Basic realm="#{realm.tr('"', "")}") controller.status = 401 controller.response_body = message end @@ -389,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 @@ -474,7 +474,7 @@ module ActionController # This removes the <tt>"</tt> characters wrapping the value. def rewrite_param_values(array_params) - array_params.each { |param| (param[1] || "".dup).gsub! %r/^"|"$/, "" } + array_params.each { |param| (param[1] || +"").gsub! %r/^"|"$/, "" } end # This method takes an authorization body and splits up the key-value @@ -511,7 +511,7 @@ module ActionController # Returns nothing. def authentication_request(controller, realm, message = nil) message ||= "HTTP Token: Access denied.\n" - controller.headers["WWW-Authenticate"] = %(Token realm="#{realm.tr('"'.freeze, "".freeze)}") + controller.headers["WWW-Authenticate"] = %(Token realm="#{realm.tr('"', "")}") controller.__send__ :render, plain: message, status: :unauthorized end end diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 2f4c8fb83c..083b762f5a 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -86,7 +86,7 @@ module ActionController # Note: SSEs are not currently supported by IE. However, they are supported # by Chrome, Firefox, Opera, and Safari. class SSE - WHITELISTED_OPTIONS = %w( retry event id ) + PERMITTED_OPTIONS = %w( retry event id ) def initialize(stream, options = {}) @stream = stream @@ -111,13 +111,13 @@ module ActionController def perform_write(json, options) current_options = @options.merge(options).stringify_keys - WHITELISTED_OPTIONS.each do |option_name| + PERMITTED_OPTIONS.each do |option_name| if (option_value = current_options[option_name]) @stream.write "#{option_name}: #{option_value}\n" end end - message = json.gsub("\n".freeze, "\ndata: ".freeze) + message = json.gsub("\n", "\ndata: ") @stream.write "data: #{message}\n\n" end end @@ -280,33 +280,35 @@ module ActionController raise error if error end - # Spawn a new thread to serve up the controller in. This is to get - # around the fact that Rack isn't based around IOs and we need to use - # a thread to stream data from the response bodies. Nobody should call - # this method except in Rails internals. Seriously! - def new_controller_thread # :nodoc: - Thread.new { - t2 = Thread.current - t2.abort_on_exception = true - yield - } + def response_body=(body) + super + response.close if response end - def log_error(exception) - logger = ActionController::Base.logger - return unless logger + private - logger.fatal do - message = "\n#{exception.class} (#{exception.message}):\n".dup - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << exception.backtrace.join("\n ") - "#{message}\n\n" + # Spawn a new thread to serve up the controller in. This is to get + # around the fact that Rack isn't based around IOs and we need to use + # a thread to stream data from the response bodies. Nobody should call + # this method except in Rails internals. Seriously! + def new_controller_thread # :nodoc: + Thread.new { + t2 = Thread.current + t2.abort_on_exception = true + yield + } end - end - def response_body=(body) - super - response.close if response - end + def log_error(exception) + logger = ActionController::Base.logger + return unless logger + + logger.fatal do + message = +"\n#{exception.class} (#{exception.message}):\n" + message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << " " << exception.backtrace.join("\n ") + "#{message}\n\n" + end + end end end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 2233b93406..118da11990 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -11,7 +11,7 @@ module ActionController #:nodoc: # @people = Person.all # end # - # That action implicitly responds to all formats, but formats can also be whitelisted: + # That action implicitly responds to all formats, but formats can also be explicitly enumerated: # # def index # @people = Person.all @@ -105,7 +105,7 @@ module ActionController #:nodoc: # # Mime::Type.register "image/jpg", :jpg # - # Respond to also allows you to specify a common block for different formats by using +any+: + # +respond_to+ also allows you to specify a common block for different formats by using +any+: # # def index # @people = Person.all @@ -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/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index a678377d4f..7361946de5 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -253,7 +253,10 @@ module ActionController # This will display the wrapped hash in the log file. request.filtered_parameters.merge! wrapped_filtered_hash end - super + ensure + # NOTE: Rescues all exceptions so they + # may be caught in ActionController::Rescue. + return super end private diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 4c2b5120eb..2804a06a58 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -105,7 +105,7 @@ module ActionController when String request.protocol + request.host_with_port + options when Proc - _compute_redirect_to_location request, options.call + _compute_redirect_to_location request, instance_eval(&options) else url_for(options) end.delete("\0\r\n") diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 6d181e6456..7d0a944381 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -40,7 +40,7 @@ module ActionController def render_to_string(*) result = super if result.respond_to?(:each) - string = "".dup + string = +"" result.each { |r| string << r } string else diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index fc9cf8aaff..cb109c6ad8 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 configuration 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 @@ -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 @@ -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. diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 5a06bf86e3..04922b0715 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -58,7 +58,7 @@ module ActionController # == Action Controller \Parameters # - # Allows you to choose which attributes should be whitelisted for mass updating + # Allows you to choose which attributes should be permitted for mass updating # and thus prevent accidentally exposing that which shouldn't be exposed. # Provides two methods for this purpose: #require and #permit. The former is # used to mark parameters as required. The latter is used to set the parameter @@ -133,6 +133,15 @@ module ActionController # Returns a hash that can be used as the JSON representation for the parameters. ## + # :method: each_key + # + # :call-seq: + # each_key() + # + # Calls block once for each key in the parameters, passing the key. + # If no block is given, an enumerator is returned instead. + + ## # :method: empty? # # :call-seq: @@ -204,7 +213,7 @@ module ActionController # # Returns a new array of the values of the parameters. delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, - :as_json, :to_s, to: :@parameters + :as_json, :to_s, :each_key, to: :@parameters # By default, never raise an UnpermittedParameters exception if these # params are present. The default includes both 'controller' and 'action' @@ -339,6 +348,14 @@ module ActionController end alias_method :each, :each_pair + # Convert all hashes in values into parameters, then yield each value in + # the same way as <tt>Hash#each_value</tt>. + def each_value(&block) + @parameters.each_pair do |key, value| + yield convert_hashes_to_parameters(key, value) + end + end + # Attribute that keeps track of converted arrays, if any, to avoid double # looping in the common use case permit + mass-assignment. Defined in a # method to instantiate it only if needed. @@ -505,7 +522,7 @@ module ActionController # # Note that if you use +permit+ in a key that points to a hash, # it won't allow all the hash. You also need to specify which - # attributes inside the hash should be whitelisted. + # attributes inside the hash should be permitted. # # params = ActionController::Parameters.new({ # person: { @@ -560,12 +577,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) @@ -637,20 +656,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 @@ -793,9 +810,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)) } @@ -906,15 +921,28 @@ module ActionController PERMITTED_SCALAR_TYPES.any? { |type| value.is_a?(type) } end - def permitted_scalar_filter(params, key) - if has_key?(key) && permitted_scalar?(self[key]) - params[key] = self[key] + # Adds existing keys to the params if their values are scalar. + # + # For example: + # + # puts self.keys #=> ["zipcode(90210i)"] + # params = {} + # + # permitted_scalar_filter(params, "zipcode") + # + # puts params.keys # => ["zipcode"] + def permitted_scalar_filter(params, permitted_key) + permitted_key = permitted_key.to_s + + if has_key?(permitted_key) && permitted_scalar?(self[permitted_key]) + params[permitted_key] = self[permitted_key] end - keys.grep(/\A#{Regexp.escape(key)}\(\d+[if]?\)\z/) do |k| - if permitted_scalar?(self[k]) - params[k] = self[k] - end + each_key do |key| + next unless key =~ /\(\d+[if]?\)\z/ + next unless $~.pre_match == permitted_key + + params[key] = self[key] if permitted_scalar?(self[key]) end end @@ -999,8 +1027,8 @@ module ActionController # # It provides an interface for protecting attributes from end-user # assignment. This makes Action Controller parameters forbidden - # to be used in Active Model mass assignment until they have been - # whitelisted. + # to be used in Active Model mass assignment until they have been explicitly + # enumerated. # # In addition, parameters can be marked as required and flow through a # predefined raise/rescue flow to end up as a <tt>400 Bad Request</tt> with no @@ -1036,7 +1064,7 @@ module ActionController # end # # In order to use <tt>accepts_nested_attributes_for</tt> with Strong \Parameters, you - # will need to specify which nested attributes should be whitelisted. You might want + # will need to specify which nested attributes should be permitted. You might want # to allow +:id+ and +:_destroy+, see ActiveRecord::NestedAttributes for more information. # # class Person @@ -1054,7 +1082,7 @@ module ActionController # private # # def person_params - # # It's mandatory to specify the nested attributes that should be whitelisted. + # # It's mandatory to specify the nested attributes that should be permitted. # # If you use `permit` with just the key that points to the nested attributes hash, # # it will return an empty hash. # params.require(:person).permit(:name, :age, pets_attributes: [ :id, :name, :category ]) diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 84dbb59a63..f077e765ab 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -44,7 +44,7 @@ module ActionController options[:original_script_name] = original_script_name else if same_origin - options[:script_name] = request.script_name.empty? ? "".freeze : request.script_name.dup + options[:script_name] = request.script_name.empty? ? "" : request.script_name.dup else options[:script_name] = script_name end diff --git a/actionpack/lib/action_controller/railties/helpers.rb b/actionpack/lib/action_controller/railties/helpers.rb index fa746fa9e8..75938108d6 100644 --- a/actionpack/lib/action_controller/railties/helpers.rb +++ b/actionpack/lib/action_controller/railties/helpers.rb @@ -7,7 +7,7 @@ module ActionController super return unless klass.respond_to?(:helpers_path=) - if namespace = klass.parents.detect { |m| m.respond_to?(:railtie_helpers_paths) } + if namespace = klass.module_parents.detect { |m| m.respond_to?(:railtie_helpers_paths) } paths = namespace.railtie_helpers_paths else paths = ActionController::Helpers.helpers_path 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_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index a8febc32b3..f67b13f657 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -4,8 +4,8 @@ module ActionDispatch module Http module Cache module Request - HTTP_IF_MODIFIED_SINCE = "HTTP_IF_MODIFIED_SINCE".freeze - HTTP_IF_NONE_MATCH = "HTTP_IF_NONE_MATCH".freeze + HTTP_IF_MODIFIED_SINCE = "HTTP_IF_MODIFIED_SINCE" + HTTP_IF_NONE_MATCH = "HTTP_IF_NONE_MATCH" def if_modified_since if since = get_header(HTTP_IF_MODIFIED_SINCE) @@ -124,8 +124,8 @@ module ActionDispatch private - DATE = "Date".freeze - LAST_MODIFIED = "Last-Modified".freeze + DATE = "Date" + LAST_MODIFIED = "Last-Modified" SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) def generate_weak_etag(validators) @@ -166,11 +166,11 @@ module ActionDispatch @cache_control = cache_control_headers end - DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate".freeze - NO_CACHE = "no-cache".freeze - PUBLIC = "public".freeze - PRIVATE = "private".freeze - MUST_REVALIDATE = "must-revalidate".freeze + DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate" + NO_CACHE = "no-cache" + PUBLIC = "public" + PRIVATE = "private" + MUST_REVALIDATE = "must-revalidate" def handle_conditional_get! # Normally default cache control setting is handled by ETag @@ -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_disposition.rb b/actionpack/lib/action_dispatch/http/content_disposition.rb new file mode 100644 index 0000000000..58164c1522 --- /dev/null +++ b/actionpack/lib/action_dispatch/http/content_disposition.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module ActionDispatch + module Http + class ContentDisposition # :nodoc: + def self.format(disposition:, filename:) + new(disposition: disposition, filename: filename).to_s + end + + attr_reader :disposition, :filename + + def initialize(disposition:, filename:) + @disposition = disposition + @filename = filename + end + + TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/ + + def ascii_filename + 'filename="' + percent_escape(I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"' + end + + RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/ + + def utf8_filename + "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR) + end + + def to_s + if filename + "#{disposition}; #{ascii_filename}; #{utf8_filename}" + else + "#{disposition}" + end + end + + private + def percent_escape(string, pattern) + string.gsub(pattern) do |char| + char.bytes.map { |byte| "%%%02X" % byte }.join + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 35041fd072..b1e5a28be5 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -5,9 +5,9 @@ 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 + CONTENT_TYPE = "Content-Type" + POLICY = "Content-Security-Policy" + POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only" def initialize(app) @app = app @@ -22,7 +22,8 @@ module ActionDispatch #:nodoc: if policy = request.content_security_policy nonce = request.content_security_policy_nonce - headers[header_name(request)] = policy.build(request.controller_instance, nonce) + context = request.controller_instance || request + headers[header_name(request)] = policy.build(context, nonce) end response @@ -50,10 +51,10 @@ module ActionDispatch #:nodoc: 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 + POLICY = "action_dispatch.content_security_policy" + POLICY_REPORT_ONLY = "action_dispatch.content_security_policy_report_only" + NONCE_GENERATOR = "action_dispatch.content_security_policy_nonce_generator" + NONCE = "action_dispatch.content_security_policy_nonce" def content_security_policy get_header(POLICY) @@ -132,7 +133,7 @@ module ActionDispatch #:nodoc: worker_src: "worker-src" }.freeze - NONCE_DIRECTIVES = %w[script-src].freeze + NONCE_DIRECTIVES = %w[script-src style-src].freeze private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES @@ -257,7 +258,8 @@ module ActionDispatch #:nodoc: if context.nil? raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}" else - context.instance_exec(&source) + resolved = context.instance_exec(&source) + resolved.is_a?(Symbol) ? apply_mapping(resolved) : resolved end else raise RuntimeError, "Unexpected content security policy source: #{source.inspect}" diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index ec012ad02d..cbb772175c 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "action_dispatch/http/parameter_filter" +require "active_support/parameter_filter" module ActionDispatch module Http @@ -28,8 +28,8 @@ module ActionDispatch # => reverses the value to all keys matching /secret/i module FilterParameters ENV_MATCH = [/RAW_POST_DATA/, "rack.request.form_vars"] # :nodoc: - NULL_PARAM_FILTER = ParameterFilter.new # :nodoc: - NULL_ENV_FILTER = ParameterFilter.new ENV_MATCH # :nodoc: + NULL_PARAM_FILTER = ActiveSupport::ParameterFilter.new # :nodoc: + NULL_ENV_FILTER = ActiveSupport::ParameterFilter.new ENV_MATCH # :nodoc: def initialize super @@ -41,6 +41,8 @@ module ActionDispatch # Returns a hash of parameters with all sensitive data replaced. def filtered_parameters @filtered_parameters ||= parameter_filter.filter(parameters) + rescue ActionDispatch::Http::Parameters::ParseError + @filtered_parameters = {} end # Returns a hash of request.env with all sensitive data replaced. @@ -69,7 +71,7 @@ module ActionDispatch end def parameter_filter_for(filters) # :doc: - ParameterFilter.new(filters) + ActiveSupport::ParameterFilter.new(filters) end KV_RE = "[^&;=]+" diff --git a/actionpack/lib/action_dispatch/http/filter_redirect.rb b/actionpack/lib/action_dispatch/http/filter_redirect.rb index 25394fe5dd..8c4e852235 100644 --- a/actionpack/lib/action_dispatch/http/filter_redirect.rb +++ b/actionpack/lib/action_dispatch/http/filter_redirect.rb @@ -3,7 +3,7 @@ module ActionDispatch module Http module FilterRedirect - FILTERED = "[FILTERED]".freeze # :nodoc: + FILTERED = "[FILTERED]" # :nodoc: def filtered_location # :nodoc: if location_filter_match? 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_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index d7435fa8df..498b1e6695 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -7,6 +7,11 @@ module ActionDispatch module MimeNegotiation extend ActiveSupport::Concern + RESCUABLE_MIME_FORMAT_ERRORS = [ + ActionController::BadRequest, + ActionDispatch::Http::Parameters::ParseError, + ] + included do mattr_accessor :ignore_accept_header, default: false end @@ -59,7 +64,7 @@ module ActionDispatch fetch_header("action_dispatch.request.formats") do |k| params_readable = begin parameters[:format] - rescue ActionController::BadRequest + rescue *RESCUABLE_MIME_FORMAT_ERRORS false end @@ -85,10 +90,7 @@ module ActionDispatch if variant.all? { |v| v.is_a?(Symbol) } @variant = ActiveSupport::ArrayInquirer.new(variant) else - raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols. " \ - "For security reasons, never directly set the variant to a user-provided value, " \ - "like params[:variant].to_sym. Check user-provided value against a whitelist first, " \ - "then set the variant: request.variant = :tablet if params[:variant] == 'tablet'" + raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols." end end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 295539281f..c3e0ea3c89 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -# -*- frozen-string-literal: true -*- - require "singleton" require "active_support/core_ext/string/starts_ends_with" @@ -74,7 +72,7 @@ module Mime def initialize(index, name, q = nil) @index = index @name = name - q ||= 0.0 if @name == "*/*".freeze # Default wildcard match to end of list. + q ||= 0.0 if @name == "*/*" # Default wildcard match to end of list. @q = ((q || 1.0).to_f * 100).to_i end diff --git a/actionpack/lib/action_dispatch/http/parameter_filter.rb b/actionpack/lib/action_dispatch/http/parameter_filter.rb index 1d58964862..ddeb3d81e2 100644 --- a/actionpack/lib/action_dispatch/http/parameter_filter.rb +++ b/actionpack/lib/action_dispatch/http/parameter_filter.rb @@ -1,86 +1,12 @@ # frozen_string_literal: true -require "active_support/core_ext/object/duplicable" +require "active_support/deprecation/constant_accessor" +require "active_support/parameter_filter" module ActionDispatch module Http - class ParameterFilter - FILTERED = "[FILTERED]".freeze # :nodoc: - - def initialize(filters = []) - @filters = filters - end - - def filter(params) - compiled_filter.call(params) - end - - private - - def compiled_filter - @compiled_filter ||= CompiledFilter.compile(@filters) - end - - class CompiledFilter # :nodoc: - def self.compile(filters) - return lambda { |params| params.dup } if filters.empty? - - strings, regexps, blocks = [], [], [] - - filters.each do |item| - case item - when Proc - blocks << item - when Regexp - regexps << item - else - strings << Regexp.escape(item.to_s) - end - end - - deep_regexps, regexps = regexps.partition { |r| r.to_s.include?("\\.".freeze) } - deep_strings, strings = strings.partition { |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? - - new regexps, deep_regexps, blocks - end - - attr_reader :regexps, :deep_regexps, :blocks - - def initialize(regexps, deep_regexps, blocks) - @regexps = regexps - @deep_regexps = deep_regexps.any? ? deep_regexps : nil - @blocks = blocks - end - - def call(original_params, parents = []) - filtered_params = original_params.class.new - - original_params.each do |key, value| - parents.push(key) if deep_regexps - if regexps.any? { |r| key =~ r } - value = FILTERED - elsif deep_regexps && (joined = parents.join(".")) && deep_regexps.any? { |r| joined =~ r } - value = FILTERED - elsif value.is_a?(Hash) - value = call(value, parents) - elsif value.is_a?(Array) - value = value.map { |v| v.is_a?(Hash) ? call(v, parents) : v } - elsif blocks.any? - key = key.dup if key.duplicable? - value = value.dup if value.duplicable? - blocks.each { |b| b.call(key, value) } - end - parents.pop if deep_regexps - - filtered_params[key] = value - end - - filtered_params - end - end - end + include ActiveSupport::Deprecation::DeprecatedConstantAccessor + deprecate_constant "ParameterFilter", "ActiveSupport::ParameterFilter", + message: "ActionDispatch::Http::ParameterFilter is deprecated and will be removed from Rails 6.1. Use ActiveSupport::ParameterFilter instead." end end diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 8d7431fd6b..13d0963a33 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -111,13 +111,23 @@ module ActionDispatch begin strategy.call(raw_post) rescue # JSON or Ruby code block errors. - my_logger = logger || ActiveSupport::Logger.new($stderr) - my_logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{raw_post}" - + log_parse_error_once raise ParseError end end + def log_parse_error_once + @parse_error_logged ||= begin + parse_logger = logger || ActiveSupport::Logger.new($stderr) + parse_logger.debug <<~MSG.chomp + Error occurred while parsing request parameters. + Contents: + + #{raw_post} + MSG + end + end + def params_parsers ActionDispatch::Request.parameter_parsers end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 3838b84a7a..44f23940d3 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -136,11 +136,11 @@ module ActionDispatch end def routes # :nodoc: - get_header("action_dispatch.routes".freeze) + get_header("action_dispatch.routes") end def routes=(routes) # :nodoc: - set_header("action_dispatch.routes".freeze, routes) + set_header("action_dispatch.routes", routes) end def engine_script_name(_routes) # :nodoc: @@ -158,11 +158,11 @@ module ActionDispatch end def controller_instance # :nodoc: - get_header("action_controller.instance".freeze) + get_header("action_controller.instance") end def controller_instance=(controller) # :nodoc: - set_header("action_controller.instance".freeze, controller) + set_header("action_controller.instance", controller) end def http_auth_salt @@ -173,7 +173,7 @@ module ActionDispatch # We're treating `nil` as "unset", and we want the default setting to be # `true`. This logic should be extracted to `env_config` and calculated # once. - !(get_header("action_dispatch.show_exceptions".freeze) == false) + !(get_header("action_dispatch.show_exceptions") == false) end # Returns a symbol form of the #request_method. @@ -280,10 +280,10 @@ module ActionDispatch end def remote_ip=(remote_ip) - set_header "action_dispatch.remote_ip".freeze, remote_ip + set_header "action_dispatch.remote_ip", remote_ip end - ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id".freeze # :nodoc: + ACTION_DISPATCH_REQUEST_ID = "action_dispatch.request_id" # :nodoc: # Returns the unique request id, which is based on either the X-Request-Id header that can # be generated by a firewall, load balancer, or web server or by the RequestId middleware @@ -383,9 +383,6 @@ module ActionDispatch end self.request_parameters = Request::Utils.normalize_encode_params(pr) end - rescue Http::Parameters::ParseError # one of the parse strategies blew up - self.request_parameters = Request::Utils.normalize_encode_params(super || {}) - raise rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}") end @@ -407,18 +404,18 @@ module ActionDispatch def request_parameters=(params) raise if params.nil? - set_header("action_dispatch.request.request_parameters".freeze, params) + set_header("action_dispatch.request.request_parameters", params) end def logger - get_header("action_dispatch.logger".freeze) + get_header("action_dispatch.logger") end def commit_flash end def ssl? - super || scheme == "wss".freeze + super || scheme == "wss" end private diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 7e50cb6d23..1d38942a31 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -78,9 +78,9 @@ module ActionDispatch # :nodoc: x end - CONTENT_TYPE = "Content-Type".freeze - SET_COOKIE = "Set-Cookie".freeze - LOCATION = "Location".freeze + CONTENT_TYPE = "Content-Type" + SET_COOKIE = "Set-Cookie" + LOCATION = "Location" NO_CONTENT_CODES = [100, 101, 102, 204, 205, 304] cattr_accessor :default_charset, default: "utf-8" @@ -105,7 +105,7 @@ module ActionDispatch # :nodoc: def body @str_body ||= begin - buf = "".dup + buf = +"" each { |chunk| buf << chunk } buf end @@ -224,16 +224,6 @@ module ActionDispatch # :nodoc: @status = Rack::Utils.status_code(status) end - # Sets the HTTP content type. - def content_type=(content_type) - return unless content_type - new_header_info = parse_content_type(content_type.to_s) - prev_header_info = parsed_content_type_header - charset = new_header_info.charset || prev_header_info.charset - charset ||= self.class.default_charset unless prev_header_info.mime_type - set_content_type new_header_info.mime_type, charset - end - # Sets the HTTP response's content MIME type. For example, in the controller # you could write this: # @@ -242,7 +232,17 @@ module ActionDispatch # :nodoc: # If a character set has been defined for this response (see charset=) then # the character set information will also be included in the content type # information. + def content_type=(content_type) + return unless content_type + new_header_info = parse_content_type(content_type.to_s) + prev_header_info = parsed_content_type_header + charset = new_header_info.charset || prev_header_info.charset + charset ||= self.class.default_charset unless prev_header_info.mime_type + set_content_type new_header_info.mime_type, charset + end + # Content type of response. + # It returns just MIME type and does NOT contain charset part. def content_type parsed_content_type_header.mime_type end 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 35ba44005a..8227749986 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -67,7 +67,7 @@ module ActionDispatch end def path_for(options) - path = options[:script_name].to_s.chomp("/".freeze) + path = options[:script_name].to_s.chomp("/") path << options[:path] if options.key?(:path) add_trailing_slash(path) if options[:trailing_slash] @@ -79,108 +79,108 @@ module ActionDispatch private - def add_params(path, params) - params = { params: params } unless params.is_a?(Hash) - params.reject! { |_, v| v.to_param.nil? } - query = params.to_query - path << "?#{query}" unless query.empty? - end - - def add_anchor(path, anchor) - if anchor - path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + def add_params(path, params) + params = { params: params } unless params.is_a?(Hash) + params.reject! { |_, v| v.to_param.nil? } + query = params.to_query + path << "?#{query}" unless query.empty? end - end - def extract_domain_from(host, tld_length) - host.split(".").last(1 + tld_length).join(".") - end + def add_anchor(path, anchor) + if anchor + path << "##{Journey::Router::Utils.escape_fragment(anchor.to_param)}" + end + end - def extract_subdomains_from(host, tld_length) - parts = host.split(".") - parts[0..-(tld_length + 2)] - end + def extract_domain_from(host, tld_length) + host.split(".").last(1 + tld_length).join(".") + end - def add_trailing_slash(path) - if path.include?("?") - path.sub!(/\?/, '/\&') - elsif !path.include?(".") - path.sub!(/[^\/]\z|\A\z/, '\&/') + def extract_subdomains_from(host, tld_length) + parts = host.split(".") + parts[0..-(tld_length + 2)] end - end - def build_host_url(host, port, protocol, options, path) - if match = host.match(HOST_REGEXP) - protocol ||= match[1] unless protocol == false - host = match[2] - port = match[3] unless options.key? :port + def add_trailing_slash(path) + if path.include?("?") + path.sub!(/\?/, '/\&') + elsif !path.include?(".") + path.sub!(/[^\/]\z|\A\z/, '\&/') + end end - protocol = normalize_protocol protocol - host = normalize_host(host, options) + def build_host_url(host, port, protocol, options, path) + if match = host.match(HOST_REGEXP) + protocol ||= match[1] unless protocol == false + host = match[2] + port = match[3] unless options.key? :port + end - result = protocol.dup + protocol = normalize_protocol protocol + host = normalize_host(host, options) - if options[:user] && options[:password] - result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" - end + result = protocol.dup - result << host - normalize_port(port, protocol) { |normalized_port| - result << ":#{normalized_port}" - } + if options[:user] && options[:password] + result << "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@" + end - result.concat path - end + result << host + normalize_port(port, protocol) { |normalized_port| + result << ":#{normalized_port}" + } - def named_host?(host) - IP_HOST_REGEXP !~ host - end + result.concat path + end - def normalize_protocol(protocol) - case protocol - when nil - "http://" - when false, "//" - "//" - when PROTOCOL_REGEXP - "#{$1}://" - else - raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + def named_host?(host) + IP_HOST_REGEXP !~ host end - end - def normalize_host(_host, options) - return _host unless named_host?(_host) + def normalize_protocol(protocol) + case protocol + when nil + "http://" + when false, "//" + "//" + when PROTOCOL_REGEXP + "#{$1}://" + else + raise ArgumentError, "Invalid :protocol option: #{protocol.inspect}" + end + end - tld_length = options[:tld_length] || @@tld_length - subdomain = options.fetch :subdomain, true - domain = options[:domain] + def normalize_host(_host, options) + return _host unless named_host?(_host) - host = "".dup - if subdomain == true - return _host if domain.nil? + tld_length = options[:tld_length] || @@tld_length + subdomain = options.fetch :subdomain, true + domain = options[:domain] - host << extract_subdomains_from(_host, tld_length).join(".") - elsif subdomain - host << subdomain.to_param + host = +"" + if subdomain == true + return _host if domain.nil? + + host << extract_subdomains_from(_host, tld_length).join(".") + elsif subdomain + host << subdomain.to_param + end + host << "." unless host.empty? + host << (domain || extract_domain_from(_host, tld_length)) + host end - host << "." unless host.empty? - host << (domain || extract_domain_from(_host, tld_length)) - host - end - def normalize_port(port, protocol) - return unless port + def normalize_port(port, protocol) + return unless port - case protocol - when "//" then yield port - when "https://" - yield port unless port.to_i == 443 - else - yield port unless port.to_i == 80 + case protocol + when "//" then yield port + when "https://" + yield port unless port.to_i == 443 + else + yield port unless port.to_i == 80 + end end - end end def initialize @@ -231,7 +231,7 @@ module ActionDispatch # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080' # req.host # => "example.com" def host - raw_host_with_port.sub(/:\d+$/, "".freeze) + raw_host_with_port.sub(/:\d+$/, "") end # Returns a \host:\port string for this request, such as "example.com" or diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 0f04839d9b..52396ec901 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -50,7 +50,7 @@ module ActionDispatch unmatched_keys = (missing_keys || []) & constraints.keys missing_keys = (missing_keys || []) - unmatched_keys - message = "No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}".dup + message = +"No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}" message << ", missing required keys: #{missing_keys.sort.inspect}" if missing_keys && !missing_keys.empty? message << ", possible unmatched constraints: #{unmatched_keys.sort.inspect}" if unmatched_keys && !unmatched_keys.empty? 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 32f632800c..086d6a3e07 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -65,12 +65,12 @@ module ActionDispatch def literal?; false; end end - %w{ Symbol Slash Dot }.each do |t| - class_eval <<-eoruby, __FILE__, __LINE__ + 1 - class #{t} < Terminal; - def type; :#{t.upcase}; end - end - eoruby + class Slash < Terminal # :nodoc: + def type; :SLASH; end + end + + class Dot < Terminal # :nodoc: + def type; :DOT; end end class Symbol < Terminal # :nodoc: @@ -89,6 +89,7 @@ module ActionDispatch regexp == DEFAULT_EXP end + def type; :SYMBOL; end def symbol?; true; end end diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 30af3ff930..89a164f968 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -15,9 +15,6 @@ require "action_dispatch/journey/path/pattern" module ActionDispatch module Journey # :nodoc: class Router # :nodoc: - class RoutingError < ::StandardError # :nodoc: - end - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb index df3f79a407..3c8b9a6eaa 100644 --- a/actionpack/lib/action_dispatch/journey/router/utils.rb +++ b/actionpack/lib/action_dispatch/journey/router/utils.rb @@ -17,11 +17,11 @@ module ActionDispatch def self.normalize_path(path) path ||= "" encoding = path.encoding - path = "/#{path}".dup - path.squeeze!("/".freeze) - path.sub!(%r{/+\Z}, "".freeze) + path = +"/#{path}" + path.squeeze!("/") + path.sub!(%r{/+\Z}, "") path.gsub!(/(%[a-f0-9]{2})/) { $1.upcase } - path = "/".dup if path == "".freeze + path = +"/" if path == "" path.force_encoding(encoding) path end @@ -29,16 +29,16 @@ module ActionDispatch # URI path and fragment escaping # https://tools.ietf.org/html/rfc3986 class UriEncoder # :nodoc: - ENCODE = "%%%02X".freeze + ENCODE = "%%%02X" US_ASCII = Encoding::US_ASCII UTF_8 = Encoding::UTF_8 - EMPTY = "".dup.force_encoding(US_ASCII).freeze + EMPTY = (+"").force_encoding(US_ASCII).freeze DEC2HEX = (0..255).to_a.map { |i| ENCODE % i }.map { |s| s.force_encoding(US_ASCII) } - ALPHA = "a-zA-Z".freeze - DIGIT = "0-9".freeze - UNRESERVED = "#{ALPHA}#{DIGIT}\\-\\._~".freeze - SUB_DELIMS = "!\\$&'\\(\\)\\*\\+,;=".freeze + ALPHA = "a-zA-Z" + DIGIT = "0-9" + UNRESERVED = "#{ALPHA}#{DIGIT}\\-\\._~" + SUB_DELIMS = "!\\$&'\\(\\)\\*\\+,;=" ESCAPED = /%[a-zA-Z0-9]{2}/.freeze 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/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 3395471a85..d2619cbf3a 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -40,7 +40,7 @@ module ActionDispatch @parameters.each do |index| param = parts[index] value = hash[param.name] - return "".freeze unless value + return "" unless value parts[index] = param.escape value end diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index c45d947904..26d3fd936f 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -9,7 +9,7 @@ require "rack/utils" module ActionDispatch class Request def cookie_jar - fetch_header("action_dispatch.cookies".freeze) do + fetch_header("action_dispatch.cookies") do self.cookie_jar = Cookies::CookieJar.build(self, cookies) end end @@ -22,11 +22,11 @@ module ActionDispatch } def have_cookie_jar? - has_header? "action_dispatch.cookies".freeze + has_header? "action_dispatch.cookies" end def cookie_jar=(jar) - set_header "action_dispatch.cookies".freeze, jar + set_header "action_dispatch.cookies", jar end def key_generator @@ -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 @@ -168,20 +172,21 @@ module ActionDispatch # * <tt>:httponly</tt> - Whether this cookie is accessible via scripting or # only HTTP. Defaults to +false+. class Cookies - HTTP_HEADER = "Set-Cookie".freeze - GENERATOR_KEY = "action_dispatch.key_generator".freeze - SIGNED_COOKIE_SALT = "action_dispatch.signed_cookie_salt".freeze - ENCRYPTED_COOKIE_SALT = "action_dispatch.encrypted_cookie_salt".freeze - ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt".freeze - AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "action_dispatch.authenticated_encrypted_cookie_salt".freeze - USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption".freeze - ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher".freeze - SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest".freeze - SECRET_TOKEN = "action_dispatch.secret_token".freeze - SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze - COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze - COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze - COOKIES_ROTATIONS = "action_dispatch.cookies_rotations".freeze + HTTP_HEADER = "Set-Cookie" + GENERATOR_KEY = "action_dispatch.key_generator" + SIGNED_COOKIE_SALT = "action_dispatch.signed_cookie_salt" + ENCRYPTED_COOKIE_SALT = "action_dispatch.encrypted_cookie_salt" + ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt" + AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "action_dispatch.authenticated_encrypted_cookie_salt" + USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption" + ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher" + SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest" + SECRET_TOKEN = "action_dispatch.secret_token" + SECRET_KEY_BASE = "action_dispatch.secret_key_base" + COOKIES_SERIALIZER = "action_dispatch.cookies_serializer" + COOKIES_DIGEST = "action_dispatch.cookies_digest" + COOKIES_ROTATIONS = "action_dispatch.cookies_rotations" + USE_COOKIES_WITH_METADATA = "action_dispatch.use_cookies_with_metadata" # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -470,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 @@ -481,7 +486,7 @@ module ActionDispatch options = { value: options } end - commit(options) + commit(name, options) @parent_jar[name] = options end @@ -497,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 @@ -519,7 +535,7 @@ module ActionDispatch end module SerializedCookieJars # :nodoc: - MARSHAL_SIGNATURE = "\x04\x08".freeze + MARSHAL_SIGNATURE = "\x04\x08" SERIALIZER = ActiveSupport::MessageEncryptor::NullSerializer protected @@ -583,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 @@ -631,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 33edad8bd9..7669767ae3 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -23,7 +23,7 @@ module ActionDispatch if clean_params.empty? "None" else - PP.pp(clean_params, "".dup, 200) + PP.pp(clean_params, +"", 200) end end @@ -152,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, @@ -190,11 +180,14 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - logger.fatal " " - logger.fatal "#{exception.class} (#{exception.message}):" - log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) - logger.fatal " " - log_array logger, trace + message = [] + message << " " + message << "#{exception.class} (#{exception.message}):" + message.concat(exception.annoted_source_code) if exception.respond_to?(:annoted_source_code) + message << " " + message.concat(trace) + + log_array(logger, message) end end diff --git a/actionpack/lib/action_dispatch/middleware/debug_locks.rb b/actionpack/lib/action_dispatch/middleware/debug_locks.rb index 03760438f7..93c6c85a71 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_locks.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_locks.rb @@ -32,7 +32,7 @@ module ActionDispatch req = ActionDispatch::Request.new env if req.get? - path = req.path_info.chomp("/".freeze) + path = req.path_info.chomp("/") if path == @path return render_details(req) end @@ -63,19 +63,19 @@ module ActionDispatch str = threads.map do |thread, info| if info[:exclusive] - lock_state = "Exclusive".dup + lock_state = +"Exclusive" elsif info[:sharing] > 0 - lock_state = "Sharing".dup + lock_state = +"Sharing" lock_state << " x#{info[:sharing]}" if info[:sharing] > 1 else - lock_state = "No lock".dup + lock_state = +"No lock" end if info[:waiting] lock_state << " (yielded share)" end - msg = "Thread #{info[:index]} [0x#{thread.__id__.to_s(16)} #{thread.status || 'dead'}] #{lock_state}\n".dup + msg = +"Thread #{info[:index]} [0x#{thread.__id__.to_s(16)} #{thread.status || 'dead'}] #{lock_state}\n" if info[:sleeper] msg << " Waiting in #{info[:sleeper]}" diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index f05c69137b..fb2b2bd3b0 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -31,11 +31,12 @@ module ActionDispatch "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 @@ -66,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 @@ -99,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 @@ -113,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 fd05eec172..cf9165d008 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -38,7 +38,7 @@ module ActionDispatch # # See docs on the FlashHash class for more details about the flash. class Flash - KEY = "action_dispatch.request.flash_hash".freeze + KEY = "action_dispatch.request.flash_hash" module RequestMethods # Access the contents of the flash. Use <tt>flash["notice"]</tt> to diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index da2871b551..fcc0c72240 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -15,7 +15,7 @@ module ActionDispatch # The unique request id can be used to trace a request end-to-end and would typically end up being part of log files # from multiple pieces of the stack. class RequestId - X_REQUEST_ID = "X-Request-Id".freeze #:nodoc: + X_REQUEST_ID = "X-Request-Id" #:nodoc: def initialize(app) @app = app @@ -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\-@]/, "").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 240269d1c7..00902ede21 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -83,7 +83,7 @@ module ActionDispatch private def set_hsts_header!(headers) - headers["Strict-Transport-Security".freeze] ||= @hsts_header + headers["Strict-Transport-Security"] ||= @hsts_header end def normalize_hsts_options(options) @@ -102,23 +102,23 @@ module ActionDispatch # https://tools.ietf.org/html/rfc6797#section-6.1 def build_hsts_header(hsts) - value = "max-age=#{hsts[:expires].to_i}".dup + value = +"max-age=#{hsts[:expires].to_i}" value << "; includeSubDomains" if hsts[:subdomains] value << "; preload" if hsts[:preload] value end def flag_cookies_as_secure!(headers) - if cookies = headers["Set-Cookie".freeze] - cookies = cookies.split("\n".freeze) + if cookies = headers["Set-Cookie"] + cookies = cookies.split("\n") - headers["Set-Cookie".freeze] = cookies.map { |cookie| - if cookie !~ /;\s*secure\s*(;|$)/i + headers["Set-Cookie"] = cookies.map { |cookie| + if !/;\s*secure\s*(;|$)/i.match?(cookie) "#{cookie}; secure" else cookie end - }.join("\n".freeze) + }.join("\n") end end @@ -141,7 +141,7 @@ module ActionDispatch host = @redirect[:host] || request.host port = @redirect[:port] || request.port - location = "https://#{host}".dup + location = +"https://#{host}" location << ":#{port}" if port != 80 && port != 443 location << request.fullpath location diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 8130bfe2e7..1f2f7757a3 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -41,7 +41,6 @@ module ActionDispatch rescue SystemCallError false end - } return ::Rack::Utils.escape_path(match).b end @@ -80,7 +79,7 @@ module ActionDispatch end def content_type(path) - ::Rack::Mime.mime_type(::File.extname(path), "text/plain".freeze) + ::Rack::Mime.mime_type(::File.extname(path), "text/plain") end def gzip_encoding_accepted?(request) @@ -117,7 +116,7 @@ module ActionDispatch req = Rack::Request.new env if req.get? || req.head? - path = req.path_info.chomp("/".freeze) + path = req.path_info.chomp("/") if match = @file_handler.match?(path) req.path_info = match return @file_handler.serve(req) 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 index e1b129ccc5..e8454acfad 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb @@ -11,11 +11,11 @@ <h2> <%= h @exception.message %> <% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> - <br />To resolve this issue run: bin/rails active_storage:install + <br />To resolve this issue run: rails active_storage:install <% end %> </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/invalid_statement.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb index 033518cf8a..e5e3196710 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.text.erb @@ -5,7 +5,7 @@ <%= @exception.message %> <% if @exception.message.match? %r{#{ActiveStorage::Blob.table_name}|#{ActiveStorage::Attachment.table_name}} %> -To resolve this issue run: bin/rails active_storage:install +To resolve this issue run: rails active_storage:install <% end %> <%= render template: "rescues/_source" %> 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/middleware/templates/routes/_table.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb index 1fa0691303..0242b706b2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb @@ -197,4 +197,7 @@ setupMatchPaths(); setupRouteToggleHelperLinks(); + + // Focus the search input after page has loaded + document.getElementById('search').focus(); </script> diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index eb6fbca6ba..efc3988bc3 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -21,6 +21,7 @@ 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 = { 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/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index bae50f6a43..413e524ef6 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -83,7 +83,7 @@ module ActionDispatch private def normalize_filter(filter) if filter[:controller] - { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + { controller: /#{filter[:controller].underscore.sub(/_?controller\z/, "")}/ } elsif filter[:grep] { controller: /#{filter[:grep]}/, action: /#{filter[:grep]}/, verb: /#{filter[:grep]}/, name: /#{filter[:grep]}/, path: /#{filter[:grep]}/ } @@ -159,7 +159,7 @@ module ActionDispatch "No routes were found for this grep pattern." end - @buffer << "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." + @buffer << "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." end end @@ -258,7 +258,7 @@ module ActionDispatch <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 diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d9dd24935b..d67044b4ac 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -50,7 +50,19 @@ module ActionDispatch private def constraint_args(constraint, request) - constraint.arity == 1 ? [request] : [request.path_parameters, request] + arity = if constraint.respond_to?(:arity) + constraint.arity + else + constraint.method(:call).arity + end + + if arity < 1 + [] + elsif arity == 1 + [request] + else + [request.path_parameters, request] + end end end @@ -148,17 +160,8 @@ module ActionDispatch end def make_route(name, precedence) - route = Journey::Route.new(name, - application, - path, - conditions, - required_defaults, - defaults, - request_method, - precedence, - @internal) - - route + Journey::Route.new(name, application, path, conditions, required_defaults, + defaults, request_method, precedence, @internal) end def application @@ -279,7 +282,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 @@ -308,8 +311,8 @@ module ActionDispatch def check_controller_and_action(path_params, controller, action) 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 = +"'#{part}' is not a supported controller name. This can lead to potential routing problems." + message << " See https://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" raise ArgumentError, message } @@ -333,7 +336,7 @@ module ActionDispatch end def split_to(to) - if to =~ /#/ + if /#/.match?(to) to.split("#") else [] @@ -342,7 +345,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("/") @@ -390,7 +393,7 @@ module ActionDispatch # for root cases, where the latter is the correct one. def self.normalize_path(path) path = Journey::Router::Utils.normalize_path(path) - path.gsub!(%r{/(\(+)/?}, '\1/') unless path =~ %r{^/\(+[^)]+\)$} + path.gsub!(%r{/(\(+)/?}, '\1/') unless path =~ %r{^/(\(+[^)]+\)){1,}$} path end @@ -553,10 +556,10 @@ module ActionDispatch # # match 'json_only', constraints: { format: 'json' }, via: :get # - # class Whitelist + # class PermitList # def matches?(request) request.remote_ip == '1.2.3.4' end # end - # match 'path', to: 'c#a', constraints: Whitelist.new, via: :get + # match 'path', to: 'c#a', constraints: PermitList.new, via: :get # # See <tt>Scoping#constraints</tt> for more examples with its scope # equivalent. @@ -644,7 +647,7 @@ module ActionDispatch # Query if the following named route was already defined. def has_named_route?(name) - @set.named_routes.key? name + @set.named_routes.key?(name) end private @@ -668,7 +671,7 @@ module ActionDispatch script_namer = ->(options) do prefix_options = options.slice(*_route.segment_keys) - prefix_options[:relative_url_root] = "".freeze + prefix_options[:relative_url_root] = "" if options[:_recall] prefix_options.reverse_merge!(options[:_recall].slice(*_route.segment_keys)) @@ -1159,10 +1162,16 @@ module ActionDispatch end def actions + if @except + available_actions - Array(@except).map(&:to_sym) + else + available_actions + end + end + + def available_actions if @only Array(@only).map(&:to_sym) - elsif @except - default_actions - Array(@except).map(&:to_sym) else default_actions end @@ -1588,7 +1597,7 @@ module ActionDispatch when Symbol options[:action] = to when String - if to =~ /#/ + if /#/.match?(to) options[:to] = to else options[:controller] = to @@ -1914,7 +1923,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 @@ -1934,9 +1943,7 @@ module ActionDispatch end def match_root_route(options) - name = has_named_route?(name_for_action(:root, nil)) ? nil : :root - args = ["/", { as: name, via: :get }.merge!(options)] - + args = ["/", { as: :root, via: :get }.merge(options)] match(*args) end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index e17ccaf986..4de5f9e2f7 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -181,8 +181,8 @@ module ActionDispatch CACHE[type].fetch(action) { build action, type } end - def self.url; CACHE["url".freeze][nil]; end - def self.path; CACHE["path".freeze][nil]; end + def self.url; CACHE["url"][nil]; end + def self.path; CACHE["path"][nil]; end def self.build(action, type) prefix = action ? "#{action}_" : "" diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 1134279a7f..2ae75b0da8 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -245,7 +245,7 @@ module ActionDispatch missing_keys << missing_key } constraints = Hash[@route.requirements.merge(params).sort_by { |k, v| k.to_s }] - message = "No route matches #{constraints.inspect}".dup + message = +"No route matches #{constraints.inspect}" message << ", missing required keys: #{missing_keys.sort.inspect}" raise ActionController::UrlGenerationError, message @@ -377,7 +377,7 @@ module ActionDispatch @prepend = [] @disable_clear_and_finalize = false @finalized = false - @env_key = "ROUTES_#{object_id}_SCRIPT_NAME".freeze + @env_key = "ROUTES_#{object_id}_SCRIPT_NAME" @set = Journey::Routes.new @router = Journey::Router.new @set @@ -584,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) @@ -729,7 +729,7 @@ module ActionDispatch # Remove leading slashes from controllers def normalize_controller! if controller - if controller.start_with?("/".freeze) + if controller.start_with?("/") @options[:controller] = controller[1..-1] else @options[:controller] = controller @@ -820,10 +820,6 @@ module ActionDispatch path, params = generate(route_name, path_options, recall) - if options.key? :params - params.merge! options[:params] - end - options[:path] = path options[:script_name] = script_name options[:params] = params diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb index d2685e0452..884fb51d18 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 @@ -65,7 +65,7 @@ module ActionDispatch end def display_image - message = "[Screenshot]: #{image_path}\n".dup + message = +"[Screenshot]: #{image_path}\n" case output_type when "artifact" diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb index e47d5020f4..600e9c733b 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -17,8 +17,11 @@ module ActionDispatch end def after_teardown - take_failed_screenshot - Capybara.reset_sessions! + begin + take_failed_screenshot + ensure + Capybara.reset_sessions! + end ensure super end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 98b1965d22..8595ea03cf 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -79,9 +79,8 @@ module ActionDispatch end def generate_response_message(expected, actual = @response.response_code) - "Expected response to be a <#{code_with_name(expected)}>,"\ - " but was a <#{code_with_name(actual)}>" - .dup.concat(location_if_redirected).concat(response_body_if_short) + (+"Expected response to be a <#{code_with_name(expected)}>,"\ + " but was a <#{code_with_name(actual)}>").concat(location_if_redirected).concat(response_body_if_short) end def response_body_if_short diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 5390581139..af41521c5c 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -9,6 +9,11 @@ module ActionDispatch module Assertions # Suite of assertions to test routes generated by \Rails and the handling of requests made to them. module RoutingAssertions + def setup # :nodoc: + @routes ||= nil + super + end + # Asserts that the routing of the given +path+ was handled correctly and that the parsed options (given in the +expected_options+ hash) # match +path+. Basically, it asserts that \Rails recognizes the route given by +expected_options+. # @@ -78,7 +83,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 +194,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 f0398dc7b1..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 @@ -216,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_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index 8ac50c730d..0b98f27f11 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -8,12 +8,12 @@ module ActionDispatch module FixtureFile # Shortcut for <tt>Rack::Test::UploadedFile.new(File.join(ActionDispatch::IntegrationTest.fixture_path, path), type)</tt>: # - # post :change_avatar, avatar: fixture_file_upload('files/spongebob.png', 'image/png') + # post :change_avatar, params: { avatar: fixture_file_upload('files/spongebob.png', 'image/png') } # # To upload binary files on Windows, pass <tt>:binary</tt> as the last parameter. # This will not affect other platforms: # - # post :change_avatar, avatar: fixture_file_upload('files/spongebob.png', 'image/png', :binary) + # post :change_avatar, params: { avatar: fixture_file_upload('files/spongebob.png', 'image/png', :binary) } def fixture_file_upload(path, mime_type = nil, binary = false) if self.class.respond_to?(:fixture_path) && self.class.fixture_path && !File.exist?(path) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index f4787ed27a..f23151e518 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -13,13 +13,6 @@ silence_warnings do Encoding.default_external = Encoding::UTF_8 end -require "drb" -begin - require "drb/unix" -rescue LoadError - puts "'drb/unix' is not available" -end - if ENV["TRAVIS"] PROCESS_COUNT = 0 else @@ -80,7 +73,7 @@ end module ActiveSupport class TestCase if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0 - parallelize_me! + parallelize(workers: PROCESS_COUNT) end end end @@ -232,6 +225,7 @@ module ActionController routes = ActionDispatch::Routing::RouteSet.new routes.draw(&block) include routes.url_helpers + routes end end @@ -358,86 +352,19 @@ class ImagesController < ResourcesController; end require "active_support/testing/method_call_assertions" -class ForkingExecutor - class Server - include DRb::DRbUndumped - - def initialize - @queue = Queue.new - end - - def record(reporter, result) - reporter.record result - end - - def <<(o) - o[2] = DRbObject.new(o[2]) if o - @queue << o - end - def pop; @queue.pop; end - end - - def initialize(size) - @size = size - @queue = Server.new - @pool = nil - @url = DRb.start_service("drbunix:", @queue).uri - end - - def <<(work); @queue << work; end - - def shutdown - pool = @size.times.map { - fork { - DRb.stop_service - queue = DRbObject.new_with_uri @url - while job = queue.pop - klass = job[0] - method = job[1] - reporter = job[2] - result = Minitest.run_one_method klass, method - if result.error? - translate_exceptions result - end - queue.record reporter, result - end - } - } - @size.times { @queue << nil } - pool.each { |pid| Process.waitpid pid } - end +class ActiveSupport::TestCase + include ActiveSupport::Testing::MethodCallAssertions private - def translate_exceptions(result) - result.failures.map! { |e| - begin - Marshal.dump e - e - rescue TypeError - ex = Exception.new e.message - ex.set_backtrace e.backtrace - Minitest::UnexpectedError.new ex - end - } + # Skips the current run on Rubinius using Minitest::Assertions#skip + def rubinius_skip(message = "") + skip message if RUBY_ENGINE == "rbx" end -end - -if RUBY_ENGINE == "ruby" && PROCESS_COUNT > 0 - # Use N processes (N defaults to 4) - Minitest.parallel_executor = ForkingExecutor.new(PROCESS_COUNT) -end -class ActiveSupport::TestCase - include ActiveSupport::Testing::MethodCallAssertions - - # Skips the current run on Rubinius using Minitest::Assertions#skip - private def rubinius_skip(message = "") - skip message if RUBY_ENGINE == "rbx" - end - # Skips the current run on JRuby using Minitest::Assertions#skip - private def jruby_skip(message = "") - skip message if defined?(JRUBY_VERSION) - end + # Skips the current run on JRuby using Minitest::Assertions#skip + def jruby_skip(message = "") + skip message if defined?(JRUBY_VERSION) + end end class DrivenByRackTest < ActionDispatch::SystemTestCase diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index a672ede1a9..558e710df9 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -138,7 +138,7 @@ class ControllerInstanceTests < ActiveSupport::TestCase response_headers = SimpleController.action("hello").call( "REQUEST_METHOD" => "GET", - "rack.input" => -> {} + "rack.input" => -> { } )[1] assert response_headers.key?("X-Frame-Options") @@ -193,7 +193,7 @@ class UrlOptionsTest < ActionController::TestCase action: "home", controller: "pages", only_path: true, - params: { "token" => "secret" } + token: "secret" } assert_equal "/home?token=secret", rs.url_for(options) diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index 34bc2c0caa..409a4ec2e6 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -342,6 +342,21 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest end end + def test_flash_usable_in_metal_without_helper + controller_class = nil + + assert_nothing_raised do + controller_class = Class.new(ActionController::Metal) do + include ActionController::Flash + end + end + + controller = controller_class.new + + assert_respond_to controller, :alert + assert_respond_to controller, :notice + end + private # Overwrite get to send SessionSecret in env hash diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 3f211cd60d..b133afb343 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -272,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/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 672aa1351c..103123f98c 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -150,7 +150,7 @@ class HttpTokenAuthenticationTest < ActionController::TestCase end test "token_and_options returns empty string with empty token" do - token = "".dup + token = +"" actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first expected = token assert_equal(expected, actual) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 9cdf04b886..39ede1442a 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -349,6 +349,16 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest 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 + def test_xml_http_request_get with_test_route_set do get "/get", xhr: true @@ -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 431fe90b23..d81c43b87d 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -304,7 +304,7 @@ module ActionController # Simulate InterlockHook ActiveSupport::Dependencies.interlock.start_running res = get :write_sleep_autoload - res.each {} + res.each { } ActiveSupport::Dependencies.interlock.done_running 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 248ef36b7c..7b53092266 100644 --- a/actionpack/test/controller/metal_test.rb +++ b/actionpack/test/controller/metal_test.rb @@ -20,7 +20,7 @@ class MetalControllerInstanceTests < ActiveSupport::TestCase response_headers = SimpleController.action("hello").call( "REQUEST_METHOD" => "GET", - "rack.input" => -> {} + "rack.input" => -> { } )[1] assert_not response_headers.key?("X-Frame-Options") diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 771eccb29b..00e1d5f3b3 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -78,7 +78,7 @@ class RespondToController < ActionController::Base def missing_templates respond_to do |type| # This test requires a block that is empty - type.json {} + type.json { } type.xml end end @@ -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" diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index b049022a06..7572d514fb 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -13,7 +13,7 @@ module BareMetalTest test "response body is a Rack-compatible response" do status, headers, body = BareController.action(:index).call(Rack::MockRequest.env_for("/")) assert_equal 200, status - string = "".dup + string = +"" body.each do |part| assert part.is_a?(String), "Each part of the body must be a String" diff --git a/actionpack/test/controller/new_base/render_context_test.rb b/actionpack/test/controller/new_base/render_context_test.rb index 07fbadae9f..5e570a1d79 100644 --- a/actionpack/test/controller/new_base/render_context_test.rb +++ b/actionpack/test/controller/new_base/render_context_test.rb @@ -32,10 +32,11 @@ module RenderContext "controller context!" end - # 3) Set view_context to self - private def view_context - self - end + private + # 3) Set view_context to self + def view_context + self + end end class RenderContextTest < Rack::TestCase diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 674b2c6266..7789e654d5 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -75,6 +75,28 @@ class ParametersAccessorsTest < ActiveSupport::TestCase end end + test "each_value carries permitted status" do + @params.permit! + @params.each_value do |value| + assert_predicate(value, :permitted?) + end + end + + test "each_value carries unpermitted status" do + @params.each_value do |value| + assert_not_predicate(value, :permitted?) + end + end + + test "each_key converts to hash for permitted" do + @params.permit! + @params.each_key { |key| assert_kind_of(String, key) if key == "person" } + end + + test "each_key converts to hash for unpermitted" do + @params.each_key { |key| assert_kind_of(String, key) if key == "person" } + end + test "empty? returns true when params contains no key/value pairs" do params = ActionController::Parameters.new assert_empty params @@ -190,6 +212,27 @@ class ParametersAccessorsTest < ActiveSupport::TestCase 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 params = ActionController::Parameters.new(city: "Chicago", state: "Illinois") assert params.value?("Chicago") diff --git a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb index fe0e5e368d..974612fb7b 100644 --- a/actionpack/test/controller/parameters/always_permitted_parameters_test.rb +++ b/actionpack/test/controller/parameters/always_permitted_parameters_test.rb @@ -20,7 +20,7 @@ class AlwaysPermittedParametersTest < ActiveSupport::TestCase end end - test "permits parameters that are whitelisted" do + test "allows both explicitly listed and always-permitted parameters" do params = ActionController::Parameters.new( book: { pages: 65 }, format: "json") diff --git a/actionpack/test/controller/parameters/nested_parameters_permit_test.rb b/actionpack/test/controller/parameters/nested_parameters_permit_test.rb index ccc6bf9807..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 diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 34b9ac0ab8..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 diff --git a/actionpack/test/controller/params_parse_test.rb b/actionpack/test/controller/params_parse_test.rb new file mode 100644 index 0000000000..440ab06fd7 --- /dev/null +++ b/actionpack/test/controller/params_parse_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ParamsParseTest < ActionController::TestCase + class UsersController < ActionController::Base + def create + head :ok + end + end + + tests UsersController + + def test_parse_error_logged_once + log_output = capture_log_output do + post :create, body: "{", as: :json + end + assert_equal <<~LOG, log_output + Error occurred while parsing request parameters. + Contents: + + { + LOG + end + + private + + def capture_log_output + output = StringIO.new + request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output) + yield + output.string + end +end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 2959dc3e4d..998498e1b2 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -5,6 +5,12 @@ require "abstract_unit" class Workshop extend ActiveModel::Naming include ActiveModel::Conversion + + OUT_OF_SCOPE_BLOCK = proc do + raise "Not executed in controller's context" unless RedirectController === self + request.original_url + end + attr_accessor :id def initialize(id) @@ -119,6 +125,10 @@ class RedirectController < ActionController::Base redirect_to proc { { action: "hello_world" } } end + def redirect_to_out_of_scope_block + redirect_to Workshop::OUT_OF_SCOPE_BLOCK + end + def redirect_with_header_break redirect_to "/lol\r\nwat" end @@ -204,6 +214,13 @@ class RedirectTest < ActionController::TestCase assert_equal "http://test.host/things/stuff", redirect_to_url end + def test_relative_url_redirect_host_with_port + request.host = "test.host:1234" + get :relative_url_redirect_with_status + assert_response 302 + assert_equal "http://test.host:1234/things/stuff", redirect_to_url + end + def test_simple_redirect_using_options get :host_redirect assert_response :redirect @@ -326,6 +343,12 @@ class RedirectTest < ActionController::TestCase assert_redirected_to "http://www.rubyonrails.org/" end + def test_redirect_to_out_of_scope_block + get :redirect_to_out_of_scope_block + assert_response :redirect + assert_redirected_to "http://test.host/redirect/redirect_to_out_of_scope_block" + end + def test_redirect_to_with_block_and_accepted_options with_routing do |set| set.draw do diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 24c5761e41..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"] @@ -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 diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 7a02c27c99..ea94a3e048 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -521,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 diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 4ed79073e5..3c39373e55 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -70,6 +70,10 @@ class RescueController < ActionController::Base render plain: "io error" end + rescue_from ActionDispatch::Http::Parameters::ParseError do + render plain: "parse error", status: :bad_request + end + before_action(only: :before_action_raises) { raise "umm nice" } def before_action_raises @@ -130,6 +134,11 @@ class RescueController < ActionController::Base raise ResourceUnavailableToRescueAsString end + def arbitrary_action + params + render plain: "arbitrary action" + end + def missing_template end @@ -306,6 +315,23 @@ class RescueControllerTest < ActionController::TestCase get :exception_with_no_handler_for_wrapper assert_response :unprocessable_entity end + + test "can rescue a ParseError" do + capture_log_output do + post :arbitrary_action, body: "{", as: :json + end + assert_response :bad_request + assert_equal "parse error", response.body + end + + private + + def capture_log_output + output = StringIO.new + request.set_header "action_dispatch.logger", ActiveSupport::Logger.new(output) + yield + output.string + end end class RescueTest < ActionDispatch::IntegrationTest diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 30bea64c55..d2146f12a5 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 @@ -854,6 +853,28 @@ class ResourcesTest < ActionController::TestCase end end + def test_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resources :products, only: [:show, :destroy], except: :destroy + end + + assert_resource_allowed_routes("products", {}, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + assert_resource_allowed_routes("products", { format: "xml" }, { id: "1" }, :show, [:index, :new, :create, :edit, :update, :destroy]) + end + end + + def test_singleton_resource_has_show_action_but_does_not_have_destroy_action + with_routing do |set| + set.draw do + resource :account, only: [:show, :destroy], except: :destroy + end + + assert_singleton_resource_allowed_routes("accounts", {}, :show, [:new, :create, :edit, :update, :destroy]) + assert_singleton_resource_allowed_routes("accounts", { format: "xml" }, :show, [:new, :create, :edit, :update, :destroy]) + end + end + def test_resource_has_only_create_action_and_named_route with_routing do |set| set.draw do @@ -1323,7 +1344,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 +1358,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 259f3b8855..30f2a23b33 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -25,8 +25,8 @@ class UriReservedCharactersRoutingTest < ActiveSupport::TestCase safe, unsafe = %w(: @ & = + $ , ;), %w(^ ? # [ ]) hex = unsafe.map { |char| "%" + char.unpack1("H2").upcase } - @segment = "#{safe.join}#{unsafe.join}".freeze - @escaped = "#{safe.join}#{hex.join}".freeze + @segment = "#{safe.join}#{unsafe.join}" + @escaped = "#{safe.join}#{hex.join}" end def test_route_generation_escapes_unsafe_path_characters @@ -309,7 +309,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase def test_specific_controller_action_failure rs.draw do - mount lambda {} => "/foo" + mount lambda { } => "/foo" end assert_raises(ActionController::UrlGenerationError) do @@ -674,7 +674,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal "/page/foo", url_for(rs, controller: "content", action: "show_page", id: "foo") assert_equal({ controller: "content", action: "show_page", id: "foo" }, rs.recognize_path("/page/foo")) - token = "\321\202\320\265\320\272\321\201\321\202".dup # 'text' in Russian + token = +"\321\202\320\265\320\272\321\201\321\202" # 'text' in Russian token.force_encoding(Encoding::BINARY) escaped_token = CGI.escape(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) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 7b1a52b277..c917cdf761 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -144,7 +144,7 @@ class SendFileTest < ActionController::TestCase get :test_send_file_headers_bang assert_equal "image/png", response.content_type - assert_equal 'disposition; filename="filename"', response.get_header("Content-Disposition") + assert_equal %(disposition; filename="filename"; filename*=UTF-8''filename), response.get_header("Content-Disposition") assert_equal "binary", response.get_header("Content-Transfer-Encoding") assert_equal "private", response.get_header("Cache-Control") end @@ -153,7 +153,7 @@ class SendFileTest < ActionController::TestCase def test_send_file_headers_with_disposition_as_a_symbol get :test_send_file_headers_with_disposition_as_a_symbol - assert_equal 'disposition; filename="filename"', response.get_header("Content-Disposition") + assert_equal %(disposition; filename="filename"; filename*=UTF-8''filename), response.get_header("Content-Disposition") end def test_send_file_headers_with_mime_lookup_with_symbol diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 734da3de9c..6fc70d6248 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 @@ -515,7 +542,7 @@ XML def test_params_passing_with_frozen_values assert_nothing_raised do get :test_params, params: { - frozen: "icy".freeze, frozens: ["icy".freeze].freeze, deepfreeze: { frozen: "icy".freeze }.freeze + frozen: -"icy", frozens: [-"icy"].freeze, deepfreeze: { frozen: -"icy" }.freeze } end parsed_params = ::JSON.parse(@response.body) diff --git a/actionpack/test/dispatch/content_disposition_test.rb b/actionpack/test/dispatch/content_disposition_test.rb new file mode 100644 index 0000000000..3f5959da6e --- /dev/null +++ b/actionpack/test/dispatch/content_disposition_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "abstract_unit" + +module ActionDispatch + class ContentDispositionTest < ActiveSupport::TestCase + test "encoding a Latin filename" do + disposition = Http::ContentDisposition.new(disposition: :inline, filename: "racecar.jpg") + + assert_equal %(filename="racecar.jpg"), disposition.ascii_filename + assert_equal "filename*=UTF-8''racecar.jpg", disposition.utf8_filename + assert_equal "inline; #{disposition.ascii_filename}; #{disposition.utf8_filename}", disposition.to_s + end + + test "encoding a Latin filename with accented characters" do + disposition = Http::ContentDisposition.new(disposition: :inline, filename: "råcëçâr.jpg") + + assert_equal %(filename="racecar.jpg"), disposition.ascii_filename + assert_equal "filename*=UTF-8''r%C3%A5c%C3%AB%C3%A7%C3%A2r.jpg", disposition.utf8_filename + assert_equal "inline; #{disposition.ascii_filename}; #{disposition.utf8_filename}", disposition.to_s + end + + test "encoding a non-Latin filename" do + disposition = Http::ContentDisposition.new(disposition: :inline, filename: "автомобиль.jpg") + + assert_equal %(filename="%3F%3F%3F%3F%3F%3F%3F%3F%3F%3F.jpg"), disposition.ascii_filename + assert_equal "filename*=UTF-8''%D0%B0%D0%B2%D1%82%D0%BE%D0%BC%D0%BE%D0%B1%D0%B8%D0%BB%D1%8C.jpg", disposition.utf8_filename + assert_equal "inline; #{disposition.ascii_filename}; #{disposition.utf8_filename}", disposition.to_s + end + + test "without filename" do + disposition = Http::ContentDisposition.new(disposition: :inline, filename: nil) + + assert_equal "inline", disposition.to_s + end + end +end diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 4f9a4ff2bd..c8c885f35c 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -260,12 +260,13 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT ROUTES.draw do scope module: "default_content_security_policy_integration_test" do get "/", to: "policy#index" + get "/redirect", to: redirect("/") end end POLICY = ActionDispatch::ContentSecurityPolicy.new do |p| - p.default_src :self - p.script_src :https + p.default_src -> { :self } + p.script_src -> { :https } end class PolicyConfigMiddleware @@ -295,14 +296,19 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT def test_adds_nonce_to_script_src_content_security_policy_only_once get "/" get "/" + assert_response :success + assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='" + end + + def test_redirect_works_with_dynamic_sources + get "/redirect" + assert_response :redirect 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" @@ -339,6 +345,11 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest p.script_src :self end + content_security_policy only: :style_src do |p| + p.default_src false + p.style_src :self + end + content_security_policy(false, only: :no_policy) content_security_policy_report_only only: :report_only @@ -363,6 +374,10 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest head :ok end + def style_src + head :ok + end + def no_policy head :ok end @@ -381,6 +396,7 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest get "/conditional", to: "policy#conditional" get "/report-only", to: "policy#report_only" get "/script-src", to: "policy#script_src" + get "/style-src", to: "policy#style_src" get "/no-policy", to: "policy#no_policy" end end @@ -441,6 +457,11 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest assert_policy "script-src 'self' 'nonce-iyhD0Yc0W+c='" end + def test_adds_nonce_to_style_src_content_security_policy + get "/style-src" + assert_policy "style-src 'self' 'nonce-iyhD0Yc0W+c='" + end + def test_generates_no_content_security_policy get "/no-policy" diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index aba778fad6..6637c2cae9 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -289,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 @@ -1274,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 @@ -1284,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 @@ -1300,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 045567ff83..37399cfd07 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -26,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) @@ -74,6 +86,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end when %r{/framework_raises} method_that_raises + when %r{/nested_exceptions} + raise_nested_exceptions else raise "puke!" end @@ -354,7 +368,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest }) assert_response 500 - assert_includes(body, CGI.escapeHTML(PP.pp(params, "".dup, 200))) + assert_includes(body, CGI.escapeHTML(PP.pp(params, +"", 200))) end test "sets the HTTP charset parameter" do @@ -440,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 @@ -454,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 @@ -467,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 @@ -497,13 +511,13 @@ 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 @@ -523,4 +537,39 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest 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 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/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/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index e9f7ad41dd..5f43e5a3c5 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -42,7 +42,7 @@ class MiddlewareStackTest < ActiveSupport::TestCase end test "use should push middleware class with block arguments onto the stack" do - proc = Proc.new {} + proc = Proc.new { } assert_difference "@stack.size" do @stack.use(BlockMiddleware, &proc) end diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 85ea04356a..7a7a201b11 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -13,7 +13,7 @@ module TestGenerationPrefix end def self.model_name - klass = "Post".dup + klass = +"Post" def klass.name; self end ActiveModel::Name.new(klass) diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 84a2d1f69e..9d1246b3a4 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -24,7 +24,7 @@ class BaseRequestTest < ActiveSupport::TestCase def stub_request(env = {}) ip_spoofing_check = env.key?(:ip_spoofing_check) ? env.delete(:ip_spoofing_check) : true @trusted_proxies ||= nil - ip_app = ActionDispatch::RemoteIp.new(Proc.new {}, ip_spoofing_check, @trusted_proxies) + ip_app = ActionDispatch::RemoteIp.new(Proc.new { }, ip_spoofing_check, @trusted_proxies) ActionDispatch::Http::URL.tld_length = env.delete(:tld_length) if env.key?(:tld_length) ip_app.call(env) @@ -1059,44 +1059,9 @@ class RequestParameters < BaseRequestTest end class RequestParameterFilter < BaseRequestTest - test "process parameter filter" do - test_hashes = [ - [{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'], - [{ "foo" => "bar" }, { "foo" => "[FILTERED]" }, %w'foo'], - [{ "foo" => "bar", "bar" => "foo" }, { "foo" => "[FILTERED]", "bar" => "foo" }, %w'foo baz'], - [{ "foo" => "bar", "baz" => "foo" }, { "foo" => "[FILTERED]", "baz" => "[FILTERED]" }, %w'foo baz'], - [{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => "[FILTERED]", "bar" => "foo" } }, %w'fo'], - [{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => "[FILTERED]" }, %w'f banana'], - [{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => "[FILTERED]", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'], - [{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => "[FILTERED]" }, "1"] }, [/foo/]]] - - test_hashes.each do |before_filter, after_filter, filter_words| - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - assert_equal after_filter, parameter_filter.filter(before_filter) - - filter_words << "blah" - filter_words << lambda { |key, value| - value.reverse! if key =~ /bargain/ - } - - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo" } } } - after_filter["barg"] = { :bargain => "niag", "blah" => "[FILTERED]", "bar" => { "bargain" => { "blah" => "[FILTERED]" } } } - - assert_equal after_filter, parameter_filter.filter(before_filter) - end - end - - test "parameter filter should maintain hash with indifferent access" do - test_hashes = [ - [{ "foo" => "bar" }.with_indifferent_access, ["blah"]], - [{ "foo" => "bar" }.with_indifferent_access, []] - ] - - test_hashes.each do |before_filter, filter_words| - parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words) - assert_instance_of ActiveSupport::HashWithIndifferentAccess, - parameter_filter.filter(before_filter) + test "parameter filter is deprecated" do + assert_deprecated do + ActionDispatch::Http::ParameterFilter.new(["blah"]) end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 6d87314e97..0f37d074af 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -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| diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb index 9150d5010b..fe1f1995d8 100644 --- a/actionpack/test/dispatch/routing/inspector_test.rb +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -368,19 +368,19 @@ module ActionDispatch assert_equal [ "No routes were found for this grep pattern.", - "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 def test_not_routes_when_expanded - output = draw(grep: "rails/dummy", formatter: ActionDispatch::Routing::ConsoleFormatter::Expanded.new) {} + 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: http://guides.rubyonrails.org/routing.html." + "For more information about routes, see the Rails guide: https://guides.rubyonrails.org/routing.html." ], output end @@ -434,7 +434,7 @@ module ActionDispatch assert_equal [ "No routes were found for this controller.", - "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 @@ -445,19 +445,19 @@ module ActionDispatch assert_equal [ "No routes were found for this grep pattern.", - "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 def test_no_routes_were_defined - output = draw(grep: "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 diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index dd6adcbfd1..4dffbd0db1 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -115,6 +115,21 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 301, status end + def test_accepts_a_constraint_object_responding_to_call + constraint = Class.new do + def call(*); true; end + def matches?(*); false; end + end + + draw do + get "/", to: "home#show", constraints: constraint.new + end + + assert_nothing_raised do + get "/" + end + end + def test_namespace_with_controller_segment assert_raise(ArgumentError) do draw do @@ -1367,6 +1382,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal "projects#index", @response.body end + def test_optionally_scoped_root_unscoped_access + draw do + scope "(:locale)" do + scope "(:platform)" do + scope "(:browser)" do + root to: "projects#index" + end + end + end + end + + assert_equal "/", root_path + get "/" + assert_equal "projects#index", @response.body + end + def test_scope_with_format_option draw do get "direct/index", as: :no_format_direct, format: false @@ -3153,7 +3184,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 @@ -3667,15 +3698,25 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - def test_multiple_roots + def test_multiple_roots_raises_error + ex = assert_raises(ArgumentError) { + draw do + root "pages#index", constraints: { host: "www.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" } + end + } + assert_match(/Invalid route name, already in use: 'root'/, ex.message) + end + + def test_multiple_named_roots draw do namespace :foo do root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end root "pages#index", constraints: { host: "www.example.com" } - root "admin/pages#index", constraints: { host: "admin.example.com" } + root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root end get "http://www.example.com/foo" diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index b69071b44b..f802abc653 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -36,30 +36,30 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "skip exceptions app if not showing exceptions" do @app = ProductionApp assert_raise RuntimeError do - get "/", headers: { "action_dispatch.show_exceptions" => false } + get "/", env: { "action_dispatch.show_exceptions" => false } end end test "rescue with error page" do @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_response 500 assert_equal "500 error fixture\n", body - get "/bad_params", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params", env: { "action_dispatch.show_exceptions" => true } assert_response 400 assert_equal "400 error fixture\n", body - get "/not_found", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "404 error fixture\n", body - get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true } + get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body - get "/unknown_http_method", headers: { "action_dispatch.show_exceptions" => true } + get "/unknown_http_method", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body end @@ -70,11 +70,11 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest begin @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_response 500 assert_equal "500 localized error fixture\n", body - get "/not_found", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "404 error fixture\n", body ensure @@ -85,14 +85,14 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "sets the HTTP charset parameter" do @app = ProductionApp - get "/", headers: { "action_dispatch.show_exceptions" => true } + get "/", env: { "action_dispatch.show_exceptions" => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end test "show registered original exception for wrapped exceptions" do @app = ProductionApp - get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_match(/404 error/, body) end @@ -106,7 +106,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true } + get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true } assert_response 404 assert_equal "YOU FAILED", body end @@ -117,7 +117,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest end @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app) - get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true } + get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true } assert_response 405 assert_equal "", body end @@ -125,12 +125,12 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest test "bad params exception is returned in the correct format" do @app = ProductionApp - get "/bad_params", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params", env: { "action_dispatch.show_exceptions" => true } assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] assert_response 400 assert_match(/400 error/, body) - get "/bad_params.json", headers: { "action_dispatch.show_exceptions" => true } + get "/bad_params.json", env: { "action_dispatch.show_exceptions" => true } assert_equal "application/json; charset=utf-8", response.headers["Content-Type"] assert_response 400 assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body) diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 6b69cd9999..d44aa00122 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -31,7 +31,7 @@ module StaticTests end def test_handles_urls_with_ascii_8bit - assert_equal "Hello, World!", get("/doorkeeper%E3E4".dup.force_encoding("ASCII-8BIT")).body + assert_equal "Hello, World!", get((+"/doorkeeper%E3E4").force_encoding("ASCII-8BIT")).body end def test_handles_urls_with_ascii_8bit_on_win_31j @@ -39,7 +39,7 @@ module StaticTests Encoding.default_internal = "Windows-31J" Encoding.default_external = "Windows-31J" end - assert_equal "Hello, World!", get("/doorkeeper%E3E4".dup.force_encoding("ASCII-8BIT")).body + assert_equal "Hello, World!", get((+"/doorkeeper%E3E4").force_encoding("ASCII-8BIT")).body end def test_handles_urls_with_null_byte diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 5a584b12e5..21169fcb5c 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -103,6 +103,12 @@ module ActionDispatch 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) diff --git a/actionpack/test/fixtures/alternate_helpers/foo_helper.rb b/actionpack/test/fixtures/alternate_helpers/foo_helper.rb index 3aadb6145e..c1a995af5f 100644 --- a/actionpack/test/fixtures/alternate_helpers/foo_helper.rb +++ b/actionpack/test/fixtures/alternate_helpers/foo_helper.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module FooHelper - redefine_method(:baz) {} + redefine_method(:baz) { } end diff --git a/actionpack/test/journey/route/definition/scanner_test.rb b/actionpack/test/journey/route/definition/scanner_test.rb index bcbe4388c3..092177d315 100644 --- a/actionpack/test/journey/route/definition/scanner_test.rb +++ b/actionpack/test/journey/route/definition/scanner_test.rb @@ -30,6 +30,12 @@ module ActionDispatch ["/~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, "("], diff --git a/actionpack/test/journey/router/utils_test.rb b/actionpack/test/journey/router/utils_test.rb index 2d09098f11..472f1bf35e 100644 --- a/actionpack/test/journey/router/utils_test.rb +++ b/actionpack/test/journey/router/utils_test.rb @@ -23,7 +23,7 @@ module ActionDispatch end def test_uri_unescape_with_utf8_string - assert_equal "Šašinková", Utils.unescape_uri("%C5%A0a%C5%A1inkov%C3%A1".dup.force_encoding(Encoding::US_ASCII)) + assert_equal "Šašinková", Utils.unescape_uri((+"%C5%A0a%C5%A1inkov%C3%A1").force_encoding(Encoding::US_ASCII)) end def test_normalize_path_not_greedy diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 183f421bcf..1f4e14aef6 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -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) |