1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Revert "removing Rack::Runtime from the default stack."

This reverts commit 37423e4ff8.

Jeremy is right that we shouldn't remove this.  The fact is that many
engines are depending on this middleware to be in the default stack.
This ties our hands and forces us to keep the middleware in the stack so
that engines will work.  To be extremely clear, I think this is another
smell of "the rack stack" that we have in place.  When manipulating
middleware, we should have meaningful names for places in the req / res
lifecycle **not** have engines depend on a particular constant be in a
particular place in the stack.  This is a weakness of the API that we
have to figure out a way to address before removing the constant.

As far as timing attacks are concerned, we can reduce the granularity
such that it isn't useful information for hackers, but is still useful
for developers.
This commit is contained in:
Aaron Patterson 2015-10-03 14:21:31 -07:00
parent 99392112c5
commit 24f9c03d52
7 changed files with 16 additions and 14 deletions

View file

@ -188,6 +188,7 @@ An API application comes with the following middlewares by default:
- `ActiveSupport::Cache::Strategy::LocalCache::Middleware`
- `ActionDispatch::RequestId`
- `Rails::Rack::Logger`
- `Rack::Runtime`
- `ActionDispatch::ShowExceptions`
- `ActionDispatch::DebugExceptions`
- `ActionDispatch::RemoteIp`

View file

@ -412,7 +412,7 @@ Ruby version 2.2.2 (x86_64-linux)
RubyGems version 2.4.6
Rack version 1.6
JavaScript Runtime Node.js (V8)
Middleware Rack::Sendfile, ActionDispatch::Static, Rack::Lock, #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x007ffd131a7c88>, Rack::MethodOverride, ActionDispatch::RequestId, Rails::Rack::Logger, ActionDispatch::ShowExceptions, ActionDispatch::DebugExceptions, ActionDispatch::RemoteIp, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, Rack::Head, Rack::ConditionalGet, Rack::ETag
Middleware Rack::Sendfile, ActionDispatch::Static, Rack::Lock, #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x007ffd131a7c88>, Rack::Runtime, Rack::MethodOverride, ActionDispatch::RequestId, Rails::Rack::Logger, ActionDispatch::ShowExceptions, ActionDispatch::DebugExceptions, ActionDispatch::RemoteIp, ActionDispatch::Reloader, ActionDispatch::Callbacks, ActiveRecord::Migration::CheckPending, ActiveRecord::ConnectionAdapters::ConnectionManagement, ActiveRecord::QueryCache, ActionDispatch::Cookies, ActionDispatch::Session::CookieStore, ActionDispatch::Flash, Rack::Head, Rack::ConditionalGet, Rack::ETag
Application root /home/foobar/commandsapp
Environment development
Database adapter sqlite3

View file

@ -106,6 +106,7 @@ use Rack::Sendfile
use ActionDispatch::Static
use Rack::Lock
use #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x000000029a0838>
use Rack::Runtime
use Rack::MethodOverride
use ActionDispatch::RequestId
use Rails::Rack::Logger

View file

@ -1,6 +1,3 @@
* Removed Rack::Runtime from the default stack. It can be added back via
`config.middleware.use ::Rack::Runtime`.
* Add fail fast to `bin/rails test`
Adding `--fail-fast` or `-f` when running tests will interrupt the run on

View file

@ -63,7 +63,7 @@ INFO
Rails.cache = ActiveSupport::Cache.lookup_store(config.cache_store)
if Rails.cache.respond_to?(:middleware)
config.middleware.insert_before(::ActionDispatch::RequestId, Rails.cache.middleware)
config.middleware.insert_before(::Rack::Runtime, Rails.cache.middleware)
end
end
end

View file

@ -47,6 +47,7 @@ module Rails
end
end
middleware.use ::Rack::Runtime
middleware.use ::Rack::MethodOverride unless config.api_only
middleware.use ::ActionDispatch::RequestId

View file

@ -27,8 +27,9 @@ module ApplicationTests
"Rack::Sendfile",
"ActionDispatch::Static",
"ActionDispatch::LoadInterlock",
"Rack::MethodOverride",
"ActiveSupport::Cache::Strategy::LocalCache",
"Rack::Runtime",
"Rack::MethodOverride",
"ActionDispatch::RequestId",
"Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
"ActionDispatch::ShowExceptions",
@ -58,6 +59,7 @@ module ApplicationTests
"ActionDispatch::Static",
"ActionDispatch::LoadInterlock",
"ActiveSupport::Cache::Strategy::LocalCache",
"Rack::Runtime",
"ActionDispatch::RequestId",
"Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
"ActionDispatch::ShowExceptions",
@ -166,19 +168,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 ActionDispatch::ShowExceptions"
add_to_config "config.middleware.insert_before(ActionDispatch::ShowExceptions, Rack::Config)"
add_to_config "config.middleware.delete Rack::Runtime"
add_to_config "config.middleware.insert_before(Rack::Runtime, Rack::Config)"
boot!
assert middleware.include?("Rack::Config")
assert_not middleware.include?("ActionDispatch::ShowExceptions")
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 ActionDispatch::ShowExceptions"
add_to_config "config.middleware.insert_after(ActionDispatch::ShowExceptions, Rack::Config)"
add_to_config "config.middleware.delete Rack::Runtime"
add_to_config "config.middleware.insert_after(Rack::Runtime, Rack::Config)"
boot!
assert middleware.include?("Rack::Config")
assert_not middleware.include?("ActionDispatch::ShowExceptions")
assert_not middleware.include?("Rack::Runtime")
end
test "includes exceptions middlewares even if action_dispatch.show_exceptions is disabled" do
@ -216,12 +218,12 @@ module ApplicationTests
test "Rails.cache does not respond to middleware" do
add_to_config "config.cache_store = :memory_store"
boot!
assert_equal "Rack::MethodOverride", middleware.fourth
assert_equal "Rack::Runtime", middleware.fourth
end
test "Rails.cache does respond to middleware" do
boot!
assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware.fifth
assert_equal "Rack::Runtime", middleware.fifth
end
test "insert middleware before" do