Add back Rack::Runtime to the default middleware stack.

We were planning to remove this middleware because we thought it could
make easier to attacker to do a Time Attack. However, while
Rack::Runtime can indeed be used to know how long a request took, and
compare with other requests, it doesn't provide any information that
can't be found in the total time of the request as well.

Instead of removing the middleware, we decided to keep it, and direct
users to instead of removing it, use its information to uncover actions
that are vulnerable to Time Attack.

This reverts commit 127dd06, reversing
changes made to 4354e3a.

Author: rafaelfranca

actionpack/lib/action_dispatch/middleware/stack.rb

@@ -5,16 +5,6 @@
 
 module ActionDispatch
   class MiddlewareStack
-    class FakeRuntime # :nodoc:
-      def initialize(app)
-        @app = app
-      end
-
-      def call(env)
-        @app.call(env)
-      end
-    end
-
     class Middleware
       attr_reader :args, :block, :klass
 
@@ -79,7 +69,6 @@ def call(env)
 
     def initialize(*args)
       @middlewares = []
-      @rack_runtime_deprecated = true
       yield(self) if block_given?
     end
 
@@ -181,31 +170,13 @@ def build(app = nil, &block)
 
     private
       def assert_index(index, where)
-        i = index.is_a?(Integer) ? index : index_of(index)
+        i = index.is_a?(Integer) ? index : middlewares.index { |m| m.klass == index }
         raise "No such middleware to insert #{where}: #{index.inspect}" unless i
         i
       end
 
       def build_middleware(klass, args, block)
-        @rack_runtime_deprecated = false if klass == Rack::Runtime
-
         Middleware.new(klass, args, block)
       end
-
-      def index_of(klass)
-        raise "ActionDispatch::MiddlewareStack::FakeRuntime can not be referenced in middleware operations" if klass == FakeRuntime
-
-        if klass == Rack::Runtime && @rack_runtime_deprecated
-          ActiveSupport::Deprecation.warn(<<-MSG.squish)
-            Rack::Runtime is removed from the default middleware stack in Rails
-            and referencing it in middleware operations without adding it back
-            is deprecated and will throw an error in Rails 7.1
-          MSG
-        end
-
-        middlewares.index do |m|
-          m.name == klass.name || (@rack_runtime_deprecated && m.klass == FakeRuntime && klass == Rack::Runtime)
-        end
-      end
   end
 end

actionpack/test/dispatch/middleware_stack_test.rb

@@ -213,27 +213,4 @@ def test_delete_works
   test "includes a middleware" do
     assert_equal true, @stack.include?(ActionDispatch::MiddlewareStack::Middleware.new(BarMiddleware, nil, nil))
   end
-
-  test "referencing Rack::Runtime is deprecated" do
-    @stack.use ActionDispatch::MiddlewareStack::FakeRuntime
-
-    assert_deprecated(/Rack::Runtime is removed/) do
-      @stack.insert_after(Rack::Runtime, BazMiddleware)
-    end
-  end
-
-  test "referencing Rack::Runtime is not deprecated if added" do
-    assert_not_deprecated do
-      @stack.use Rack::Runtime
-      @stack.insert_before(Rack::Runtime, BazMiddleware)
-    end
-  end
-
-  test "referencing FakeRuntime throws an error" do
-    @stack.use ActionDispatch::MiddlewareStack::FakeRuntime
-
-    assert_raises RuntimeError do
-      @stack.insert_after ActionDispatch::MiddlewareStack::FakeRuntime, BazMiddleware
-    end
-  end
 end

guides/source/api_app.md

@@ -204,6 +204,7 @@ An API application comes with the following middleware by default:
 - `ActionDispatch::Static`
 - `ActionDispatch::Executor`
 - `ActiveSupport::Cache::Strategy::LocalCache::Middleware`
+- `Rack::Runtime`
 - `ActionDispatch::RequestId`
 - `ActionDispatch::RemoteIp`
 - `Rails::Rack::Logger`

guides/source/command_line.md

@@ -446,7 +446,7 @@ Ruby version              2.7.0 (x86_64-linux)
 RubyGems version          2.7.3
 Rack version              2.0.4
 JavaScript Runtime        Node.js (V8)
-Middleware:               Rack::Sendfile, ActionDispatch::Static, ActionDispatch::Executor, ActiveSupport::Cache::Strategy::LocalCache::Middleware, Rack::MethodOverride, ActionDispatch::RequestId, ActionDispatch::RemoteIp, Sprockets::Rails::QuietAssets, Rails::Rack::Logger, ActionDispatch::ShowExceptions, WebConsole::Middleware, ActionDispatch::DebugExceptions, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, Rack::Head, Rack::ConditionalGet, Rack::ETag
+Middleware:               Rack::Sendfile, ActionDispatch::Static, ActionDispatch::Executor, ActiveSupport::Cache::Strategy::LocalCache::Middleware, Rack::Runtime, Rack::MethodOverride, ActionDispatch::RequestId, ActionDispatch::RemoteIp, Sprockets::Rails::QuietAssets, Rails::Rack::Logger, ActionDispatch::ShowExceptions, WebConsole::Middleware, ActionDispatch::DebugExceptions, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, Rack::Head, Rack::ConditionalGet, Rack::ETag
 Application root          /home/foobar/commandsapp
 Environment               development
 Database adapter          sqlite3

guides/source/configuring.md

@@ -414,6 +414,10 @@ Allows thread safe code reloading. Disabled if `config.allow_concurrency` is `fa
 
 Serves as a basic memory backed cache. This cache is not thread safe and is intended only for serving as a temporary memory cache for a single thread.
 
+#### `Rack::Runtime`
+
+Sets an `X-Runtime` header, containing the time (in seconds) taken to execute the request.
+
 #### `Rails::Rack::Logger`
 
 Notifies the logs that the request has begun. After request is complete, flushes all the logs.
@@ -1503,7 +1507,7 @@ The image analyzers can extract width and height of an image blob; the video ana
 
 #### `config.active_storage.previewers`
 
-Accepts an array of classes indicating the image previewers available in Active Storage blobs. 
+Accepts an array of classes indicating the image previewers available in Active Storage blobs.
 By default, this is defined as:
 
 ```ruby
@@ -2212,7 +2216,7 @@ Below is a comprehensive list of all the initializers found in Rails in the orde
 
 * `initialize_logger`: Initializes the logger (an `ActiveSupport::Logger` object) for the application and makes it accessible at `Rails.logger`, provided that no initializer inserted before this point has defined `Rails.logger`.
 
-* `initialize_cache`: If `Rails.cache` isn't set yet, initializes the cache by referencing the value in `config.cache_store` and stores the outcome as `Rails.cache`. If this object responds to the `middleware` method, its middleware is inserted after `ActionDispatch::Executor` in the middleware stack.
+* `initialize_cache`: If `Rails.cache` isn't set yet, initializes the cache by referencing the value in `config.cache_store` and stores the outcome as `Rails.cache`. If this object responds to the `middleware` method, its middleware is inserted before `Rack::Runtime` in the middleware stack.
 
 * `set_clear_dependencies_hook`: This initializer - which runs only if `cache_classes` is set to `false` - uses `ActionDispatch::Callbacks.after` to remove the constants which have been referenced during the request from the object space so that they will be reloaded during the following request.
 

guides/source/rails_on_rack.md

@@ -107,6 +107,7 @@ use Rack::Sendfile
 use ActionDispatch::Static
 use ActionDispatch::Executor
 use ActiveSupport::Cache::Strategy::LocalCache::Middleware
+use Rack::Runtime
 use Rack::MethodOverride
 use ActionDispatch::RequestId
 use ActionDispatch::RemoteIp
@@ -174,10 +175,10 @@ Add the following lines to your application configuration:
 
 ```ruby
 # config/application.rb
-config.middleware.delete ActionDispatch::Executor
+config.middleware.delete Rack::Runtime
 ```
 
-And now if you inspect the middleware stack, you'll find that `ActionDispatch::Executor` is
+And now if you inspect the middleware stack, you'll find that `Rack::Runtime` is
 not a part of it.
 
 ```bash
@@ -236,6 +237,10 @@ Much of Action Controller's functionality is implemented as Middlewares. The fol
 
 * Used for memory caching. This cache is not thread safe.
 
+**`Rack::Runtime`**
+
+* Sets an X-Runtime header, containing the time (in seconds) taken to execute the request.
+
 **`Rack::MethodOverride`**
 
 * Allows the method to be overridden if `params[:_method]` is set. This is the middleware which supports the PUT and DELETE HTTP method types.

railties/lib/rails/application/bootstrap.rb

@@ -59,7 +59,7 @@ module Bootstrap
           Rails.cache = ActiveSupport::Cache.lookup_store(*config.cache_store)
 
           if Rails.cache.respond_to?(:middleware)
-            config.middleware.insert_after(ActionDispatch::Executor, Rails.cache.middleware)
+            config.middleware.insert_before(::Rack::Runtime, Rails.cache.middleware)
           end
         end
       end

railties/lib/rails/application/default_middleware_stack.rb

@@ -42,7 +42,7 @@ def build_stack
 
           middleware.use ::ActionDispatch::Executor, app.executor
 
-          middleware.use ::ActionDispatch::MiddlewareStack::FakeRuntime
+          middleware.use ::Rack::Runtime
           middleware.use ::Rack::MethodOverride unless config.api_only
           middleware.use ::ActionDispatch::RequestId, header: config.action_dispatch.request_id_header
           middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies

railties/test/application/middleware_test.rb

@@ -30,7 +30,7 @@ def app
         "ActionDispatch::Static",
         "ActionDispatch::Executor",
         "ActiveSupport::Cache::Strategy::LocalCache",
-        "ActionDispatch::MiddlewareStack::FakeRuntime",
+        "Rack::Runtime",
         "Rack::MethodOverride",
         "ActionDispatch::RequestId",
         "ActionDispatch::RemoteIp",
@@ -64,7 +64,7 @@ def app
         "ActionDispatch::Static",
         "ActionDispatch::Executor",
         "ActiveSupport::Cache::Strategy::LocalCache",
-        "ActionDispatch::MiddlewareStack::FakeRuntime",
+        "Rack::Runtime",
         "Rack::MethodOverride",
         "ActionDispatch::RequestId",
         "ActionDispatch::RemoteIp",
@@ -98,7 +98,7 @@ def app
         "ActionDispatch::Static",
         "ActionDispatch::Executor",
         "ActiveSupport::Cache::Strategy::LocalCache",
-        "ActionDispatch::MiddlewareStack::FakeRuntime",
+        "Rack::Runtime",
         "ActionDispatch::RequestId",
         "ActionDispatch::RemoteIp",
         "Rails::Rack::Logger",
@@ -222,19 +222,19 @@ def app
     end
 
     test "can delete a middleware from the stack even if insert_before is added after delete" do
-      add_to_config "config.middleware.delete Rack::Head"
-      add_to_config "config.middleware.insert_before(Rack::Head, Rack::Config)"
+      add_to_config "config.middleware.delete Rack::Runtime"
+      add_to_config "config.middleware.insert_before(Rack::Runtime, Rack::Config)"
       boot!
       assert_includes middleware, "Rack::Config"
-      assert_not middleware.include?("Rack::Head")
+      assert_not middleware.include?("Rack::Runtime")
     end
 
     test "can delete a middleware from the stack even if insert_after is added after delete" do
-      add_to_config "config.middleware.delete Rack::Head"
-      add_to_config "config.middleware.insert_after(Rack::Head, Rack::Config)"
+      add_to_config "config.middleware.delete Rack::Runtime"
+      add_to_config "config.middleware.insert_after(Rack::Runtime, Rack::Config)"
       boot!
       assert_includes middleware, "Rack::Config"
-      assert_not middleware.include?("Rack::Head")
+      assert_not middleware.include?("Rack::Runtime")
     end
 
     test "includes exceptions middlewares even if action_dispatch.show_exceptions is disabled" do
@@ -272,14 +272,14 @@ def app
     test "Rails.cache does not respond to middleware" do
       add_to_config "config.cache_store = :memory_store, { timeout: 10 }"
       boot!
-      assert_equal "ActionDispatch::MiddlewareStack::FakeRuntime", middleware[4]
+      assert_equal "Rack::Runtime", middleware[4]
       assert_instance_of ActiveSupport::Cache::MemoryStore, Rails.cache
     end
 
     test "Rails.cache does respond to middleware" do
       boot!
       assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware[4]
-      assert_equal "ActionDispatch::MiddlewareStack::FakeRuntime", middleware[5]
+      assert_equal "Rack::Runtime", middleware[5]
     end
 
     test "insert middleware before" do