diff --git a/.rubocop.yml b/.rubocop.yml index 2ced30df..705a6a63 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 59f28928..8c62e7de 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -30,50 +30,56 @@ class << self 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) } || diff --git a/lib/rack/attack/blocklist.rb b/lib/rack/attack/blocklist.rb index ee576563..08adcc6d 100644 --- a/lib/rack/attack/blocklist.rb +++ b/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 diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb index 2e5c64df..ef4ad783 100644 --- a/lib/rack/attack/check.rb +++ b/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 diff --git a/lib/rack/attack/safelist.rb b/lib/rack/attack/safelist.rb index d335be2a..b3abeaf2 100644 --- a/lib/rack/attack/safelist.rb +++ b/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 diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index f3fbc6cf..360e2198 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -24,31 +24,35 @@ def initialize(client) 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 @@ def delete(key) 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 diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_proxy/redis_proxy.rb index 69fa19d7..9fe2bc8c 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_proxy.rb @@ -19,34 +19,40 @@ def self.handle?(store) 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 diff --git a/lib/rack/attack/store_proxy/redis_store_proxy.rb b/lib/rack/attack/store_proxy/redis_store_proxy.rb index 359b542f..6be54128 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb @@ -11,17 +11,15 @@ def self.handle?(store) 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 diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb index c688a0a8..4d5316a6 100644 --- a/lib/rack/attack/throttle.rb +++ b/lib/rack/attack/throttle.rb @@ -6,10 +6,10 @@ class Throttle 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 diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb index dba16cab..dc7afb85 100644 --- a/lib/rack/attack/track.rb +++ b/lib/rack/attack/track.rb @@ -5,13 +5,13 @@ class Attack 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 diff --git a/rack-attack.gemspec b/rack-attack.gemspec index bbee3e05..98eb88d4 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -1,4 +1,3 @@ -# -*- encoding: utf-8 -*- # frozen_string_literal: true lib = File.expand_path('../lib/', __FILE__) diff --git a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb index 6415572e..ce994993 100644 --- a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb +++ b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb @@ -22,11 +22,9 @@ 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 @@ def increment(key, count, options = {}) 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 @@ def increment(key, count, options = {}) 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 diff --git a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb index 2f2ef121..6f330eee 100644 --- a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb +++ b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb @@ -22,11 +22,9 @@ 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 @@ def increment(key, count, options = {}) 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 @@ def increment(key, count, options = {}) 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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b46161ed..553cb882 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,10 +15,9 @@ end def safe_require(name) - begin - require name - rescue LoadError - end + require name +rescue LoadError + nil end safe_require "connection_pool"