diff options
Diffstat (limited to 'actionpack')
74 files changed, 1128 insertions, 332 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4f95a9bab9..93f75ba635 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,72 @@ +* Add `ActionController::Parameters#include?` + + *Justin Coyne* + +## Rails 5.0.0.beta1 (December 18, 2015) ## + +* Deprecate `redirect_to :back` in favor of `redirect_back`, which accepts a + required `fallback_location` argument, thus eliminating the possibility of a + `RedirectBackError`. + + *Derek Prior* + +* Add `redirect_back` method to `ActionController::Redirecting` to provide a + way to safely redirect to the `HTTP_REFERER` if it is present, falling back + to a provided redirect otherwise. + + *Derek Prior* + +* `ActionController::TestCase` will be moved to it's own gem in Rails 5.1 + + With the speed improvements made to `ActionDispatch::IntegrationTest` we no + longer need to keep two separate code bases for testing controllers. In + Rails 5.1 `ActionController::TestCase` will be deprecated and moved into a + gem outside of Rails source. + + This is a documentation deprecation so that going forward so new tests will use + `ActionDispatch::IntegrationTest` instead of `ActionController::TestCase`. + + *Eileen M. Uchitelle* + +* Add a `response_format` option to `ActionDispatch::DebugExceptions` + to configure the format of the response when errors occur in + development mode. + + If `response_format` is `:default` the debug info will be rendered + in an HTML page. In the other hand, if the provided value is `:api` + the debug info will be rendered in the original response format. + + *Jorge Bejar* + +* Change the `protect_from_forgery` prepend default to `false`. + + Per this comment + https://github.com/rails/rails/pull/18334#issuecomment-69234050 we want + `protect_from_forgery` to default to `prepend: false`. + + `protect_from_forgery` will now be insterted into the callback chain at the + point it is called in your application. This is useful for cases where you + want to `protect_from_forgery` after you perform required authentication + callbacks or other callbacks that are required to run after forgery protection. + + If you want `protect_from_forgery` callbacks to always run first, regardless of + position they are called in your application then you can add `prepend: true` + to your `protect_from_forgery` call. + + Example: + + ```ruby + protect_from_forgery prepend: true + ``` + + *Eileen M. Uchitelle* + +* In url_for, never append a question mark to the URL when the query string + is empty anyway. (It used to do that when called like `url_for(controller: + 'x', action: 'y', q: {})`.) + + *Paul Grayson* + * Catch invalid UTF-8 querystring values and respond with BadRequest Check querystring params for invalid UTF-8 characters, and raise an @@ -17,31 +86,29 @@ *Agis Anastasopoulos* -* Deprecate `config.static_cache_control` in favor of - `config.public_file_server.headers` - - *Yuki Nishijima* - -* Add the ability of returning arbitrary headers to ActionDispatch::Static +* Add the ability of returning arbitrary headers to `ActionDispatch::Static`. Now ActionDispatch::Static can accept HTTP headers so that developers will have control of returning arbitrary headers like 'Access-Control-Allow-Origin' when a response is delivered. They can be configured with `#config`: - config.public_file_server.headers = { - "Cache-Control" => "public, max-age=60", - "Access-Control-Allow-Origin" => "http://rubyonrails.org" - } + Example: + + config.public_file_server.headers = { + "Cache-Control" => "public, max-age=60", + "Access-Control-Allow-Origin" => "http://rubyonrails.org" + } *Yuki Nishijima* * Allow multiple `root` routes in same scope level. Example: - ```ruby - root 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } - root 'landing#show' - ``` + Example: + + root 'blog#show', constraints: ->(req) { Hostname.blog_site?(req.host) } + root 'landing#show' + *Rafael Sales* * Fix regression in mounted engine named routes generation for app deployed to @@ -52,12 +119,12 @@ *Matthew Erhard* -* ActionDispatch::Response#new no longer applies default headers. If you want +* `ActionDispatch::Response#new` no longer applies default headers. If you want default headers applied to the response object, then call - `ActionDispatch::Response.create`. This change only impacts people who are + `ActionDispatch::Response.create`. This change only impacts people who are directly constructing an `ActionDispatch::Response` object. -* Accessing mime types via constants like `Mime::HTML` is deprecated. Please +* Accessing mime types via constants like `Mime::HTML` is deprecated. Please change code like this: Mime::HTML @@ -110,7 +177,7 @@ *Jeremy Friesen* -* Using strings or symbols for middleware class names is deprecated. Convert +* Using strings or symbols for middleware class names is deprecated. Convert things like this: middleware.use "Foo::Bar" @@ -119,10 +186,10 @@ middleware.use Foo::Bar -* ActionController::TestSession now accepts a default value as well as +* `ActionController::TestSession` now accepts a default value as well as a block for generating a default value based off the key provided. - This fixes calls to session#fetch in ApplicationController instances that + This fixes calls to `session#fetch` in `ApplicationController` instances that take more two arguments or a block from raising `ArgumentError: wrong number of arguments (2 for 1)` when performing controller tests. @@ -173,10 +240,10 @@ *Grey Baker* * Add support for API only apps. - ActionController::API is added as a replacement of - ActionController::Base for this kind of applications. + `ActionController::API` is added as a replacement of + `ActionController::Base` for this kind of applications. - *Santiago Pastorino & Jorge Bejar* + *Santiago Pastorino*, *Jorge Bejar* * Remove `assigns` and `assert_template`. Both methods have been extracted into a gem at https://github.com/rails/rails-controller-testing. @@ -259,7 +326,7 @@ *Peter Schröder* -* Drop request class from RouteSet constructor. +* Drop request class from `RouteSet` constructor. If you would like to use a custom request class, please subclass and implement the `request_class` method. @@ -288,7 +355,7 @@ *Jeremy Kemper*, *Yves Senn* -* Deprecate AbstractController#skip_action_callback in favor of individual skip_callback methods +* Deprecate `AbstractController#skip_action_callback` in favor of individual skip_callback methods (which can be made to raise an error if no callback was removed). *Iain Beeston* @@ -494,9 +561,7 @@ Fixes an issue where when an exception is raised in the request the additional payload data is not available. - See: - * #14903 - * https://github.com/roidrage/lograge/issues/37 + See #14903. *Dieter Komendera*, *Margus Pärt* diff --git a/actionpack/README.rdoc b/actionpack/README.rdoc index 44c980b070..0720c66cb9 100644 --- a/actionpack/README.rdoc +++ b/actionpack/README.rdoc @@ -28,7 +28,7 @@ can be used outside of Rails. The latest version of Action Pack can be installed with RubyGems: - % gem install actionpack + $ gem install actionpack Source code can be downloaded as part of the Rails project on GitHub @@ -55,4 +55,3 @@ Bug reports can be filed for the Ruby on Rails project here: Feature requests should be discussed on the rails-core mailing list here: * https://groups.google.com/forum/?fromgroups#!forum/rubyonrails-core - diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 4501202b8c..8edea0f52b 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -49,7 +49,7 @@ module AbstractController # instance methods on that abstract class. Public instance methods of # a controller would normally be considered action methods, so methods # declared on abstract classes are being removed. - # (ActionController::Metal and ActionController::Base are defined as abstract) + # (<tt>ActionController::Metal</tt> and ActionController::Base are defined as abstract) def internal_methods controller = self @@ -80,7 +80,7 @@ module AbstractController # action_methods are cached and there is sometimes need to refresh # them. ::clear_action_methods! allows you to do that, so next time - # you run action_methods, they will be recalculated + # you run action_methods, they will be recalculated. def clear_action_methods! @action_methods = nil end @@ -149,7 +149,7 @@ module AbstractController # ==== Parameters # * <tt>action_name</tt> - The name of an action to be tested def available_action?(action_name) - _find_action_name(action_name).present? + _find_action_name(action_name) end # Returns true if the given controller is capable of rendering diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 287550db42..d63ce9c1c3 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -39,8 +39,8 @@ module AbstractController # except: :index, if: -> { true } # the :except option will be ignored. # # ==== Options - # * <tt>only</tt> - The callback should be run only for this action - # * <tt>except</tt> - The callback should be run for all actions except this action + # * <tt>only</tt> - The callback should be run only for this action. + # * <tt>except</tt> - The callback should be run for all actions except this action. def _normalize_callback_options(options) _normalize_callback_option(options, :only, :if) _normalize_callback_option(options, :except, :unless) @@ -48,7 +48,8 @@ module AbstractController def _normalize_callback_option(options, from, to) # :nodoc: if from = options[from] - from = Array(from).map {|o| "action_name == '#{o}'"}.join(" || ") + _from = Array(from).map(&:to_s).to_set + from = proc {|c| _from.include? c.action_name } options[to] = Array(options[to]).unshift(from) end end @@ -59,7 +60,7 @@ module AbstractController # * <tt>names</tt> - A list of valid names that could be used for # callbacks. Note that skipping uses Ruby equality, so it's # impossible to skip a callback defined using an anonymous proc - # using #skip_action_callback + # using #skip_action_callback. def skip_action_callback(*names) ActiveSupport::Deprecation.warn('`skip_action_callback` is deprecated and will be removed in Rails 5.1. Please use skip_before_action, skip_after_action or skip_around_action instead.') skip_before_action(*names, raise: false) @@ -82,8 +83,8 @@ module AbstractController # * <tt>block</tt> - A proc that should be added to the callbacks. # # ==== Block Parameters - # * <tt>name</tt> - The callback to be added - # * <tt>options</tt> - A hash of options to be used when adding the callback + # * <tt>name</tt> - The callback to be added. + # * <tt>options</tt> - A hash of options to be used when adding the callback. def _insert_callbacks(callbacks, block = nil) options = callbacks.extract_options! _normalize_callback_options(options) diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 2694d4c12f..b9ad51a9cf 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -14,12 +14,57 @@ module ActionController # # expire_fragment('name_of_cache') module Fragments + extend ActiveSupport::Concern + + included do + if respond_to?(:class_attribute) + class_attribute :fragment_cache_keys + else + mattr_writer :fragment_cache_keys + end + + self.fragment_cache_keys = [] + + helper_method :fragment_cache_key if respond_to?(:helper_method) + end + + module ClassMethods + # Allows you to specify controller-wide key prefixes for + # cache fragments. Pass either a constant +value+, or a block + # which computes a value each time a cache key is generated. + # + # For example, you may want to prefix all fragment cache keys + # with a global version identifier, so you can easily + # invalidate all caches. + # + # class ApplicationController + # fragment_cache_key "v1" + # end + # + # When it's time to invalidate all fragments, simply change + # the string constant. Or, progressively roll out the cache + # invalidation using a computed value: + # + # class ApplicationController + # fragment_cache_key do + # @account.id.odd? ? "v1" : "v2" + # end + # end + def fragment_cache_key(value = nil, &key) + self.fragment_cache_keys += [key || ->{ value }] + end + end + # Given a key (as described in +expire_fragment+), returns # a key suitable for use in reading, writing, or expiring a - # cached fragment. All keys are prefixed with <tt>views/</tt> and uses - # ActiveSupport::Cache.expand_cache_key for the expansion. + # cached fragment. All keys begin with <tt>views/</tt>, + # followed by any controller-wide key prefix values, ending + # with the specified +key+ value. The key is expanded using + # ActiveSupport::Cache.expand_cache_key. def fragment_cache_key(key) - ActiveSupport::Cache.expand_cache_key(key.is_a?(Hash) ? url_for(key).split("://").last : key, :views) + head = self.class.fragment_cache_keys.map { |k| instance_exec(&k) } + tail = key.is_a?(Hash) ? url_for(key).split("://").last : key + ActiveSupport::Cache.expand_cache_key([*head, *tail], :views) end # Writes +content+ to the location signified by diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 8e040bb465..f6a93a8940 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -166,7 +166,7 @@ module ActionController alias :response_code :status # :nodoc: - # Basic url_for that can be overridden for more robust functionality + # Basic url_for that can be overridden for more robust functionality. def url_for(string) string end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 89d589c486..f8e0d9cf6c 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -66,7 +66,7 @@ module ActionController # # You can also pass an object that responds to +maximum+, such as a # collection of active records. In this case +last_modified+ will be set by - # calling +maximum(:updated_at)+ on the collection (the timestamp of the + # calling <tt>maximum(:updated_at)</tt> on the collection (the timestamp of the # most recently updated record) and the +etag+ by passing the object itself. # # def index @@ -228,7 +228,7 @@ module ActionController expires_in 100.years, public: public yield if stale?(etag: "#{version}-#{request.fullpath}", - last_modified: Time.parse('2011-01-01').utc, + last_modified: Time.new(2011, 1, 1).utc, public: public) end diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index 5260dc0336..5c0ada37be 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -3,12 +3,19 @@ module ActionController end class BadRequest < ActionControllerError #:nodoc: - attr_reader :original_exception - def initialize(msg = nil, e = nil) + if e + ActiveSupport::Deprecation.warn("Passing #original_exception is deprecated and has no effect. " \ + "Exceptions will automatically capture the original exception.", caller) + end + super(msg) - @original_exception = e - set_backtrace e.backtrace if e + set_backtrace $!.backtrace if $! + end + + def original_exception + ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller) + cause end end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 0a36fecd27..2ac6e37e34 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -397,7 +397,7 @@ module ActionController # RewriteRule ^(.*)$ dispatch.fcgi [E=X-HTTP_AUTHORIZATION:%{HTTP:Authorization},QSA,L] module Token TOKEN_KEY = 'token=' - TOKEN_REGEX = /^(Token|Bearer) / + TOKEN_REGEX = /^(Token|Bearer)\s+/ AUTHN_PAIR_DELIMITERS = /(?:,|;|\t+)/ extend self diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 7db8d13e24..e3c540bf5f 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -37,7 +37,7 @@ module ActionController module ClassMethods def make_response!(request) - if request.env["HTTP_VERSION"] == "HTTP/1.0" + if request.get_header("HTTP_VERSION") == "HTTP/1.0" super else Live::Response.new.tap do |res| @@ -222,12 +222,6 @@ module ActionController jar.write self unless committed? end - def before_sending - super - request.cookie_jar.commit! - headers.freeze - end - def build_buffer(response, body) buf = Live::Buffer.new response body.each { |part| buf.write part } @@ -292,9 +286,5 @@ module ActionController super response.close if response end - - def set_response!(request) - @_response = self.class.make_response! request - end end end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 0febc905f1..b13ba06962 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -20,8 +20,6 @@ module ActionController # * <tt>String</tt> starting with <tt>protocol://</tt> (like <tt>http://</tt>) or a protocol relative reference (like <tt>//</tt>) - Is passed straight through as the target for redirection. # * <tt>String</tt> not containing a protocol - The current protocol and host is prepended to the string. # * <tt>Proc</tt> - A block that will be executed in the controller's context. Should return any option accepted by +redirect_to+. - # * <tt>:back</tt> - Back to the page that issued the request. Useful for forms that are triggered from multiple places. - # Short-hand for <tt>redirect_to(request.env["HTTP_REFERER"])</tt> # # === Examples: # @@ -30,7 +28,6 @@ module ActionController # redirect_to "http://www.rubyonrails.org" # redirect_to "/images/screenshot.jpg" # redirect_to articles_url - # redirect_to :back # redirect_to proc { edit_post_url(@post) } # # The redirection happens as a "302 Found" header unless otherwise specified using the <tt>:status</tt> option: @@ -61,13 +58,8 @@ module ActionController # redirect_to post_url(@post), status: 301, flash: { updated_post_id: @post.id } # redirect_to({ action: 'atom' }, alert: "Something serious happened") # - # When using <tt>redirect_to :back</tt>, if there is no referrer, - # <tt>ActionController::RedirectBackError</tt> will be raised. You - # may specify some fallback behavior for this case by rescuing - # <tt>ActionController::RedirectBackError</tt>. def redirect_to(options = {}, response_status = {}) #:doc: raise ActionControllerError.new("Cannot redirect to nil!") unless options - raise ActionControllerError.new("Cannot redirect to a parameter hash!") if options.is_a?(ActionController::Parameters) raise AbstractController::DoubleRenderError if response_body self.status = _extract_redirect_to_status(options, response_status) @@ -75,6 +67,32 @@ module ActionController self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" end + # Redirects the browser to the page that issued the request (the referrer) + # if possible, otherwise redirects to the provided default fallback + # location. + # + # The referrer information is pulled from the HTTP `Referer` (sic) header on + # the request. This is an optional header and its presence on the request is + # subject to browser security settings and user preferences. If the request + # is missing this header, the <tt>fallback_location</tt> will be used. + # + # redirect_back fallback_location: { action: "show", id: 5 } + # redirect_back fallback_location: post + # redirect_back fallback_location: "http://www.rubyonrails.org" + # redirect_back fallback_location: "/images/screenshot.jpg" + # redirect_back fallback_location: articles_url + # redirect_back fallback_location: proc { edit_post_url(@post) } + # + # All options that can be passed to <tt>redirect_to</tt> are accepted as + # options and the behavior is indetical. + def redirect_back(fallback_location:, **args) + if referer = request.headers["Referer"] + redirect_to referer, **args + else + redirect_to fallback_location, **args + end + end + def _compute_redirect_to_location(request, options) #:nodoc: case options # The scheme name consist of a letter followed by any combination of @@ -87,6 +105,12 @@ module ActionController when String request.protocol + request.host_with_port + options when :back + ActiveSupport::Deprecation.warn(<<-MESSAGE.squish) + `redirect_to :back` is deprecated and will be removed from Rails 5.1. + Please use `redirect_back(fallback_location: fallback_location)` where + `fallback_location` represents the location to use if the request has + no HTTP referer information. + MESSAGE request.headers["Referer"] or raise RedirectBackError when Proc _compute_redirect_to_location request, options.call diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 64f6f7cf51..26c4550f89 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -77,6 +77,10 @@ module ActionController #:nodoc: config_accessor :log_warning_on_csrf_failure self.log_warning_on_csrf_failure = true + # Controls whether the Origin header is checked in addition to the CSRF token. + config_accessor :forgery_protection_origin_check + self.forgery_protection_origin_check = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -98,13 +102,13 @@ module ActionController #:nodoc: # # Valid Options: # - # * <tt>:only/:except</tt> - Only apply forgery protection to a subset of actions. Like <tt>only: [ :create, :create_all ]</tt>. + # * <tt>:only/:except</tt> - Only apply forgery protection to a subset of actions. For example <tt>only: [ :create, :create_all ]</tt>. # * <tt>:if/:unless</tt> - Turn off the forgery protection entirely depending on the passed Proc or method reference. - # * <tt>:prepend</tt> - By default, the verification of the authentication token is added to the front of the - # callback chain. If you need to make the verification depend on other callbacks, like authentication methods - # (say cookies vs OAuth), this might not work for you. Pass <tt>prepend: false</tt> to just add the - # verification callback in the position of the protect_from_forgery call. This means any callbacks added - # before are run first. + # * <tt>:prepend</tt> - By default, the verification of the authentication token will be added at the position of the + # protect_from_forgery call in your application. This means any callbacks added before are run first. This is useful + # when you want your forgery protection to depend on other callbacks, like authentication methods (Oauth vs Cookie auth). + # + # If you need to add verification to the beginning of the callback chain, use <tt>prepend: true</tt>. # * <tt>:with</tt> - Set the method to handle unverified request. # # Valid unverified request handling methods are: @@ -112,7 +116,7 @@ module ActionController #:nodoc: # * <tt>:reset_session</tt> - Resets the session. # * <tt>:null_session</tt> - Provides an empty session during request but doesn't reset it completely. Used as default if <tt>:with</tt> option is not specified. def protect_from_forgery(options = {}) - options = options.reverse_merge(prepend: true) + options = options.reverse_merge(prepend: false) self.forgery_protection_strategy = protection_method_class(options[:with] || :null_session) self.request_forgery_protection_token ||= :authenticity_token @@ -257,8 +261,19 @@ module ActionController #:nodoc: # * Does the X-CSRF-Token header match the form_authenticity_token def verified_request? !protect_against_forgery? || request.get? || request.head? || - valid_authenticity_token?(session, form_authenticity_param) || - valid_authenticity_token?(session, request.headers['X-CSRF-Token']) + (valid_request_origin? && any_authenticity_token_valid?) + end + + # Checks if any of the authenticity tokens from the request are valid. + def any_authenticity_token_valid? + request_authenticity_tokens.any? do |token| + valid_authenticity_token?(session, token) + end + end + + # Possible authenticity tokens sent in the request. + def request_authenticity_tokens + [form_authenticity_param, request.x_csrf_token] end # Sets the token value for the current session. @@ -336,5 +351,16 @@ module ActionController #:nodoc: def protect_against_forgery? allow_forgery_protection end + + # Checks if the request originated from the same origin by looking at the + # Origin header. + def valid_request_origin? + if forgery_protection_origin_check + # We accept blank origin headers because some user agents don't send it. + request.origin.nil? || request.origin == request.base_url + else + true + end + end end end diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 68cc9a9c9b..81b9a7b9ed 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -7,10 +7,8 @@ module ActionController #:nodoc: include ActiveSupport::Rescuable def rescue_with_handler(exception) - if (exception.respond_to?(:original_exception) && - (orig_exception = exception.original_exception) && - handler_for_rescue(orig_exception)) - exception = orig_exception + if exception.cause && handler_for_rescue(exception.cause) + exception = exception.cause end super(exception) end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 130ba61786..7e2745c75a 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -1,8 +1,10 @@ require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/hash/transform_values' require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/string/filters' require 'active_support/rescuable' require 'action_dispatch/http/upload' +require 'rack/test' require 'stringio' require 'set' @@ -107,7 +109,7 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :empty?, :inspect, to: :@parameters + delegate :keys, :key?, :has_key?, :empty?, :include?, :inspect, to: :@parameters # By default, never raise an UnpermittedParameters exception if these # params are present. The default includes both 'controller' and 'action' @@ -161,8 +163,8 @@ module ActionController end end - # Returns a safe +Hash+ representation of this parameter with all - # unpermitted keys removed. + # Returns a safe <tt>ActiveSupport::HashWithIndifferentAccess</tt> + # representation of this parameter with all unpermitted keys removed. # # params = ActionController::Parameters.new({ # name: 'Senjougahara Hitagi', @@ -174,15 +176,17 @@ module ActionController # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} def to_h if permitted? - @parameters.to_h + convert_parameters_to_hashes(@parameters) else slice(*self.class.always_permitted_parameters).permit!.to_h end end - # Returns an unsafe, unfiltered +Hash+ representation of this parameter. + # Returns an unsafe, unfiltered + # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this + # parameter. def to_unsafe_h - @parameters.to_h + convert_parameters_to_hashes(@parameters) end alias_method :to_unsafe_hash, :to_unsafe_h @@ -591,6 +595,21 @@ module ActionController end end + def convert_parameters_to_hashes(value) + case value + when Array + value.map { |v| convert_parameters_to_hashes(v) } + when Hash + value.transform_values do |v| + convert_parameters_to_hashes(v) + end.with_indifferent_access + when Parameters + value.to_h + else + value + end + end + def convert_hashes_to_parameters(key, value) converted = convert_value_to_parameters(value) @parameters[key] = converted unless converted.equal?(value) diff --git a/actionpack/lib/action_controller/metal/testing.rb b/actionpack/lib/action_controller/metal/testing.rb index b2b3b4283f..ac37b00010 100644 --- a/actionpack/lib/action_controller/metal/testing.rb +++ b/actionpack/lib/action_controller/metal/testing.rb @@ -4,9 +4,6 @@ module ActionController # Behavior specific to functional tests module Functional # :nodoc: - def set_response!(request) - end - def recycle! @_url_options = nil self.formats = nil diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 2cada1f68a..c55720859e 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -7,6 +7,9 @@ require 'action_controller/template_assertions' require 'rails-dom-testing' module ActionController + # :stopdoc: + # ActionController::TestCase will be deprecated and moved to a gem in Rails 5.1. + # Please use ActionDispatch::IntegrationTest going forward. class TestRequest < ActionDispatch::TestRequest #:nodoc: DEFAULT_ENV = ActionDispatch::TestRequest::DEFAULT_ENV.dup DEFAULT_ENV.delete 'PATH_INFO' @@ -210,11 +213,11 @@ module ActionController # # Simulate a POST response with the given HTTP parameters. # post(:create, params: { book: { title: "Love Hina" }}) # - # # Assert that the controller tried to redirect us to + # # Asserts that the controller tried to redirect us to # # the created book's URI. # assert_response :found # - # # Assert that the controller really put the book in the database. + # # Asserts that the controller really put the book in the database. # assert_not_nil Book.find_by(title: "Love Hina") # end # end @@ -509,15 +512,14 @@ module ActionController end end - @controller.request = @request - @controller.response = @response - @request.fetch_header("SCRIPT_NAME") do |k| @request.set_header k, @controller.config.relative_url_root end @controller.recycle! - @controller.process(action) + @controller.dispatch(action, @request, @response) + @request = @controller.request + @response = @controller.response @request.delete_header 'HTTP_COOKIE' @@ -659,4 +661,5 @@ module ActionController include Behavior end + # :startdoc: end diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 7acf91902d..e9b25339dc 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -69,6 +69,8 @@ module ActionDispatch Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts + elsif extension_format = format_from_path_extension + [extension_format] elsif xhr? [Mime[:js]] else @@ -160,6 +162,13 @@ module ActionDispatch def use_accept_header !self.class.ignore_accept_header end + + def format_from_path_extension + path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] + if match = path && path.match(/\.(\w+)\z/) + Mime[match.captures.first] + end + end end end end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index b64f660ec5..b8d395854c 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -47,15 +47,10 @@ module Mime def const_missing(sym) ext = sym.downcase if Mime[ext] - ActiveSupport::Deprecation.warn <<-eow -Accessing mime types via constants is deprecated. Please change: - - `Mime::#{sym}` - -to: - - `Mime[:#{ext}]` - eow + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Accessing mime types via constants is deprecated. + Please change `Mime::#{sym}` to `Mime[:#{ext}]`. + MSG Mime[ext] else super @@ -65,15 +60,10 @@ to: def const_defined?(sym, inherit = true) ext = sym.downcase if Mime[ext] - ActiveSupport::Deprecation.warn <<-eow -Accessing mime types via constants is deprecated. Please change: - - `Mime.const_defined?(#{sym})` - -to: - - `Mime[:#{ext}]` - eow + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Accessing mime types via constants is deprecated. + Please change `Mime.const_defined?(#{sym})` to `Mime[:#{ext}]`. + MSG true else super diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 248ecfd676..cca7376ffa 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -43,7 +43,7 @@ module ActionDispatch # # {'action' => 'my_action', 'controller' => 'my_controller'} def path_parameters - get_header(PARAMETERS_KEY) || {} + get_header(PARAMETERS_KEY) || set_header(PARAMETERS_KEY, {}) end private @@ -55,11 +55,11 @@ module ActionDispatch begin strategy.call(raw_post) - rescue => e # JSON or Ruby code block errors + 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}" - raise ParamsParser::ParseError.new(e.message, e) + raise ParamsParser::ParseError end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 35e3ac304f..29cf821090 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -36,8 +36,8 @@ module ActionDispatch HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_NEGOTIATE HTTP_PRAGMA HTTP_CLIENT_IP - HTTP_X_FORWARDED_FOR HTTP_VERSION - HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST + HTTP_X_FORWARDED_FOR HTTP_ORIGIN HTTP_VERSION + HTTP_X_CSRF_TOKEN HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST SERVER_ADDR ].freeze @@ -49,6 +49,10 @@ module ActionDispatch METHOD end + def self.empty + new({}) + end + def initialize(env) super @method = nil @@ -59,6 +63,9 @@ module ActionDispatch @ip = nil end + def commit_cookie_jar! # :nodoc: + end + def check_path_parameters! # If any of the path parameters has an invalid encoding then # raise since it's likely to trigger errors further on. @@ -306,10 +313,16 @@ module ActionDispatch end end - # Returns true if the request's content MIME type is - # +application/x-www-form-urlencoded+ or +multipart/form-data+. + # Determine whether the request body contains form-data by checking + # the request Content-Type for one of the media-types: + # "application/x-www-form-urlencoded" or "multipart/form-data". The + # list of form-data media types can be modified through the + # +FORM_DATA_MEDIA_TYPES+ array. + # + # A request body is not assumed to contain form-data when no + # Content-Type header is provided and the request_method is POST. def form_data? - FORM_DATA_MEDIA_TYPES.include?(content_mime_type.to_s) + FORM_DATA_MEDIA_TYPES.include?(media_type) end def body_stream #:nodoc: @@ -344,7 +357,7 @@ module ActionDispatch set_header k, Request::Utils.normalize_encode_params(rack_query_params) end rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e - raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}", e) + raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}") end alias :query_parameters :GET @@ -360,7 +373,7 @@ module ActionDispatch 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}", e) + raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}") end alias :request_parameters :POST diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index f0127aa276..9b11111a67 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -412,6 +412,8 @@ module ActionDispatch # :nodoc: end def before_sending + headers.freeze + request.commit_cookie_jar! unless committed? end def build_buffer(response, body) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 92b10b6d3b..37f41ae988 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -81,7 +81,8 @@ module ActionDispatch def add_params(path, params) params = { params: params } unless params.is_a?(Hash) params.reject! { |_,v| v.to_param.nil? } - path << "?#{params.to_query}" unless params.empty? + query = params.to_query + path << "?#{query}" unless query.empty? end def add_anchor(path, anchor) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 2889acaeb8..3477aa8b29 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -12,6 +12,12 @@ module ActionDispatch end # :stopdoc: + prepend Module.new { + def commit_cookie_jar! + cookie_jar.commit! + end + } + def have_cookie_jar? has_header? 'action_dispatch.cookies'.freeze end @@ -77,6 +83,12 @@ module ActionDispatch # # It can be read using the signed method `cookies.signed[:name]` # cookies.signed[:user_id] = current_user.id # + # # Sets an encrypted cookie value before sending it to the client which + # # prevent users from reading and tampering with its value. + # # The cookie is signed by your app's `secrets.secret_key_base` value. + # # It can be read using the encrypted method `cookies.encrypted[:name]` + # cookies.encrypted[:discount] = 45 + # # # Sets a "permanent" cookie (which expires in 20 years from now). # cookies.permanent[:login] = "XJ-122" # @@ -89,6 +101,7 @@ module ActionDispatch # cookies.size # => 2 # JSON.parse(cookies[:lat_lon]) # => [47.68, -122.37] # cookies.signed[:login] # => "XJ-122" + # cookies.encrypted[:discount] # => 45 # # Example for deleting: # diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 66bb74b9c5..b55c937e0c 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -38,9 +38,10 @@ module ActionDispatch end end - def initialize(app, routes_app = nil) - @app = app - @routes_app = routes_app + def initialize(app, routes_app = nil, response_format = :default) + @app = app + @routes_app = routes_app + @response_format = response_format end def call(env) @@ -66,41 +67,79 @@ module ActionDispatch log_error(request, wrapper) if request.get_header('action_dispatch.show_detailed_exceptions') - traces = wrapper.traces - - trace_to_show = 'Application Trace' - if traces[trace_to_show].empty? && wrapper.rescue_template != 'routing_error' - trace_to_show = 'Full Trace' + case @response_format + when :api + render_for_api_application(request, wrapper) + when :default + render_for_default_application(request, wrapper) end + else + raise exception + end + end - if source_to_show = traces[trace_to_show].first - source_to_show_id = source_to_show[:id] - end + def render_for_default_application(request, wrapper) + template = create_template(request, wrapper) + file = "rescues/#{wrapper.rescue_template}" - template = DebugView.new([RESCUES_TEMPLATE_PATH], - request: request, - exception: wrapper.exception, - traces: traces, - show_source_idx: source_to_show_id, - trace_to_show: trace_to_show, - routes_inspector: routes_inspector(exception), - source_extracts: wrapper.source_extracts, - line_number: wrapper.line_number, - file: wrapper.file - ) - file = "rescues/#{wrapper.rescue_template}" - - if request.xhr? - body = template.render(template: file, layout: false, formats: [:text]) - format = "text/plain" - else - body = template.render(template: file, layout: 'rescues/layout') - format = "text/html" - end - render(wrapper.status_code, body, format) + if request.xhr? + body = template.render(template: file, layout: false, formats: [:text]) + format = "text/plain" else - raise exception + body = template.render(template: file, layout: 'rescues/layout') + format = "text/html" end + render(wrapper.status_code, body, format) + end + + def render_for_api_application(request, wrapper) + body = { + status: wrapper.status_code, + error: Rack::Utils::HTTP_STATUS_CODES.fetch( + wrapper.status_code, + Rack::Utils::HTTP_STATUS_CODES[500] + ), + exception: wrapper.exception.inspect, + traces: wrapper.traces + } + + content_type = request.formats.first + to_format = "to_#{content_type.to_sym}" + + if content_type && body.respond_to?(to_format) + formatted_body = body.public_send(to_format) + format = content_type + else + formatted_body = body.to_json + format = Mime[:json] + end + + render(wrapper.status_code, formatted_body, format) + 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.exception, + traces: traces, + show_source_idx: source_to_show_id, + trace_to_show: trace_to_show, + routes_inspector: routes_inspector(wrapper.exception), + source_extracts: wrapper.source_extracts, + line_number: wrapper.line_number, + file: wrapper.file + ) end def render(status, body, format) diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 5fd984cd07..3b61824cc9 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -37,7 +37,7 @@ module ActionDispatch @backtrace_cleaner = backtrace_cleaner @exception = original_exception(exception) - expand_backtrace if exception.is_a?(SyntaxError) || exception.try(:original_exception).try(:is_a?, SyntaxError) + expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError) end def rescue_template @@ -106,17 +106,13 @@ module ActionDispatch end def original_exception(exception) - if registered_original_exception?(exception) - exception.original_exception + if @@rescue_responses.has_key?(exception.cause.class.name) + exception.cause else exception end end - def registered_original_exception?(exception) - exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name) - end - def clean_backtrace(*args) if backtrace_cleaner backtrace_cleaner.clean(backtrace, *args) diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 18af0a583a..c2a4f46e67 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -10,11 +10,25 @@ module ActionDispatch # Raised when raw data from the request cannot be parsed by the parser # defined for request's content mime type. class ParseError < StandardError - attr_reader :original_exception - def initialize(message, original_exception) - super(message) - @original_exception = original_exception + def initialize(message = nil, original_exception = nil) + if message + ActiveSupport::Deprecation.warn("Passing #message is deprecated and has no effect. " \ + "#{self.class} will automatically capture the message " \ + "of the original exception.", caller) + end + + if original_exception + ActiveSupport::Deprecation.warn("Passing #original_exception is deprecated and has no effect. " \ + "Exceptions will automatically capture the original exception.", caller) + end + + super($!.message) + end + + def original_exception + ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller) + cause end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index aee2334da9..31b75498b6 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -43,7 +43,7 @@ module ActionDispatch # Create a new +RemoteIp+ middleware instance. # - # The +check_ip_spoofing+ option is on by default. When on, an exception + # The +ip_spoofing_check+ option is on by default. When on, an exception # is raised if it looks like the client is trying to lie about its own IP # address. It makes sense to turn off this check on sites aimed at non-IP # clients (like WAP devices), or behind proxies that set headers in an @@ -57,9 +57,9 @@ module ActionDispatch # with your proxy servers after it. If your proxies aren't removed, pass # them in via the +custom_proxies+ parameter. That way, the middleware will # ignore those IP addresses, and return the one that you want. - def initialize(app, check_ip_spoofing = true, custom_proxies = nil) + def initialize(app, ip_spoofing_check = true, custom_proxies = nil) @app = app - @check_ip = check_ip_spoofing + @check_ip = ip_spoofing_check @proxies = if custom_proxies.blank? TRUSTED_PROXIES elsif custom_proxies.respond_to?(:any?) @@ -116,10 +116,18 @@ module ActionDispatch forwarded_ips = ips_from(@req.x_forwarded_for).reverse # +Client-Ip+ and +X-Forwarded-For+ should not, generally, both be set. - # If they are both set, it means that this request passed through two - # proxies with incompatible IP header conventions, and there is no way - # for us to determine which header is the right one after the fact. - # Since we have no idea, we give up and explode. + # If they are both set, it means that either: + # + # 1) This request passed through two proxies with incompatible IP header + # conventions. + # 2) The client passed one of +Client-Ip+ or +X-Forwarded-For+ + # (whichever the proxy servers weren't using) themselves. + # + # Either way, there is no way for us to determine which header is the + # right one after the fact. Since we have no idea, if we are concerned + # about IP spoofing we need to give up and explode. (If you're not + # concerned about IP spoofing you can turn the +ip_spoofing_check+ + # option off.) should_check_ip = @check_ip && client_ips.last && forwarded_ips.last if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 9e50fea3fc..5fb5953811 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -7,14 +7,22 @@ require 'action_dispatch/request/session' module ActionDispatch module Session class SessionRestoreError < StandardError #:nodoc: - attr_reader :original_exception - def initialize(const_error) - @original_exception = const_error + def initialize(const_error = nil) + if const_error + ActiveSupport::Deprecation.warn("Passing #original_exception is deprecated and has no effect. " \ + "Exceptions will automatically capture the original exception.", caller) + end super("Session contains objects whose class definition isn't available.\n" + "Remember to require the classes for all objects kept in the session.\n" + - "(Original exception: #{const_error.message} [#{const_error.class}])\n") + "(Original exception: #{$!.message} [#{$!.class}])\n") + set_backtrace $!.backtrace + end + + def original_exception + ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller) + cause end end @@ -59,8 +67,8 @@ module ActionDispatch begin # Note that the regexp does not allow $1 to end with a ':' $1.constantize - rescue LoadError, NameError => e - raise ActionDispatch::Session::SessionRestoreError, e, e.backtrace + rescue LoadError, NameError + raise ActionDispatch::Session::SessionRestoreError end retry else diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 0e636b8257..429a98f236 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -36,7 +36,7 @@ module ActionDispatch # development: # secret_key_base: 'secret key' # - # To generate a secret key for an existing application, run `rake secret`. + # To generate a secret key for an existing application, run `rails secret`. # # If you are upgrading an existing Rails 3 app, you should leave your # existing secret_token in place and simply add the new secret_key_base. diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 47f475559a..735b5939dd 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -1,19 +1,23 @@ module ActionDispatch - # This middleware is added to the stack when `config.force_ssl = true`. - # It does three jobs to enforce secure HTTP requests: + # This middleware is added to the stack when `config.force_ssl = true`, and is passed + # the options set in `config.ssl_options`. It does three jobs to enforce secure HTTP + # requests: # - # 1. TLS redirect. http:// requests are permanently redirected to https:// - # with the same URL host, path, etc. Pass `:host` and/or `:port` to - # modify the destination URL. This is always enabled. + # 1. TLS redirect: Permanently redirects http:// requests to https:// + # with the same URL host, path, etc. Enabled by default. Set `config.ssl_options` + # to modify the destination URL + # (e.g. `redirect: { host: "secure.widgets.com", port: 8080 }`), or set + # `redirect: false` to disable this feature. # - # 2. Secure cookies. Sets the `secure` flag on cookies to tell browsers they - # mustn't be sent along with http:// requests. This is always enabled. + # 2. Secure cookies: Sets the `secure` flag on cookies to tell browsers they + # mustn't be sent along with http:// requests. Enabled by default. Set + # `config.ssl_options` with `secure_cookies: false` to disable this feature. # - # 3. HTTP Strict Transport Security (HSTS). Tells the browser to remember + # 3. HTTP Strict Transport Security (HSTS): Tells the browser to remember # this site as TLS-only and automatically redirect non-TLS requests. - # Enabled by default. Pass `hsts: false` to disable. + # Enabled by default. Configure `config.ssl_options` with `hsts: false` to disable. # - # Configure HSTS with `hsts: { … }`: + # Set `config.ssl_options` with `hsts: { … }` to configure HSTS: # * `expires`: How long, in seconds, these settings will stick. Defaults to # `180.days` (recommended). The minimum required to qualify for browser # preload lists is `18.weeks`. @@ -26,10 +30,10 @@ module ActionDispatch # gap, browser vendors include a baked-in list of HSTS-enabled sites. # Go to https://hstspreload.appspot.com to submit your site for inclusion. # - # Disabling HSTS: To turn off HSTS, omitting the header is not enough. - # Browsers will remember the original HSTS directive until it expires. - # Instead, use the header to tell browsers to expire HSTS immediately. - # Setting `hsts: false` is a shortcut for `hsts: { expires: 0 }`. + # To turn off HSTS, omitting the header is not enough. Browsers will remember the + # original HSTS directive until it expires. Instead, use the header to tell browsers to + # expire HSTS immediately. Setting `hsts: false` is a shortcut for + # `hsts: { expires: 0 }`. class SSL # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/ # and greater than the 18-week requirement for browser preload lists. @@ -39,7 +43,7 @@ module ActionDispatch { expires: HSTS_EXPIRES_IN, subdomains: false, preload: false } end - def initialize(app, redirect: {}, hsts: {}, **options) + def initialize(app, redirect: {}, hsts: {}, secure_cookies: true, **options) @app = app if options[:host] || options[:port] @@ -52,6 +56,7 @@ module ActionDispatch @redirect = redirect end + @secure_cookies = secure_cookies @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end @@ -61,10 +66,11 @@ module ActionDispatch if request.ssl? @app.call(env).tap do |status, headers, body| set_hsts_header! headers - flag_cookies_as_secure! headers + flag_cookies_as_secure! headers if @secure_cookies end else - redirect_to_https request + return redirect_to_https request if @redirect + @app.call(env) end end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 90e2ae6802..0b4bee5462 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -14,8 +14,21 @@ module ActionDispatch def name; klass.name; end + def ==(middleware) + case middleware + when Middleware + klass == middleware.klass + when Class + klass == middleware + end + end + def inspect - klass.to_s + if klass.is_a?(Class) + klass.to_s + else + klass.class.to_s + end end def build(app) diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 75f8e05a3f..ea9ab3821d 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -15,7 +15,6 @@ module ActionDispatch class FileHandler def initialize(root, index: 'index', headers: {}) @root = root.chomp('/') - @compiled_root = /^#{Regexp.escape(root)}/ @file_server = ::Rack::File.new(@root, headers) @index = index end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb index e7b913bbe4..e7b913bbe4 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb new file mode 100644 index 0000000000..23a9c7ba3f --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.text.erb @@ -0,0 +1,8 @@ +<% @source_extracts.first(3).each do |source_extract| %> +<% if source_extract[:code] %> +Extracted source (around line #<%= source_extract[:line_number] %>): + +<% source_extract[:code].each do |line, source| -%> +<%= line == source_extract[:line_number] ? "*#{line}" : "##{line}" -%> <%= source -%><% end -%> +<% end %> +<% end %> 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 c1e8b6cae3..5060da9369 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 @@ -1,6 +1,6 @@ <header> <h1> - <%= @exception.original_exception.class.to_s %> in + <%= @exception.cause.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> </h1> </header> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb index 77bcd26726..78d52acd96 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb @@ -1,4 +1,4 @@ -<%= @exception.original_exception.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> +<%= @exception.cause.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: <%= @exception.message %> diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 59c3f9248f..d00b2c3eb5 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -237,7 +237,7 @@ module ActionDispatch # # == View a list of all your routes # - # rake routes + # rails routes # # Target specific controllers by prefixing the command with <tt>CONTROLLER=x</tt>. # diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 7c0404ca62..18cd205bad 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -11,7 +11,7 @@ module ActionDispatch class Mapper URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port] - class Constraints < Endpoint #:nodoc: + class Constraints < Routing::Endpoint #:nodoc: attr_reader :app, :constraints SERVE = ->(app, req) { app.serve req } @@ -600,17 +600,20 @@ module ActionDispatch def mount(app, options = nil) if options path = options.delete(:at) - else - unless Hash === app - raise ArgumentError, "must be called with mount point" - end - + elsif Hash === app options = app app, path = options.find { |k, _| k.respond_to?(:call) } options.delete(app) if app end - raise "A rack application must be specified" unless path + raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call) + raise ArgumentError, <<-MSG.strip_heredoc unless path + Must be called with mount point + + mount SomeRackApp, at: "some_route" + or + mount(SomeRackApp => "some_route") + MSG rails_app = rails_app? app options[:as] ||= app_name(app, rails_app) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5f54ea130b..2bd2e53252 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -30,9 +30,9 @@ module ActionDispatch controller = controller req res = controller.make_response! req dispatch(controller, params[:action], req, res) - rescue NameError => e + rescue ActionController::RoutingError if @raise_on_name_error - raise ActionController::RoutingError, e.message, e.backtrace + raise else return [404, {'X-Cascade' => 'pass'}, []] end @@ -42,6 +42,8 @@ module ActionDispatch def controller(req) req.controller_class + rescue NameError => e + raise ActionController::RoutingError, e.message, e.backtrace end def dispatch(controller, action, req, res) @@ -371,10 +373,6 @@ module ActionDispatch end def eval_block(block) - if block.arity == 1 - raise "You are using the old router DSL which has been removed in Rails 3.1. " << - "Please check how to update your routes file at: http://www.engineyard.com/blog/2010/the-lowdown-on-routes-in-rails-3/" - end mapper = Mapper.new(self) if default_scope mapper.with_default_scope(default_scope, &block) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index b6c031dcf4..f91679593e 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -172,8 +172,11 @@ module ActionDispatch _routes.url_for(options.symbolize_keys.reverse_merge!(url_options), route_name) when ActionController::Parameters + unless options.permitted? + raise ArgumentError.new("Generating an URL from non sanitized request parameters is insecure!") + end route_name = options.delete :use_route - _routes.url_for(options.to_unsafe_h.symbolize_keys. + _routes.url_for(options.to_h.symbolize_keys. reverse_merge!(url_options), route_name) when String options diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index b6e21b0d28..c138660a21 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -21,15 +21,17 @@ module ActionDispatch # or its symbolic equivalent <tt>assert_response(:not_implemented)</tt>. # See Rack::Utils::SYMBOL_TO_STATUS_CODE for a full list. # - # # assert that the response was a redirection + # # Asserts that the response was a redirection # assert_response :redirect # - # # assert that the response code was status code 401 (unauthorized) + # # Asserts that the response code was status code 401 (unauthorized) # assert_response 401 def assert_response(type, message = nil) + message ||= generate_response_message(type) + if Symbol === type if [:success, :missing, :redirect, :error].include?(type) - assert_predicate @response, RESPONSE_PREDICATES[type], message + assert @response.send(RESPONSE_PREDICATES[type]), message else code = Rack::Utils::SYMBOL_TO_STATUS_CODE[type] if code.nil? @@ -42,20 +44,20 @@ module ActionDispatch end end - # Assert that the redirection options passed in match those of the redirect called in the latest action. + # Asserts that the redirection options passed in match those of the redirect called in the latest action. # This match can be partial, such that <tt>assert_redirected_to(controller: "weblog")</tt> will also # match the redirection of <tt>redirect_to(controller: "weblog", action: "show")</tt> and so on. # - # # assert that the redirection was to the "index" action on the WeblogController + # # Asserts that the redirection was to the "index" action on the WeblogController # assert_redirected_to controller: "weblog", action: "index" # - # # assert that the redirection was to the named route login_url + # # Asserts that the redirection was to the named route login_url # assert_redirected_to login_url # - # # assert that the redirection was to the url for @customer + # # Asserts that the redirection was to the url for @customer # assert_redirected_to @customer # - # # asserts that the redirection matches the regular expression + # # Asserts that the redirection matches the regular expression # assert_redirected_to %r(\Ahttp://example.org) def assert_redirected_to(options = {}, message=nil) assert_response(:redirect, message) @@ -82,6 +84,17 @@ module ActionDispatch handle._compute_redirect_to_location(@request, fragment) end end + + def generate_response_message(type, code = @response.response_code) + "Expected response to be a <#{type}>, but was a <#{code}>" + .concat location_if_redirected + end + + def location_if_redirected + return '' unless @response.redirection? && @response.location.present? + location = normalize_argument_to_redirection(@response.location) + " redirect to <#{location}>" + end end end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 54e24ed6bf..78ef860548 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -14,14 +14,14 @@ module ActionDispatch # requiring a specific HTTP method. The hash should contain a :path with the incoming request path # and a :method containing the required HTTP verb. # - # # assert that POSTing to /items will call the create action on ItemsController + # # Asserts that POSTing to /items will call the create action on ItemsController # assert_recognizes({controller: 'items', action: 'create'}, {path: 'items', method: :post}) # # You can also pass in +extras+ with a hash containing URL parameters that would normally be in the query string. This can be used - # to assert that values in the query string string will end up in the params hash correctly. To test query strings you must use the + # to assert that values in the query string will end up in the params hash correctly. To test query strings you must use the # extras argument, appending the query string on the path directly will not work. For example: # - # # assert that a path of '/items/list/1?view=print' returns the correct options + # # Asserts that a path of '/items/list/1?view=print' returns the correct options # assert_recognizes({controller: 'items', action: 'list', id: '1', view: 'print'}, 'items/list/1', { view: "print" }) # # The +message+ parameter allows you to pass in an error message that is displayed upon failure. @@ -104,13 +104,13 @@ module ActionDispatch # The +extras+ hash allows you to specify options that would normally be provided as a query string to the action. The # +message+ parameter allows you to specify a custom error message to display upon failure. # - # # Assert a basic route: a controller with the default action (index) + # # Asserts a basic route: a controller with the default action (index) # assert_routing '/home', controller: 'home', action: 'index' # # # Test a route generated with a specific controller, action, and parameter (id) # assert_routing '/entries/show/23', controller: 'entries', action: 'show', id: 23 # - # # Assert a basic route (controller + default action), with an error message if it fails + # # Asserts a basic route (controller + default action), with an error message if it fails # assert_routing '/store', { controller: 'store', action: 'index' }, {}, {}, 'Route for store index not generated properly' # # # Tests a route, providing a defaults hash diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 790f9ea5d2..711ca10419 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -375,6 +375,7 @@ module ActionDispatch @request = ActionDispatch::Request.new(session.last_request.env) response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) + @response.request = @request @html_document = nil @url_options = nil diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index c28d701b48..eca0439909 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -26,7 +26,7 @@ module ActionDispatch @response.redirect_url end - # Shortcut for <tt>Rack::Test::UploadedFile.new(File.join(ActionController::TestCase.fixture_path, path), type)</tt>: + # 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') # diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 255ac9f4ed..5cfb5f02d8 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "alpha" + PRE = "beta1" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index 82c747680d..841fa6aaad 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -6,7 +6,12 @@ module ActionDispatch class ResponseAssertionsTest < ActiveSupport::TestCase include ResponseAssertions - FakeResponse = Struct.new(:response_code) do + FakeResponse = Struct.new(:response_code, :location) do + def initialize(*) + super + self.location ||= "http://test.example.com/posts" + end + [:successful, :not_found, :redirection, :server_error].each do |sym| define_method("#{sym}?") do sym == response_code @@ -14,6 +19,11 @@ module ActionDispatch end end + def setup + @controller = nil + @request = nil + end + def test_assert_response_predicate_methods [:success, :missing, :redirect, :error].each do |sym| @response = FakeResponse.new RESPONSE_PREDICATES[sym].to_s.sub(/\?/, '').to_sym @@ -58,6 +68,37 @@ module ActionDispatch assert_response :succezz } end + + def test_error_message_shows_404_when_404_asserted_for_success + @response = ActionDispatch::Response.new + @response.status = 404 + + error = assert_raises(Minitest::Assertion) { assert_response :success } + expected = "Expected response to be a <success>, but was a <404>" + assert_match expected, error.message + end + + def test_error_message_shows_302_redirect_when_302_asserted_for_success + @response = ActionDispatch::Response.new + @response.status = 302 + @response.location = 'http://test.host/posts/redirect/1' + + error = assert_raises(Minitest::Assertion) { assert_response :success } + expected = "Expected response to be a <success>, but was a <302>" \ + " redirect to <http://test.host/posts/redirect/1>" + assert_match expected, error.message + end + + def test_error_message_shows_302_redirect_when_302_asserted_for_301 + @response = ActionDispatch::Response.new + @response.status = 302 + @response.location = 'http://test.host/posts/redirect/2' + + error = assert_raises(Minitest::Assertion) { assert_response 301 } + expected = "Expected response to be a <301>, but was a <302>" \ + " redirect to <http://test.host/posts/redirect/2>" + assert_match expected, error.message + end end end end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index fb60dbd993..e3f669dbb5 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -93,7 +93,7 @@ end class ControllerInstanceTests < ActiveSupport::TestCase def setup @empty = EmptyController.new - @empty.set_request!(ActionDispatch::Request.new({})) + @empty.set_request!(ActionDispatch::Request.empty) @empty.set_response!(EmptyController.make_response!(@empty.request)) @contained = Submodule::ContainedEmptyController.new @empty_controllers = [@empty, @contained] diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index bc0ffd3eaa..d19b3810c2 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -419,3 +419,28 @@ class AutomaticCollectionCacheTest < ActionController::TestCase assert_equal 1, @controller.partial_rendered_times end end + +class FragmentCacheKeyTestController < CachingController + attr_accessor :account_id + + fragment_cache_key "v1" + fragment_cache_key { account_id } +end + +class FragmentCacheKeyTest < ActionController::TestCase + def setup + super + @store = ActiveSupport::Cache::MemoryStore.new + @controller = FragmentCacheKeyTestController.new + @controller.perform_caching = true + @controller.cache_store = @store + end + + def test_fragment_cache_key + @controller.account_id = "123" + assert_equal 'views/v1/123/what a key', @controller.fragment_cache_key('what a key') + + @controller.account_id = nil + assert_equal 'views/v1//what a key', @controller.fragment_cache_key('what a key') + end +end diff --git a/actionpack/test/controller/helper_test.rb b/actionpack/test/controller/helper_test.rb index 3ecfedefd1..feb882a2b3 100644 --- a/actionpack/test/controller/helper_test.rb +++ b/actionpack/test/controller/helper_test.rb @@ -141,20 +141,10 @@ class HelperTest < ActiveSupport::TestCase def test_helper_for_nested_controller assert_equal 'hello: Iz guuut!', call_controller(Fun::GamesController, "render_hello_world").last.body - # request = ActionController::TestRequest.new - # - # resp = Fun::GamesController.action(:render_hello_world).call(request.env) - # assert_equal 'hello: Iz guuut!', resp.last.body end def test_helper_for_acronym_controller assert_equal "test: baz", call_controller(Fun::PdfController, "test").last.body - # - # request = ActionController::TestRequest.new - # response = ActionDispatch::TestResponse.new - # request.action = 'test' - # - # assert_equal 'test: baz', Fun::PdfController.process(request, response).body end def test_default_helpers_only diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 9c5a01c318..98e3c891a7 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -94,6 +94,14 @@ class HttpTokenAuthenticationTest < ActionController::TestCase assert_response :success end + test "authentication request with tab in header" do + @request.env['HTTP_AUTHORIZATION'] = "Token\ttoken=\"lifo\"" + get :index + + assert_response :success + assert_equal 'Hello Secret', @response.body + end + test "authentication request without credential" do get :display diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 4224ac2a1b..aab2d9545d 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -1,5 +1,5 @@ require 'abstract_unit' -require 'concurrent/atomics' +require 'concurrent/atomic/count_down_latch' Thread.abort_on_exception = true module ActionController @@ -442,3 +442,42 @@ module ActionController end end end + +class LiveStreamRouterTest < ActionDispatch::IntegrationTest + class TestController < ActionController::Base + include ActionController::Live + + def index + response.headers['Content-Type'] = 'text/event-stream' + sse = SSE.new(response.stream) + sse.write("{\"name\":\"John\"}") + sse.write({ name: "Ryan" }) + ensure + sse.close + end + end + + def self.call(env) + routes.call(env) + end + + def self.routes + @routes ||= ActionDispatch::Routing::RouteSet.new + end + + routes.draw do + get '/test' => 'live_stream_router_test/test#index' + end + + def app + self.class + end + + test "streaming served through the router" do + get "/test" + + assert_response :ok + assert_match(/data: {\"name\":\"John\"}/, response.body) + assert_match(/data: {\"name\":\"Ryan\"}/, response.body) + end +end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 7835d2768a..6ae33be3c8 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -170,7 +170,7 @@ class ACLogSubscriberTest < ActionController::TestCase def test_process_action_with_view_runtime get :show wait - assert_match(/Completed 200 OK in [\d]ms/, logs[1]) + assert_match(/Completed 200 OK in \d+ms/, logs[1]) end def test_append_info_to_payload_is_called_even_with_exception diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index c025c7fa00..76e2d3ff43 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -661,10 +661,6 @@ class RespondToControllerTest < ActionController::TestCase end def test_variant_inline_syntax - get :variant_inline_syntax, format: :js - assert_equal "text/javascript", @response.content_type - assert_equal "js", @response.body - get :variant_inline_syntax assert_equal "text/html", @response.content_type assert_equal "none", @response.body @@ -674,6 +670,12 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "phone", @response.body end + def test_variant_inline_syntax_with_format + get :variant_inline_syntax, format: :js + assert_equal "text/javascript", @response.content_type + assert_equal "js", @response.body + end + def test_variant_inline_syntax_without_block get :variant_inline_syntax_without_block, params: { v: :phone } assert_equal "text/html", @response.content_type diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index e61f4d241b..c226fa57ee 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -26,7 +26,7 @@ module BareMetalTest test "response_body value is wrapped in an array when the value is a String" do controller = BareController.new - controller.set_request!(ActionDispatch::Request.new({})) + controller.set_request!(ActionDispatch::Request.empty) controller.set_response!(BareController.make_response!(controller.request)) controller.index assert_equal ["Hello world"], controller.response_body diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index 9f7d14e85d..598cac5156 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -256,7 +256,7 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_h returns empty hash on unpermitted params" do - assert @params.to_h.is_a? Hash + assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess assert_not @params.to_h.is_a? ActionController::Parameters assert @params.to_h.empty? end @@ -264,7 +264,7 @@ class ParametersPermitTest < ActiveSupport::TestCase test "to_h returns converted hash on permitted params" do @params.permit! - assert @params.to_h.is_a? Hash + assert @params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess assert_not @params.to_h.is_a? ActionController::Parameters end @@ -273,7 +273,7 @@ class ParametersPermitTest < ActiveSupport::TestCase ActionController::Parameters.permit_all_parameters = true params = ActionController::Parameters.new(crab: "Senjougahara Hitagi") - assert params.to_h.is_a? Hash + assert params.to_h.is_a? ActiveSupport::HashWithIndifferentAccess assert_not @params.to_h.is_a? ActionController::Parameters assert_equal({ "crab" => "Senjougahara Hitagi" }, params.to_h) ensure @@ -294,7 +294,41 @@ class ParametersPermitTest < ActiveSupport::TestCase end test "to_unsafe_h returns unfiltered params" do - assert @params.to_h.is_a? Hash - assert_not @params.to_h.is_a? ActionController::Parameters + assert @params.to_unsafe_h.is_a? ActiveSupport::HashWithIndifferentAccess + assert_not @params.to_unsafe_h.is_a? ActionController::Parameters + end + + test "to_h only deep dups Ruby collections" do + company = Class.new do + attr_reader :dupped + def dup; @dupped = true; end + end.new + + params = ActionController::Parameters.new(prem: { likes: %i( dancing ) }) + assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.permit!.to_h) + + params = ActionController::Parameters.new(companies: [ company, :acme ]) + assert_equal({ 'companies' => [ company, :acme ] }, params.permit!.to_h) + assert_not company.dupped + end + + test "to_unsafe_h only deep dups Ruby collections" do + company = Class.new do + attr_reader :dupped + def dup; @dupped = true; end + end.new + + params = ActionController::Parameters.new(prem: { likes: %i( dancing ) }) + assert_equal({ 'prem' => { 'likes' => %i( dancing ) } }, params.to_unsafe_h) + + params = ActionController::Parameters.new(companies: [ company, :acme ]) + assert_equal({ 'companies' => [ company, :acme ] }, params.to_unsafe_h) + assert_not company.dupped + end + + test "included? returns true when the key is present" do + assert @params.include? :person + assert @params.include? 'person' + assert_not @params.include? :gorilla end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index 631ff7d02a..0b184eace9 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -42,6 +42,10 @@ class RedirectController < ActionController::Base redirect_to :back, :status => 307 end + def redirect_back_with_status + redirect_back(fallback_location: "/things/stuff", status: 307) + end + def host_redirect redirect_to :action => "other_host", :only_path => false, :host => 'other.test.host' end @@ -187,7 +191,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_status @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back_with_status + + assert_deprecated do + get :redirect_to_back_with_status + end + assert_response 307 assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -236,7 +244,11 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back @request.env["HTTP_REFERER"] = "http://www.example.com/coming/from" - get :redirect_to_back + + assert_deprecated do + get :redirect_to_back + end + assert_response :redirect assert_equal "http://www.example.com/coming/from", redirect_to_url end @@ -244,10 +256,32 @@ class RedirectTest < ActionController::TestCase def test_redirect_to_back_with_no_referer assert_raise(ActionController::RedirectBackError) { @request.env["HTTP_REFERER"] = nil + + assert_deprecated do + get :redirect_to_back + end + get :redirect_to_back } end + def test_redirect_back + referer = "http://www.example.com/coming/from" + @request.env["HTTP_REFERER"] = referer + + get :redirect_back_with_status + + assert_response 307 + assert_equal referer, redirect_to_url + end + + def test_redirect_back_with_no_referer + get :redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do @@ -273,10 +307,10 @@ class RedirectTest < ActionController::TestCase end def test_redirect_to_params - error = assert_raise(ActionController::ActionControllerError) do + error = assert_raise(ArgumentError) do get :redirect_to_params end - assert_equal "Cannot redirect to a parameter hash!", error.message + assert_equal "Generating an URL from non sanitized request parameters is insecure!", error.message end def test_redirect_to_with_block diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 94ffbe3cd0..87a8ed3dc9 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -304,6 +304,41 @@ module RequestForgeryProtectionTests assert_not_blocked { put :index } end + def test_should_allow_post_with_origin_checking_and_correct_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://test.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_allow_post_with_origin_checking_and_no_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_block_post_with_origin_checking_and_wrong_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://bad.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + def test_should_warn_on_missing_csrf_token old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new @@ -405,6 +440,16 @@ module RequestForgeryProtectionTests def assert_cross_origin_not_blocked assert_not_blocked { yield } end + + def forgery_protection_origin_check + old_setting = ActionController::Base.forgery_protection_origin_check + ActionController::Base.forgery_protection_origin_check = true + begin + yield + ensure + ActionController::Base.forgery_protection_origin_check = old_setting + end + end end # OK let's get our test on @@ -495,10 +540,10 @@ class PrependProtectForgeryBaseControllerTest < ActionController::TestCase assert_equal(expected_callback_order, @controller.called_callbacks) end - def test_verify_authenticity_token_is_prepended_by_default + def test_verify_authenticity_token_is_not_prepended_by_default @controller = PrependDefaultController.new get :index - expected_callback_order = ["verify_authenticity_token", "custom_action"] + expected_callback_order = ["custom_action", "verify_authenticity_token"] assert_equal(expected_callback_order, @controller.called_callbacks) end end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index f53f061e10..f42bef883f 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -132,11 +132,19 @@ class RescueController < ActionController::Base end def io_error_in_view - raise ActionView::TemplateError.new(nil, IOError.new('this is io error')) + begin + raise IOError.new('this is io error') + rescue + raise ActionView::TemplateError.new(nil) + end end def zero_division_error_in_view - raise ActionView::TemplateError.new(nil, ZeroDivisionError.new('this is zero division error')) + begin + raise ZeroDivisionError.new('this is zero division error') + rescue + raise ActionView::TemplateError.new(nil) + end end protected diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 4a2b02a003..a39fede5b9 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -289,12 +289,6 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal({:id=>"1", :filters=>"foo", :format=>"js"}, params) end - def test_draw_with_block_arity_one_raises - assert_raise(RuntimeError) do - rs.draw { |map| map.match '/:controller(/:action(/:id))' } - end - end - def test_specific_controller_action_failure rs.draw do mount lambda {} => "/foo" diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index f1a296682d..b9caddcdb7 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -75,7 +75,7 @@ class TestCaseTest < ActionController::TestCase end def test_headers - render plain: request.headers.env.to_json + render plain: ::JSON.dump(request.headers.env) end def test_html_output @@ -172,7 +172,7 @@ XML before_action { @dynamic_opt = 'opt' } def test_url_options_reset - render plain: url_for(params) + render plain: url_for end def default_url_options diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 78e883f134..67212fea38 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -375,6 +375,13 @@ module AbstractController assert_equal({'query[person][position][]' => 'prof' }.to_query, params[3]) end + def test_url_action_controller_parameters + add_host! + assert_raise(ArgumentError) do + W.new.url_for(ActionController::Parameters.new(:controller => 'c', :action => 'a', protocol: 'javascript', f: '%0Aeval(name)')) + end + end + def test_path_generation_for_symbol_parameter_keys assert_generates("/image", :controller=> :image) end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 84c244c72a..dfcef14344 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -7,7 +7,7 @@ class CookieJarTest < ActiveSupport::TestCase attr_reader :request def setup - @request = ActionDispatch::Request.new({}) + @request = ActionDispatch::Request.empty end def test_fetch diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 93258fbceb..159bf10545 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -42,7 +42,11 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest when "/unprocessable_entity" raise ActionController::InvalidAuthenticityToken when "/not_found_original_exception" - raise ActionView::Template::Error.new('template', AbstractController::ActionNotFound.new) + begin + raise AbstractController::ActionNotFound.new + rescue + raise ActionView::Template::Error.new('template') + end when "/missing_template" raise ActionView::MissingTemplate.new(%w(foo), 'foo/index', %w(foo), false, 'mailer') when "/bad_request" @@ -56,12 +60,12 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest when "/syntax_error_into_view" begin eval 'broke_syntax =' - rescue Exception => e + rescue Exception template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, {}) - raise ActionView::Template::Error.new(template, e) + raise ActionView::Template::Error.new(template) end when "/framework_raises" method_that_raises @@ -71,6 +75,13 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest end end + class BoomerAPI < Boomer + def call(env) + env['action_dispatch.show_detailed_exceptions'] = @detailed + raise "puke!" + end + end + RoutesApp = Struct.new(:routes).new(SharedTestRoutes) ProductionApp = ActionDispatch::DebugExceptions.new(Boomer.new(false), RoutesApp) DevelopmentApp = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp) @@ -162,6 +173,14 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_equal "text/plain", response.content_type assert_match(/RuntimeError\npuke/, body) + Rails.stub :root, Pathname.new('.') do + get "/", headers: xhr_request_env + + assert_response 500 + assert_match 'Extracted source (around line #', body + assert_select 'pre', { count: 0 }, body + end + get "/not_found", headers: xhr_request_env assert_response 404 assert_no_match(/<body>/, body) @@ -193,6 +212,68 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end + test "rescue with json error for API request" do + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) + + get "/", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_no_match(/<header>/, body) + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/not_found", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 404 + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/#{AbstractController::ActionNotFound.name}/, body) + + get "/method_not_allowed", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 405 + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::MethodNotAllowed/, body) + + get "/unknown_http_method", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 405 + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::UnknownHttpMethod/, body) + + get "/bad_request", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 400 + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::BadRequest/, body) + + get "/parameter_missing", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 400 + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/ActionController::ParameterMissing/, body) + end + + test "rescue with json on API request returns only allowed formats or json as a fallback" do + @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api) + + get "/index.json", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/index.html", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_no_match(/<header>/, body) + assert_no_match(/<body>/, body) + assert_equal "application/json", response.content_type + assert_match(/RuntimeError: puke/, body) + + get "/index.xml", headers: { 'action_dispatch.show_exceptions' => true } + assert_response 500 + assert_equal "application/xml", response.content_type + assert_match(/RuntimeError: puke/, body) + end + test "does not show filtered parameters" do @app = DevelopmentApp @@ -360,7 +441,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_select 'pre code a:first', %r{test/dispatch/debug_exceptions_test\.rb:\d+:in `call} end - # assert framework trace that that threw the error is first + # assert framework trace that threw the error is first assert_select '#Framework-Trace' do assert_select 'pre code a:first', /method_that_raises/ end diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index 55becc1c91..e4475f4233 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -1,12 +1,12 @@ require 'abstract_unit' -require 'concurrent/atomics' +require 'concurrent/atomic/count_down_latch' module ActionController module Live class ResponseTest < ActiveSupport::TestCase def setup @response = Live::Response.new - @response.request = ActionDispatch::Request.new({}) #yolo + @response.request = ActionDispatch::Request.empty end def test_header_merge diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index e783df855e..df27e41997 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -158,7 +158,7 @@ module ActionDispatch assert_equal '/*path.:format', fakeset.asts.first.to_s end - def test_raising_helpful_error_on_invalid_arguments + def test_raising_error_when_path_is_not_passed fakeset = FakeSet.new mapper = Mapper.new fakeset app = lambda { |env| [200, {}, [""]] } @@ -166,6 +166,18 @@ module ActionDispatch mapper.mount app end end + + def test_raising_error_when_rack_app_is_not_passed + fakeset = FakeSet.new + mapper = Mapper.new fakeset + assert_raises ArgumentError do + mapper.mount 10, as: "exciting" + end + + assert_raises ArgumentError do + mapper.mount as: "exciting" + end + end end end end diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index 33aa616474..a8c8e0784f 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -131,4 +131,20 @@ class MiddlewareStackTest < ActiveSupport::TestCase assert_equal BazMiddleware, @stack.last.klass end end + + test "can check if Middleware are equal - Class" do + assert_equal @stack.last, BarMiddleware + end + + test "includes a class" do + assert_equal true, @stack.include?(BarMiddleware) + end + + test "can check if Middleware are equal - Middleware" do + assert_equal @stack.last, @stack.last + end + + test "includes a middleware" do + assert_equal true, @stack.include?(ActionDispatch::MiddlewareStack::Middleware.new(BarMiddleware, nil, nil)) + end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index c2300a0142..a3992ad008 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -37,6 +37,13 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest ) end + test "parses json params for application/vnd.api+json" do + assert_parses( + {"person" => {"name" => "David"}}, + "{\"person\": {\"name\": \"David\"}}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + end + test "nils are stripped from collections" do assert_parses( {"person" => []}, @@ -69,8 +76,8 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest $stderr = StringIO.new # suppress the log json = "[\"person]\": {\"name\": \"David\"}}" exception = assert_raise(ActionDispatch::ParamsParser::ParseError) { post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => false} } - assert_equal JSON::ParserError, exception.original_exception.class - assert_equal exception.original_exception.message, exception.message + assert_equal JSON::ParserError, exception.cause.class + assert_equal exception.cause.message, exception.message ensure $stderr = STDERR end @@ -136,6 +143,13 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest ) end + test "parses json params for application/vnd.api+json" do + assert_parses( + {"user" => {"username" => "sikachu"}, "username" => "sikachu"}, + "{\"username\": \"sikachu\"}", { 'CONTENT_TYPE' => 'application/vnd.api+json' } + ) + end + test "parses json with non-object JSON content" do assert_parses( {"user" => {"_json" => "string content" }, "_json" => "string content" }, diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index ae0e7e93ed..7dcbcc5c21 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -7,7 +7,7 @@ module ActionDispatch attr_reader :req def setup - @req = ActionDispatch::Request.new({}) + @req = ActionDispatch::Request.empty end def test_create_adds_itself_to_env diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index e9896a71f4..0edad72fd9 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -4,9 +4,13 @@ class BaseRequestTest < ActiveSupport::TestCase def setup @env = { :ip_spoofing_check => true, - :tld_length => 1, "rack.input" => "foo" } + @original_tld_length = ActionDispatch::Http::URL.tld_length + end + + def teardown + ActionDispatch::Http::URL.tld_length = @original_tld_length end def url_for(options = {}) @@ -19,9 +23,9 @@ class BaseRequestTest < ActiveSupport::TestCase 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) - tld_length = env.key?(:tld_length) ? env.delete(:tld_length) : 1 + ActionDispatch::Http::URL.tld_length = env.delete(:tld_length) if env.key?(:tld_length) + ip_app.call(env) - ActionDispatch::Http::URL.tld_length = tld_length env = @env.merge(env) ActionDispatch::Request.new(env) @@ -254,15 +258,6 @@ end class RequestDomain < BaseRequestTest test "domains" do - request = stub_request 'HTTP_HOST' => 'www.rubyonrails.org' - assert_equal "rubyonrails.org", request.domain - - request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" - assert_equal "rubyonrails.co.uk", request.domain(2) - - request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk", :tld_length => 2 - assert_equal "rubyonrails.co.uk", request.domain - request = stub_request 'HTTP_HOST' => "192.168.1.200" assert_nil request.domain @@ -271,25 +266,18 @@ class RequestDomain < BaseRequestTest request = stub_request 'HTTP_HOST' => "192.168.1.200.com" assert_equal "200.com", request.domain - end - test "subdomains" do - request = stub_request 'HTTP_HOST' => "www.rubyonrails.org" - assert_equal %w( www ), request.subdomains - assert_equal "www", request.subdomain + request = stub_request 'HTTP_HOST' => 'www.rubyonrails.org' + assert_equal "rubyonrails.org", request.domain request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" - assert_equal %w( www ), request.subdomains(2) - assert_equal "www", request.subdomain(2) - - request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk" - assert_equal %w( dev www ), request.subdomains(2) - assert_equal "dev.www", request.subdomain(2) + assert_equal "rubyonrails.co.uk", request.domain(2) - request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk", :tld_length => 2 - assert_equal %w( dev www ), request.subdomains - assert_equal "dev.www", request.subdomain + request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk", :tld_length => 2 + assert_equal "rubyonrails.co.uk", request.domain + end + test "subdomains" do request = stub_request 'HTTP_HOST' => "foobar.foobar.com" assert_equal %w( foobar ), request.subdomains assert_equal "foobar", request.subdomain @@ -309,6 +297,22 @@ class RequestDomain < BaseRequestTest request = stub_request 'HTTP_HOST' => nil assert_equal [], request.subdomains assert_equal "", request.subdomain + + request = stub_request 'HTTP_HOST' => "www.rubyonrails.org" + assert_equal %w( www ), request.subdomains + assert_equal "www", request.subdomain + + request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" + assert_equal %w( www ), request.subdomains(2) + assert_equal "www", request.subdomain(2) + + request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk" + assert_equal %w( dev www ), request.subdomains(2) + assert_equal "dev.www", request.subdomain(2) + + request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk", :tld_length => 2 + assert_equal %w( dev www ), request.subdomains + assert_equal "dev.www", request.subdomain end end @@ -893,6 +897,27 @@ class RequestFormat < BaseRequestTest ActionDispatch::Request.ignore_accept_header = old_ignore_accept_header end end + + test "format taken from the path extension" do + request = stub_request 'PATH_INFO' => '/foo.xml' + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [Mime[:xml]], request.formats + end + + request = stub_request 'PATH_INFO' => '/foo.123' + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [Mime[:html]], request.formats + end + end + + test "formats from accept headers have higher precedence than path extension" do + request = stub_request 'HTTP_ACCEPT' => 'application/json', + 'PATH_INFO' => '/foo.xml' + + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [Mime[:json]], request.formats + end + end end class RequestMimeType < BaseRequestTest @@ -1012,8 +1037,8 @@ class RequestParameters < BaseRequestTest request.parameters end - assert e.original_exception - assert_equal e.original_exception.backtrace, e.backtrace + assert_not_nil e.cause + assert_equal e.cause.backtrace, e.backtrace end end @@ -1212,3 +1237,23 @@ class RequestVariant < BaseRequestTest end end end + +class RequestFormData < BaseRequestTest + test 'media_type is from the FORM_DATA_MEDIA_TYPES array' do + assert stub_request('CONTENT_TYPE' => 'application/x-www-form-urlencoded').form_data? + assert stub_request('CONTENT_TYPE' => 'multipart/form-data').form_data? + end + + test 'media_type is not from the FORM_DATA_MEDIA_TYPES array' do + assert !stub_request('CONTENT_TYPE' => 'application/xml').form_data? + assert !stub_request('CONTENT_TYPE' => 'multipart/related').form_data? + end + + test 'no Content-Type header is provided and the request_method is POST' do + request = stub_request('REQUEST_METHOD' => 'POST') + + assert_equal '', request.media_type + assert_equal 'POST', request.request_method + assert !request.form_data? + end +end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 981d820ccf..c37679bc5f 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -5,6 +5,7 @@ require 'rack/content_length' class ResponseTest < ActiveSupport::TestCase def setup @response = ActionDispatch::Response.create + @response.request = ActionDispatch::Request.empty end def test_can_wait_until_commit @@ -39,6 +40,7 @@ class ResponseTest < ActiveSupport::TestCase def test_response_body_encoding body = ["hello".encode(Encoding::UTF_8)] response = ActionDispatch::Response.new 200, {}, body + response.request = ActionDispatch::Request.empty assert_equal Encoding::UTF_8, response.body.encoding end @@ -261,6 +263,7 @@ class ResponseTest < ActiveSupport::TestCase test "can be explicitly destructured into status, headers and an enumerable body" do response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found']) + response.request = ActionDispatch::Request.empty status, headers, body = *response assert_equal 404, status @@ -356,6 +359,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest resp.cache_control[:public] = true resp.etag = '123' resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty }.to_a } @@ -392,6 +396,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest resp.charset = 'utf-16' resp.content_type = Mime[:xml] resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty }.to_a } diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 8972f3e74d..82222a141c 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4592,3 +4592,44 @@ class TestDefaultUrlOptions < ActionDispatch::IntegrationTest assert_equal '/en/posts/2014/12/13', archived_posts_path(2014, 12, 13) end end + +class TestErrorsInController < ActionDispatch::IntegrationTest + class ::PostsController < ActionController::Base + def foo + nil.i_do_not_exist + end + + def bar + NonExistingClass.new + end + end + + Routes = ActionDispatch::Routing::RouteSet.new + Routes.draw do + get '/:controller(/:action)' + end + + APP = build_app Routes + + def app + APP + end + + def test_legit_no_method_errors_are_not_caught + get '/posts/foo' + assert_equal 500, response.status + end + + def test_legit_name_errors_are_not_caught + get '/posts/bar' + assert_equal 500, response.status + end + + def test_legit_routing_not_found_responses + get '/posts/baz' + assert_equal 404, response.status + + get '/i_do_not_exist' + assert_equal 404, response.status + end +end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 15dd702161..14894d4b82 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -8,14 +8,22 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest case req.path when "/not_found" raise AbstractController::ActionNotFound - when "/bad_params" - raise ActionDispatch::ParamsParser::ParseError.new("", StandardError.new) + when "/bad_params", "/bad_params.json" + begin + raise StandardError.new + rescue + raise ActionDispatch::ParamsParser::ParseError + end when "/method_not_allowed" raise ActionController::MethodNotAllowed, 'PUT' when "/unknown_http_method" raise ActionController::UnknownHttpMethod when "/not_found_original_exception" - raise ActionView::Template::Error.new('template', AbstractController::ActionNotFound.new) + begin + raise AbstractController::ActionNotFound.new + rescue + raise ActionView::Template::Error.new('template') + end else raise "puke!" end @@ -112,4 +120,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_response 405 assert_equal "", body end + + test "bad params exception is returned in the correct format" do + @app = ProductionApp + + get "/bad_params", headers: { '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 } + assert_equal "application/json; charset=utf-8", response.headers["Content-Type"] + assert_response 400 + assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body) + end end diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 7a5b8393dc..11251c86e7 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -12,25 +12,31 @@ class SSLTest < ActionDispatch::IntegrationTest end class RedirectSSLTest < SSLTest - def assert_not_redirected(url, headers: {}) - self.app = build_app + + def assert_not_redirected(url, headers: {}, redirect: {}, deprecated_host: nil, + deprecated_port: nil) + + self.app = build_app ssl_options: { redirect: redirect, + host: deprecated_host, port: deprecated_port + } + get url, headers: headers assert_response :ok end - def assert_redirected(host: nil, port: nil, status: 301, body: [], - deprecated_host: nil, deprecated_port: nil, + def assert_redirected(redirect: {}, deprecated_host: nil, deprecated_port: nil, from: 'http://a/b?c=d', to: from.sub('http', 'https')) - self.app = build_app ssl_options: { - redirect: { host: host, port: port, status: status, body: body }, + redirect = { status: 301, body: [] }.merge(redirect) + + self.app = build_app ssl_options: { redirect: redirect, host: deprecated_host, port: deprecated_port } get from - assert_response status + assert_response redirect[:status] || 301 assert_redirected_to to - assert_equal body.join, @response.body + assert_equal redirect[:body].join, @response.body end test 'https is not redirected' do @@ -46,31 +52,31 @@ class RedirectSSLTest < SSLTest end test 'redirect with non-301 status' do - assert_redirected status: 307 + assert_redirected redirect: { status: 307 } end test 'redirect with custom body' do - assert_redirected body: ['foo'] + assert_redirected redirect: { body: ['foo'] } end test 'redirect to specific host' do - assert_redirected host: 'ssl', to: 'https://ssl/b?c=d' + assert_redirected redirect: { host: 'ssl' }, to: 'https://ssl/b?c=d' end test 'redirect to default port' do - assert_redirected port: 443 + assert_redirected redirect: { port: 443 } end test 'redirect to non-default port' do - assert_redirected port: 8443, to: 'https://a:8443/b?c=d' + assert_redirected redirect: { port: 8443 }, to: 'https://a:8443/b?c=d' end test 'redirect to different host and non-default port' do - assert_redirected host: 'ssl', port: 8443, to: 'https://ssl:8443/b?c=d' + assert_redirected redirect: { host: 'ssl', port: 8443 }, to: 'https://ssl:8443/b?c=d' end test 'redirect to different host including port' do - assert_redirected host: 'ssl:443', to: 'https://ssl:443/b?c=d' + assert_redirected redirect: { host: 'ssl:443' }, to: 'https://ssl:443/b?c=d' end test ':host is deprecated, moved within redirect: { host: … }' do @@ -84,6 +90,10 @@ class RedirectSSLTest < SSLTest assert_redirected deprecated_port: 1, to: 'https://a:1/b?c=d' end end + + test 'no redirect with redirect set to false' do + assert_not_redirected 'http://example.org', redirect: false + end end class StrictTransportSecurityTest < SSLTest @@ -187,6 +197,11 @@ class SecureCookiesTest < SSLTest assert_cookies 'problem=def; path=/; Secure; HttpOnly' end + def test_cookies_as_not_secure_with_secure_cookies_disabled + get headers: { 'Set-Cookie' => DEFAULT }, ssl_options: { secure_cookies: false } + assert_cookies *DEFAULT.split("\n") + end + def test_no_cookies get assert_nil response.headers['Set-Cookie'] diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index fd4ede4d1b..8c9782bb90 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -129,6 +129,13 @@ module TestUrlGeneration ) end + test "generating URLS with empty querystring" do + assert_equal "/bars.json", bars_path( + a: {}, + format: 'json' + ) + end + end end |