Merge pull request #410 from grzuy/style

Code style updates

.rubocop.yml

@@ -41,16 +41,35 @@ Style/BlockDelimiters:
 Style/BracesAroundHashParameters:
   Enabled: true
 
+Style/Encoding:
+  Enabled: true
+
+Style/EmptyMethod:
+  Enabled: true
+
 Style/FrozenStringLiteralComment:
   Enabled: true
 
 Style/HashSyntax:
   Enabled: true
 
+Style/OptionalArguments:
+  Enabled: true
+
+Style/RaiseArgs:
+  Enabled: true
+
+Style/RedundantBegin:
+  Enabled: true
+
 Style/RedundantFreeze:
   Enabled: true
 
-# TODO
-# Remove cop disabling and fix offenses
-Lint/HandleExceptions:
-  Enabled: false
+Style/RedundantSelf:
+  Enabled: true
+
+Style/Semicolon:
+  Enabled: true
+
+Style/SingleLineMethods:
+  Enabled: true

lib/rack/attack.rb

@@ -30,50 +30,56 @@ class Rack::Attack
     attr_accessor :notifier, :blocklisted_response, :throttled_response, :anonymous_blocklists, :anonymous_safelists
 
     def safelist(name = nil, &block)
-      safelist = Safelist.new(name, block)
+      safelist = Safelist.new(name, &block)
 
       if name
-        self.safelists[name] = safelist
+        safelists[name] = safelist
       else
         anonymous_safelists << safelist
       end
     end
 
     def blocklist(name = nil, &block)
-      blocklist = Blocklist.new(name, block)
+      blocklist = Blocklist.new(name, &block)
 
       if name
-        self.blocklists[name] = blocklist
+        blocklists[name] = blocklist
       else
         anonymous_blocklists << blocklist
       end
     end
 
     def blocklist_ip(ip_address)
-      ip_blocklist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
-      anonymous_blocklists << Blocklist.new(nil, ip_blocklist_proc)
+      anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
     end
 
     def safelist_ip(ip_address)
-      ip_safelist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
-      anonymous_safelists << Safelist.new(nil, ip_safelist_proc)
+      anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
     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
+    def safelists
+      @safelists  ||= {}
+    end
 
-    def blocklists; @blocklists ||= {}; end
+    def blocklists
+      @blocklists ||= {}
+    end
 
-    def throttles;  @throttles  ||= {}; end
+    def throttles
+      @throttles  ||= {}
+    end
 
-    def tracks;     @tracks     ||= {}; end
+    def tracks
+      @tracks     ||= {}
+    end
 
     def safelisted?(request)
       anonymous_safelists.any? { |safelist| safelist.matched_by?(request) } ||

lib/rack/attack/blocklist.rb

@@ -3,7 +3,7 @@
 module Rack
   class Attack
     class Blocklist < Check
-      def initialize(name, block)
+      def initialize(name = nil, &block)
         super
         @type = :blocklist
       end

lib/rack/attack/check.rb

@@ -4,7 +4,7 @@ module Rack
   class Attack
     class Check
       attr_reader :name, :block, :type
-      def initialize(name, options = {}, block)
+      def initialize(name, options = {}, &block)
         @name, @block = name, block
         @type = options.fetch(:type, nil)
       end

lib/rack/attack/safelist.rb

@@ -3,7 +3,7 @@
 module Rack
   class Attack
     class Safelist < Check
-      def initialize(name, block)
+      def initialize(name = nil, &block)
         super
         @type = :safelist
       end

lib/rack/attack/store_proxy/dalli_proxy.rb

@@ -24,31 +24,35 @@ module Rack
         end
 
         def read(key)
-          with do |client|
-            client.get(key)
+          rescuing do
+            with do |client|
+              client.get(key)
+            end
           end
-        rescue Dalli::DalliError
         end
 
         def write(key, value, options = {})
-          with do |client|
-            client.set(key, value, options.fetch(:expires_in, 0), raw: true)
+          rescuing do
+            with do |client|
+              client.set(key, value, options.fetch(:expires_in, 0), raw: true)
+            end
           end
-        rescue Dalli::DalliError
         end
 
         def increment(key, amount, options = {})
-          with do |client|
-            client.incr(key, amount, options.fetch(:expires_in, 0), amount)
+          rescuing do
+            with do |client|
+              client.incr(key, amount, options.fetch(:expires_in, 0), amount)
+            end
           end
-        rescue Dalli::DalliError
         end
 
         def delete(key)
-          with do |client|
-            client.delete(key)
+          rescuing do
+            with do |client|
+              client.delete(key)
+            end
           end
-        rescue Dalli::DalliError
         end
 
         private
@@ -56,10 +60,18 @@ module Rack
         def stub_with_if_missing
           unless __getobj__.respond_to?(:with)
             class << self
-              def with; yield __getobj__; end
+              def with
+                yield __getobj__
+              end
             end
           end
         end
+
+        def rescuing
+          yield
+        rescue Dalli::DalliError
+          nil
+        end
       end
     end
   end

lib/rack/attack/store_proxy/redis_proxy.rb

@@ -19,34 +19,40 @@ module Rack
         end
 
         def read(key)
-          get(key)
-        rescue Redis::BaseError
+          rescuing { get(key) }
         end
 
         def write(key, value, options = {})
           if (expires_in = options[:expires_in])
-            setex(key, expires_in, value)
+            rescuing { setex(key, expires_in, value) }
           else
-            set(key, value)
+            rescuing { set(key, value) }
           end
-        rescue Redis::BaseError
         end
 
         def increment(key, amount, options = {})
           count = nil
 
-          pipelined do
-            count = incrby(key, amount)
-            expire(key, options[:expires_in]) if options[:expires_in]
+          rescuing do
+            pipelined do
+              count = incrby(key, amount)
+              expire(key, options[:expires_in]) if options[:expires_in]
+            end
           end
 
           count.value if count
-        rescue Redis::BaseError
         end
 
         def delete(key, _options = {})
-          del(key)
+          rescuing { del(key) }
+        end
+
+        private
+
+        def rescuing
+          yield
         rescue Redis::BaseError
+          nil
         end
       end
     end

lib/rack/attack/store_proxy/redis_store_proxy.rb

@@ -11,17 +11,15 @@ module Rack
         end
 
         def read(key)
-          get(key, raw: true)
-        rescue Redis::BaseError
+          rescuing { get(key, raw: true) }
         end
 
         def write(key, value, options = {})
           if (expires_in = options[:expires_in])
-            setex(key, expires_in, value, raw: true)
+            rescuing { setex(key, expires_in, value, raw: true) }
           else
-            set(key, value, raw: true)
+            rescuing { set(key, value, raw: true) }
           end
-        rescue Redis::BaseError
         end
       end
     end

lib/rack/attack/throttle.rb

@@ -6,10 +6,10 @@ module Rack
       MANDATORY_OPTIONS = [:limit, :period].freeze
 
       attr_reader :name, :limit, :period, :block, :type
-      def initialize(name, options, block)
+      def initialize(name, options, &block)
         @name, @block = name, 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

lib/rack/attack/track.rb

@@ -5,13 +5,13 @@ module Rack
     class Track
       attr_reader :filter
 
-      def initialize(name, options = {}, block)
+      def initialize(name, options = {}, &block)
         options[:type] = :track
 
         if options[:limit] && options[:period]
-          @filter = Throttle.new(name, options, block)
+          @filter = Throttle.new(name, options, &block)
         else
-          @filter = Check.new(name, options, block)
+          @filter = Check.new(name, options, &block)
         end
       end
 

rack-attack.gemspec

@@ -1,4 +1,3 @@
-# -*- encoding: utf-8 -*-
 # frozen_string_literal: true
 
 lib = File.expand_path('../lib/', __FILE__)

spec/acceptance/cache_store_config_for_allow2ban_spec.rb

@@ -22,11 +22,9 @@ describe "Cache store config when using allow2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def write(key, value)
-      end
+      def write(key, value); end
 
-      def increment(key, count, options = {})
-      end
+      def increment(key, count, options = {}); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do
@@ -44,11 +42,9 @@ describe "Cache store config when using allow2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def read(key)
-      end
+      def read(key); end
 
-      def increment(key, count, options = {})
-      end
+      def increment(key, count, options = {}); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do
@@ -66,11 +62,9 @@ describe "Cache store config when using allow2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def read(key)
-      end
+      def read(key); end
 
-      def write(key, value)
-      end
+      def write(key, value); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -22,11 +22,9 @@ describe "Cache store config when using fail2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def write(key, value)
-      end
+      def write(key, value); end
 
-      def increment(key, count, options = {})
-      end
+      def increment(key, count, options = {}); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do
@@ -44,11 +42,9 @@ describe "Cache store config when using fail2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def read(key)
-      end
+      def read(key); end
 
-      def increment(key, count, options = {})
-      end
+      def increment(key, count, options = {}); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do
@@ -66,11 +62,9 @@ describe "Cache store config when using fail2ban" do
     raised_exception = nil
 
     fake_store_class = Class.new do
-      def read(key)
-      end
+      def read(key); end
 
-      def write(key, value)
-      end
+      def write(key, value); end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do

spec/spec_helper.rb

@@ -15,10 +15,9 @@ if RUBY_ENGINE == "ruby"
 end
 
 def safe_require(name)
-  begin
-    require name
-  rescue LoadError
-  end
+  require name
+rescue LoadError
+  nil
 end
 
 safe_require "connection_pool"