From 20d668211e9bd64d918e987b6053462e4bdb570e Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 28 Feb 2019 21:17:36 -0300 Subject: [PATCH 01/13] style: fix Lint/HandleExceptions rubocop --- .rubocop.yml | 5 ----- lib/rack/attack/store_proxy/dalli_proxy.rb | 4 ++++ lib/rack/attack/store_proxy/redis_proxy.rb | 4 ++++ lib/rack/attack/store_proxy/redis_store_proxy.rb | 2 ++ spec/spec_helper.rb | 1 + 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 2ced30d..f21b1b1 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -49,8 +49,3 @@ Style/HashSyntax: Style/RedundantFreeze: Enabled: true - -# TODO -# Remove cop disabling and fix offenses -Lint/HandleExceptions: - Enabled: false diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index f3fbc6c..0b6cfb0 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -28,6 +28,7 @@ module Rack client.get(key) end rescue Dalli::DalliError + nil end def write(key, value, options = {}) @@ -35,6 +36,7 @@ module Rack client.set(key, value, options.fetch(:expires_in, 0), raw: true) end rescue Dalli::DalliError + nil end def increment(key, amount, options = {}) @@ -42,6 +44,7 @@ module Rack client.incr(key, amount, options.fetch(:expires_in, 0), amount) end rescue Dalli::DalliError + nil end def delete(key) @@ -49,6 +52,7 @@ module Rack client.delete(key) end rescue Dalli::DalliError + nil end private diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_proxy/redis_proxy.rb index 69fa19d..5572d0c 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_proxy.rb @@ -21,6 +21,7 @@ module Rack def read(key) get(key) rescue Redis::BaseError + nil end def write(key, value, options = {}) @@ -30,6 +31,7 @@ module Rack set(key, value) end rescue Redis::BaseError + nil end def increment(key, amount, options = {}) @@ -42,11 +44,13 @@ module Rack count.value if count rescue Redis::BaseError + nil end def delete(key, _options = {}) del(key) 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 359b542..d7b753c 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb @@ -13,6 +13,7 @@ module Rack def read(key) get(key, raw: true) rescue Redis::BaseError + nil end def write(key, value, options = {}) @@ -22,6 +23,7 @@ module Rack set(key, value, raw: true) end rescue Redis::BaseError + nil end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b46161e..ad377bf 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,7 @@ def safe_require(name) begin require name rescue LoadError + nil end end From 04eeeb9a338bb81145b799565ad472e52f7b6cb5 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 28 Feb 2019 22:51:57 -0300 Subject: [PATCH 02/13] refactor: avoid rescuing pattern repetition --- lib/rack/attack/store_proxy/dalli_proxy.rb | 38 +++++++++++-------- lib/rack/attack/store_proxy/redis_proxy.rb | 28 +++++++------- .../attack/store_proxy/redis_store_proxy.rb | 10 ++--- 3 files changed, 40 insertions(+), 36 deletions(-) diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index 0b6cfb0..fef6179 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -24,35 +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 - nil 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 - nil 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 - nil end def delete(key) - with do |client| - client.delete(key) + rescuing do + with do |client| + client.delete(key) + end end - rescue Dalli::DalliError - nil end private @@ -64,6 +64,12 @@ module Rack 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 5572d0c..9fe2bc8 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_proxy.rb @@ -19,36 +19,38 @@ module Rack end def read(key) - get(key) - rescue Redis::BaseError - nil + 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 - nil 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 - nil end def delete(key, _options = {}) - del(key) + rescuing { del(key) } + end + + private + + def rescuing + yield rescue Redis::BaseError nil end diff --git a/lib/rack/attack/store_proxy/redis_store_proxy.rb b/lib/rack/attack/store_proxy/redis_store_proxy.rb index d7b753c..6be5412 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb @@ -11,19 +11,15 @@ module Rack end def read(key) - get(key, raw: true) - rescue Redis::BaseError - nil + 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 - nil end end end From 7dbb490c017be06ffaa908feb61c3609b1f9ef96 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:09:46 -0300 Subject: [PATCH 03/13] style: enabled Style/EmptyMethod rubocop --- .rubocop.yml | 3 +++ .../cache_store_config_for_allow2ban_spec.rb | 18 ++++++------------ .../cache_store_config_for_fail2ban_spec.rb | 18 ++++++------------ 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f21b1b1..96dcfe5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -41,6 +41,9 @@ Style/BlockDelimiters: Style/BracesAroundHashParameters: Enabled: true +Style/EmptyMethod: + Enabled: true + Style/FrozenStringLiteralComment: Enabled: true diff --git a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb index 6415572..ce99499 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 @@ 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 diff --git a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb index 2f2ef12..6f330ee 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 @@ 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 From 8e3077c845c492a76173fab291f85cc71eaf13e0 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:10:40 -0300 Subject: [PATCH 04/13] style: enabled Style/RedundantBegin rubocop --- .rubocop.yml | 3 +++ spec/spec_helper.rb | 8 +++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 96dcfe5..cca3e40 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -50,5 +50,8 @@ Style/FrozenStringLiteralComment: Style/HashSyntax: Enabled: true +Style/RedundantBegin: + Enabled: true + Style/RedundantFreeze: Enabled: true diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ad377bf..553cb88 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,11 +15,9 @@ if RUBY_ENGINE == "ruby" end def safe_require(name) - begin - require name - rescue LoadError - nil - end + require name +rescue LoadError + nil end safe_require "connection_pool" From a0259fb14a438c22aff8887eb4f6e21a1bd77801 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:14:47 -0300 Subject: [PATCH 05/13] style: enable Style/SingleLineMethods rubocop --- .rubocop.yml | 3 +++ lib/rack/attack.rb | 16 ++++++++++++---- lib/rack/attack/store_proxy/dalli_proxy.rb | 4 +++- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index cca3e40..ad708a2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -55,3 +55,6 @@ Style/RedundantBegin: Style/RedundantFreeze: Enabled: true + +Style/SingleLineMethods: + Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 59f2892..d2571a2 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -67,13 +67,21 @@ class Rack::Attack self.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/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index fef6179..095bfb6 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -60,7 +60,9 @@ 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 From 92bc56b7b7479c076e269633affdb9a349ee281b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:15:50 -0300 Subject: [PATCH 06/13] style: enable Style/RedundantSelf rubocop --- .rubocop.yml | 3 +++ lib/rack/attack.rb | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ad708a2..e470c08 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -56,5 +56,8 @@ Style/RedundantBegin: Style/RedundantFreeze: Enabled: true +Style/RedundantSelf: + Enabled: true + Style/SingleLineMethods: Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index d2571a2..4ac4c44 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -33,7 +33,7 @@ class Rack::Attack safelist = Safelist.new(name, block) if name - self.safelists[name] = safelist + safelists[name] = safelist else anonymous_safelists << safelist end @@ -43,7 +43,7 @@ class Rack::Attack blocklist = Blocklist.new(name, block) if name - self.blocklists[name] = blocklist + blocklists[name] = blocklist else anonymous_blocklists << blocklist end @@ -60,11 +60,11 @@ class Rack::Attack 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; From 2240e8f2c63360ae764e2e0f0672b536bdc1f18a Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:19:06 -0300 Subject: [PATCH 07/13] style: enable Style/RaiseArgs rubocop --- .rubocop.yml | 3 +++ lib/rack/attack/throttle.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index e470c08..365ca05 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -50,6 +50,9 @@ Style/FrozenStringLiteralComment: Style/HashSyntax: Enabled: true +Style/RaiseArgs: + Enabled: true + Style/RedundantBegin: Enabled: true diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb index c688a0a..2f6f421 100644 --- a/lib/rack/attack/throttle.rb +++ b/lib/rack/attack/throttle.rb @@ -9,7 +9,7 @@ module Rack 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 From 7daeac340147aa2a5804e30f2efb88466d84a52b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:27:08 -0300 Subject: [PATCH 08/13] style: enable Style/Encoding rubocop --- .rubocop.yml | 3 +++ rack-attack.gemspec | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 365ca05..0d4f3c6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -41,6 +41,9 @@ Style/BlockDelimiters: Style/BracesAroundHashParameters: Enabled: true +Style/Encoding: + Enabled: true + Style/EmptyMethod: Enabled: true diff --git a/rack-attack.gemspec b/rack-attack.gemspec index bbee3e0..98eb88d 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__) From 5a42fd3ac789350cb7f78ac7ef0d5a9e29579da3 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 21:51:15 -0300 Subject: [PATCH 09/13] style: enable Style/OptionalArguments rubocop --- .rubocop.yml | 3 +++ lib/rack/attack.rb | 10 +++++----- lib/rack/attack/blocklist.rb | 2 +- lib/rack/attack/check.rb | 2 +- lib/rack/attack/safelist.rb | 2 +- lib/rack/attack/track.rb | 4 ++-- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0d4f3c6..8e210a5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -53,6 +53,9 @@ Style/FrozenStringLiteralComment: Style/HashSyntax: Enabled: true +Style/OptionalArguments: + Enabled: true + Style/RaiseArgs: Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 4ac4c44..1c5c2ab 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -30,7 +30,7 @@ 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 safelists[name] = safelist @@ -40,7 +40,7 @@ class Rack::Attack end def blocklist(name = nil, &block) - blocklist = Blocklist.new(name, block) + blocklist = Blocklist.new(name, &block) if name blocklists[name] = blocklist @@ -51,12 +51,12 @@ class Rack::Attack 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(nil, &ip_blocklist_proc) 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(nil, &ip_safelist_proc) end def throttle(name, options, &block) @@ -64,7 +64,7 @@ class Rack::Attack end def track(name, options = {}, &block) - tracks[name] = Track.new(name, options, block) + tracks[name] = Track.new(name, options, &block) end def safelists; diff --git a/lib/rack/attack/blocklist.rb b/lib/rack/attack/blocklist.rb index ee57656..bd7fa11 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, &block) super @type = :blocklist end diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb index 2e5c64d..ef4ad78 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 d335be2..0bd1453 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, &block) super @type = :safelist end diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb index dba16ca..9e7ff2e 100644 --- a/lib/rack/attack/track.rb +++ b/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) else - @filter = Check.new(name, options, block) + @filter = Check.new(name, options, &block) end end From 3639afc196249c1b143327c559086af5a9d55bc3 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 22:01:17 -0300 Subject: [PATCH 10/13] refactor: remove unnecessary block local variable --- lib/rack/attack.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 1c5c2ab..f6ab929 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -50,13 +50,11 @@ class Rack::Attack 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(nil) { |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(nil) { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } end def throttle(name, options, &block) From 0e8dff4c8809dfb3a1879058e6c8e1dd165906c1 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 22:12:12 -0300 Subject: [PATCH 11/13] refactor: make Throttle.new consistent with Blocklist/Safelist.new --- lib/rack/attack.rb | 2 +- lib/rack/attack/throttle.rb | 2 +- lib/rack/attack/track.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index f6ab929..26b6725 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -58,7 +58,7 @@ class Rack::Attack end def throttle(name, options, &block) - throttles[name] = Throttle.new(name, options, block) + throttles[name] = Throttle.new(name, options, &block) end def track(name, options = {}, &block) diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb index 2f6f421..4d5316a 100644 --- a/lib/rack/attack/throttle.rb +++ b/lib/rack/attack/throttle.rb @@ -6,7 +6,7 @@ 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, "Must pass #{opt.inspect} option" unless options[opt] diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb index 9e7ff2e..dc7afb8 100644 --- a/lib/rack/attack/track.rb +++ b/lib/rack/attack/track.rb @@ -9,7 +9,7 @@ module Rack 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) end From fcb89a6c12a129d7298b0234f42ae03e11e912bf Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 22:22:11 -0300 Subject: [PATCH 12/13] refactor: avoid unnecessary nil argument passing --- lib/rack/attack.rb | 4 ++-- lib/rack/attack/blocklist.rb | 2 +- lib/rack/attack/safelist.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 26b6725..8066622 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -50,11 +50,11 @@ class Rack::Attack end def blocklist_ip(ip_address) - anonymous_blocklists << Blocklist.new(nil) { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } end def safelist_ip(ip_address) - anonymous_safelists << Safelist.new(nil) { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } + anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } end def throttle(name, options, &block) diff --git a/lib/rack/attack/blocklist.rb b/lib/rack/attack/blocklist.rb index bd7fa11..08adcc6 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/safelist.rb b/lib/rack/attack/safelist.rb index 0bd1453..b3abeaf 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 From 6541634fb0d828efb7e44d54511061519233056f Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 1 Mar 2019 22:25:27 -0300 Subject: [PATCH 13/13] style: enable Style/Semicolon rubocop --- .rubocop.yml | 3 +++ lib/rack/attack.rb | 16 ++++++++-------- lib/rack/attack/store_proxy/dalli_proxy.rb | 4 ++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8e210a5..705a6a6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -68,5 +68,8 @@ Style/RedundantFreeze: 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 8066622..8c62e7d 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -65,20 +65,20 @@ class Rack::Attack tracks[name] = Track.new(name, options, &block) end - def safelists; - @safelists ||= {}; + def safelists + @safelists ||= {} end - def blocklists; - @blocklists ||= {}; + def blocklists + @blocklists ||= {} end - def throttles; - @throttles ||= {}; + def throttles + @throttles ||= {} end - def tracks; - @tracks ||= {}; + def tracks + @tracks ||= {} end def safelisted?(request) diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index 095bfb6..360e219 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -60,8 +60,8 @@ module Rack def stub_with_if_missing unless __getobj__.respond_to?(:with) class << self - def with; - yield __getobj__; + def with + yield __getobj__ end end end