diff options
Diffstat (limited to 'actionpack/lib')
7 files changed, 107 insertions, 33 deletions
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/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/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 6548ce326b..700317614f 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -176,7 +176,7 @@ module ActionController def initialize(session = {}) super(nil, nil) @id = SecureRandom.hex(16) - @data = session.with_indifferent_access + @data = stringify_keys(session) @loaded = true end diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index fee08fc3db..cfd6681dd1 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -82,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/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 735b5939dd..711d8b016a 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 @@ -49,7 +49,7 @@ 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 @@ -57,6 +57,17 @@ module ActionDispatch 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 diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index 38d0da3e67..42890225fa 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 @@ -61,7 +61,7 @@ module ActionDispatch def initialize(by, req) @by = by @req = req - @delegate = {}.with_indifferent_access + @delegate = {} @loaded = false @exists = nil # we haven't checked yet end @@ -88,13 +88,13 @@ module ActionDispatch # nil if the given key is not found in the session. def [](key) load_for_read! - @delegate[key] + @delegate[key.to_s] end # Returns true if the session has the given key or false. def has_key?(key) load_for_read! - @delegate.key?(key) + @delegate.key?(key.to_s) end alias :key? :has_key? alias :include? :has_key? @@ -112,7 +112,7 @@ module ActionDispatch # Writes given value to given key of the session. def []=(key, value) load_for_write! - @delegate[key] = value + @delegate[key.to_s] = value end # Clears the session. @@ -139,13 +139,13 @@ module ActionDispatch # # => {"session_id"=>"e29b9ea315edf98aad94cc78c34cc9b2", "foo" => "bar"} def update(hash) load_for_write! - @delegate.update hash + @delegate.update stringify_keys(hash) end # Deletes given key from the session. def delete(key) load_for_write! - @delegate.delete key + @delegate.delete key.to_s end # Returns value of the given key from the session, or raises +KeyError+ @@ -165,9 +165,9 @@ module ActionDispatch def fetch(key, default=Unspecified, &block) load_for_read! if default == Unspecified - @delegate.fetch(key, &block) + @delegate.fetch(key.to_s, &block) else - @delegate.fetch(key, default, &block) + @delegate.fetch(key.to_s, default, &block) end end @@ -211,9 +211,15 @@ module ActionDispatch def load! id, session = @by.load_session @req options[:id] = id - @delegate.replace(session) + @delegate.replace(stringify_keys(session)) @loaded = true end + + def stringify_keys(other) + other.each_with_object({}) { |(key, value), hash| + hash[key.to_s] = value + } + end end end end diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 6f651a5689..5d30a545a2 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -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) |