Merge branch 'master' into guides_session_guidelines_2

16 files changed, 90 insertions, 69 deletions

diff --git a/actionpack/lib/abstract_controller.rb b/actionpack/lib/abstract_controller.rb
index 0477e7f1c9..3a98931167 100644
--- a/actionpack/lib/abstract_controller.rb
+++ b/actionpack/lib/abstract_controller.rb
@@ -7,6 +7,7 @@ require "active_support/i18n"
 module AbstractController
   extend ActiveSupport::Autoload
 
+  autoload :ActionNotFound, "abstract_controller/base"
   autoload :Base
   autoload :Caching
   autoload :Callbacks
diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb
index 93ffff1bd6..c276ee57c0 100644
--- a/actionpack/lib/action_controller/api.rb
+++ b/actionpack/lib/action_controller/api.rb
@@ -12,7 +12,7 @@ module ActionController
   #
   # An API Controller is different from a normal controller in the sense that
   # by default it doesn't include a number of features that are usually required
-  # by browser access only: layouts and templates rendering, cookies, sessions,
+  # by browser access only: layouts and templates rendering,
   # flash, assets, and so on. This makes the entire controller stack thinner,
   # suitable for API applications. It doesn't mean you won't have such
   # features if you need them: they're all available for you to include in
diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb
index afbd38e7fe..d8b04d8ddb 100644
--- a/actionpack/lib/action_controller/log_subscriber.rb
+++ b/actionpack/lib/action_controller/log_subscriber.rb
@@ -56,7 +56,7 @@ module ActionController
     def unpermitted_parameters(event)
       debug do
         unpermitted_keys = event.payload[:keys]
-        "Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}"
+        color("Unpermitted parameter#{'s' if unpermitted_keys.size > 1}: #{unpermitted_keys.map { |e| ":#{e}" }.join(", ")}", RED)
       end
     end
 
diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb
index 1482b2999a..083b762f5a 100644
--- a/actionpack/lib/action_controller/metal/live.rb
+++ b/actionpack/lib/action_controller/metal/live.rb
@@ -280,33 +280,35 @@ module ActionController
       raise error if error
     end
 
-    # Spawn a new thread to serve up the controller in. This is to get
-    # around the fact that Rack isn't based around IOs and we need to use
-    # a thread to stream data from the response bodies. Nobody should call
-    # this method except in Rails internals. Seriously!
-    def new_controller_thread # :nodoc:
-      Thread.new {
-        t2 = Thread.current
-        t2.abort_on_exception = true
-        yield
-      }
+    def response_body=(body)
+      super
+      response.close if response
     end
 
-    def log_error(exception)
-      logger = ActionController::Base.logger
-      return unless logger
+    private
 
-      logger.fatal do
-        message = +"\n#{exception.class} (#{exception.message}):\n"
-        message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
-        message << "  " << exception.backtrace.join("\n  ")
-        "#{message}\n\n"
+      # Spawn a new thread to serve up the controller in. This is to get
+      # around the fact that Rack isn't based around IOs and we need to use
+      # a thread to stream data from the response bodies. Nobody should call
+      # this method except in Rails internals. Seriously!
+      def new_controller_thread # :nodoc:
+        Thread.new {
+          t2 = Thread.current
+          t2.abort_on_exception = true
+          yield
+        }
       end
-    end
 
-    def response_body=(body)
-      super
-      response.close if response
-    end
+      def log_error(exception)
+        logger = ActionController::Base.logger
+        return unless logger
+
+        logger.fatal do
+          message = +"\n#{exception.class} (#{exception.message}):\n"
+          message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
+          message << "  " << exception.backtrace.join("\n  ")
+          "#{message}\n\n"
+        end
+      end
   end
 end
diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb
index a678377d4f..7361946de5 100644
--- a/actionpack/lib/action_controller/metal/params_wrapper.rb
+++ b/actionpack/lib/action_controller/metal/params_wrapper.rb
@@ -253,7 +253,10 @@ module ActionController
         # This will display the wrapped hash in the log file.
         request.filtered_parameters.merge! wrapped_filtered_hash
       end
-      super
+    ensure
+      # NOTE: Rescues all exceptions so they
+      # may be caught in ActionController::Rescue.
+      return super
     end
 
     private
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index c1272ce667..04922b0715 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -352,7 +352,7 @@ module ActionController
     # the same way as <tt>Hash#each_value</tt>.
     def each_value(&block)
       @parameters.each_pair do |key, value|
-        yield [convert_hashes_to_parameters(key, value)]
+        yield convert_hashes_to_parameters(key, value)
       end
     end
 
diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb
index 50953e32b5..b1e5a28be5 100644
--- a/actionpack/lib/action_dispatch/http/content_security_policy.rb
+++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb
@@ -22,7 +22,8 @@ module ActionDispatch #:nodoc:
 
         if policy = request.content_security_policy
           nonce = request.content_security_policy_nonce
-          headers[header_name(request)] = policy.build(request.controller_instance, nonce)
+          context = request.controller_instance || request
+          headers[header_name(request)] = policy.build(context, nonce)
         end
 
         response
@@ -257,7 +258,8 @@ module ActionDispatch #:nodoc:
           if context.nil?
             raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}"
           else
-            context.instance_exec(&source)
+            resolved = context.instance_exec(&source)
+            resolved.is_a?(Symbol) ? apply_mapping(resolved) : resolved
           end
         else
           raise RuntimeError, "Unexpected content security policy source: #{source.inspect}"
diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb
index 9f8f178402..cbb772175c 100644
--- a/actionpack/lib/action_dispatch/http/filter_parameters.rb
+++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb
@@ -41,6 +41,8 @@ module ActionDispatch
       # Returns a hash of parameters with all sensitive data replaced.
       def filtered_parameters
         @filtered_parameters ||= parameter_filter.filter(parameters)
+      rescue ActionDispatch::Http::Parameters::ParseError
+        @filtered_parameters = {}
       end
 
       # Returns a hash of request.env with all sensitive data replaced.
diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
index be129965d1..498b1e6695 100644
--- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb
+++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb
@@ -7,6 +7,11 @@ module ActionDispatch
     module MimeNegotiation
       extend ActiveSupport::Concern
 
+      RESCUABLE_MIME_FORMAT_ERRORS = [
+        ActionController::BadRequest,
+        ActionDispatch::Http::Parameters::ParseError,
+      ]
+
       included do
         mattr_accessor :ignore_accept_header, default: false
       end
@@ -59,7 +64,7 @@ module ActionDispatch
         fetch_header("action_dispatch.request.formats") do |k|
           params_readable = begin
                               parameters[:format]
-                            rescue ActionController::BadRequest
+                            rescue *RESCUABLE_MIME_FORMAT_ERRORS
                               false
                             end
 
diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb
index 8d7431fd6b..13d0963a33 100644
--- a/actionpack/lib/action_dispatch/http/parameters.rb
+++ b/actionpack/lib/action_dispatch/http/parameters.rb
@@ -111,13 +111,23 @@ module ActionDispatch
           begin
             strategy.call(raw_post)
           rescue # JSON or Ruby code block errors.
-            my_logger = logger || ActiveSupport::Logger.new($stderr)
-            my_logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{raw_post}"
-
+            log_parse_error_once
             raise ParseError
           end
         end
 
+        def log_parse_error_once
+          @parse_error_logged ||= begin
+            parse_logger = logger || ActiveSupport::Logger.new($stderr)
+            parse_logger.debug <<~MSG.chomp
+              Error occurred while parsing request parameters.
+              Contents:
+
+              #{raw_post}
+            MSG
+          end
+        end
+
         def params_parsers
           ActionDispatch::Request.parameter_parsers
         end
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 7bc364d370..44f23940d3 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -383,9 +383,6 @@ module ActionDispatch
         end
         self.request_parameters = Request::Utils.normalize_encode_params(pr)
       end
-    rescue Http::Parameters::ParseError # one of the parse strategies blew up
-      self.request_parameters = Request::Utils.normalize_encode_params(super || {})
-      raise
     rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
       raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}")
     end
diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb
index 32f632800c..086d6a3e07 100644
--- a/actionpack/lib/action_dispatch/journey/nodes/node.rb
+++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb
@@ -65,12 +65,12 @@ module ActionDispatch
         def literal?; false; end
       end
 
-      %w{ Symbol Slash Dot }.each do |t|
-        class_eval <<-eoruby, __FILE__, __LINE__ + 1
-          class #{t} < Terminal;
-            def type; :#{t.upcase}; end
-          end
-        eoruby
+      class Slash < Terminal # :nodoc:
+        def type; :SLASH; end
+      end
+
+      class Dot < Terminal # :nodoc:
+        def type; :DOT; end
       end
 
       class Symbol < Terminal # :nodoc:
@@ -89,6 +89,7 @@ module ActionDispatch
           regexp == DEFAULT_EXP
         end
 
+        def type; :SYMBOL; end
         def symbol?; true; end
       end
 
diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb
index 30af3ff930..89a164f968 100644
--- a/actionpack/lib/action_dispatch/journey/router.rb
+++ b/actionpack/lib/action_dispatch/journey/router.rb
@@ -15,9 +15,6 @@ require "action_dispatch/journey/path/pattern"
 module ActionDispatch
   module Journey # :nodoc:
     class Router # :nodoc:
-      class RoutingError < ::StandardError # :nodoc:
-      end
-
       attr_accessor :routes
 
       def initialize(routes)
diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index 5f5fdbc66a..7669767ae3 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -180,11 +180,14 @@ module ActionDispatch
         trace = wrapper.framework_trace if trace.empty?
 
         ActiveSupport::Deprecation.silence do
-          logger.fatal "  "
-          logger.fatal "#{exception.class} (#{exception.message}):"
-          log_array logger, exception.annoted_source_code if exception.respond_to?(:annoted_source_code)
-          logger.fatal "  "
-          log_array logger, trace
+          message = []
+          message << "  "
+          message << "#{exception.class} (#{exception.message}):"
+          message.concat(exception.annoted_source_code) if exception.respond_to?(:annoted_source_code)
+          message << "  "
+          message.concat(trace)
+
+          log_array(logger, message)
         end
       end
 
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 99f3b4c2cd..d67044b4ac 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -160,17 +160,8 @@ module ActionDispatch
         end
 
         def make_route(name, precedence)
-          route = Journey::Route.new(name,
-                            application,
-                            path,
-                            conditions,
-                            required_defaults,
-                            defaults,
-                            request_method,
-                            precedence,
-                            @internal)
-
-          route
+          Journey::Route.new(name, application, path, conditions, required_defaults,
+                             defaults, request_method, precedence, @internal)
         end
 
         def application
@@ -656,7 +647,7 @@ module ActionDispatch
 
         # Query if the following named route was already defined.
         def has_named_route?(name)
-          @set.named_routes.key? name
+          @set.named_routes.key?(name)
         end
 
         private
@@ -1171,10 +1162,16 @@ module ActionDispatch
           end
 
           def actions
+            if @except
+              available_actions - Array(@except).map(&:to_sym)
+            else
+              available_actions
+            end
+          end
+
+          def available_actions
             if @only
               Array(@only).map(&:to_sym)
-            elsif @except
-              default_actions - Array(@except).map(&:to_sym)
             else
               default_actions
             end
@@ -1946,9 +1943,7 @@ module ActionDispatch
           end
 
           def match_root_route(options)
-            name = has_named_route?(name_for_action(:root, nil)) ? nil : :root
-            args = ["/", { as: name, via: :get }.merge!(options)]
-
+            args = ["/", { as: :root, via: :get }.merge(options)]
             match(*args)
           end
       end
diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
index e47d5020f4..600e9c733b 100644
--- a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
+++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
@@ -17,8 +17,11 @@ module ActionDispatch
         end
 
         def after_teardown
-          take_failed_screenshot
-          Capybara.reset_sessions!
+          begin
+            take_failed_screenshot
+          ensure
+            Capybara.reset_sessions!
+          end
         ensure
           super
         end