diff options
Diffstat (limited to 'actionpack/lib')
57 files changed, 902 insertions, 393 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 7f349f2741..8edea0f52b 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -49,7 +49,7 @@ module AbstractController # instance methods on that abstract class. Public instance methods of # a controller would normally be considered action methods, so methods # declared on abstract classes are being removed. - # (ActionController::Metal and ActionController::Base are defined as abstract) + # (<tt>ActionController::Metal</tt> and ActionController::Base are defined as abstract) def internal_methods controller = self @@ -80,7 +80,7 @@ module AbstractController # action_methods are cached and there is sometimes need to refresh # them. ::clear_action_methods! allows you to do that, so next time - # you run action_methods, they will be recalculated + # you run action_methods, they will be recalculated. def clear_action_methods! @action_methods = nil end 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/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index a73f188623..e765d73ce4 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -82,7 +82,13 @@ module AbstractController # <tt>render :file => "foo/bar"</tt>. # :api: plugin def _normalize_args(action=nil, options={}) - if action.is_a? Hash + if action.respond_to?(:permitted?) + if action.permitted? + action + else + raise ArgumentError, "render parameters are not permitted" + end + elsif action.is_a?(Hash) action else options diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 3d3af555c9..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 @@ -41,13 +44,12 @@ module ActionController autoload :UrlFor end + autoload_under "api" do + autoload :ApiRendering + end + 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 1a46d49a49..ff12705abe 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -112,7 +112,7 @@ module ActionController UrlFor, Redirecting, - Rendering, + ApiRendering, Renderers::All, ConditionalGet, BasicImplicitRender, diff --git a/actionpack/lib/action_controller/api/api_rendering.rb b/actionpack/lib/action_controller/api/api_rendering.rb new file mode 100644 index 0000000000..3a08d28c39 --- /dev/null +++ b/actionpack/lib/action_controller/api/api_rendering.rb @@ -0,0 +1,14 @@ +module ActionController + module ApiRendering + extend ActiveSupport::Concern + + included do + include Rendering + end + + def render_to_body(options = {}) + _process_options(options) + super + end + end +end 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 8e040bb465..f6e67b02d7 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -166,7 +166,7 @@ module ActionController alias :response_code :status # :nodoc: - # Basic url_for that can be overridden for more robust functionality + # Basic url_for that can be overridden for more robust functionality. def url_for(string) string end @@ -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 d86a793e4c..f8e0d9cf6c 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -228,7 +228,7 @@ module ActionController expires_in 100.years, public: public yield if stale?(etag: "#{version}-#{request.fullpath}", - last_modified: Time.parse('2011-01-01').utc, + last_modified: Time.new(2011, 1, 1).utc, public: public) end diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index b2110bf946..5e9832fd4e 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -50,7 +50,6 @@ module ActionController end private - # :nodoc: def include_content?(status) case status when 100..199 diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 2ac6e37e34..35be6d9300 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -1,4 +1,5 @@ require 'base64' +require 'active_support/security_utils' module ActionController # Makes it dead easy to do HTTP Basic, Digest and Token authentication. @@ -68,7 +69,11 @@ module ActionController def http_basic_authenticate_with(options = {}) before_action(options.except(:name, :password, :realm)) do authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password| - name == options[:name] && password == options[:password] + # This comparison uses & so that it doesn't short circuit and + # uses `variable_size_secure_compare` so that length information + # isn't leaked. + ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) & + ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password]) 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..6b540d42c7 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,29 +1,80 @@ +require 'active_support/core_ext/string/strip' + module ActionController + # Handles implicit rendering for a controller action when it did not + # explicitly indicate an appropiate response via methods such as +render+, + # +respond_to+, +redirect+ or +head+. + # + # For API controllers, the implicit render always renders "204 No Content" + # and does not account for any templates. + # + # For other controllers, the following conditions are checked: + # + # First, if a template exists for the controller action, it is rendered. + # This template lookup takes into account the action name, locales, format, + # variant, template handlers, etc. (see +render+ for details). + # + # Second, if other templates exist for the controller action but is not in + # the right format (or variant, etc.), an <tt>ActionController::UnknownFormat</tt> + # is raised. The list of available templates is assumed to be a complete + # enumeration of all the possible formats (or variants, etc.); that is, + # having only HTML and JSON templates indicate that the controller action is + # not meant to handle XML requests. + # + # Third, if the current request is an "interactive" browser request (the user + # navigated here by entering the URL in the address bar, submiting a form, + # clicking on a link, etc. as opposed to an XHR or non-browser API request), + # <tt>ActionView::UnknownFormat</tt> is raised to display a helpful error + # message. + # + # Finally, it falls back to the same "204 No Content" behavior as API controllers. 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) - 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 + elsif any_templates?(action_name.to_s, _prefixes) + message = "#{self.class.name}\##{action_name} does not know how to respond " \ + "to this request. There are other templates available for this controller " \ + "action but none of them were suitable for this request.\n\n" \ + "This usually happens when the client requested an unsupported format " \ + "(e.g. requesting HTML content from a JSON endpoint or vice versa), but " \ + "it might also be failing due to other constraints, such as locales or" \ + "variants.\n" + + if request.formats.any? + message << "\nRequested format(s): #{request.formats.join(", ")}" end + + if request.variant.any? + message << "\nRequested variant(s): #{request.variant.join(", ")}" + end + + raise ActionController::UnknownFormat, message + elsif interactive_browser_request? + message = "You did not define any templates for #{self.class.name}\##{action_name}. " \ + "This is not necessarily a problem (e.g. you might be building an API endpoint " \ + "that does not require any templates), and the controller would usually respond " \ + "with `head :no_content` for your convenience.\n\n" \ + "However, you appear to have navigated here from an interactive browser request – " \ + "such as by navigating to this URL directly, clicking on a link or submitting a form. " \ + "Rendering a `head :no_content` in this case could have resulted in unexpected UI " \ + "behavior in the browser.\n\n" \ + "If you expected the `head :no_content` response, you do not need to take any " \ + "actions – requests coming from an XHR (AJAX) request or other non-browser clients " \ + "will receive the \"204 No Content\" response as expected.\n\n" \ + "If you did not expect this behavior, you can resolve this error by adding a " \ + "template for this controller action (usually `#{action_name}.html.erb`) or " \ + "otherwise indicate the appropriate response in the action using `render`, " \ + "`redirect_to`, `head`, etc.\n" + + raise ActionController::UnknownFormat, message + else + logger.info "No template found for #{self.class.name}\##{action_name}, rendering head :no_content" if logger + super end end @@ -32,5 +83,11 @@ module ActionController "default_render" end end + + private + + def interactive_browser_request? + 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..bf74b39ac4 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -19,9 +19,9 @@ module ActionController :controller => self.class.name, :action => self.action_name, :params => request.filtered_parameters, - :format => request.format.try(:ref), + :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) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index e3c540bf5f..fc20e7a421 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -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 6e346fadfe..173a14a1d2 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -9,6 +9,13 @@ module ActionController #:nodoc: # @people = Person.all # end # + # That action implicitly responds to all formats, but formats can also be whitelisted: + # + # def index + # @people = Person.all + # respond_to :html, :js + # end + # # Here's the same action, with web-service support baked in: # # def index @@ -16,11 +23,12 @@ module ActionController #:nodoc: # # respond_to do |format| # format.html + # format.js # format.xml { render xml: @people } # end # end # - # What that says is, "if the client wants HTML in response to this action, just respond as we + # What that says is, "if the client wants HTML or JS in response to this action, just respond as we # would have before, but if the client wants XML, return them the list of people in XML format." # (Rails determines the desired response format from the HTTP Accept header submitted by the client.) # @@ -180,9 +188,6 @@ module ActionController #:nodoc: # format.html.none # format.html.phone # this gets rendered # end - # - # Be sure to check the documentation of <tt>ActionController::MimeResponds.respond_to</tt> - # for more examples. def respond_to(*mimes) raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 22e0bb5955..90fb34e386 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -11,6 +11,7 @@ module ActionController Renderers.remove(key) end + # See <tt>Responder#api_behavior</tt> class MissingRenderer < LoadError def initialize(format) super "No renderer defined for format: #{format}" @@ -20,40 +21,25 @@ module ActionController module Renderers extend ActiveSupport::Concern + # A Set containing renderer names that correspond to available renderer procs. + # Default values are <tt>:json</tt>, <tt>:js</tt>, <tt>:xml</tt>. + RENDERERS = Set.new + included do class_attribute :_renderers self._renderers = Set.new.freeze end - module ClassMethods - def use_renderers(*args) - renderers = _renderers + args - self._renderers = renderers.freeze - end - alias use_renderer use_renderers - end - - def render_to_body(options) - _render_to_body_with_renderer(options) || super - end + # Used in <tt>ActionController::Base</tt> + # and <tt>ActionController::API</tt> to include all + # renderers by default. + module All + extend ActiveSupport::Concern + include Renderers - def _render_to_body_with_renderer(options) - _renderers.each do |name| - if options.key?(name) - _process_options(options) - method_name = Renderers._render_with_renderer_method_name(name) - return send(method_name, options.delete(name), options) - end + included do + self._renderers = RENDERERS end - nil - end - - # A Set containing renderer names that correspond to available renderer procs. - # Default values are <tt>:json</tt>, <tt>:js</tt>, <tt>:xml</tt>. - RENDERERS = Set.new - - def self._render_with_renderer_method_name(key) - "_render_with_renderer_#{key}" end # Adds a new renderer to call within controller actions. @@ -103,13 +89,70 @@ module ActionController remove_method(method_name) if method_defined?(method_name) end - module All - extend ActiveSupport::Concern - include Renderers + def self._render_with_renderer_method_name(key) + "_render_with_renderer_#{key}" + end - included do - self._renderers = RENDERERS + module ClassMethods + + # Adds, by name, a renderer or renderers to the +_renderers+ available + # to call within controller actions. + # + # It is useful when rendering from an <tt>ActionController::Metal</tt> controller or + # otherwise to add an available renderer proc to a specific controller. + # + # 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>. + # + # Since <tt>ActionController::Metal</tt> controllers cannot render, the controller + # must include <tt>AbstractController::Rendering</tt>, <tt>ActionController::Rendering</tt>, + # and <tt>ActionController::Renderers</tt>, and have at lest one renderer. + # + # Rather than including <tt>ActionController::Renderers::All</tt> and including all renderers, + # you may specify which renderers to include by passing the renderer name or names to + # +use_renderers+. For example, a controller that includes only the <tt>:json</tt> renderer + # (+_render_with_renderer_json+) might look like: + # + # class MetalRenderingController < ActionController::Metal + # include AbstractController::Rendering + # include ActionController::Rendering + # include ActionController::Renderers + # + # use_renderers :json + # + # def show + # render json: record + # end + # end + # + # You must specify a +use_renderer+, else the +controller.renderer+ and + # +controller._renderers+ will be <tt>nil</tt>, and the action will fail. + def use_renderers(*args) + renderers = _renderers + args + self._renderers = renderers.freeze end + alias use_renderer use_renderers + end + + # Called by +render+ in <tt>AbstractController::Rendering</tt> + # which sets the return value as the +response_body+. + # + # If no renderer is found, +super+ returns control to + # <tt>ActionView::Rendering.render_to_body</tt>, if present. + def render_to_body(options) + _render_to_body_with_renderer(options) || super + end + + def _render_to_body_with_renderer(options) + _renderers.each do |name| + if options.key?(name) + _process_options(options) + method_name = Renderers._render_with_renderer_method_name(name) + return send(method_name, options.delete(name), options) + end + end + nil end add :json do |json, options| diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 26c4550f89..b2f0b382b9 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -81,6 +81,10 @@ module ActionController #:nodoc: config_accessor :forgery_protection_origin_check self.forgery_protection_origin_check = false + # Controls whether form-action/method specific CSRF tokens are used. + config_accessor :per_form_csrf_tokens + self.per_form_csrf_tokens = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -277,16 +281,25 @@ module ActionController #:nodoc: end # Sets the token value for the current session. - def form_authenticity_token - masked_authenticity_token(session) + def form_authenticity_token(form_options: {}) + masked_authenticity_token(session, form_options: form_options) end # Creates a masked version of the authenticity token that varies # on each request. The masking is used to mitigate SSL attacks # like BREACH. - def masked_authenticity_token(session) + def masked_authenticity_token(session, form_options: {}) + action, method = form_options.values_at(:action, :method) + + raw_token = if per_form_csrf_tokens && action && method + action_path = normalize_action_path(action) + per_form_csrf_token(session, action_path, method) + else + real_csrf_token(session) + end + one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH) - encrypted_csrf_token = xor_byte_strings(one_time_pad, real_csrf_token(session)) + encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token) masked_token = one_time_pad + encrypted_csrf_token Base64.strict_encode64(masked_token) end @@ -316,30 +329,58 @@ module ActionController #:nodoc: compare_with_real_token masked_token, session elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2 - # Split the token into the one-time pad and the encrypted - # value and decrypt it - one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] - encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] - csrf_token = xor_byte_strings(one_time_pad, encrypted_csrf_token) - - compare_with_real_token csrf_token, session + csrf_token = unmask_token(masked_token) + compare_with_real_token(csrf_token, session) || + valid_per_form_csrf_token?(csrf_token, session) else false # Token is malformed end end + def unmask_token(masked_token) + # Split the token into the one-time pad and the encrypted + # value and decrypt it + one_time_pad = masked_token[0...AUTHENTICITY_TOKEN_LENGTH] + encrypted_csrf_token = masked_token[AUTHENTICITY_TOKEN_LENGTH..-1] + xor_byte_strings(one_time_pad, encrypted_csrf_token) + end + def compare_with_real_token(token, session) ActiveSupport::SecurityUtils.secure_compare(token, real_csrf_token(session)) end + def valid_per_form_csrf_token?(token, session) + if per_form_csrf_tokens + correct_token = per_form_csrf_token( + session, + normalize_action_path(request.fullpath), + request.request_method + ) + + ActiveSupport::SecurityUtils.secure_compare(token, correct_token) + else + false + end + end + def real_csrf_token(session) session[:_csrf_token] ||= SecureRandom.base64(AUTHENTICITY_TOKEN_LENGTH) Base64.strict_decode64(session[:_csrf_token]) end + def per_form_csrf_token(session, action_path, method) + OpenSSL::HMAC.digest( + OpenSSL::Digest::SHA256.new, + real_csrf_token(session), + [action_path, method.downcase].join("#") + ) + 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. @@ -362,5 +403,9 @@ module ActionController #:nodoc: true end end + + def normalize_action_path(action_path) + action_path.split('?').first.to_s.chomp('/') + end end end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 957aa746c0..a01110d474 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -109,7 +109,8 @@ module ActionController cattr_accessor :permit_all_parameters, instance_accessor: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false - delegate :keys, :key?, :has_key?, :empty?, :inspect, to: :@parameters + delegate :keys, :key?, :has_key?, :values, :has_value?, :value?, :empty?, :include?, + :as_json, to: :@parameters # By default, never raise an UnpermittedParameters exception if these # params are present. The default includes both 'controller' and 'action' @@ -121,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>. @@ -153,13 +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 - @parameters == other_hash + @parameters == other end end @@ -176,7 +175,7 @@ module ActionController # safe_params.to_h # => {"name"=>"Senjougahara Hitagi"} def to_h if permitted? - convert_parameters_to_hashes(@parameters) + convert_parameters_to_hashes(@parameters, :to_h) else slice(*self.class.always_permitted_parameters).permit!.to_h end @@ -186,7 +185,7 @@ module ActionController # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this # parameter. def to_unsafe_h - convert_parameters_to_hashes(@parameters) + convert_parameters_to_hashes(@parameters, :to_unsafe_h) end alias_method :to_unsafe_hash, :to_unsafe_h @@ -419,7 +418,7 @@ module ActionController # params.fetch(:none) # => ActionController::ParameterMissing: param is missing or the value is empty: none # params.fetch(:none, 'Francesco') # => "Francesco" # params.fetch(:none) { 'Francesco' } # => "Francesco" - def fetch(key, *args, &block) + def fetch(key, *args) convert_value_to_parameters( @parameters.fetch(key) { if block_given? @@ -514,7 +513,7 @@ module ActionController # to key. If the key is not found, returns the default value. If the # optional code block is given and the key is not found, pass in the key # and return the result of block. - def delete(key, &block) + def delete(key) convert_value_to_parameters(@parameters.delete(key)) end @@ -579,13 +578,37 @@ 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 + Method #{method_sym} is deprecated and will be removed in Rails 5.1, + as `ActionController::Parameters` no longer inherits from + hash. Using this deprecated behavior exposes potential security + problems. If you continue to use this method you may be creating + a security vulnerability in your app that can be exploited. Instead, + consider using one of these documented methods which are not + deprecated: http://api.rubyonrails.org/v#{ActionPack.version}/classes/ActionController/Parameters.html + DEPRECATE + ActiveSupport::Deprecation.warn(message) + @parameters.public_send(method_sym, *args, &block) + else + super + end + 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 @@ -595,16 +618,16 @@ module ActionController end end - def convert_parameters_to_hashes(value) + def convert_parameters_to_hashes(value, using) case value when Array - value.map { |v| convert_parameters_to_hashes(v) } + value.map { |v| convert_parameters_to_hashes(v, using) } when Hash value.transform_values do |v| - convert_parameters_to_hashes(v) + convert_parameters_to_hashes(v, using) end.with_indifferent_access when Parameters - value.to_h + value.send(using) else value end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index c55720859e..700317614f 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -8,6 +8,21 @@ require 'rails-dom-testing' module ActionController # :stopdoc: + class Metal + 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: @@ -37,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 @@ -90,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 @@ -455,16 +470,16 @@ module ActionController parameters = nil end - if parameters.present? || session.present? || flash.present? + if parameters || session || flash non_kwarg_request_warning end end - if body.present? + if body @request.set_header 'RAW_POST_DATA', body end - if http_method.present? + if http_method http_method = http_method.to_s.upcase else http_method = "GET" @@ -472,16 +487,12 @@ module ActionController parameters ||= {} - if format.present? + if format parameters[:format] = format end @html_document = nil - unless @controller.respond_to?(:recycle!) - @controller.extend(Testing::Functional) - end - self.cookies.update @request.cookies self.cookies.update_cookies_from_jar @request.set_header 'HTTP_COOKIE', cookies.to_header diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index f6336c8c7a..1e4df07d6e 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2015 David Heinemeier Hansson +# Copyright (c) 2004-2016 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -95,6 +95,7 @@ module ActionDispatch autoload :TestProcess autoload :TestRequest autoload :TestResponse + autoload :AssertionResponse end end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 30ade14c26..4bd727c14e 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -80,9 +80,17 @@ module ActionDispatch set_header DATE, utc_time.httpdate end + # This method allows you to set the ETag for cached content, which + # will be returned to the end user. + # + # By default, Action Dispatch sets all ETags to be weak. + # This ensures that if the content changes only semantically, + # the whole page doesn't have to be regenerated from scratch + # by the web server. With strong ETags, pages are compared + # byte by byte, and are regenerated only if they are not exactly equal. def etag=(etag) key = ActiveSupport::Cache.expand_cache_key(etag) - super %("#{Digest::MD5.hexdigest(key)}") + super %(W/"#{Digest::MD5.hexdigest(key)}") end def etag?; etag; end @@ -91,7 +99,7 @@ module ActionDispatch DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze - SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public must-revalidate]) + SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) def cache_control_segments if cache_control = _cache_control diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 12f81dc1a5..8e899174c6 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 diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 0152c17ed4..e9b25339dc 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -67,10 +67,10 @@ module ActionDispatch v = if params_readable Array(Mime[parameters[:format]]) - elsif format = format_from_path_extension - Array(Mime[format]) elsif use_accept_header && valid_accept_header accepts + elsif extension_format = format_from_path_extension + [extension_format] elsif xhr? [Mime[:js]] else @@ -166,7 +166,7 @@ module ActionDispatch def format_from_path_extension path = @env['action_dispatch.original_path'] || @env['PATH_INFO'] if match = path && path.match(/\.(\w+)\z/) - match.captures.first + Mime[match.captures.first] end end end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index b8d395854c..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' @@ -31,7 +33,7 @@ module Mime SET = Mimes.new EXTENSION_LOOKUP = {} - LOOKUP = Hash.new { |h, k| h[k] = Type.new(k) unless k.blank? } + LOOKUP = {} class << self def [](type) @@ -106,70 +108,58 @@ module Mime result = @index <=> item.index if result == 0 result end - - def ==(item) - @name == item.to_s - 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 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 = 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 - exchange_xml_items if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list - delete_at(text_xml_idx) # delete text_xml from the list + if app_xml_idx > text_xml_idx # make sure app_xml is ahead of text_xml in the list + 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 + list.delete_at(text_xml_idx) # delete text_xml from the list elsif text_xml_idx - text_xml.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 = 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 - @app_xml_idx = idx + 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 text_xml_idx - @text_xml_idx ||= index('text/xml') - end - - def app_xml_idx - @app_xml_idx ||= index(Mime[:xml].to_s) - end - - def text_xml - self[text_xml_idx] - end - - def app_xml - self[app_xml_idx] - end - - def exchange_xml_items - self[app_xml_idx], self[text_xml_idx] = text_xml, app_xml - @app_xml_idx, @text_xml_idx = text_xml_idx, app_xml_idx - end + 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) @@ -177,7 +167,7 @@ module Mime end def lookup(string) - LOOKUP[string] + LOOKUP[string] || Type.new(string) end def lookup_by_extension(extension) @@ -209,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 @@ -255,9 +246,12 @@ module Mime end end + attr_reader :hash + def initialize(string, symbol = nil, synonyms = []) @symbol, @synonyms = symbol, synonyms @string = string + @hash = [@string, @synonyms, @symbol].hash end def to_s @@ -291,6 +285,13 @@ module Mime end end + def eql?(other) + super || (self.class == other.class && + @string == other.string && + @synonyms == other.synonyms && + @symbol == other.symbol) + end + def =~(mime_type) return false unless mime_type regexp = Regexp.new(Regexp.quote(mime_type.to_s)) @@ -303,6 +304,10 @@ module Mime def all?; false; end + protected + + attr_reader :string, :synonyms + private def to_ary; end diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 87715205d9..66cea88256 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -14,6 +14,7 @@ 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) @@ -27,7 +28,8 @@ 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 c9df787351..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") @@ -43,7 +52,7 @@ module ActionDispatch # # {'action' => 'my_action', 'controller' => 'my_controller'} def path_parameters - get_header(PARAMETERS_KEY) || {} + get_header(PARAMETERS_KEY) || set_header(PARAMETERS_KEY, {}) end private @@ -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 29cf821090..316a9f08b7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -259,7 +259,7 @@ module ActionDispatch end # Returns the IP address of client as a +String+, - # usually set by the RemoteIp middleware. + # usually set by the RemoteIp middleware. def remote_ip @remote_ip ||= (get_header("action_dispatch.remote_ip") || ip).to_s end @@ -403,6 +403,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 9b11111a67..fa4c54701a 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: @@ -232,7 +233,7 @@ module ActionDispatch # :nodoc: end # Sets the HTTP character set. In case of nil parameter - # it sets the charset to utf-8. + # it sets the charset to utf-8. # # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' @@ -412,6 +413,13 @@ module ActionDispatch # :nodoc: end def before_sending + # Normally we've already committed by now, but it's possible + # (e.g., if the controller action tries to read back its own + # response) to get here before that. In that case, we must force + # an "early" commit: we're about to freeze the headers, so this is + # our last chance. + commit! unless committed? + headers.freeze request.commit_cookie_jar! unless committed? 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/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 5ee8810066..018b89a2b7 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -124,7 +124,7 @@ module ActionDispatch end def captures - (length - 1).times.map { |i| self[i + 1] } + Array.new(length - 1) { |i| self[i + 1] } end def [](x) 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/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..51a471fb23 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -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 diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index c2a4f46e67..5841c978af 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ 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 = {}) + 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/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 47f475559a..711d8b016a 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -1,35 +1,39 @@ module ActionDispatch - # This middleware is added to the stack when `config.force_ssl = true`. - # It does three jobs to enforce secure HTTP requests: + # This middleware is added to the stack when `config.force_ssl = true`, and is passed + # the options set in `config.ssl_options`. It does three jobs to enforce secure HTTP + # requests: # - # 1. TLS redirect. http:// requests are permanently redirected to https:// - # with the same URL host, path, etc. Pass `:host` and/or `:port` to - # modify the destination URL. This is always enabled. + # 1. TLS redirect: Permanently redirects http:// requests to https:// + # with the same URL host, path, etc. Enabled by default. Set `config.ssl_options` + # to modify the destination URL + # (e.g. `redirect: { host: "secure.widgets.com", port: 8080 }`), or set + # `redirect: false` to disable this feature. # - # 2. Secure cookies. Sets the `secure` flag on cookies to tell browsers they - # mustn't be sent along with http:// requests. This is always enabled. + # 2. Secure cookies: Sets the `secure` flag on cookies to tell browsers they + # mustn't be sent along with http:// requests. Enabled by default. Set + # `config.ssl_options` with `secure_cookies: false` to disable this feature. # - # 3. HTTP Strict Transport Security (HSTS). Tells the browser to remember + # 3. HTTP Strict Transport Security (HSTS): Tells the browser to remember # this site as TLS-only and automatically redirect non-TLS requests. - # Enabled by default. Pass `hsts: false` to disable. + # Enabled by default. Configure `config.ssl_options` with `hsts: false` to disable. # - # Configure HSTS with `hsts: { … }`: + # Set `config.ssl_options` with `hsts: { … }` to configure HSTS: # * `expires`: How long, in seconds, these settings will stick. Defaults to # `180.days` (recommended). The minimum required to qualify for browser # preload lists is `18.weeks`. # * `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 # gap, browser vendors include a baked-in list of HSTS-enabled sites. # Go to https://hstspreload.appspot.com to submit your site for inclusion. # - # Disabling HSTS: To turn off HSTS, omitting the header is not enough. - # Browsers will remember the original HSTS directive until it expires. - # Instead, use the header to tell browsers to expire HSTS immediately. - # Setting `hsts: false` is a shortcut for `hsts: { expires: 0 }`. + # To turn off HSTS, omitting the header is not enough. Browsers will remember the + # original HSTS directive until it expires. Instead, use the header to tell browsers to + # expire HSTS immediately. Setting `hsts: false` is a shortcut for + # `hsts: { expires: 0 }`. class SSL # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/ # and greater than the 18-week requirement for browser preload lists. @@ -39,19 +43,31 @@ module ActionDispatch { expires: HSTS_EXPIRES_IN, subdomains: false, preload: false } end - def initialize(app, redirect: {}, hsts: {}, **options) + def initialize(app, redirect: {}, hsts: {}, secure_cookies: true, **options) @app = app if options[:host] || options[:port] 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 + @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 @@ -61,10 +77,11 @@ module ActionDispatch if request.ssl? @app.call(env).tap do |status, headers, body| set_hsts_header! headers - flag_cookies_as_secure! headers + flag_cookies_as_secure! headers if @secure_cookies end else - redirect_to_https request + return redirect_to_https request if @redirect + @app.call(env) end end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 44fc1ee736..0b4bee5462 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -14,6 +14,15 @@ module ActionDispatch def name; klass.name; end + def ==(middleware) + case middleware + when Middleware + klass == middleware.klass + when Class + klass == middleware + end + end + def inspect if klass.is_a?(Class) klass.to_s diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index ea9ab3821d..41c220236a 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -27,7 +27,7 @@ module ActionDispatch # in the server's `public/` directory (see Static#call). def match?(path) path = ::Rack::Utils.unescape_path path - return false unless path.valid_encoding? + return false unless valid_path?(path) path = Rack::Utils.clean_path_info path paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] @@ -94,6 +94,10 @@ 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/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 9e7fcbd849..42890225fa 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -109,7 +109,7 @@ module ActionDispatch @delegate.values end - # Writes given value to given key of the session. + # Writes given value to given key of the session. def []=(key, value) load_for_write! @delegate[key.to_s] = value @@ -148,8 +148,8 @@ module ActionDispatch @delegate.delete key.to_s end - # Returns value of given key from the session, or raises +KeyError+ - # if can't find given key in case of not setted dafault value. + # Returns value of the given key from the session, or raises +KeyError+ + # if can't find the given key and no default value is set. # Returns default value if specified. # # session.fetch(:foo) diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index d00b2c3eb5..dcf800b215 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -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 @@ -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 diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index f3a5268d2e..5d30a545a2 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -41,7 +41,7 @@ module ActionDispatch 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 + formatter.no_routes(collect_routes(@routes)) return formatter.result end @@ -82,9 +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 - @routes.select { |route| route.defaults[:controller] == filter } + @routes.select do |route| + route_wrapper = RouteWrapper.new(route) + filter.any? { |default, value| route_wrapper.send(default) =~ value } + end else @routes end @@ -136,14 +145,18 @@ module ActionDispatch @buffer << draw_header(routes) end - def no_routes - @buffer << <<-MESSAGE.strip_heredoc + def no_routes(routes) + @buffer << + if routes.none? + <<-MESSAGE.strip_heredoc You don't have any routes defined! Please add some routes in config/routes.rb. - - For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. MESSAGE + else + "No routes were found for this controller" + end + @buffer << "For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html." end private @@ -187,7 +200,7 @@ module ActionDispatch def header(routes) end - def no_routes + def no_routes(*) @buffer << <<-MESSAGE.strip_heredoc <p>You don't have any routes defined!</p> <ul> diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 18cd205bad..16b430c36e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -107,6 +107,7 @@ module ActionDispatch @ast = ast @anchor = anchor @via = via + @internal = options[:internal] path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -148,7 +149,8 @@ module ActionDispatch required_defaults, defaults, request_method, - precedence) + precedence, + @internal) route end @@ -184,26 +186,32 @@ module ActionDispatch def build_path(ast, requirements, anchor) pattern = Journey::Path::Pattern.new(ast, requirements, JOINED_SEPARATORS, anchor) - # Get all the symbol nodes followed by literals that are not the - # dummy node. - symbols = ast.find_all { |n| - n.cat? && n.left.symbol? && n.right.cat? && n.right.left.literal? - }.map(&:left) - - # Get all the symbol nodes preceded by literals. - symbols.concat ast.find_all { |n| - n.cat? && n.left.literal? && n.right.cat? && n.right.left.symbol? - }.map { |n| n.right.left } + # Find all the symbol nodes that are adjacent to literal nodes and alter + # the regexp so that Journey will partition them into custom routes. + ast.find_all { |node| + next unless node.cat? + + if node.left.literal? && node.right.symbol? + symbol = node.right + elsif node.left.literal? && node.right.cat? && node.right.left.symbol? + symbol = node.right.left + elsif node.left.symbol? && node.right.literal? + symbol = node.left + elsif node.left.symbol? && node.right.cat? && node.right.left.literal? + symbol = node.left + else + next + end - symbols.each { |x| - x.regexp = /(?:#{Regexp.union(x.regexp, '-')})+/ + if symbol + symbol.regexp = /(?:#{Regexp.union(symbol.regexp, '-')})+/ + end } pattern end private :build_path - private def add_wildcard_options(options, formatted, path_ast) # Add a constraint for wildcard route to make it non-greedy and match the @@ -387,24 +395,6 @@ module ActionDispatch end module Base - # You can specify what Rails should route "/" to with the root method: - # - # root to: 'pages#main' - # - # For options, see +match+, as +root+ uses it internally. - # - # You can also pass a string which will expand - # - # root 'pages#main' - # - # You should put the root route at the top of <tt>config/routes.rb</tt>, - # because this means it will be matched first. As this is the most popular route - # of most Rails applications, this is beneficial. - def root(options = {}) - name = has_named_route?(:root) ? nil : :root - match '/', { as: name, via: :get }.merge!(options) - end - # Matches a url pattern to one or more routes. # # You should not use the +match+ method in your router @@ -1689,7 +1679,20 @@ to this: @set.add_route(mapping, ast, as, anchor) end - def root(path, options={}) + # You can specify what Rails should route "/" to with the root method: + # + # root to: 'pages#main' + # + # For options, see +match+, as +root+ uses it internally. + # + # You can also pass a string which will expand + # + # root 'pages#main' + # + # You should put the root route at the top of <tt>config/routes.rb</tt>, + # because this means it will be matched first. As this is the most popular route + # of most Rails applications, this is beneficial. + def root(path, options = {}) if path.is_a?(String) options[:to] = path elsif path.is_a?(Hash) and options.empty? @@ -1701,11 +1704,11 @@ to this: if @scope.resources? with_scope_level(:root) do path_scope(parent_resource.path) do - super(options) + match_root_route(options) end end else - super(options) + match_root_route(options) end end @@ -1900,6 +1903,11 @@ to this: ensure @scope = @scope.parent end + + def match_root_route(options) + name = has_named_route?(:root) ? nil : :root + match '/', { :as => name, :via => :get }.merge!(options) + end end # Routing Concerns allow you to declare common routes that can be reused diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 2bd2e53252..310e98f584 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -281,8 +281,17 @@ module ActionDispatch helper = UrlHelper.create(route, opts, route_key, url_strategy) mod.module_eval do define_method(name) do |*args| - options = nil - options = args.pop if args.last.is_a? Hash + last = args.last + options = case last + when Hash + args.pop + when ActionController::Parameters + if last.permitted? + args.pop.to_h + else + raise ArgumentError, "Generating a URL from non sanitized request parameters is insecure!" + end + end helper.call self, args, options end end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index f91679593e..28be189f93 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("Generating a URL from non sanitized request parameters is insecure!") 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 new file mode 100644 index 0000000000..3fb81ff083 --- /dev/null +++ b/actionpack/lib/action_dispatch/testing/assertion_response.rb @@ -0,0 +1,49 @@ +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 + class AssertionResponse + attr_reader :code, :name + + GENERIC_RESPONSE_CODES = { # :nodoc: + success: "2XX", + missing: "404", + redirect: "3XX", + error: "5XX" + } + + def initialize(code_or_name) + if code_or_name.is_a?(Symbol) + @name = code_or_name + @code = code_from_name(code_or_name) + else + @name = name_from_code(code_or_name) + @code = code_or_name + end + + raise ArgumentError, "Invalid response name: #{name}" if @code.nil? + raise ArgumentError, "Invalid response code: #{code}" if @name.nil? + end + + def code_and_name + "#{code}: #{name}" + end + + private + + def code_from_name(name) + GENERIC_RESPONSE_CODES[name] || Rack::Utils::SYMBOL_TO_STATUS_CODE[name] + end + + def name_from_code(code) + GENERIC_RESPONSE_CODES.invert[code] || Rack::Utils::HTTP_STATUS_CODES[code] + end + end +end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index c138660a21..cd55b7d975 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -1,4 +1,3 @@ - module ActionDispatch module Assertions # A small suite of assertions that test responses from \Rails applications. @@ -29,18 +28,10 @@ module ActionDispatch def assert_response(type, message = nil) message ||= generate_response_message(type) - if Symbol === type - if [:success, :missing, :redirect, :error].include?(type) - assert @response.send(RESPONSE_PREDICATES[type]), message - else - code = Rack::Utils::SYMBOL_TO_STATUS_CODE[type] - if code.nil? - raise ArgumentError, "Invalid response type :#{type}" - end - assert_equal code, @response.response_code, message - end + if RESPONSE_PREDICATES.keys.include?(type) + assert @response.send(RESPONSE_PREDICATES[type]), message else - assert_equal type, @response.response_code, message + assert_equal AssertionResponse.new(type).code, @response.response_code, message end end @@ -85,8 +76,9 @@ module ActionDispatch end end - def generate_response_message(type, code = @response.response_code) - "Expected response to be a <#{type}>, but was a <#{code}>" + def generate_response_message(expected, actual = @response.response_code) + "Expected response to be a <#{code_with_name(expected)}>,"\ + " but was a <#{code_with_name(actual)}>" .concat location_if_redirected end @@ -95,6 +87,14 @@ module ActionDispatch location = normalize_argument_to_redirection(@response.location) " redirect to <#{location}>" end + + def code_with_name(code_or_name) + if RESPONSE_PREDICATES.values.include?("#{code_or_name}?".to_sym) + code_or_name = RESPONSE_PREDICATES.invert["#{code_or_name}?".to_sym] + end + + AssertionResponse.new(code_or_name).code_and_name + end end end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 78ef860548..e7af27463c 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 } diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 711ca10419..f4534b4173 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 @@ -321,7 +321,9 @@ module ActionDispatch 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 +332,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 +352,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 +381,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 +393,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,6 +699,39 @@ 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 @@ -671,5 +760,9 @@ module ActionDispatch def document_root_element html_document.root end + + def self.register_encoder(*args) + Integration::Session::RequestEncoder.register_encoder(*args) + end 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_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.rb b/actionpack/lib/action_pack.rb index f664dab620..941877d10d 100644 --- a/actionpack/lib/action_pack.rb +++ b/actionpack/lib/action_pack.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2015 David Heinemeier Hansson +# Copyright (c) 2004-2016 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/actionpack/lib/action_pack/gem_version.rb b/actionpack/lib/action_pack/gem_version.rb index 5cfb5f02d8..157f401f54 100644 --- a/actionpack/lib/action_pack/gem_version.rb +++ b/actionpack/lib/action_pack/gem_version.rb @@ -8,7 +8,7 @@ module ActionPack MAJOR = 5 MINOR = 0 TINY = 0 - PRE = "beta1" + PRE = "beta3" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end |