diff options
Diffstat (limited to 'actionpack/lib')
73 files changed, 972 insertions, 566 deletions
diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index 56c4033387..1e57cbaac4 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -6,6 +6,7 @@ module AbstractController extend ActiveSupport::Autoload autoload :Base + autoload :Caching autoload :Callbacks autoload :Collector autoload :DoubleRenderError, "abstract_controller/rendering" @@ -15,4 +16,9 @@ module AbstractController autoload :Translation autoload :AssetPaths autoload :UrlFor + + def self.eager_load! + super + AbstractController::Caching.eager_load! + end end diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 8edea0f52b..aa06f70433 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -1,13 +1,11 @@ require 'erubis' +require 'abstract_controller/error' require 'active_support/configurable' require 'active_support/descendants_tracker' require 'active_support/core_ext/module/anonymous' require 'active_support/core_ext/module/attr_internal' module AbstractController - class Error < StandardError #:nodoc: - end - # Raised when a non-existing controller action is triggered. class ActionNotFound < StandardError end @@ -78,7 +76,7 @@ module AbstractController end end - # action_methods are cached and there is sometimes need to refresh + # action_methods are cached and there is sometimes a need to refresh # them. ::clear_action_methods! allows you to do that, so next time # you run action_methods, they will be recalculated. def clear_action_methods! @@ -152,6 +150,13 @@ module AbstractController _find_action_name(action_name) end + # Tests if a response body is set. Used to determine if the + # +process_action+ callback needs to be terminated in + # +AbstractController::Callbacks+. + def performed? + response_body + end + # Returns true if the given controller is capable of rendering # a path. A subclass of +AbstractController::Base+ # may return false. An Email controller for example does not diff --git a/actionpack/lib/abstract_controller/caching.rb b/actionpack/lib/abstract_controller/caching.rb new file mode 100644 index 0000000000..0dea50889a --- /dev/null +++ b/actionpack/lib/abstract_controller/caching.rb @@ -0,0 +1,62 @@ +module AbstractController + module Caching + extend ActiveSupport::Concern + extend ActiveSupport::Autoload + + eager_autoload do + autoload :Fragments + end + + module ConfigMethods + def cache_store + config.cache_store + end + + def cache_store=(store) + config.cache_store = ActiveSupport::Cache.lookup_store(store) + end + + private + def cache_configured? + perform_caching && cache_store + end + end + + include ConfigMethods + include AbstractController::Caching::Fragments + + included do + extend ConfigMethods + + config_accessor :default_static_extension + self.default_static_extension ||= '.html' + + config_accessor :perform_caching + self.perform_caching = true if perform_caching.nil? + + class_attribute :_view_cache_dependencies + self._view_cache_dependencies = [] + helper_method :view_cache_dependencies if respond_to?(:helper_method) + end + + module ClassMethods + def view_cache_dependency(&dependency) + self._view_cache_dependencies += [dependency] + end + end + + def view_cache_dependencies + self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact + end + + protected + # Convenience accessor. + def cache(key, options = {}, &block) + if cache_configured? + cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) + else + yield + end + end + end +end diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/abstract_controller/caching/fragments.rb index b9ad51a9cf..3257a731ed 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/abstract_controller/caching/fragments.rb @@ -1,4 +1,4 @@ -module ActionController +module AbstractController module Caching # Fragment caching is used for caching various blocks within # views without caching the entire action as a whole. This is @@ -135,13 +135,8 @@ module ActionController end def instrument_fragment_cache(name, key) # :nodoc: - payload = { - controller: controller_name, - action: action_name, - key: key - } - - ActiveSupport::Notifications.instrument("#{name}.action_controller", payload) { yield } + payload = instrument_payload(key) + ActiveSupport::Notifications.instrument("#{name}.#{instrument_name}", payload) { yield } end end end diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index d63ce9c1c3..3ef8da86fa 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -9,7 +9,7 @@ module AbstractController included do define_callbacks :process_action, - terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.response_body }, + terminator: ->(controller, result_lambda) { result_lambda.call if result_lambda.is_a?(Proc); controller.performed? }, skip_after_callbacks_if_terminated: true end diff --git a/actionpack/lib/abstract_controller/error.rb b/actionpack/lib/abstract_controller/error.rb new file mode 100644 index 0000000000..7fafce4dd4 --- /dev/null +++ b/actionpack/lib/abstract_controller/error.rb @@ -0,0 +1,4 @@ +module AbstractController + class Error < StandardError #:nodoc: + end +end diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index d84c238a62..ab4355296b 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -38,7 +38,8 @@ module AbstractController end # Declare a controller method as a helper. For example, the following - # makes the +current_user+ controller method available to the view: + # makes the +current_user+ and +logged_in?+ controller methods available + # to the view: # class ApplicationController < ActionController::Base # helper_method :current_user, :logged_in? # diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index e765d73ce4..4ba2c26949 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -1,3 +1,4 @@ +require 'abstract_controller/error' require 'active_support/concern' require 'active_support/core_ext/class/attribute' require 'action_view' @@ -59,9 +60,7 @@ module AbstractController end DEFAULT_PROTECTED_INSTANCE_VARIABLES = Set.new %i( - @_action_name @_response_body @_formats @_prefixes @_config - @_view_context_class @_view_renderer @_lookup_context - @_routes @_db_runtime + @_action_name @_response_body @_formats @_prefixes ) # This method should return a hash with assigns. @@ -123,7 +122,7 @@ module AbstractController def _normalize_render(*args, &block) options = _normalize_args(*args, &block) #TODO: remove defined? when we restore AP <=> AV dependency - if defined?(request) && request.variant.present? + if defined?(request) && !request.nil? && request.variant.present? options[:variant] = request.variant end _normalize_options(options) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 40f33a9de0..62f5905205 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -9,12 +9,15 @@ module ActionController autoload :API autoload :Base - autoload :Caching autoload :Metal autoload :Middleware autoload :Renderer autoload :FormBuilder + eager_autoload do + autoload :Caching + end + autoload_under "metal" do autoload :ConditionalGet autoload :Cookies @@ -47,11 +50,6 @@ module ActionController autoload :TestCase, 'action_controller/test_case' autoload :TemplateAssertions, 'action_controller/test_case' - - def self.eager_load! - super - ActionController::Caching.eager_load! - end end # Common Active Support usage in Action Controller diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index ff12705abe..6bbebb7b4c 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -14,22 +14,22 @@ module ActionController # flash, assets, and so on. This makes the entire controller stack thinner, # suitable for API applications. It doesn't mean you won't have such # features if you need them: they're all available for you to include in - # your application, they're just not part of the default API Controller stack. + # your application, they're just not part of the default API controller stack. # - # By default, only the ApplicationController in a \Rails application inherits - # from <tt>ActionController::API</tt>. All other controllers in turn inherit - # from ApplicationController. + # Normally, +ApplicationController+ is the only controller that inherits from + # <tt>ActionController::API</tt>. All other controllers in turn inherit from + # +ApplicationController+. # # A sample controller could look like this: # # class PostsController < ApplicationController # def index - # @posts = Post.all - # render json: @posts + # posts = Post.all + # render json: posts # end # end # - # Request, response and parameters objects all work the exact same way as + # Request, response, and parameters objects all work the exact same way as # <tt>ActionController::Base</tt>. # # == Renders @@ -37,18 +37,18 @@ module ActionController # The default API Controller stack includes all renderers, which means you # can use <tt>render :json</tt> and brothers freely in your controllers. Keep # in mind that templates are not going to be rendered, so you need to ensure - # your controller is calling either <tt>render</tt> or <tt>redirect</tt> in - # all actions, otherwise it will return 204 No Content response. + # your controller is calling either <tt>render</tt> or <tt>redirect_to</tt> in + # all actions, otherwise it will return 204 No Content. # # def show - # @post = Post.find(params[:id]) - # render json: @post + # post = Post.find(params[:id]) + # render json: post # end # # == Redirects # # Redirects are used to move from one action to another. You can use the - # <tt>redirect</tt> method in your controllers in the same way as + # <tt>redirect_to</tt> method in your controllers in the same way as in # <tt>ActionController::Base</tt>. For example: # # def create @@ -56,7 +56,7 @@ module ActionController # # do stuff here # end # - # == Adding new behavior + # == Adding New Behavior # # In some scenarios you may want to add back some functionality provided by # <tt>ActionController::Base</tt> that is not present by default in @@ -72,18 +72,19 @@ module ActionController # # class PostsController < ApplicationController # def index - # @posts = Post.all + # posts = Post.all # # respond_to do |format| - # format.json { render json: @posts } - # format.xml { render xml: @posts } + # format.json { render json: posts } + # format.xml { render xml: posts } # end # end # end # - # Quite straightforward. Make sure to check <tt>ActionController::Base</tt> - # available modules if you want to include any other functionality that is - # not provided by <tt>ActionController::API</tt> out of the box. + # Quite straightforward. Make sure to check the modules included in + # <tt>ActionController::Base</tt> if you want to use any other + # functionality that is not provided by <tt>ActionController::API</tt> + # out of the box. class API < Metal abstract! diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 04e5922ce8..d546d7260c 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -229,7 +229,7 @@ module ActionController HttpAuthentication::Digest::ControllerMethods, HttpAuthentication::Token::ControllerMethods, - # Before callbacks should also be executed the earliest as possible, so + # Before callbacks should also be executed as early as possible, so # also include them at the bottom. AbstractController::Callbacks, @@ -251,9 +251,10 @@ module ActionController setup_renderer! # Define some internal variables that should not be propagated to the view. - PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + [ - :@_params, :@_response, :@_request, - :@_view_runtime, :@_stream, :@_url_options, :@_action_has_layout ] + PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + %i( + @_params @_response @_request @_config @_url_options @_action_has_layout @_view_context_class + @_view_renderer @_lookup_context @_routes @_view_runtime @_db_runtime @_helper_proxy + ) def _protected_ivars # :nodoc: PROTECTED_IVARS diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 0b8fa2ea09..a9a8508abc 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -1,6 +1,3 @@ -require 'fileutils' -require 'uri' - module ActionController # \Caching is a cheap way of speeding up slow applications by keeping the result of # calculations, renderings, and database calls around for subsequent requests. @@ -23,65 +20,25 @@ module ActionController # config.action_controller.cache_store = :mem_cache_store, Memcached::Rails.new('localhost:11211') # config.action_controller.cache_store = MyOwnStore.new('parameter') module Caching - extend ActiveSupport::Concern extend ActiveSupport::Autoload - - eager_autoload do - autoload :Fragments - end - - module ConfigMethods - def cache_store - config.cache_store - end - - def cache_store=(store) - config.cache_store = ActiveSupport::Cache.lookup_store(store) - end - - private - def cache_configured? - perform_caching && cache_store - end - end - - include AbstractController::Callbacks - - include ConfigMethods - include Fragments + extend ActiveSupport::Concern included do - extend ConfigMethods - - config_accessor :default_static_extension - self.default_static_extension ||= '.html' - - config_accessor :perform_caching - self.perform_caching = true if perform_caching.nil? - - class_attribute :_view_cache_dependencies - self._view_cache_dependencies = [] - helper_method :view_cache_dependencies if respond_to?(:helper_method) + include AbstractController::Caching end - module ClassMethods - def view_cache_dependency(&dependency) - self._view_cache_dependencies += [dependency] - end - end + private - def view_cache_dependencies - self.class._view_cache_dependencies.map { |dep| instance_exec(&dep) }.compact - end + def instrument_payload(key) + { + controller: controller_name, + action: action_name, + key: key + } + end - protected - # Convenience accessor. - def cache(key, options = {}, &block) - if cache_configured? - cache_store.fetch(ActiveSupport::Cache.expand_cache_key(key, :controller), options, &block) - else - yield - end + def instrument_name + "action_controller" end end end diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb index 4c9f14e409..a0917b4fdb 100644 --- a/actionpack/lib/action_controller/log_subscriber.rb +++ b/actionpack/lib/action_controller/log_subscriber.rb @@ -25,7 +25,9 @@ module ActionController status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name) end message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms" - message << " (#{additions.join(" | ".freeze)})" unless additions.blank? + message << " (#{additions.join(" | ".freeze)})" unless additions.empty? + message << "\n\n" if defined?(Rails.env) && Rails.env.development? + message end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index f6a93a8940..f6e67b02d7 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -174,10 +174,8 @@ module ActionController def response_body=(body) body = [body] unless body.nil? || body.respond_to?(:each) response.reset_body! - body.each { |part| - next if part.empty? - response.write part - } + return unless body + response.body = body super end diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb index 6c6f8381ff..cef65a362c 100644 --- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb +++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb @@ -1,5 +1,5 @@ module ActionController - module BasicImplicitRender + module BasicImplicitRender # :nodoc: def send_action(method, *args) super.tap { default_render unless performed? } end diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index f8e0d9cf6c..e21449f376 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -36,8 +36,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -86,12 +101,16 @@ module ActionController # # before_action { fresh_when @article, template: 'widgets/show' } # - def fresh_when(object = nil, etag: object, last_modified: nil, public: false, template: nil) + def fresh_when(object = nil, etag: nil, weak_etag: nil, strong_etag: nil, last_modified: nil, public: false, template: nil) + weak_etag ||= etag || object unless strong_etag last_modified ||= object.try(:updated_at) || object.try(:maximum, :updated_at) - if etag || template - response.etag = combine_etags(etag: etag, last_modified: last_modified, - public: public, template: template) + if strong_etag + response.strong_etag = combine_etags strong_etag, + last_modified: last_modified, public: public, template: template + elsif weak_etag || template + response.weak_etag = combine_etags weak_etag, + last_modified: last_modified, public: public, template: template end response.last_modified = last_modified if last_modified @@ -107,8 +126,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -180,12 +214,12 @@ module ActionController # super if stale? @article, template: 'widgets/show' # end # - def stale?(object = nil, etag: object, last_modified: nil, public: nil, template: nil) - fresh_when(object, etag: etag, last_modified: last_modified, public: public, template: template) + def stale?(object = nil, **freshness_kwargs) + fresh_when(object, **freshness_kwargs) !request.fresh?(response) end - # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a +private+ + # Sets an HTTP 1.1 Cache-Control header. Defaults to issuing a +private+ # instruction, so that intermediate caches must not cache the response. # # expires_in 20.minutes @@ -195,7 +229,7 @@ module ActionController # This method will overwrite an existing Cache-Control header. # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities. # - # The method will also ensure a HTTP Date header for client compatibility. + # The method will also ensure an HTTP Date header for client compatibility. def expires_in(seconds, options = {}) response.cache_control.merge!( :max_age => seconds, @@ -208,7 +242,7 @@ module ActionController response.date = Time.now unless response.date? end - # Sets a HTTP 1.1 Cache-Control header of <tt>no-cache</tt> so no caching should + # Sets an HTTP 1.1 Cache-Control header of <tt>no-cache</tt> so no caching should # occur by the browser or intermediate caches (like caching proxy servers). def expires_now response.cache_control.replace(:no_cache => true) @@ -216,26 +250,23 @@ module ActionController # Cache or yield the block. The cache is supposed to never expire. # - # You can use this method when you have a HTTP response that never changes, + # You can use this method when you have an HTTP response that never changes, # and the browser and proxies should cache it indefinitely. # # * +public+: By default, HTTP responses are private, cached only on the # user's web browser. To allow proxies to cache the response, set +true+ to # indicate that they can serve the cached response to all users. - # - # * +version+: the version passed as a key for the cache. - def http_cache_forever(public: false, version: 'v1') + def http_cache_forever(public: false) expires_in 100.years, public: public - yield if stale?(etag: "#{version}-#{request.fullpath}", + yield if stale?(etag: request.fullpath, last_modified: Time.new(2011, 1, 1).utc, public: public) end private - def combine_etags(options) - etags = etaggers.map { |etagger| instance_exec(options, &etagger) }.compact - etags.unshift options[:etag] + def combine_etags(validator, options) + [validator, *etaggers.map { |etagger| instance_exec(options, &etagger) }].compact end end end diff --git a/actionpack/lib/action_controller/metal/cookies.rb b/actionpack/lib/action_controller/metal/cookies.rb index f8efb2b076..44925641a1 100644 --- a/actionpack/lib/action_controller/metal/cookies.rb +++ b/actionpack/lib/action_controller/metal/cookies.rb @@ -3,7 +3,7 @@ module ActionController #:nodoc: extend ActiveSupport::Concern included do - helper_method :cookies + helper_method :cookies if defined?(helper_method) end private diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 957e7a3019..6cd6130032 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -25,14 +25,13 @@ module ActionController #:nodoc: # * <tt>:filename</tt> - suggests a filename for the browser to use. # Defaults to <tt>File.basename(path)</tt>. # * <tt>:type</tt> - specifies an HTTP content type. - # You can specify either a string or a symbol for a registered type register with - # <tt>Mime::Type.register</tt>, for example :json - # If omitted, type will be guessed from the file extension specified in <tt>:filename</tt>. - # If no content type is registered for the extension, default type 'application/octet-stream' will be used. + # You can specify either a string or a symbol for a registered type with <tt>Mime::Type.register</tt>, for example :json. + # If omitted, the type will be inferred from the file extension specified in <tt>:filename</tt>. + # If no content type is registered for the extension, the default type 'application/octet-stream' will be used. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. # Valid values are 'inline' and 'attachment' (default). # * <tt>:status</tt> - specifies the status code to send with the response. Defaults to 200. - # * <tt>:url_based_filename</tt> - set to +true+ if you want the browser guess the filename from + # * <tt>:url_based_filename</tt> - set to +true+ if you want the browser to guess the filename from # the URL, which is necessary for i18n filenames on certain browsers # (setting <tt>:filename</tt> overrides this option). # @@ -79,14 +78,14 @@ module ActionController #:nodoc: # <tt>render plain: data</tt>, but also allows you to specify whether # the browser should display the response as a file attachment (i.e. in a # download dialog) or as inline data. You may also set the content type, - # the apparent file name, and other things. + # the file name, and other things. # # Options: # * <tt>:filename</tt> - suggests a filename for the browser to use. - # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. You can specify - # either a string or a symbol for a registered type register with <tt>Mime::Type.register</tt>, for example :json - # If omitted, type will be guessed from the file extension specified in <tt>:filename</tt>. - # If no content type is registered for the extension, default type 'application/octet-stream' will be used. + # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. + # You can specify either a string or a symbol for a registered type with <tt>Mime::Type.register</tt>, for example :json. + # If omitted, type will be inferred from the file extension specified in <tt>:filename</tt>. + # If no content type is registered for the extension, the default type 'application/octet-stream' will be used. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. # Valid values are 'inline' and 'attachment' (default). # * <tt>:status</tt> - specifies the status code to send with the response. Defaults to 200. diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb index e31d65aac2..ea8e91ce24 100644 --- a/actionpack/lib/action_controller/metal/force_ssl.rb +++ b/actionpack/lib/action_controller/metal/force_ssl.rb @@ -2,17 +2,17 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/slice' module ActionController - # This module provides a method which will redirect browser to use HTTPS + # This module provides a method which will redirect the browser to use HTTPS # protocol. This will ensure that user's sensitive information will be - # transferred safely over the internet. You _should_ always force browser + # transferred safely over the internet. You _should_ always force the browser # to use HTTPS when you're transferring sensitive information such as # user authentication, account information, or credit card information. # # Note that if you are really concerned about your application security, # you might consider using +config.force_ssl+ in your config file instead. # That will ensure all the data transferred via HTTPS protocol and prevent - # user from getting session hijacked when accessing the site under unsecured - # HTTP protocol. + # the user from getting their session hijacked when accessing the site over + # unsecured HTTP protocol. module ForceSSL extend ActiveSupport::Concern include AbstractController::Callbacks diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index d3853e2e83..295f0cb66f 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -5,7 +5,7 @@ module ActionController # # In addition to using the standard template helpers provided, creating custom helpers to # extract complicated logic or reusable functionality is strongly encouraged. By default, each controller - # will include all helpers. These helpers are only accessible on the controller through <tt>.helpers</tt> + # will include all helpers. These helpers are only accessible on the controller through <tt>#helpers</tt> # # In previous versions of \Rails the controller will include a helper which # matches the name of the controller, e.g., <tt>MyController</tt> will automatically @@ -71,7 +71,7 @@ module ActionController attrs.flatten.each { |attr| helper_method(attr, "#{attr}=") } end - # Provides a proxy to access helpers methods from outside the view. + # Provides a proxy to access helper methods from outside the view. def helpers @helper_proxy ||= begin proxy = ActionView::Base.new @@ -113,5 +113,10 @@ module ActionController all_helpers_from_path(helpers_path) end end + + # Provides a proxy to access helper methods from outside the view. + def helpers + @_helper_proxy ||= view_context + end end end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 35be6d9300..4639348509 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -310,9 +310,9 @@ module ActionController end # Might want a shorter timeout depending on whether the request - # is a PATCH, PUT, or POST, and if client is browser or web service. + # is a PATCH, PUT, or POST, and if the client is a browser or web service. # Can be much shorter if the Stale directive is implemented. This would - # allow a user to use new nonce without prompting user again for their + # allow a user to use new nonce without prompting the user again for their # username and password. def validate_nonce(secret_key, request, value, seconds_to_timeout=5*60) return false if value.nil? @@ -347,7 +347,12 @@ module ActionController # private # def authenticate # authenticate_or_request_with_http_token do |token, options| - # token == TOKEN + # # Compare the tokens in a time-constant manner, to mitigate + # # timing attacks. + # ActiveSupport::SecurityUtils.secure_compare( + # ::Digest::SHA256.hexdigest(token), + # ::Digest::SHA256.hexdigest(TOKEN) + # ) # end # end # end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 17fcc2fa02..6192fc0f9c 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,29 +1,62 @@ +require 'active_support/core_ext/string/strip' + module ActionController + # Handles implicit rendering for a controller action that does not + # explicitly respond with +render+, +respond_to+, +redirect+, or +head+. + # + # For API controllers, the implicit response is always 204 No Content. + # + # For all other controllers, we use these heuristics to decide whether to + # render a template, raise an error for a missing template, or respond with + # 204 No Content: + # + # First, if we DO find a template, it's rendered. Template lookup accounts + # for the action name, locales, format, variant, template handlers, and more + # (see +render+ for details). + # + # Second, if we DON'T find a template but the controller action does have + # templates for other formats, variants, etc., then we trust that you meant + # to provide a template for this response, too, and we raise + # <tt>ActionController::UnknownFormat</tt> with an explanation. + # + # Third, if we DON'T find a template AND the request is a page load in a web + # browser (technically, a non-XHR GET request for an HTML response) where + # you reasonably expect to have rendered a template, then we raise + # <tt>ActionView::UnknownFormat</tt> with an explanation. + # + # Finally, if we DON'T find a template AND the request isn't a browser page + # load, then we implicitly respond with 204 No Content. module ImplicitRender + # :stopdoc: include BasicImplicitRender - # Renders the template corresponding to the controller action, if it exists. - # The action name, format, and variant are all taken into account. - # For example, the "new" action with an HTML format and variant "phone" - # would try to render the <tt>new.html+phone.erb</tt> template. - # - # If no template is found <tt>ActionController::BasicImplicitRender</tt>'s implementation is called, unless - # a block is passed. In that case, it will override the super implementation. - # - # default_render do - # head 404 # No template was found - # end def default_render(*args) if template_exists?(action_name.to_s, _prefixes, variants: request.variant) render(*args) + elsif any_templates?(action_name.to_s, _prefixes) + message = "#{self.class.name}\##{action_name} is missing a template " \ + "for this request format and variant.\n" \ + "\nrequest.formats: #{request.formats.map(&:to_s).inspect}" \ + "\nrequest.variant: #{request.variant.inspect}" + + raise ActionController::UnknownFormat, message + elsif interactive_browser_request? + message = "#{self.class.name}\##{action_name} is missing a template " \ + "for this request format and variant.\n\n" \ + "request.formats: #{request.formats.map(&:to_s).inspect}\n" \ + "request.variant: #{request.variant.inspect}\n\n" \ + "NOTE! For XHR/Ajax or API requests, this action would normally " \ + "respond with 204 No Content: an empty white screen. Since you're " \ + "loading it in a web browser, we assume that you expected to " \ + "actually render a template, not… nothing, so we're showing an " \ + "error to be extra-clear. If you expect 204 No Content, carry on. " \ + "That's what you'll get from an XHR or API request. Give it a shot." + + raise ActionController::UnknownFormat, message else - if block_given? - yield(*args) - else - logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger - super - end + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super end end @@ -32,5 +65,10 @@ module ActionController "default_render" end end + + private + def interactive_browser_request? + request.get? && request.format == Mime[:html] && !request.xhr? + end end end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 3dbf34eb2a..624a6d5b76 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,9 +19,10 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :headers => request.headers, + :format => request.format.ref, :method => request.request_method, - :path => (request.fullpath rescue "unknown") + :path => request.fullpath } ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) @@ -74,8 +75,8 @@ module ActionController ActiveSupport::Notifications.instrument("halted_callback.action_controller", :filter => filter) end - # A hook which allows you to clean up any time taken into account in - # views wrongly, like database querying time. + # A hook which allows you to clean up any time, wrongly taken into account in + # views, like database querying time. # # def cleanup_view_runtime # super - time_taken_in_something_expensive diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index e3c540bf5f..5d395cd8bd 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -3,7 +3,7 @@ require 'delegate' require 'active_support/json' module ActionController - # Mix this module in to your controller, and all actions in that controller + # Mix this module into your controller, and all actions in that controller # will be able to stream data to the client as it's written. # # class MyController < ActionController::Base @@ -20,7 +20,7 @@ module ActionController # end # end # - # There are a few caveats with this use. You *cannot* write headers after the + # There are a few caveats with this module. You *cannot* write headers after the # response has been committed (Response#committed? will return truthy). # Calling +write+ or +close+ on the response stream will cause the response # object to be committed. Make sure all headers are set before calling write @@ -163,14 +163,6 @@ module ActionController end end - def each - @response.sending! - while str = @buf.pop - yield str - end - @response.sent! - end - # Write a 'close' event to the buffer; the producer/writing thread # uses this to notify us that it's finished supplying content. # @@ -210,6 +202,14 @@ module ActionController def call_on_error @error_callback.call end + + private + + def each_chunk(&block) + while str = @buf.pop + yield str + end + end end class Response < ActionDispatch::Response #:nodoc: all @@ -237,39 +237,55 @@ module ActionController # This processes the action in a child thread. It lets us return the # response code and headers back up the rack stack, and still process # the body in parallel with sending data to the client - Thread.new { - t2 = Thread.current - t2.abort_on_exception = true - - # Since we're processing the view in a different thread, copy the - # thread locals from the main thread to the child thread. :'( - locals.each { |k,v| t2[k] = v } - - begin - super(name) - rescue => e - if @_response.committed? - begin - @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html - @_response.stream.call_on_error - rescue => exception - log_error(exception) - ensure - log_error(e) - @_response.stream.close + new_controller_thread { + ActiveSupport::Dependencies.interlock.running do + t2 = Thread.current + + # Since we're processing the view in a different thread, copy the + # thread locals from the main thread to the child thread. :'( + locals.each { |k,v| t2[k] = v } + + begin + super(name) + rescue => e + if @_response.committed? + begin + @_response.stream.write(ActionView::Base.streaming_completion_on_exception) if request.format == :html + @_response.stream.call_on_error + rescue => exception + log_error(exception) + ensure + log_error(e) + @_response.stream.close + end + else + error = e end - else - error = e + ensure + @_response.commit! end - ensure - @_response.commit! end } - @_response.await_commit + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + @_response.await_commit + end + raise error if error end + # Spawn a new thread to serve up the controller in. This is to get + # around the fact that Rack isn't based around IOs and we need to use + # a thread to stream data from the response bodies. Nobody should call + # this method except in Rails internals. Seriously! + def new_controller_thread # :nodoc: + Thread.new { + t2 = Thread.current + t2.abort_on_exception = true + yield + } + end + def log_error(exception) logger = ActionController::Base.logger return unless logger diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 173a14a1d2..2e89af1a5e 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -198,7 +198,7 @@ module ActionController #:nodoc: _process_format(format) _set_rendered_content_type format response = collector.response - response ? response.call : render({}) + response.call if response else raise ActionController::UnknownFormat end diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index b13ba06962..3c7cc15627 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -84,7 +84,7 @@ module ActionController # 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. + # options and the behavior is identical. def redirect_back(fallback_location:, **args) if referer = request.headers["Referer"] redirect_to referer, **args diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 90fb34e386..1735609cd9 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -103,7 +103,7 @@ module ActionController # # Both <tt>ActionController::Base</tt> and <tt>ActionController::API</tt> # include <tt>ActionController::Renderers::All</tt>, making all renderers - # avaialable in the controller. See <tt>Renderers::RENDERERS</tt> and <tt>Renderers.add</tt>. + # available in the controller. See <tt>Renderers::RENDERERS</tt> and <tt>Renderers.add</tt>. # # Since <tt>ActionController::Metal</tt> controllers cannot render, the controller # must include <tt>AbstractController::Rendering</tt>, <tt>ActionController::Rendering</tt>, diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 91b3403ad5..0559fbc6ce 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -213,7 +213,7 @@ module ActionController #:nodoc: if !verified_request? if logger && log_warning_on_csrf_failure - logger.warn "Can't verify CSRF token authenticity" + logger.warn "Can't verify CSRF token authenticity." end handle_unverified_request end @@ -235,7 +235,9 @@ module ActionController #:nodoc: # we aren't serving an unauthorized cross-origin response. def verify_same_origin_request if marked_for_same_origin_verification? && non_xhr_javascript_response? - logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING if logger + if logger && log_warning_on_csrf_failure + logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING + end raise ActionController::InvalidCrossOriginRequest, CROSS_ORIGIN_JAVASCRIPT_WARNING end end @@ -378,7 +380,9 @@ module ActionController #:nodoc: end def xor_byte_strings(s1, s2) - s1.bytes.zip(s2.bytes).map { |(c1,c2)| c1 ^ c2 }.pack('c*') + s2_bytes = s2.bytes + s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } + s2_bytes.pack('C*') end # The form's authenticity parameter. Override to provide your own. @@ -403,7 +407,8 @@ module ActionController #:nodoc: end def normalize_action_path(action_path) - action_path.split('?').first.to_s.chomp('/') + uri = URI.parse(action_path) + uri.path.chomp('/') end end end diff --git a/actionpack/lib/action_controller/metal/rescue.rb b/actionpack/lib/action_controller/metal/rescue.rb index 81b9a7b9ed..17f4030f25 100644 --- a/actionpack/lib/action_controller/metal/rescue.rb +++ b/actionpack/lib/action_controller/metal/rescue.rb @@ -1,18 +1,11 @@ module ActionController #:nodoc: - # This module is responsible to provide `rescue_from` helpers - # to controllers and configure when detailed exceptions must be + # This module is responsible for providing `rescue_from` helpers + # to controllers and configuring when detailed exceptions must be # shown. module Rescue extend ActiveSupport::Concern include ActiveSupport::Rescuable - def rescue_with_handler(exception) - if exception.cause && handler_for_rescue(exception.cause) - exception = exception.cause - end - super(exception) - end - # Override this method if you want to customize when detailed # exceptions must be shown. This method is only called when # consider_all_requests_local is false. By default, it returns @@ -27,7 +20,7 @@ module ActionController #:nodoc: super rescue Exception => exception request.env['action_dispatch.show_detailed_exceptions'] ||= show_detailed_exceptions? - rescue_with_handler(exception) || raise(exception) + rescue_with_handler(exception) || raise end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index d3382ef296..46589901fd 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -43,7 +43,7 @@ module ActionController # == Action Controller \Parameters # - # Allows to choose which attributes should be whitelisted for mass updating + # Allows you to choose which attributes should be whitelisted for mass updating # and thus prevent accidentally exposing that which shouldn't be exposed. # Provides two methods for this purpose: #require and #permit. The former is # used to mark parameters as required. The latter is used to set the parameter @@ -109,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?, :values, :has_value?, :value?, :empty?, :include?, :inspect, + delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these @@ -122,16 +122,6 @@ module ActionController cattr_accessor :always_permitted_parameters self.always_permitted_parameters = %w( controller action ) - def self.const_missing(const_name) - return super unless const_name == :NEVER_UNPERMITTED_PARAMS - ActiveSupport::Deprecation.warn(<<-MSG.squish) - `ActionController::Parameters::NEVER_UNPERMITTED_PARAMS` has been deprecated. - Use `ActionController::Parameters.always_permitted_parameters` instead. - MSG - - always_permitted_parameters - end - # Returns a new instance of <tt>ActionController::Parameters</tt>. # Also, sets the +permitted+ attribute to the default value of # <tt>ActionController::Parameters.permit_all_parameters</tt>. @@ -154,17 +144,21 @@ module ActionController end # Returns true if another +Parameters+ object contains the same content and - # permitted flag, or other Hash-like object contains the same content. This - # override is in place so you can perform a comparison with `Hash`. - def ==(other_hash) - if other_hash.respond_to?(:permitted?) - super + # permitted flag. + def ==(other) + if other.respond_to?(:permitted?) + self.permitted? == other.permitted? && self.parameters == other.parameters + elsif other.is_a?(Hash) + ActiveSupport::Deprecation.warn <<-WARNING.squish + Comparing equality between `ActionController::Parameters` and a + `Hash` is deprecated and will be removed in Rails 5.1. Please only do + comparisons between instances of `ActionController::Parameters`. If + you need to compare to a hash, first convert it using + `ActionController::Parameters#new`. + WARNING + @parameters == other.with_indifferent_access else - if other_hash.is_a?(Hash) - @parameters == other_hash.with_indifferent_access - else - @parameters == other_hash - end + @parameters == other end end @@ -190,12 +184,19 @@ module ActionController # Returns an unsafe, unfiltered # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this # parameter. + # + # params = ActionController::Parameters.new({ + # name: 'Senjougahara Hitagi', + # oddity: 'Heavy stone crab' + # }) + # params.to_unsafe_h + # # => {"name"=>"Senjougahara Hitagi", "oddity" => "Heavy stone crab"} def to_unsafe_h convert_parameters_to_hashes(@parameters, :to_unsafe_h) end alias_method :to_unsafe_hash, :to_unsafe_h - # Convert all hashes in values into parameters, then yield each pair like + # Convert all hashes in values into parameters, then yield each pair in # the same way as <tt>Hash#each_pair</tt> def each_pair(&block) @parameters.each_pair do |key, value| @@ -277,7 +278,7 @@ module ActionController # params = ActionController::Parameters.new(user: { ... }, profile: { ... }) # user_params, profile_params = params.require(:user, :profile) # - # Otherwise, the method reraises the first exception found: + # Otherwise, the method re-raises the first exception found: # # params = ActionController::Parameters.new(user: {}, profile: {}) # user_params, profile_params = params.require(:user, :profile) @@ -436,6 +437,21 @@ module ActionController ) end + if Hash.method_defined?(:dig) + # Extracts the nested parameter from the given +keys+ by calling +dig+ + # at each step. Returns +nil+ if any intermediate step is +nil+. + # + # params = ActionController::Parameters.new(foo: { bar: { baz: 1 } }) + # params.dig(:foo, :bar, :baz) # => 1 + # params.dig(:foo, :zot, :xyz) # => nil + # + # params2 = ActionController::Parameters.new(foo: [10, 11, 12]) + # params2.dig(:foo, 1) # => 11 + def dig(*keys) + convert_value_to_parameters(@parameters.dig(*keys)) + end + end + # Returns a new <tt>ActionController::Parameters</tt> instance that # includes only the given +keys+. If the given +keys+ # don't exist, returns an empty hash. @@ -584,6 +600,10 @@ module ActionController dup end + def inspect + "<#{self.class} #{@parameters} permitted: #{@permitted}>" + end + def method_missing(method_sym, *args, &block) if @parameters.respond_to?(method_sym) message = <<-DEPRECATE.squish @@ -603,12 +623,14 @@ module ActionController end protected + attr_reader :parameters + def permitted=(new_permitted) @permitted = new_permitted end def fields_for_style? - @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) } + @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } end private @@ -734,6 +756,10 @@ module ActionController end end + def non_scalar?(value) + value.is_a?(Array) || value.is_a?(Parameters) + end + EMPTY_ARRAY = [] def hash_filter(params, filter) filter = filter.with_indifferent_access @@ -748,7 +774,7 @@ module ActionController array_of_permitted_scalars?(self[key]) do |val| params[key] = val end - else + elsif non_scalar?(value) # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| element.permit(*Array.wrap(filter[key])) @@ -771,7 +797,7 @@ module ActionController # # class PeopleController < ActionController::Base # # Using "Person.create(params[:person])" would raise an - # # ActiveModel::ForbiddenAttributes exception because it'd + # # ActiveModel::ForbiddenAttributesError exception because it'd # # be using mass assignment without an explicit permit step. # # This is the recommended form: # def create @@ -799,7 +825,8 @@ module ActionController # end # # In order to use <tt>accepts_nested_attributes_for</tt> with Strong \Parameters, you - # will need to specify which nested attributes should be whitelisted. + # will need to specify which nested attributes should be whitelisted. You might want + # to allow +:id+ and +:_destroy+, see ActiveRecord::NestedAttributes for more information. # # class Person # has_many :pets @@ -819,7 +846,7 @@ module ActionController # # It's mandatory to specify the nested attributes that should be whitelisted. # # If you use `permit` with just the key that points to the nested attributes hash, # # it will return an empty hash. - # params.require(:person).permit(:name, :age, pets_attributes: [ :name, :category ]) + # params.require(:person).permit(:name, :age, pets_attributes: [ :id, :name, :category ]) # end # end # diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index e4d19e9dba..a8c8d66682 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -1,7 +1,7 @@ require 'active_support/core_ext/hash/keys' module ActionController - # ActionController::Renderer allows to render arbitrary templates + # ActionController::Renderer allows you to render arbitrary templates # without requirement of being in controller actions. # # You get a concrete renderer class by invoking ActionController::Base#renderer. @@ -13,11 +13,11 @@ module ActionController # # ApplicationController.renderer.render template: '...' # - # You can use a shortcut on controller to replace previous example with: + # You can use this shortcut in a controller, instead of the previous example: # # ApplicationController.render template: '...' # - # #render method allows you to use any options as when rendering in controller. + # #render allows you to use the same options that you can use when rendering in a controller. # For example, # # FooController.render :action, locals: { ... }, assigns: { ... } @@ -45,7 +45,7 @@ module ActionController }.freeze # Create a new renderer instance for a specific controller class. - def self.for(controller, env = {}, defaults = DEFAULTS) + def self.for(controller, env = {}, defaults = DEFAULTS.dup) new(controller, env, defaults) end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index b43bb9dc17..b1b3e87934 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -12,6 +12,17 @@ module ActionController include Testing::Functional end + module Live + # Disable controller / rendering threads in tests. User tests can access + # the database on the main thread, so they could open a txn, then the + # controller thread will open a new connection and try to access data + # that's only visible to the main thread's txn. This is the problem in #23483 + remove_method :new_controller_thread + def new_controller_thread # :nodoc: + yield + end + end + # 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: @@ -41,7 +52,7 @@ module ActionController self.session = session self.session_options = TestSession::DEFAULT_OPTIONS @custom_param_parsers = { - Mime[:xml] => lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } + xml: lambda { |raw_post| Hash.from_xml(raw_post)['hash'] } } end @@ -94,7 +105,7 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - @custom_param_parsers[content_mime_type] = ->(_) { non_path_parameters } + @custom_param_parsers[content_mime_type.symbol] = ->(_) { non_path_parameters } data = non_path_parameters.to_query end end @@ -417,7 +428,7 @@ module ActionController end alias xhr :xml_http_request - # Simulate a HTTP request to +action+ by specifying request method, + # Simulate an HTTP request to +action+ by specifying request method, # parameters and set/volley the response. # # - +action+: The controller action to call. @@ -516,32 +527,37 @@ module ActionController @request.set_header k, @controller.config.relative_url_root end - @controller.recycle! - @controller.dispatch(action, @request, @response) - @request = @controller.request - @response = @controller.response + begin + @controller.recycle! + @controller.dispatch(action, @request, @response) + ensure + @request = @controller.request + @response = @controller.response - @request.delete_header 'HTTP_COOKIE' + @request.delete_header 'HTTP_COOKIE' - if @request.have_cookie_jar? - unless @request.cookie_jar.committed? - @request.cookie_jar.write(@response) - self.cookies.update(@request.cookie_jar.instance_variable_get(:@cookies)) + if @request.have_cookie_jar? + unless @request.cookie_jar.committed? + @request.cookie_jar.write(@response) + self.cookies.update(@request.cookie_jar.instance_variable_get(:@cookies)) + end end - end - @response.prepare! + @response.prepare! - if flash_value = @request.flash.to_session_value - @request.session['flash'] = flash_value - else - @request.session.delete('flash') - end + if flash_value = @request.flash.to_session_value + @request.session['flash'] = flash_value + else + @request.session.delete('flash') + end - if xhr - @request.delete_header 'HTTP_X_REQUESTED_WITH' - @request.delete_header 'HTTP_ACCEPT' + if xhr + @request.delete_header 'HTTP_X_REQUESTED_WITH' + @request.delete_header 'HTTP_ACCEPT' + end + @request.query_string = '' + + @response.sent! end - @request.query_string = '' @response end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1e4df07d6e..01d49475de 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -51,8 +51,8 @@ module ActionDispatch autoload :Cookies autoload :DebugExceptions autoload :ExceptionWrapper + autoload :Executor autoload :Flash - autoload :LoadInterlock autoload :ParamsParser autoload :PublicExceptions autoload :Reloader diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 0f7898a3f8..9fa2e38ae3 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -17,9 +17,7 @@ module ActionDispatch end def if_none_match_etags - (if_none_match ? if_none_match.split(/\s*,\s*/) : []).collect do |etag| - etag.gsub(/^\"|\"$/, "") - end + if_none_match ? if_none_match.split(/\s*,\s*/) : [] end def not_modified?(modified_at) @@ -28,8 +26,8 @@ module ActionDispatch def etag_matches?(etag) if etag - etag = etag.gsub(/^\"|\"$/, "") - if_none_match_etags.include?(etag) + validators = if_none_match_etags + validators.include?(etag) || validators.include?('*') end end @@ -80,19 +78,63 @@ module ActionDispatch set_header DATE, utc_time.httpdate end - def etag=(etag) - key = ActiveSupport::Cache.expand_cache_key(etag) - super %(W/"#{Digest::MD5.hexdigest(key)}") + # This method sets a weak ETag validator on the response so browsers + # and proxies may cache the response, keyed on the ETag. On subsequent + # requests, the If-None-Match header is set to the cached ETag. If it + # matches the current ETag, we can return a 304 Not Modified response + # with no body, letting the browser or proxy know that their cache is + # current. Big savings in request time and network bandwidth. + # + # Weak ETags are considered to be semantically equivalent but not + # byte-for-byte identical. This is perfect for browser caching of HTML + # pages where we don't care about exact equality, just what the user + # is viewing. + # + # Strong ETags are considered byte-for-byte identical. They allow a + # browser or proxy cache to support Range requests, useful for paging + # through a PDF file or scrubbing through a video. Some CDNs only + # support strong ETags and will ignore weak ETags entirely. + # + # Weak ETags are what we almost always need, so they're the default. + # Check out `#strong_etag=` to provide a strong ETag validator. + def etag=(weak_validators) + self.weak_etag = weak_validators + end + + def weak_etag=(weak_validators) + set_header 'ETag', generate_weak_etag(weak_validators) + end + + def strong_etag=(strong_validators) + set_header 'ETag', generate_strong_etag(strong_validators) end def etag?; etag; end + # True if an ETag is set and it's a weak validator (preceded with W/) + def weak_etag? + etag? && etag.starts_with?('W/"') + end + + # True if an ETag is set and it isn't a weak validator (not preceded with W/) + def strong_etag? + etag? && !weak_etag? + end + private DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) + def generate_weak_etag(validators) + "W/#{generate_strong_etag(validators)}" + end + + def generate_strong_etag(validators) + %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(validators))}") + end + def cache_control_segments if cache_control = _cache_control cache_control.delete(' ').split(',') diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 9dcab79c3a..041eca48ca 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -4,9 +4,11 @@ module ActionDispatch module Http # Allows you to specify sensitive parameters which will be replaced from # the request log by looking in the query string of the request and all - # sub-hashes of the params hash to filter. If a block is given, each key and - # value of the params hash and all sub-hashes is passed to it, the value - # or key can be replaced using String#replace or similar method. + # sub-hashes of the params hash to filter. Filtering only certain sub-keys + # from a hash is possible by using the dot notation: 'credit_card.number'. + # If a block is given, each key and value of the params hash and all + # sub-hashes is passed to it, the value or key can be replaced using + # String#replace or similar method. # # env["action_dispatch.parameter_filter"] = [:password] # => replaces the value to all keys matching /password/i with "[FILTERED]" @@ -14,6 +16,10 @@ module ActionDispatch # env["action_dispatch.parameter_filter"] = [:foo, "bar"] # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]" # + # env["action_dispatch.parameter_filter"] = [ "credit_card.code" ] + # => replaces { credit_card: {code: "xxxx"} } with "[FILTERED]", does not + # change { file: { code: "xxxx"} } + # # env["action_dispatch.parameter_filter"] = -> (k, v) do # v.reverse! if k =~ /secret/i # end diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 12f81dc1a5..69a934b7cd 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -2,9 +2,23 @@ module ActionDispatch module Http # Provides access to the request's HTTP headers from the environment. # - # env = { "CONTENT_TYPE" => "text/plain" } + # env = { "CONTENT_TYPE" => "text/plain", "HTTP_USER_AGENT" => "curl/7.43.0" } # headers = ActionDispatch::Http::Headers.new(env) # headers["Content-Type"] # => "text/plain" + # headers["User-Agent"] # => "curl/7.43.0" + # + # Also note that when headers are mapped to CGI-like variables by the Rack + # server, both dashes and underscores are converted to underscores. This + # ambiguity cannot be resolved at this stage anymore. Both underscores and + # dashes have to be interpreted as if they were originally sent as dashes. + # + # # GET / HTTP/1.1 + # # ... + # # User-Agent: curl/7.43.0 + # # X_Custom_Header: token + # + # headers["X_Custom_Header"] # => nil + # headers["X-Custom-Header"] # => "token" class Headers CGI_VARIABLES = Set.new(%W[ AUTH_TYPE @@ -101,7 +115,7 @@ module ActionDispatch private - # Converts a HTTP header name to an environment variable name if it is + # Converts an HTTP header name to an environment variable name if it is # not contained within the headers hash. def env_name(key) key = key.to_s diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index e9b25339dc..0a58ce2b96 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -164,7 +164,7 @@ module ActionDispatch end def format_from_path_extension - path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] + path = get_header('action_dispatch.original_path') || get_header('PATH_INFO') if match = path && path.match(/\.(\w+)\z/) Mime[match.captures.first] end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 463b5fe405..4672ea7199 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,3 +1,5 @@ +# -*- frozen-string-literal: true -*- + require 'singleton' require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/starts_ends_with' @@ -108,57 +110,56 @@ module Mime end end - class AcceptList < Array #:nodoc: - def assort! - sort! + class AcceptList #:nodoc: + def self.sort!(list) + list.sort! - text_xml_idx = find_item_by_name self, 'text/xml' - app_xml_idx = find_item_by_name self, Mime[:xml].to_s + text_xml_idx = find_item_by_name list, 'text/xml' + app_xml_idx = find_item_by_name list, Mime[:xml].to_s # Take care of the broken text/xml entry by renaming or deleting it if text_xml_idx && app_xml_idx - app_xml = self[app_xml_idx] - text_xml = self[text_xml_idx] + app_xml = list[app_xml_idx] + text_xml = list[text_xml_idx] app_xml.q = [text_xml.q, app_xml.q].max # set the q value to the max of the two if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml + list[app_xml_idx], list[text_xml_idx] = text_xml, app_xml app_xml_idx, text_xml_idx = text_xml_idx, app_xml_idx end - delete_at(text_xml_idx) # delete text_xml from the list + list.delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - self[text_xml_idx].name = Mime[:xml].to_s + list[text_xml_idx].name = Mime[:xml].to_s end # Look for more specific XML-based types and sort them ahead of app/xml if app_xml_idx - app_xml = self[app_xml_idx] + app_xml = list[app_xml_idx] idx = app_xml_idx - while idx < length - type = self[idx] + while idx < list.length + type = list[idx] break if type.q < app_xml.q if type.name.ends_with? '+xml' - self[app_xml_idx], self[idx] = self[idx], app_xml + list[app_xml_idx], list[idx] = list[idx], app_xml app_xml_idx = idx end idx += 1 end end - map! { |i| Mime::Type.lookup(i.name) }.uniq! - to_a + list.map! { |i| Mime::Type.lookup(i.name) }.uniq! + list end - private - def find_item_by_name(array, name) + def self.find_item_by_name(array, name) array.index { |item| item.name == name } end end class << self - TRAILING_STAR_REGEXP = /(text|application)\/\*/ + TRAILING_STAR_REGEXP = /^(text|application)\/\*/ PARAMETER_SEPARATOR_REGEXP = /;\s*\w+="?\w+"?/ def register_callback(&block) @@ -198,21 +199,22 @@ module Mime accept_header = accept_header.split(PARAMETER_SEPARATOR_REGEXP).first parse_trailing_star(accept_header) || [Mime::Type.lookup(accept_header)].compact else - list, index = AcceptList.new, 0 + list, index = [], 0 accept_header.split(',').each do |header| params, q = header.split(PARAMETER_SEPARATOR_REGEXP) - if params.present? - params.strip! - params = parse_trailing_star(params) || [params] + next unless params + params.strip! + next if params.empty? + + params = parse_trailing_star(params) || [params] - params.each do |m| - list << AcceptItem.new(index, m.to_s, q) - index += 1 - end + params.each do |m| + list << AcceptItem.new(index, m.to_s, q) + index += 1 end end - list.assort! + AcceptList.sort! list end end diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 87715205d9..8b04174f1f 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -14,20 +14,22 @@ Mime::Type.register "image/jpeg", :jpeg, [], %w(jpg jpeg jpe pjpeg) Mime::Type.register "image/gif", :gif, [], %w(gif) Mime::Type.register "image/bmp", :bmp, [], %w(bmp) Mime::Type.register "image/tiff", :tiff, [], %w(tif tiff) +Mime::Type.register "image/svg+xml", :svg Mime::Type.register "video/mpeg", :mpeg, [], %w(mpg mpeg mpe) Mime::Type.register "application/xml", :xml, %w( text/xml application/x-xml ) Mime::Type.register "application/rss+xml", :rss Mime::Type.register "application/atom+xml", :atom -Mime::Type.register "application/x-yaml", :yaml, %w( text/yaml ) +Mime::Type.register "application/x-yaml", :yaml, %w( text/yaml ), %w(yml yaml) Mime::Type.register "multipart/form-data", :multipart_form Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form # http://www.ietf.org/rfc/rfc4627.txt # http://www.json.org/JSONRequest.html -Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest application/vnd.api+json ) +Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) Mime::Type.register "application/pdf", :pdf, [], %w(pdf) Mime::Type.register "application/zip", :zip, [], %w(zip) +Mime::Type.register "application/gzip", :gzip, %w(application/x-gzip), %w(gz) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index cca7376ffa..ff5031d7d5 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -1,22 +1,31 @@ module ActionDispatch module Http module Parameters + extend ActiveSupport::Concern + PARAMETERS_KEY = 'action_dispatch.request.path_parameters' DEFAULT_PARSERS = { - Mime[:json] => lambda { |raw_post| + Mime[:json].symbol => -> (raw_post) { data = ActiveSupport::JSON.decode(raw_post) data.is_a?(Hash) ? data : {:_json => data} } } - def self.included(klass) - class << klass - attr_accessor :parameter_parsers + included do + class << self + attr_reader :parameter_parsers end - klass.parameter_parsers = DEFAULT_PARSERS + self.parameter_parsers = DEFAULT_PARSERS end + + module ClassMethods + def parameter_parsers=(parsers) # :nodoc: + @parameter_parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } + end + end + # Returns both GET and POST \parameters in a single hash. def parameters params = get_header("action_dispatch.request.parameters") @@ -51,7 +60,7 @@ module ActionDispatch def parse_formatted_parameters(parsers) return yield if content_length.zero? - strategy = parsers.fetch(content_mime_type) { return yield } + strategy = parsers.fetch(content_mime_type.symbol) { return yield } begin strategy.call(raw_post) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 5427425ef7..b0ed681623 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -337,7 +337,6 @@ module ActionDispatch else self.session = {} end - self.flash = nil end def session=(session) #:nodoc: @@ -403,6 +402,10 @@ module ActionDispatch def commit_flash end + def ssl? + super || scheme == 'wss'.freeze + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 14f86c7c07..1515d59df3 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/module/attribute_accessors' require 'action_dispatch/http/filter_redirect' +require 'action_dispatch/http/cache' require 'monitor' module ActionDispatch # :nodoc: @@ -67,7 +68,13 @@ module ActionDispatch # :nodoc: alias_method :headers, :header delegate :[], :[]=, :to => :@header - delegate :each, :to => :@stream + + def each(&block) + sending! + x = @stream.each(&block) + sent! + x + end CONTENT_TYPE = "Content-Type".freeze SET_COOKIE = "Set-Cookie".freeze @@ -96,10 +103,10 @@ module ActionDispatch # :nodoc: def body @str_body ||= begin - buf = '' - each { |chunk| buf << chunk } - buf - end + buf = '' + each { |chunk| buf << chunk } + buf + end end def write(string) @@ -111,10 +118,13 @@ module ActionDispatch # :nodoc: end def each(&block) - @response.sending! - x = @buf.each(&block) - @response.sent! - x + if @str_body + return enum_for(:each) unless block_given? + + yield @str_body + else + each_chunk(&block) + end end def abort @@ -128,6 +138,12 @@ module ActionDispatch # :nodoc: def closed? @closed end + + private + + def each_chunk(&block) + @buf.each(&block) # extract into own method + end end def self.create(status = 200, header = {}, body = [], default_headers: self.default_headers) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 37f41ae988..7a1350a46d 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -217,7 +217,7 @@ module ActionDispatch @protocol ||= ssl? ? 'https://' : 'http://' end - # Returns the \host for this request, such as "example.com". + # Returns the \host and port for this request, such as "example.com:8080". # # class Request < Rack::Request # include ActionDispatch::Http::URL @@ -226,6 +226,9 @@ module ActionDispatch # req = Request.new 'HTTP_HOST' => 'example.com' # req.raw_host_with_port # => "example.com" # + # req = Request.new 'HTTP_HOST' => 'example.com:80' + # req.raw_host_with_port # => "example.com:80" + # # req = Request.new 'HTTP_HOST' => 'example.com:8080' # req.raw_host_with_port # => "example.com:8080" def raw_host_with_port @@ -236,7 +239,7 @@ module ActionDispatch end end - # Returns the host for this request, such as example.com. + # Returns the host for this request, such as "example.com". # # class Request < Rack::Request # include ActionDispatch::Http::URL @@ -249,12 +252,16 @@ module ActionDispatch end # Returns a \host:\port string for this request, such as "example.com" or - # "example.com:8080". + # "example.com:8080". Port is only included if it is not a default port + # (80 or 443) # # class Request < Rack::Request # include ActionDispatch::Http::URL # end # + # req = Request.new 'HTTP_HOST' => 'example.com' + # req.host_with_port # => "example.com" + # # req = Request.new 'HTTP_HOST' => 'example.com:80' # req.host_with_port # => "example.com" # @@ -347,6 +354,17 @@ module ActionDispatch standard_port? ? '' : ":#{port}" end + # Returns the requested port, such as 8080, based on SERVER_PORT + # + # class Request < Rack::Request + # include ActionDispatch::Http::URL + # end + # + # req = Request.new 'SERVER_PORT' => '80' + # req.server_port # => 80 + # + # req = Request.new 'SERVER_PORT' => '8080' + # req.server_port # => 8080 def server_port get_header('SERVER_PORT').to_i end diff --git a/actionpack/lib/action_dispatch/journey/backwards.rb b/actionpack/lib/action_dispatch/journey/backwards.rb deleted file mode 100644 index 3bd20fdf81..0000000000 --- a/actionpack/lib/action_dispatch/journey/backwards.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Rack # :nodoc: - Mount = ActionDispatch::Journey::Router - Mount::RouteSet = ActionDispatch::Journey::Router - Mount::RegexpWithNamedGroups = ActionDispatch::Journey::Path::Pattern -end diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 0323360faa..200477b002 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -32,8 +32,13 @@ module ActionDispatch defaults = route.defaults required_parts = route.required_parts - parameterized_parts.keep_if do |key, value| - (defaults[key].nil? && value.present?) || value.to_s != defaults[key].to_s || required_parts.include?(key) + + route.parts.reverse_each do |key| + break if defaults[key].nil? && parameterized_parts[key].present? + break if parameterized_parts[key].to_s != defaults[key].to_s + break if required_parts.include?(key) + + parameterized_parts.delete(key) end return [route.format(parameterized_parts), params] diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 35c2b1b86e..cfd6681dd1 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -3,7 +3,7 @@ module ActionDispatch class Route # :nodoc: attr_reader :app, :path, :defaults, :name, :precedence - attr_reader :constraints + attr_reader :constraints, :internal alias :conditions :constraints module VerbMatchers @@ -55,7 +55,7 @@ module ActionDispatch ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, internal = false) @name = name @app = app @path = path @@ -70,6 +70,7 @@ module ActionDispatch @decorated_ast = nil @precedence = precedence @path_formatter = @path.build_formatter + @internal = internal end def ast @@ -81,7 +82,7 @@ module ActionDispatch end def requirements # :nodoc: - # needed for rails `rake routes` + # needed for rails `rails routes` @defaults.merge(path.requirements).delete_if { |_,v| /.+?/ == v } diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index f649588520..06cdce1724 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -16,9 +16,6 @@ module ActionDispatch class RoutingError < ::StandardError # :nodoc: end - # :nodoc: - VERSION = '2.0.0' - attr_accessor :routes def initialize(routes) diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb index f80df78582..c782779b34 100644 --- a/actionpack/lib/action_dispatch/middleware/callbacks.rb +++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb @@ -7,7 +7,16 @@ module ActionDispatch define_callbacks :call class << self - delegate :to_prepare, :to_cleanup, :to => "ActionDispatch::Reloader" + def to_prepare(*args, &block) + ActiveSupport::Reloader.to_prepare(*args, &block) + end + + def to_cleanup(*args, &block) + ActiveSupport::Reloader.to_complete(*args, &block) + end + + deprecate to_prepare: 'use ActiveSupport::Reloader.to_prepare instead', + to_cleanup: 'use ActiveSupport::Reloader.to_complete instead' def before(*args, &block) set_callback(:call, :before, *args, &block) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 3477aa8b29..f2f3150b56 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/key_generator' require 'active_support/message_verifier' require 'active_support/json' +require 'rack/utils' module ActionDispatch class Request @@ -337,7 +338,7 @@ module ActionDispatch end def to_header - @cookies.map { |k,v| "#{k}=#{v}" }.join ';' + @cookies.map { |k,v| "#{escape(k)}=#{escape(v)}" }.join '; ' end def handle_options(options) #:nodoc: @@ -419,6 +420,10 @@ module ActionDispatch private + def escape(string) + ::Rack::Utils.escape(string) + end + def make_set_cookie_header(header) header = @set_cookies.inject(header) { |m, (k, v)| if write_cookie?(v) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index b55c937e0c..5f758d641a 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -67,18 +67,19 @@ module ActionDispatch log_error(request, wrapper) if request.get_header('action_dispatch.show_detailed_exceptions') - case @response_format - when :api - render_for_api_application(request, wrapper) - when :default - render_for_default_application(request, wrapper) + content_type = request.formats.first + + if api_request?(content_type) + render_for_api_request(content_type, wrapper) + else + render_for_browser_request(request, wrapper) end else raise exception end end - def render_for_default_application(request, wrapper) + def render_for_browser_request(request, wrapper) template = create_template(request, wrapper) file = "rescues/#{wrapper.rescue_template}" @@ -92,7 +93,7 @@ module ActionDispatch render(wrapper.status_code, body, format) end - def render_for_api_application(request, wrapper) + def render_for_api_request(content_type, wrapper) body = { status: wrapper.status_code, error: Rack::Utils::HTTP_STATUS_CODES.fetch( @@ -103,7 +104,6 @@ module ActionDispatch traces: wrapper.traces } - content_type = request.formats.first to_format = "to_#{content_type.to_sym}" if content_type && body.respond_to?(to_format) @@ -156,15 +156,20 @@ module ActionDispatch trace = wrapper.framework_trace if trace.empty? ActiveSupport::Deprecation.silence do - message = "\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) - message << " " << trace.join("\n ") - logger.fatal("#{message}\n\n") + logger.fatal " " + logger.fatal "#{exception.class} (#{exception.message}):" + log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code) + logger.fatal " " + log_array logger, trace end end + def log_array(logger, array) + array.map { |line| logger.fatal line } + end + def logger(request) - request.logger || stderr_logger + request.logger || ActionView::Base.logger || stderr_logger end def stderr_logger @@ -176,5 +181,9 @@ module ActionDispatch ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) end end + + def api_request?(content_type) + @response_format == :api && !content_type.html? + end end end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 3b61824cc9..59edc66086 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -1,4 +1,3 @@ -require 'action_controller/metal/exceptions' require 'active_support/core_ext/module/attribute_accessors' require 'rack/utils' diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb b/actionpack/lib/action_dispatch/middleware/executor.rb new file mode 100644 index 0000000000..06245b403b --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/executor.rb @@ -0,0 +1,19 @@ +require 'rack/body_proxy' + +module ActionDispatch + class Executor + def initialize(app, executor) + @app, @executor = app, executor + end + + def call(env) + state = @executor.run! + begin + response = @app.call(env) + returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! } + ensure + state.complete! unless returned + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index c51dcd542a..80703940ed 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -70,6 +70,11 @@ module ActionDispatch session.delete('flash') end end + + def reset_session # :nodoc + super + self.flash = nil + end end class FlashNow #:nodoc: @@ -128,7 +133,7 @@ module ActionDispatch def to_session_value #:nodoc: flashes_to_keep = @flashes.except(*@discard) return nil if flashes_to_keep.empty? - {'flashes' => flashes_to_keep} + { 'discard' => [], 'flashes' => flashes_to_keep } end def initialize(flashes = {}, discard = []) #:nodoc: diff --git a/actionpack/lib/action_dispatch/middleware/load_interlock.rb b/actionpack/lib/action_dispatch/middleware/load_interlock.rb deleted file mode 100644 index 07f498319c..0000000000 --- a/actionpack/lib/action_dispatch/middleware/load_interlock.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'active_support/dependencies' -require 'rack/body_proxy' - -module ActionDispatch - class LoadInterlock - def initialize(app) - @app = app - end - - def call(env) - interlock = ActiveSupport::Dependencies.interlock - interlock.start_running - response = @app.call(env) - body = Rack::BodyProxy.new(response[2]) { interlock.done_running } - response[2] = body - response - ensure - interlock.done_running unless body - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..faf3262b8f 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,8 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + ActiveSupport::Deprecation.warn('ActionDispatch::ParamsParser is deprecated and will be removed in Rails 5.1. Configure the parameter parsing in ActionDispatch::Request.parameter_parsers.') + parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app end diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb index af9a29eb07..112bde6596 100644 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ b/actionpack/lib/action_dispatch/middleware/reloader.rb @@ -23,74 +23,32 @@ module ActionDispatch # middleware stack, but are executed only when <tt>ActionDispatch::Reloader.prepare!</tt> # or <tt>ActionDispatch::Reloader.cleanup!</tt> are called manually. # - class Reloader - include ActiveSupport::Callbacks - include ActiveSupport::Deprecation::Reporting - - define_callbacks :prepare - define_callbacks :cleanup - - # Add a prepare callback. Prepare callbacks are run before each request, prior - # to ActionDispatch::Callback's before callbacks. + class Reloader < Executor def self.to_prepare(*args, &block) - unless block_given? - warn "to_prepare without a block is deprecated. Please use a block" - end - set_callback(:prepare, *args, &block) + ActiveSupport::Reloader.to_prepare(*args, &block) end - # Add a cleanup callback. Cleanup callbacks are run after each request is - # complete (after #close is called on the response body). def self.to_cleanup(*args, &block) - unless block_given? - warn "to_cleanup without a block is deprecated. Please use a block" - end - set_callback(:cleanup, *args, &block) + ActiveSupport::Reloader.to_complete(*args, &block) end - # Execute all prepare callbacks. def self.prepare! - new(nil).prepare! + default_reloader.prepare! end - # Execute all cleanup callbacks. def self.cleanup! - new(nil).cleanup! - end - - def initialize(app, condition=nil) - @app = app - @condition = condition || lambda { true } - @validated = true + default_reloader.reload! end - def call(env) - @validated = @condition.call - prepare! - - response = @app.call(env) - response[2] = ::Rack::BodyProxy.new(response[2]) { cleanup! } + class << self + attr_accessor :default_reloader # :nodoc: - response - rescue Exception - cleanup! - raise + deprecate to_prepare: 'use ActiveSupport::Reloader.to_prepare instead', + to_cleanup: 'use ActiveSupport::Reloader.to_complete instead', + prepare!: 'use Rails.application.reloader.prepare! instead', + cleanup!: 'use Rails.application.reloader.reload! instead of cleanup + prepare' end - def prepare! #:nodoc: - run_callbacks :prepare if validated? - end - - def cleanup! #:nodoc: - run_callbacks :cleanup if validated? - ensure - @validated = true - end - - private - - def validated? #:nodoc: - @validated - end + self.default_reloader = ActiveSupport::Reloader end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 429a98f236..dec9c60ef2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -23,7 +23,7 @@ module ActionDispatch # goes a step further than signed cookies in that encrypted cookies cannot # be altered or read by users. This is the default starting in Rails 4. # - # If you have both secret_token and secret_key base set, your cookies will + # If you have both secret_token and secret_key_base set, your cookies will # be encrypted, and signed cookies generated by Rails 3 will be # transparently read and encrypted to provide a smooth upgrade path. # diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 735b5939dd..ab3077b308 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -23,7 +23,7 @@ module ActionDispatch # preload lists is `18.weeks`. # * `subdomains`: Set to `true` to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a - # vulnerable site on a subdomain. Defaults to `false`. + # vulnerable site on a subdomain. Defaults to `true`. # * `preload`: Advertise that this site may be included in browsers' # preloaded HSTS lists. HSTS protects your site on every visit *except the # first visit* since it hasn't seen your HSTS header yet. To close this @@ -34,6 +34,10 @@ module ActionDispatch # 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 }`. + # + # Requests can opt-out of redirection with `exclude`: + # + # config.ssl_options = { redirect: { exclude: -> request { request.path =~ /healthcheck/ } } } 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. @@ -49,14 +53,26 @@ module ActionDispatch if options[:host] || options[:port] ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc The `:host` and `:port` options are moving within `:redirect`: - `config.ssl_options = { redirect: { host: …, port: … }}`. + `config.ssl_options = { redirect: { host: …, port: … } }`. end_warning @redirect = options.slice(:host, :port) else @redirect = redirect end + @exclude = @redirect && @redirect[:exclude] || proc { !@redirect } @secure_cookies = secure_cookies + + if hsts != true && hsts != false && hsts[:subdomains].nil? + hsts[:subdomains] = false + + ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc + In Rails 5.1, The `:subdomains` option of HSTS config will be treated as true if + unspecified. Set `config.ssl_options = { hsts: { subdomains: false } }` to opt out + of this behavior. + end_warning + end + @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end @@ -69,7 +85,7 @@ module ActionDispatch flag_cookies_as_secure! headers if @secure_cookies end else - return redirect_to_https request if @redirect + return redirect_to_https request unless @exclude.call(request) @app.call(env) end end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 41c220236a..2c5721dc22 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -27,8 +27,8 @@ module ActionDispatch # in the server's `public/` directory (see Static#call). def match?(path) path = ::Rack::Utils.unescape_path path - return false unless valid_path?(path) - path = Rack::Utils.clean_path_info path + return false unless ::Rack::Utils.valid_path? path + path = ::Rack::Utils.clean_path_info path paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] @@ -94,10 +94,6 @@ module ActionDispatch false end end - - def valid_path?(path) - path.valid_encoding? && !path.include?("\0") - end end # This middleware will attempt to return the contents of a file's body from diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index ddeea24bb3..e9e6a2e597 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -39,6 +39,8 @@ module ActionDispatch config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie + ActionDispatch::Reloader.default_reloader = app.reloader + ActionDispatch.test_app = app end end diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 42890225fa..47568f6ad0 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -9,7 +9,7 @@ module ActionDispatch # Singleton object used to determine if an optional param wasn't specified Unspecified = Object.new - + # Creates a session hash, merging the properties of the previous session if any def self.create(store, req, default_options) session_was = find req @@ -198,6 +198,10 @@ module ActionDispatch @delegate.merge!(other) end + def each(&block) + to_hash.each(&block) + end + private def load_for_read! diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index d00b2c3eb5..61ebd0b8db 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -73,14 +73,14 @@ module ActionDispatch # get 'post/:id' => 'posts#show' # post 'post/:id' => 'posts#create_comment' # + # Now, if you POST to <tt>/posts/:id</tt>, it will route to the <tt>create_comment</tt> action. A GET on the same + # URL will route to the <tt>show</tt> action. + # # If your route needs to respond to more than one HTTP method (or all methods) then using the # <tt>:via</tt> option on <tt>match</tt> is preferable. # # match 'post/:id' => 'posts#show', via: [:get, :post] # - # Now, if you POST to <tt>/posts/:id</tt>, it will route to the <tt>create_comment</tt> action. A GET on the same - # URL will route to the <tt>show</tt> action. - # # == Named routes # # Routes can be named by passing an <tt>:as</tt> option, @@ -89,7 +89,7 @@ module ActionDispatch # # Example: # - # # In routes.rb + # # In config/routes.rb # get '/login' => 'accounts#login', as: 'login' # # # With render, redirect_to, tests, etc. @@ -101,7 +101,7 @@ module ActionDispatch # # Use <tt>root</tt> as a shorthand to name a route for the root path "/". # - # # In routes.rb + # # In config/routes.rb # root to: 'blogs#index' # # # would recognize http://www.example.com/ as @@ -114,15 +114,15 @@ module ActionDispatch # Note: when using +controller+, the route is simply named after the # method you call on the block parameter rather than map. # - # # In routes.rb + # # In config/routes.rb # controller :blog do # get 'blog/show' => :list # get 'blog/delete' => :delete - # get 'blog/edit/:id' => :edit + # get 'blog/edit' => :edit # end # # # provides named routes for show, delete, and edit - # link_to @article.title, show_path(id: @article.id) + # link_to @article.title, blog_show_path(id: @article.id) # # == Pretty URLs # @@ -159,7 +159,7 @@ module ActionDispatch # # controller 'geocode' do # get 'geocode/:postalcode' => :show, constraints: { - # postalcode: /# Postcode format + # postalcode: /# Postalcode format # \d{5} #Prefix # (-\d{4})? #Suffix # /x @@ -196,7 +196,7 @@ module ActionDispatch # # Rails.application.reload_routes! # - # This will clear all named routes and reload routes.rb if the file has been modified from + # This will clear all named routes and reload config/routes.rb if the file has been modified from # last load. To absolutely force reloading, use <tt>reload!</tt>. # # == Testing Routes @@ -239,7 +239,7 @@ module ActionDispatch # # rails routes # - # Target specific controllers by prefixing the command with <tt>CONTROLLER=x</tt>. + # Target specific controllers by prefixing the command with <tt>-c</tt> option. # module Routing extend ActiveSupport::Autoload @@ -252,5 +252,14 @@ module ActionDispatch SEPARATORS = %w( / . ? ) #:nodoc: HTTP_METHODS = [:get, :head, :post, :patch, :put, :delete, :options] #:nodoc: + + #:stopdoc: + INSECURE_URL_PARAMETERS_MESSAGE = <<-MSG.squish + Attempting to generate a URL from non-sanitized request parameters! + + An attacker can inject malicious data into the generated URL, such as + changing the host. Whitelist and sanitize passed parameters to be secure. + MSG + #:startdoc: end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 69e6dd5215..2459a45827 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -33,15 +33,15 @@ module ActionDispatch end def controller - requirements[:controller] || ':controller' + parts.include?(:controller) ? ':controller' : requirements[:controller] end def action - requirements[:action] || ':action' + parts.include?(:action) ? ':action' : requirements[:action] end def internal? - controller.to_s =~ %r{\Arails/(info|mailers|welcome)} + internal end def engine? @@ -51,7 +51,7 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes` or looks at the RoutingError page. + # executes `rails routes` or looks at the RoutingError page. # People should not use this class. class RoutesInspector # :nodoc: def initialize(routes) @@ -60,12 +60,10 @@ module ActionDispatch end def format(formatter, filter = nil) - routes_to_display = filter_routes(filter) - + routes_to_display = filter_routes(normalize_filter(filter)) routes = collect_routes(routes_to_display) - if routes.none? - formatter.no_routes(collect_routes(@routes), filter) + formatter.no_routes(collect_routes(@routes)) return formatter.result end @@ -82,10 +80,20 @@ module ActionDispatch private + def normalize_filter(filter) + if filter.is_a?(Hash) && filter[:controller] + { controller: /#{filter[:controller].downcase.sub(/_?controller\z/, '').sub('::', '/')}/ } + elsif filter + { controller: /#{filter}/, action: /#{filter}/, verb: /#{filter}/, name: /#{filter}/, path: /#{filter}/ } + end + end + def filter_routes(filter) if filter - filter_name = filter.underscore.sub(/_controller$/, '') - @routes.select { |route| route.defaults[:controller] == filter_name } + @routes.select do |route| + route_wrapper = RouteWrapper.new(route) + filter.any? { |default, value| route_wrapper.send(default) =~ value } + end else @routes end @@ -137,7 +145,7 @@ module ActionDispatch @buffer << draw_header(routes) end - def no_routes(routes, filter) + def no_routes(routes) @buffer << if routes.none? <<-MESSAGE.strip_heredoc @@ -145,8 +153,6 @@ module ActionDispatch Please add some routes in config/routes.rb. MESSAGE - elsif missing_controller?(filter) - "The controller #{filter} does not exist!" else "No routes were found for this controller" end @@ -154,10 +160,6 @@ module ActionDispatch end private - def missing_controller?(controller_name) - [ controller_name.camelize, "#{controller_name.camelize}Controller" ].none?(&:safe_constantize) - end - def draw_section(routes) header_lengths = ['Prefix', 'Verb', 'URI Pattern'].map(&:length) name_width, verb_width, path_width = widths(routes).zip(header_lengths).map(&:max) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index afbaa45d20..40b6500553 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1,4 +1,3 @@ -require 'active_support/core_ext/hash/reverse_merge' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/enumerable' require 'active_support/core_ext/array/extract_options' @@ -107,6 +106,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via + @internal = options.delete(:internal) path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -120,7 +120,7 @@ module ActionDispatch if options_constraints.is_a?(Hash) @defaults = Hash[options_constraints.find_all { |key, default| - URL_OPTIONS.include?(key) && (String === default || Fixnum === default) + URL_OPTIONS.include?(key) && (String === default || Integer === default) }].merge @defaults @blocks = blocks constraints.merge! options_constraints @@ -137,6 +137,10 @@ module ActionDispatch @conditions = Hash[conditions] @defaults = formats[:defaults].merge(@defaults).merge(normalize_defaults(options)) + if path_params.include?(:action) && !@requirements.key?(:action) + @defaults[:action] ||= 'index' + end + @required_defaults = (split_options[:required_defaults] || []).map(&:first) end @@ -148,7 +152,8 @@ module ActionDispatch required_defaults, defaults, request_method, - precedence) + precedence, + @internal) route end @@ -819,10 +824,10 @@ module ActionDispatch if options[:constraints].is_a?(Hash) defaults = options[:constraints].select do |k, v| - URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) + URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Integer)) end - (options[:defaults] ||= {}).reverse_merge!(defaults) + options[:defaults] = defaults.merge(options[:defaults] || {}) else block, options[:constraints] = options[:constraints], {} end @@ -1596,7 +1601,7 @@ module ActionDispatch route_options = options.dup if _path && option_path ActiveSupport::Deprecation.warn <<-eowarn -Specifying strings for both :path and the route path is deprecated. Change things like this: +Specifying strings for both :path and the route path is deprecated. Change things like this: match #{_path.inspect}, :path => #{option_path.inspect} @@ -2083,8 +2088,7 @@ to this: def each node = self - loop do - break if node.equal? NULL + until node.equal? NULL yield node node = node.parent end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 846b5fa1fc..ed7130b58e 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -289,7 +289,7 @@ module ActionDispatch if last.permitted? args.pop.to_h else - raise ArgumentError, "Generating an URL from non sanitized request parameters is insecure!" + raise ArgumentError, ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE end end helper.call self, args, options @@ -513,6 +513,21 @@ module ActionDispatch route = @set.add_route(name, mapping) named_routes[name] = route if name + + if route.segment_keys.include?(:controller) + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Using a dynamic :controller segment in a route is deprecated and + will be removed in Rails 5.1. + MSG + end + + if route.segment_keys.include?(:action) + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Using a dynamic :action segment in a route is deprecated and + will be removed in Rails 5.1. + MSG + end + route end @@ -533,12 +548,10 @@ module ActionDispatch @recall = recall @set = set - normalize_recall! normalize_options! normalize_controller_action_id! use_relative_controller! normalize_controller! - normalize_action! end def controller @@ -557,11 +570,6 @@ module ActionDispatch end end - # Set 'index' as default action for recall - def normalize_recall! - @recall[:action] ||= 'index' - end - def normalize_options! # If an explicit :controller was given, always make :action explicit # too, so that action expiry works as expected for things like @@ -615,13 +623,6 @@ module ActionDispatch end end - # Move 'index' action from options to recall - def normalize_action! - if @options[:action] == 'index'.freeze - @recall[:action] = @options.delete(:action) - end - end - # Generates a path from routes, returns [path, params]. # If no route is generated the formatter will raise ActionController::UrlGenerationError def generate diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index f91679593e..5ee138e6c6 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -173,7 +173,7 @@ module ActionDispatch route_name) when ActionController::Parameters unless options.permitted? - raise ArgumentError.new("Generating an URL from non sanitized request parameters is insecure!") + raise ArgumentError.new(ActionDispatch::Routing::INSECURE_URL_PARAMETERS_MESSAGE) end route_name = options.delete :use_route _routes.url_for(options.to_h.symbolize_keys. diff --git a/actionpack/lib/action_dispatch/testing/assertion_response.rb b/actionpack/lib/action_dispatch/testing/assertion_response.rb index 3fb81ff083..404b96bbcd 100644 --- a/actionpack/lib/action_dispatch/testing/assertion_response.rb +++ b/actionpack/lib/action_dispatch/testing/assertion_response.rb @@ -1,14 +1,7 @@ module ActionDispatch - # This is a class that abstracts away an asserted response. - # It purposely does not inherit from Response, because it doesn't need it. - # That means it does not have headers or a body. - # - # As an input to the initializer, we take a Fixnum, a String, or a Symbol. - # If it's a Fixnum or String, we figure out what its symbolized name. - # If it's a Symbol, we figure out what its corresponding code is. - # The resulting code will be a Fixnum, for real HTTP codes, and it will - # be a String for the pseudo-HTTP codes, such as: - # :success, :missing, :redirect and :error + # This is a class that abstracts away an asserted response. It purposely + # does not inherit from Response because it doesn't need it. That means it + # does not have headers or a body. class AssertionResponse attr_reader :code, :name @@ -19,6 +12,9 @@ module ActionDispatch error: "5XX" } + # Accepts a specific response status code as an Integer (404) or String + # ('404') or a response status range as a Symbol pseudo-code (:success, + # indicating any 200-299 status code). def initialize(code_or_name) if code_or_name.is_a?(Symbol) @name = code_or_name diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 78ef860548..44ad2c10d8 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -86,6 +86,7 @@ module ActionDispatch end # Load routes.rb if it hasn't been loaded. + options = options.clone generated_path, query_string_keys = @routes.generate_extras(options, defaults) found_extras = options.reject { |k, _| ! query_string_keys.include? k } @@ -116,7 +117,7 @@ module ActionDispatch # # Tests a route, providing a defaults hash # assert_routing 'controller/action/9', {id: "9", item: "square"}, {controller: "controller", action: "action"}, {}, {item: "square"} # - # # Tests a route with a HTTP method + # # Tests a route with an HTTP method # assert_routing({ method: 'put', path: '/product/321' }, { controller: "product", action: "update", id: "321" }) def assert_routing(path, options, defaults={}, extras={}, message=nil) assert_recognizes(options, path, extras, message) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 711ca10419..8777666f9f 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -71,7 +71,7 @@ module ActionDispatch end # Performs an XMLHttpRequest request with the given parameters, mirroring - # a request from the Prototype library. + # an AJAX request made from JavaScript. # # The request_method is +:get+, +:post+, +:patch+, +:put+, +:delete+ or # +:head+; the parameters are +nil+, a hash, or a url-encoded or multipart @@ -95,7 +95,7 @@ module ActionDispatch ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) xhr and xml_http_request methods are deprecated in favor of - `get "/posts", xhr: true` and `post "/posts/1", xhr: true` + `get "/posts", xhr: true` and `post "/posts/1", xhr: true`. MSG process(request_method, path, params: params, headers: headers, xhr: true) @@ -122,6 +122,7 @@ module ActionDispatch # params: { ref_id: 14 }, # headers: { "X-Test-Header" => "testvalue" } def request_via_redirect(http_method, path, *args) + ActiveSupport::Deprecation.warn('`request_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') process_with_kwargs(http_method, path, *args) follow_redirect! while redirect? @@ -131,35 +132,35 @@ module ActionDispatch # Performs a GET request, following any subsequent redirect. # See +request_via_redirect+ for more information. def get_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`get_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`get_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:get, path, *args) end # Performs a POST request, following any subsequent redirect. # See +request_via_redirect+ for more information. def post_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`post_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`post_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:post, path, *args) end # Performs a PATCH request, following any subsequent redirect. # See +request_via_redirect+ for more information. def patch_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`patch_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`patch_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:patch, path, *args) end # Performs a PUT request, following any subsequent redirect. # See +request_via_redirect+ for more information. def put_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`put_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`put_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:put, path, *args) end # Performs a DELETE request, following any subsequent redirect. # See +request_via_redirect+ for more information. def delete_via_redirect(path, *args) - ActiveSupport::Deprecation.warn('`delete_via_redirect` is deprecated and will be removed in Rails 5.1. Please use follow_redirect! manually after the request call for the same behavior.') + ActiveSupport::Deprecation.warn('`delete_via_redirect` is deprecated and will be removed in Rails 5.1. Please use `follow_redirect!` manually after the request call for the same behavior.') request_via_redirect(:delete, path, *args) end end @@ -299,7 +300,7 @@ module ActionDispatch end end - REQUEST_KWARGS = %i(params headers env xhr) + REQUEST_KWARGS = %i(params headers env xhr as) def kwarg_request?(args) args[0].respond_to?(:keys) && args[0].keys.any? { |k| REQUEST_KWARGS.include?(k) } end @@ -316,12 +317,15 @@ module ActionDispatch params: { id: 1 }, headers: { 'X-Extra-Header' => '123' }, env: { 'action_dispatch.custom' => 'custom' }, - xhr: true + xhr: true, + as: :json MSG end # Performs the actual request. - def process(method, path, params: nil, headers: nil, env: nil, xhr: false) + def process(method, path, params: nil, headers: nil, env: nil, xhr: false, as: nil) + request_encoder = RequestEncoder.encoder(as) + if path =~ %r{://} location = URI.parse(path) https! URI::HTTPS === location if location.scheme @@ -330,14 +334,17 @@ module ActionDispatch url_host += ":#{location.port}" if default != location.port host! url_host end - path = location.query ? "#{location.path}?#{location.query}" : location.path + path = request_encoder.append_format_to location.path + path = location.query ? "#{path}?#{location.query}" : path + else + path = request_encoder.append_format_to path end hostname, port = host.split(':') request_env = { :method => method, - :params => params, + :params => request_encoder.encode_params(params), "SERVER_NAME" => hostname, "SERVER_PORT" => port || (https? ? "443" : "80"), @@ -347,7 +354,7 @@ module ActionDispatch "REQUEST_URI" => path, "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, - "CONTENT_TYPE" => "application/x-www-form-urlencoded", + "CONTENT_TYPE" => request_encoder.content_type, "HTTP_ACCEPT" => accept } @@ -376,6 +383,7 @@ module ActionDispatch response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) @response.request = @request + @response.response_parser = RequestEncoder.parser(@response.content_type) @html_document = nil @url_options = nil @@ -387,6 +395,56 @@ module ActionDispatch def build_full_uri(path, env) "#{env['rack.url_scheme']}://#{env['SERVER_NAME']}:#{env['SERVER_PORT']}#{path}" end + + class RequestEncoder # :nodoc: + @encoders = {} + + attr_reader :response_parser + + def initialize(mime_name, param_encoder, response_parser, url_encoded_form = false) + @mime = Mime[mime_name] + + unless @mime + raise ArgumentError, "Can't register a request encoder for " \ + "unregistered MIME Type: #{mime_name}. See `Mime::Type.register`." + end + + @url_encoded_form = url_encoded_form + @path_format = ".#{@mime.symbol}" unless @url_encoded_form + @response_parser = response_parser || -> body { body } + @param_encoder = param_encoder || :"to_#{@mime.symbol}".to_proc + end + + def append_format_to(path) + path << @path_format unless @url_encoded_form + path + end + + def content_type + @mime.to_s + end + + def encode_params(params) + @param_encoder.call(params) + end + + def self.parser(content_type) + mime = Mime::Type.lookup(content_type) + encoder(mime ? mime.ref : nil).response_parser + end + + def self.encoder(name) + @encoders[name] || WWWFormEncoder + end + + def self.register_encoder(mime_name, param_encoder: nil, response_parser: nil) + @encoders[mime_name] = new(mime_name, param_encoder, response_parser) + end + + register_encoder :json, response_parser: -> body { JSON.parse(body) } + + WWWFormEncoder = new(:url_encoded_form, -> params { params }, nil, true) + end end module Runner @@ -643,33 +701,85 @@ module ActionDispatch # end # end # + # You can also test your JSON API easily by setting what the request should + # be encoded as: + # + # require 'test_helper' + # + # class ApiTest < ActionDispatch::IntegrationTest + # test 'creates articles' do + # assert_difference -> { Article.count } do + # post articles_path, params: { article: { title: 'Ahoy!' } }, as: :json + # end + # + # assert_response :success + # assert_equal({ id: Arcticle.last.id, title: 'Ahoy!' }, response.parsed_body) + # end + # end + # + # The `as` option sets the format to JSON, sets the content type to + # 'application/json' and encodes the parameters as JSON. + # + # Calling `parsed_body` on the response parses the response body as what + # the last request was encoded as. If the request wasn't encoded `as` something, + # it's the same as calling `body`. + # + # For any custom MIME Types you've registered, you can even add your own encoders with: + # + # ActionDispatch::IntegrationTest.register_encoder :wibble, + # param_encoder: -> params { params.to_wibble }, + # response_parser: -> body { body } + # + # Where `param_encoder` defines how the params should be encoded and + # `response_parser` defines how the response body should be parsed through + # `parsed_body`. + # # Consult the Rails Testing Guide for more. class IntegrationTest < ActiveSupport::TestCase - include Integration::Runner - include ActionController::TemplateAssertions - include ActionDispatch::Routing::UrlFor + module UrlOptions + extend ActiveSupport::Concern + def url_options + integration_session.url_options + end + end - @@app = nil + module Behavior + extend ActiveSupport::Concern - def self.app - @@app || ActionDispatch.test_app - end + include Integration::Runner + include ActionController::TemplateAssertions - def self.app=(app) - @@app = app - end + included do + include ActionDispatch::Routing::UrlFor + include UrlOptions # don't let UrlFor override the url_options method + ActiveSupport.run_load_hooks(:action_dispatch_integration_test, self) + @@app = nil + end - def app - super || self.class.app - end + module ClassMethods + def app + defined?(@@app) ? @@app : ActionDispatch.test_app + end - def url_options - integration_session.url_options - end + def app=(app) + @@app = app + end - def document_root_element - html_document.root + def register_encoder(*args) + Integration::Session::RequestEncoder.register_encoder(*args) + end + end + + def app + super || self.class.app + end + + def document_root_element + html_document.root + end end + + include Behavior end end diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index eca0439909..1ecd7d14a7 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -1,6 +1,5 @@ require 'action_dispatch/middleware/cookies' require 'action_dispatch/middleware/flash' -require 'active_support/core_ext/hash/indifferent_access' module ActionDispatch module TestProcess diff --git a/actionpack/lib/action_dispatch/testing/test_request.rb b/actionpack/lib/action_dispatch/testing/test_request.rb index ad1a7f7109..46523a8600 100644 --- a/actionpack/lib/action_dispatch/testing/test_request.rb +++ b/actionpack/lib/action_dispatch/testing/test_request.rb @@ -22,23 +22,23 @@ module ActionDispatch private_class_method :default_env def request_method=(method) - @env['REQUEST_METHOD'] = method.to_s.upcase + set_header('REQUEST_METHOD', method.to_s.upcase) end def host=(host) - @env['HTTP_HOST'] = host + set_header('HTTP_HOST', host) end def port=(number) - @env['SERVER_PORT'] = number.to_i + set_header('SERVER_PORT', number.to_i) end def request_uri=(uri) - @env['REQUEST_URI'] = uri + set_header('REQUEST_URI', uri) end def path=(path) - @env['PATH_INFO'] = path + set_header('PATH_INFO', path) end def action=(action_name) @@ -46,24 +46,24 @@ module ActionDispatch end def if_modified_since=(last_modified) - @env['HTTP_IF_MODIFIED_SINCE'] = last_modified + set_header('HTTP_IF_MODIFIED_SINCE', last_modified) end def if_none_match=(etag) - @env['HTTP_IF_NONE_MATCH'] = etag + set_header('HTTP_IF_NONE_MATCH', etag) end def remote_addr=(addr) - @env['REMOTE_ADDR'] = addr + set_header('REMOTE_ADDR', addr) end def user_agent=(user_agent) - @env['HTTP_USER_AGENT'] = user_agent + set_header('HTTP_USER_AGENT', user_agent) end def accept=(mime_types) - @env.delete('action_dispatch.request.accepts') - @env['HTTP_ACCEPT'] = Array(mime_types).collect(&:to_s).join(",") + delete_header('action_dispatch.request.accepts') + set_header('HTTP_ACCEPT', Array(mime_types).collect(&:to_s).join(",")) end end end diff --git a/actionpack/lib/action_dispatch/testing/test_response.rb b/actionpack/lib/action_dispatch/testing/test_response.rb index 4b79a90242..9d4b73a43d 100644 --- a/actionpack/lib/action_dispatch/testing/test_response.rb +++ b/actionpack/lib/action_dispatch/testing/test_response.rb @@ -18,5 +18,11 @@ module ActionDispatch # Was there a server-side error? alias_method :error?, :server_error? + + attr_writer :response_parser # :nodoc: + + def parsed_body + @parsed_body ||= @response_parser.call(body) + end end end diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 3a1410d492..d8f86630b1 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -6,9 +6,9 @@ module ActionPack module VERSION MAJOR = 5 - MINOR = 0 + MINOR = 1 TINY = 0 - PRE = "beta1.1" + PRE = "alpha" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end |