From e3056e737f68a9e15df0ec578d2d7f5e6dec9100 Mon Sep 17 00:00:00 2001
From: Gonzalo Rodriguez <grzuy0@gmail.com>
Date: Tue, 29 Oct 2019 15:45:26 -0300
Subject: [PATCH] fix: avoid unintended effects on load_config_initializers and
 other gems load order

Because of the sort algorithm rails uses to satisfy `after` and `before`
constraints, gems can have unintended effects on others. See
https://github.com/rails/rails/commit/0a120a818d413c64ff9867125f0b03788fc306f8

Prefer making rack-attack middleware idempotent instead of relying on
the load order and the contents of the middleware stack too much.

closes #452
closes #456
---
 lib/rack/attack.rb                       |  3 ++-
 lib/rack/attack/railtie.rb               | 12 ++----------
 spec/acceptance/rails_middleware_spec.rb |  6 ------
 spec/spec_helper.rb                      |  2 ++
 4 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb
index b33092d4..add91c7e 100644
--- a/lib/rack/attack.rb
+++ b/lib/rack/attack.rb
@@ -153,8 +153,9 @@ def initialize(app)
     end
 
     def call(env)
-      return @app.call(env) unless self.class.enabled
+      return @app.call(env) if !self.class.enabled || env["rack.attack.called"]
 
+      env["rack.attack.called"] = true
       env['PATH_INFO'] = PathNormalizer.normalize_path(env['PATH_INFO'])
       request = Rack::Attack::Request.new(env)
 
diff --git a/lib/rack/attack/railtie.rb b/lib/rack/attack/railtie.rb
index ceed9f7d..398ac6c2 100644
--- a/lib/rack/attack/railtie.rb
+++ b/lib/rack/attack/railtie.rb
@@ -3,17 +3,9 @@
 module Rack
   class Attack
     class Railtie < ::Rails::Railtie
-      initializer 'rack.attack.middleware', after: :load_config_initializers, before: :build_middleware_stack do |app|
+      initializer "rack-attack.middleware" do |app|
         if Gem::Version.new(::Rails::VERSION::STRING) >= Gem::Version.new("5.1")
-          middlewares = app.config.middleware
-          operations = middlewares.send(:operations) + middlewares.send(:delete_operations)
-
-          use_middleware = operations.none? do |operation|
-            middleware = operation[1]
-            middleware.include?(Rack::Attack)
-          end
-
-          middlewares.use(Rack::Attack) if use_middleware
+          app.middleware.use(Rack::Attack)
         end
       end
     end
diff --git a/spec/acceptance/rails_middleware_spec.rb b/spec/acceptance/rails_middleware_spec.rb
index 2c69b7e1..8e7b014a 100644
--- a/spec/acceptance/rails_middleware_spec.rb
+++ b/spec/acceptance/rails_middleware_spec.rb
@@ -18,12 +18,6 @@
         assert_equal 1, @app.middleware.count(Rack::Attack)
       end
 
-      it "is not added when it was added explicitly" do
-        @app.config.middleware.use(Rack::Attack)
-        @app.initialize!
-        assert_equal 1, @app.middleware.count(Rack::Attack)
-      end
-
       it "is not added when it was explicitly deleted" do
         @app.config.middleware.delete(Rack::Attack)
         @app.initialize!
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index a1728cb6..0389bd01 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -46,6 +46,8 @@ def app
     Rack::Builder.new do
       # Use Rack::Lint to test that rack-attack is complying with the rack spec
       use Rack::Lint
+      # Intentionally added twice to test idempotence property
+      use Rack::Attack
       use Rack::Attack
       use Rack::Lint