From 975c4635d257ea3c415b06f595c36dbb67948bf4 Mon Sep 17 00:00:00 2001
From: Lucas Mansur <lucas.mansur2@gmail.com>
Date: Tue, 20 Feb 2018 00:30:44 -0300
Subject: [PATCH] Fix several code style offenses

---
 lib/rack/attack.rb                            | 38 ++++++++++---------
 lib/rack/attack/allow2ban.rb                  |  5 +--
 lib/rack/attack/blocklist.rb                  |  1 -
 lib/rack/attack/cache.rb                      |  5 +--
 lib/rack/attack/check.rb                      | 10 ++---
 lib/rack/attack/fail2ban.rb                   | 15 ++++----
 lib/rack/attack/safelist.rb                   |  1 -
 lib/rack/attack/store_proxy.rb                |  2 +-
 lib/rack/attack/store_proxy/dalli_proxy.rb    | 12 +++---
 .../attack/store_proxy/mem_cache_proxy.rb     | 14 +++----
 .../attack/store_proxy/redis_store_proxy.rb   |  6 +--
 lib/rack/attack/throttle.rb                   | 14 ++++---
 lib/rack/attack/track.rb                      | 10 ++---
 13 files changed, 64 insertions(+), 69 deletions(-)

diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb
index 8c9df482..cb3f112e 100644
--- a/lib/rack/attack.rb
+++ b/lib/rack/attack.rb
@@ -21,11 +21,10 @@ class MissingStoreError < StandardError; end
   autoload :Request,         'rack/attack/request'
 
   class << self
-
     attr_accessor :notifier, :blocklisted_response, :throttled_response
 
     def safelist(name, &block)
-      self.safelists[name] = Safelist.new(name, block)
+      safelists[name] = Safelist.new(name, block)
     end
 
     def whitelist(name, &block)
@@ -34,7 +33,7 @@ def whitelist(name, &block)
     end
 
     def blocklist(name, &block)
-      self.blocklists[name] = Blocklist.new(name, block)
+      blocklists[name] = Blocklist.new(name, block)
     end
 
     def blacklist(name, &block)
@@ -43,11 +42,11 @@ def blacklist(name, &block)
     end
 
     def throttle(name, options, &block)
-      self.throttles[name] = Throttle.new(name, options, block)
+      throttles[name] = Throttle.new(name, options, block)
     end
 
     def track(name, options = {}, &block)
-      self.tracks[name] = Track.new(name, options, block)
+      tracks[name] = Track.new(name, options, block)
     end
 
     def safelists;  @safelists  ||= {}; end
@@ -66,7 +65,7 @@ def blacklists
     end
 
     def safelisted?(req)
-      safelists.any? do |name, safelist|
+      safelists.any? do |_name, safelist|
         safelist[req]
       end
     end
@@ -77,7 +76,7 @@ def whitelisted?(req)
     end
 
     def blocklisted?(req)
-      blocklists.any? do |name, blocklist|
+      blocklists.any? do |_name, blocklist|
         blocklist[req]
       end
     end
@@ -88,7 +87,7 @@ def blacklisted?(req)
     end
 
     def throttled?(req)
-      throttles.any? do |name, throttle|
+      throttles.any? do |_name, throttle|
         throttle[req]
       end
     end
@@ -108,27 +107,29 @@ def cache
     end
 
     def clear!
-      @safelists, @blocklists, @throttles, @tracks = {}, {}, {}, {}
+      @safelists = {}
+      @blocklists = {}
+      @throttles = {}
+      @tracks = {}
     end
 
     def blacklisted_response=(res)
       warn "[DEPRECATION] 'Rack::Attack.blacklisted_response=' is deprecated.  Please use 'blocklisted_response=' instead."
-      self.blocklisted_response=(res)
+      self.blocklisted_response = res
     end
 
     def blacklisted_response
       warn "[DEPRECATION] 'Rack::Attack.blacklisted_response' is deprecated.  Please use 'blocklisted_response' instead."
       blocklisted_response
     end
-
   end
 
   # Set defaults
   @notifier             = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications)
-  @blocklisted_response = lambda {|env| [403, {'Content-Type' => 'text/plain'}, ["Forbidden\n"]] }
-  @throttled_response   = lambda {|env|
+  @blocklisted_response = ->(_env) { [403, { 'Content-Type' => 'text/plain' }, ["Forbidden\n"]] }
+  @throttled_response   = lambda { |env|
     retry_after = (env['rack.attack.match_data'] || {})[:period]
-    [429, {'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s}, ["Retry later\n"]]
+    [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]]
   }
 
   def initialize(app)
@@ -152,8 +153,9 @@ def call(env)
   end
 
   extend Forwardable
-  def_delegators self, :safelisted?,
-                       :blocklisted?,
-                       :throttled?,
-                       :tracked?
+  def_delegators self,
+                 :safelisted?,
+                 :blocklisted?,
+                 :throttled?,
+                 :tracked?
 end
diff --git a/lib/rack/attack/allow2ban.rb b/lib/rack/attack/allow2ban.rb
index 763253b8..b79e210b 100644
--- a/lib/rack/attack/allow2ban.rb
+++ b/lib/rack/attack/allow2ban.rb
@@ -3,6 +3,7 @@ class Attack
     class Allow2Ban < Fail2Ban
       class << self
         protected
+
         def key_prefix
           'allow2ban'
         end
@@ -11,9 +12,7 @@ def key_prefix
         # (blocking the request) if they have tripped the limit.
         def fail!(discriminator, bantime, findtime, maxretry)
           count = cache.count("#{key_prefix}:count:#{discriminator}", findtime)
-          if count >= maxretry
-            ban!(discriminator, bantime)
-          end
+          ban!(discriminator, bantime) if count >= maxretry
           # we may not block them this time, but they're banned for next time
           false
         end
diff --git a/lib/rack/attack/blocklist.rb b/lib/rack/attack/blocklist.rb
index b61a29fe..3cfba540 100644
--- a/lib/rack/attack/blocklist.rb
+++ b/lib/rack/attack/blocklist.rb
@@ -5,7 +5,6 @@ def initialize(name, block)
         super
         @type = :blocklist
       end
-
     end
   end
 end
diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb
index aec048b5..2d4d4017 100644
--- a/lib/rack/attack/cache.rb
+++ b/lib/rack/attack/cache.rb
@@ -1,7 +1,6 @@
 module Rack
   class Attack
     class Cache
-
       attr_accessor :prefix
 
       def initialize
@@ -24,11 +23,11 @@ def read(unprefixed_key)
       end
 
       def write(unprefixed_key, value, expires_in)
-        store.write("#{prefix}:#{unprefixed_key}", value, :expires_in => expires_in)
+        store.write("#{prefix}:#{unprefixed_key}", value, expires_in: expires_in)
       end
 
       def reset_count(unprefixed_key, period)
-        key, _ = key_and_expiry(unprefixed_key, period)
+        key, _expires_in = key_and_expiry(unprefixed_key, period)
         store.delete(key)
       end
 
diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb
index 21451ded..e9fb3099 100644
--- a/lib/rack/attack/check.rb
+++ b/lib/rack/attack/check.rb
@@ -2,22 +2,22 @@ module Rack
   class Attack
     class Check
       attr_reader :name, :block, :type
+
       def initialize(name, options = {}, block)
-        @name, @block = name, block
+        @name = name
+        @block = block
         @type = options.fetch(:type, nil)
       end
 
       def [](req)
-        block[req].tap {|match|
+        block[req].tap do |match|
           if match
             req.env["rack.attack.matched"] = name
             req.env["rack.attack.match_type"] = type
             Rack::Attack.instrument(req)
           end
-        }
+        end
       end
-
     end
   end
 end
-
diff --git a/lib/rack/attack/fail2ban.rb b/lib/rack/attack/fail2ban.rb
index 76dc59fa..a1c7801f 100644
--- a/lib/rack/attack/fail2ban.rb
+++ b/lib/rack/attack/fail2ban.rb
@@ -3,9 +3,9 @@ class Attack
     class Fail2Ban
       class << self
         def filter(discriminator, options)
-          bantime   = options[:bantime]   or raise ArgumentError, "Must pass bantime option"
-          findtime  = options[:findtime]  or raise ArgumentError, "Must pass findtime option"
-          maxretry  = options[:maxretry]  or raise ArgumentError, "Must pass maxretry option"
+          bantime   = options[:bantime]   || raise(ArgumentError, "Must pass bantime option")
+          findtime  = options[:findtime]  || raise(ArgumentError, "Must pass findtime option")
+          maxretry  = options[:maxretry]  || raise(ArgumentError, "Must pass maxretry option")
 
           if banned?(discriminator)
             # Return true for blocklist
@@ -16,7 +16,7 @@ def filter(discriminator, options)
         end
 
         def reset(discriminator, options)
-          findtime = options[:findtime] or raise ArgumentError, "Must pass findtime option"
+          findtime = options[:findtime] || raise(ArgumentError, "Must pass findtime option")
           cache.reset_count("#{key_prefix}:count:#{discriminator}", findtime)
           # Clear ban flag just in case it's there
           cache.delete("#{key_prefix}:ban:#{discriminator}")
@@ -27,21 +27,20 @@ def banned?(discriminator)
         end
 
         protected
+
         def key_prefix
           'fail2ban'
         end
 
         def fail!(discriminator, bantime, findtime, maxretry)
           count = cache.count("#{key_prefix}:count:#{discriminator}", findtime)
-          if count >= maxretry
-            ban!(discriminator, bantime)
-          end
+          ban!(discriminator, bantime) if count >= maxretry
 
           true
         end
 
-
         private
+
         def ban!(discriminator, bantime)
           cache.write("#{key_prefix}:ban:#{discriminator}", 1, bantime)
         end
diff --git a/lib/rack/attack/safelist.rb b/lib/rack/attack/safelist.rb
index 748d422b..e548b0be 100644
--- a/lib/rack/attack/safelist.rb
+++ b/lib/rack/attack/safelist.rb
@@ -5,7 +5,6 @@ def initialize(name, block)
         super
         @type = :safelist
       end
-
     end
   end
 end
diff --git a/lib/rack/attack/store_proxy.rb b/lib/rack/attack/store_proxy.rb
index 5ce0f52f..d8edddb4 100644
--- a/lib/rack/attack/store_proxy.rb
+++ b/lib/rack/attack/store_proxy.rb
@@ -12,8 +12,8 @@ def self.build(store)
         klass ? klass.new(client) : client
       end
 
-
       private
+
       def self.unwrap_active_support_stores(store)
         # ActiveSupport::Cache::RedisStore doesn't expose any way to set an expiry,
         # so use the raw Redis::Store instead.
diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb
index 703f2d66..e624ebb1 100644
--- a/lib/rack/attack/store_proxy/dalli_proxy.rb
+++ b/lib/rack/attack/store_proxy/dalli_proxy.rb
@@ -28,14 +28,14 @@ def read(key)
         rescue Dalli::DalliError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           with do |client|
             client.set(key, value, options.fetch(:expires_in, 0), raw: true)
           end
         rescue Dalli::DalliError
         end
 
-        def increment(key, amount, options={})
+        def increment(key, amount, options = {})
           with do |client|
             client.incr(key, amount, options.fetch(:expires_in, 0), amount)
           end
@@ -52,13 +52,11 @@ def delete(key)
         private
 
         def stub_with_if_missing
-          unless __getobj__.respond_to?(:with)
-            class << self
-              def with; yield __getobj__; end
-            end
+          return if __getobj__.respond_to?(:with)
+          class << self
+            def with; yield __getobj__; end
           end
         end
-
       end
     end
   end
diff --git a/lib/rack/attack/store_proxy/mem_cache_proxy.rb b/lib/rack/attack/store_proxy/mem_cache_proxy.rb
index 098e0480..ddff6447 100644
--- a/lib/rack/attack/store_proxy/mem_cache_proxy.rb
+++ b/lib/rack/attack/store_proxy/mem_cache_proxy.rb
@@ -17,18 +17,18 @@ def read(key)
           rescue MemCache::MemCacheError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           # Third argument: writing raw value
           set(key, value, options.fetch(:expires_in, 0), true)
         rescue MemCache::MemCacheError
         end
 
-        def increment(key, amount, options={})
+        def increment(key, amount, _options = {})
           incr(key, amount)
         rescue MemCache::MemCacheError
         end
 
-        def delete(key, options={})
+        def delete(key, _options = {})
           with do |client|
             client.delete(key)
           end
@@ -38,13 +38,11 @@ def delete(key, options={})
         private
 
         def stub_with_if_missing
-          unless __getobj__.respond_to?(:with)
-            class << self
-              def with; yield __getobj__; end
-            end
+          return if __getobj__.respond_to?(:with)
+          class << self
+            def with; yield __getobj__; end
           end
         end
-
       end
     end
   end
diff --git a/lib/rack/attack/store_proxy/redis_store_proxy.rb b/lib/rack/attack/store_proxy/redis_store_proxy.rb
index 1a7c20a4..1741b992 100644
--- a/lib/rack/attack/store_proxy/redis_store_proxy.rb
+++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb
@@ -24,7 +24,7 @@ def read(key)
         rescue Redis::BaseError
         end
 
-        def write(key, value, options={})
+        def write(key, value, options = {})
           if (expires_in = options[:expires_in])
             setex(key, expires_in, value, raw: true)
           else
@@ -33,7 +33,7 @@ def write(key, value, options={})
         rescue Redis::BaseError
         end
 
-        def increment(key, amount, options={})
+        def increment(key, amount, options = {})
           count = nil
 
           pipelined do
@@ -45,7 +45,7 @@ def increment(key, amount, options={})
         rescue Redis::BaseError
         end
 
-        def delete(key, options={})
+        def delete(key, _options = {})
           del(key)
         rescue Redis::BaseError
         end
diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb
index 19a65196..f4decc18 100644
--- a/lib/rack/attack/throttle.rb
+++ b/lib/rack/attack/throttle.rb
@@ -1,13 +1,15 @@
 module Rack
   class Attack
     class Throttle
-      MANDATORY_OPTIONS = [:limit, :period].freeze
+      MANDATORY_OPTIONS = %i[limit period].freeze
 
       attr_reader :name, :limit, :period, :block, :type
+
       def initialize(name, options, block)
-        @name, @block = name, block
+        @name = name
+        @block = block
         MANDATORY_OPTIONS.each do |opt|
-          raise ArgumentError.new("Must pass #{opt.inspect} option") unless options[opt]
+          raise ArgumentError, "Must pass #{opt.inspect} option" unless options[opt]
         end
         @limit  = options[:limit]
         @period = options[:period].respond_to?(:call) ? options[:period] : options[:period].to_i
@@ -28,9 +30,9 @@ def [](req)
         count          = cache.count(key, current_period)
 
         data = {
-          :count => count,
-          :period => current_period,
-          :limit => current_limit
+          count: count,
+          period: current_period,
+          limit: current_limit
         }
         (req.env['rack.attack.throttle_data'] ||= {})[name] = data
 
diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb
index e0039e3b..0057caff 100644
--- a/lib/rack/attack/track.rb
+++ b/lib/rack/attack/track.rb
@@ -8,11 +8,11 @@ class Track
       def initialize(name, options = {}, block)
         options[:type] = :track
 
-        if options[:limit] && options[:period]
-          @filter = Throttle.new(name, options, block)
-        else
-          @filter = Check.new(name, options, block)
-        end
+        @filter = if options[:limit] && options[:period]
+                    Throttle.new(name, options, block)
+                  else
+                    Check.new(name, options, block)
+                  end
       end
 
       def_delegator :@filter, :[]