From 7bfcf4b3137b72cc6930bfb866f37015bfe67762 Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Mon, 12 Apr 2021 14:31:12 -0400 Subject: [PATCH] Remove Rack::Runtime and deprecate referencing it Previous discussion: #38412, #38325, 37423e4, 24f9c03 - Rack::Runtime is replaced by FakeRuntime, which is a dummy middleware that just passes requests on and cannot be used in middleware operations - Using Rack::Runtime in middleware operations (relative inserts, moves, etc.) throws a deprecation warning and uses FakeRuntime instead - if an application adds Rack::Runtime explicitly (use, unshift, etc.), then the deprecation warning does not happen and FakeRuntime is ignored - docs are updated to no longer reference Rack::Runtime --- .../lib/action_dispatch/middleware/stack.rb | 31 ++++++++++++++++++- .../test/dispatch/middleware_stack_test.rb | 23 ++++++++++++++ guides/source/api_app.md | 1 - guides/source/command_line.md | 2 +- guides/source/configuring.md | 3 +- guides/source/rails_on_rack.md | 9 ++---- railties/CHANGELOG.md | 5 +++ railties/lib/rails/application/bootstrap.rb | 2 +- .../application/default_middleware_stack.rb | 2 +- railties/test/application/middleware_test.rb | 22 ++++++------- 10 files changed, 75 insertions(+), 25 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index fc06c28d8a..a3baf489f9 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -5,6 +5,16 @@ require "active_support/dependencies" module ActionDispatch class MiddlewareStack + class FakeRuntime + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + end + end + class Middleware attr_reader :args, :block, :klass @@ -69,6 +79,7 @@ module ActionDispatch def initialize(*args) @middlewares = [] + @rack_runtime_deprecated = true yield(self) if block_given? end @@ -153,13 +164,31 @@ module ActionDispatch private def assert_index(index, where) - i = index.is_a?(Integer) ? index : middlewares.index { |m| m.klass == index } + i = index.is_a?(Integer) ? index : index_of(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(index) + raise "ActionDispatch::MiddlewareStack::FakeRuntime can not be referenced in middleware operations" if index == FakeRuntime + + if index == 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.klass == index || (@rack_runtime_deprecated && m.klass == FakeRuntime && index == Rack::Runtime) + end + end end end diff --git a/actionpack/test/dispatch/middleware_stack_test.rb b/actionpack/test/dispatch/middleware_stack_test.rb index a01eab82a7..52c40511d9 100644 --- a/actionpack/test/dispatch/middleware_stack_test.rb +++ b/actionpack/test/dispatch/middleware_stack_test.rb @@ -195,4 +195,27 @@ class MiddlewareStackTest < ActiveSupport::TestCase 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 diff --git a/guides/source/api_app.md b/guides/source/api_app.md index 912c71ea18..a7ef2aca0d 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -204,7 +204,6 @@ 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` diff --git a/guides/source/command_line.md b/guides/source/command_line.md index 6cc751ebd3..d2ac04e8a1 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -458,7 +458,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::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 +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 Application root /home/foobar/commandsapp Environment development Database adapter sqlite3 diff --git a/guides/source/configuring.md b/guides/source/configuring.md index d290924aa2..575f10728b 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -274,7 +274,6 @@ Every Rails application comes with a standard set of middleware which it uses in * `ActionDispatch::Static` is used to serve static assets. Disabled if `config.public_file_server.enabled` is `false`. Set `config.public_file_server.index_name` if you need to serve a static directory index file that is not named `index`. For example, to serve `main.html` instead of `index.html` for directory requests, set `config.public_file_server.index_name` to `"main"`. * `ActionDispatch::Executor` allows thread safe code reloading. Disabled if `config.allow_concurrency` is `false`, which causes `Rack::Lock` to be loaded. `Rack::Lock` wraps the app in mutex so it can only be called by a single thread at a time. * `ActiveSupport::Cache::Strategy::LocalCache` 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. * `ActionDispatch::ShowExceptions` rescues any exception returned by the application and renders nice exception pages if the request is local or if `config.consider_all_requests_local` is set to `true`. If `config.action_dispatch.show_exceptions` is set to `false`, exceptions will be raised regardless. * `ActionDispatch::RequestId` makes a unique X-Request-Id header available to the response and enables the `ActionDispatch::Request#uuid` method. Configurable with `config.action_dispatch.request_id_header`. @@ -1582,7 +1581,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 before `Rack::Runtime` 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 after `ActionDispatch::Executor` 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. diff --git a/guides/source/rails_on_rack.md b/guides/source/rails_on_rack.md index b6ff179ae9..3ab8512cf0 100644 --- a/guides/source/rails_on_rack.md +++ b/guides/source/rails_on_rack.md @@ -107,7 +107,6 @@ 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 @@ -175,10 +174,10 @@ Add the following lines to your application configuration: ```ruby # config/application.rb -config.middleware.delete Rack::Runtime +config.middleware.delete ActionDispatch::Executor ``` -And now if you inspect the middleware stack, you'll find that `Rack::Runtime` is +And now if you inspect the middleware stack, you'll find that `ActionDispatch::Executor` is not a part of it. ```bash @@ -230,10 +229,6 @@ 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. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index b87df6aa02..259bb60b8e 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Remove Rack::Runtime from the default middleware stack and deprecate + referencing it in middleware operations without adding it back + + *Hartley McGuire* + * Allow adding additional authorized hosts in development via `ENV['RAILS_DEVELOPMENT_HOSTS']` *Josh Abernathy*, *Debbie Milburn* diff --git a/railties/lib/rails/application/bootstrap.rb b/railties/lib/rails/application/bootstrap.rb index 295c073791..e6f24eff69 100644 --- a/railties/lib/rails/application/bootstrap.rb +++ b/railties/lib/rails/application/bootstrap.rb @@ -59,7 +59,7 @@ module Rails Rails.cache = ActiveSupport::Cache.lookup_store(*config.cache_store) if Rails.cache.respond_to?(:middleware) - config.middleware.insert_before(::Rack::Runtime, Rails.cache.middleware) + config.middleware.insert_after(ActionDispatch::Executor, Rails.cache.middleware) end end end diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 5a766dd60a..2b5cba4726 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -42,7 +42,7 @@ module Rails middleware.use ::ActionDispatch::Executor, app.executor - middleware.use ::Rack::Runtime + middleware.use ::ActionDispatch::MiddlewareStack::FakeRuntime 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 diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index ebd37a35a8..868a0f7997 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -31,7 +31,7 @@ module ApplicationTests "ActionDispatch::Static", "ActionDispatch::Executor", "ActiveSupport::Cache::Strategy::LocalCache", - "Rack::Runtime", + "ActionDispatch::MiddlewareStack::FakeRuntime", "Rack::MethodOverride", "ActionDispatch::RequestId", "ActionDispatch::RemoteIp", @@ -66,7 +66,7 @@ module ApplicationTests "ActionDispatch::Static", "ActionDispatch::Executor", "ActiveSupport::Cache::Strategy::LocalCache", - "Rack::Runtime", + "ActionDispatch::MiddlewareStack::FakeRuntime", "Rack::MethodOverride", "ActionDispatch::RequestId", "ActionDispatch::RemoteIp", @@ -101,7 +101,7 @@ module ApplicationTests "ActionDispatch::Static", "ActionDispatch::Executor", "ActiveSupport::Cache::Strategy::LocalCache", - "Rack::Runtime", + "ActionDispatch::MiddlewareStack::FakeRuntime", "ActionDispatch::RequestId", "ActionDispatch::RemoteIp", "Rails::Rack::Logger", @@ -225,19 +225,19 @@ module ApplicationTests 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::Runtime" - add_to_config "config.middleware.insert_before(Rack::Runtime, Rack::Config)" + add_to_config "config.middleware.delete Rack::Head" + add_to_config "config.middleware.insert_before(Rack::Head, Rack::Config)" boot! assert_includes middleware, "Rack::Config" - assert_not middleware.include?("Rack::Runtime") + assert_not middleware.include?("Rack::Head") 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::Runtime" - add_to_config "config.middleware.insert_after(Rack::Runtime, Rack::Config)" + add_to_config "config.middleware.delete Rack::Head" + add_to_config "config.middleware.insert_after(Rack::Head, Rack::Config)" boot! assert_includes middleware, "Rack::Config" - assert_not middleware.include?("Rack::Runtime") + assert_not middleware.include?("Rack::Head") end test "includes exceptions middlewares even if action_dispatch.show_exceptions is disabled" do @@ -275,14 +275,14 @@ module ApplicationTests test "Rails.cache does not respond to middleware" do add_to_config "config.cache_store = :memory_store, { timeout: 10 }" boot! - assert_equal "Rack::Runtime", middleware[5] + assert_equal "ActionDispatch::MiddlewareStack::FakeRuntime", middleware[5] 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[5] - assert_equal "Rack::Runtime", middleware[6] + assert_equal "ActionDispatch::MiddlewareStack::FakeRuntime", middleware[6] end test "insert middleware before" do