diff --git a/.rubocop.yml b/.rubocop.yml index cc0e049..14b26e5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -40,6 +40,9 @@ Security: Lint: Enabled: true +Style/BlockDelimiters: + Enabled: true + Style/BracesAroundHashParameters: Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 4a96778..30a9ccf 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -81,40 +81,40 @@ class Rack::Attack blocklists end - def safelisted?(req) - ip_safelists.any? { |safelist| safelist.match?(req) } || - safelists.any? { |_name, safelist| safelist.match?(req) } + def safelisted?(request) + ip_safelists.any? { |safelist| safelist.matched_by?(request) } || + safelists.any? { |_name, safelist| safelist.matched_by?(request) } end - def whitelisted?(req) + def whitelisted?(request) warn "[DEPRECATION] 'Rack::Attack.whitelisted?' is deprecated. Please use 'safelisted?' instead." - safelisted?(req) + safelisted?(request) end - def blocklisted?(req) - ip_blocklists.any? { |blocklist| blocklist.match?(req) } || - blocklists.any? { |_name, blocklist| blocklist.match?(req) } + def blocklisted?(request) + ip_blocklists.any? { |blocklist| blocklist.matched_by?(request) } || + blocklists.any? { |_name, blocklist| blocklist.matched_by?(request) } end - def blacklisted?(req) + def blacklisted?(request) warn "[DEPRECATION] 'Rack::Attack.blacklisted?' is deprecated. Please use 'blocklisted?' instead." - blocklisted?(req) + blocklisted?(request) end - def throttled?(req) + def throttled?(request) throttles.any? do |_name, throttle| - throttle[req] + throttle.matched_by?(request) end end - def tracked?(req) - tracks.each_value do |tracker| - tracker[req] + def tracked?(request) + tracks.each_value do |track| + track.matched_by?(request) end end - def instrument(req) - notifier.instrument('rack.attack', req) if notifier + def instrument(request) + notifier.instrument('rack.attack', request) if notifier end def cache @@ -167,16 +167,16 @@ class Rack::Attack def call(env) env['PATH_INFO'] = PathNormalizer.normalize_path(env['PATH_INFO']) - req = Rack::Attack::Request.new(env) + request = Rack::Attack::Request.new(env) - if safelisted?(req) + if safelisted?(request) @app.call(env) - elsif blocklisted?(req) + elsif blocklisted?(request) self.class.blocklisted_response.call(env) - elsif throttled?(req) + elsif throttled?(request) self.class.throttled_response.call(env) else - tracked?(req) + tracked?(request) @app.call(env) end end diff --git a/lib/rack/attack/check.rb b/lib/rack/attack/check.rb index 49857cf..b24a873 100644 --- a/lib/rack/attack/check.rb +++ b/lib/rack/attack/check.rb @@ -7,17 +7,15 @@ module Rack @type = options.fetch(:type, nil) end - def [](req) - block[req].tap { |match| + def matched_by?(request) + block.call(request).tap do |match| if match - req.env["rack.attack.matched"] = name - req.env["rack.attack.match_type"] = type - Rack::Attack.instrument(req) + request.env["rack.attack.matched"] = name + request.env["rack.attack.match_type"] = type + Rack::Attack.instrument(request) end - } + end end - - alias_method :match?, :[] end end end diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb index 19a6519..b89e114 100644 --- a/lib/rack/attack/throttle.rb +++ b/lib/rack/attack/throttle.rb @@ -18,12 +18,12 @@ module Rack Rack::Attack.cache end - def [](req) - discriminator = block[req] + def matched_by?(request) + discriminator = block.call(request) return false unless discriminator - current_period = period.respond_to?(:call) ? period.call(req) : period - current_limit = limit.respond_to?(:call) ? limit.call(req) : limit + current_period = period.respond_to?(:call) ? period.call(request) : period + current_limit = limit.respond_to?(:call) ? limit.call(request) : limit key = "#{name}:#{discriminator}" count = cache.count(key, current_period) @@ -32,15 +32,15 @@ module Rack :period => current_period, :limit => current_limit } - (req.env['rack.attack.throttle_data'] ||= {})[name] = data + (request.env['rack.attack.throttle_data'] ||= {})[name] = data (count > current_limit).tap do |throttled| if throttled - req.env['rack.attack.matched'] = name - req.env['rack.attack.match_discriminator'] = discriminator - req.env['rack.attack.match_type'] = type - req.env['rack.attack.match_data'] = data - Rack::Attack.instrument(req) + request.env['rack.attack.matched'] = name + request.env['rack.attack.match_discriminator'] = discriminator + request.env['rack.attack.match_type'] = type + request.env['rack.attack.match_data'] = data + Rack::Attack.instrument(request) end end end diff --git a/lib/rack/attack/track.rb b/lib/rack/attack/track.rb index e0039e3..123bfa4 100644 --- a/lib/rack/attack/track.rb +++ b/lib/rack/attack/track.rb @@ -1,8 +1,6 @@ module Rack class Attack class Track - extend Forwardable - attr_reader :filter def initialize(name, options = {}, block) @@ -15,7 +13,9 @@ module Rack end end - def_delegator :@filter, :[] + def matched_by?(request) + filter.matched_by?(request) + end end end end diff --git a/spec/integration/offline_spec.rb b/spec/integration/offline_spec.rb index f47fb31..ebaf16a 100644 --- a/spec/integration/offline_spec.rb +++ b/spec/integration/offline_spec.rb @@ -20,24 +20,24 @@ end describe 'when Redis is offline' do include OfflineExamples - before { + before do @cache = Rack::Attack::Cache.new # Use presumably unused port for Redis client @cache.store = ActiveSupport::Cache::RedisStore.new(:host => '127.0.0.1', :port => 3333) - } + end end describe 'when Memcached is offline' do include OfflineExamples - before { + before do Dalli.logger.level = Logger::FATAL @cache = Rack::Attack::Cache.new @cache.store = Dalli::Client.new('127.0.0.1:22122') - } + end - after { + after do Dalli.logger.level = Logger::INFO - } + end end diff --git a/spec/integration/rack_attack_cache_spec.rb b/spec/integration/rack_attack_cache_spec.rb index 2e9f9e5..98f5568 100644 --- a/spec/integration/rack_attack_cache_spec.rb +++ b/spec/integration/rack_attack_cache_spec.rb @@ -37,13 +37,13 @@ describe Rack::Attack::Cache do store = Rack::Attack::StoreProxy.build(store) describe "with #{store.class}" do - before { + before do @cache = Rack::Attack::Cache.new @key = "rack::attack:cache-test-key" @expires_in = 1 @cache.store = store delete(@key) - } + end after { delete(@key) } diff --git a/spec/rack_attack_track_spec.rb b/spec/rack_attack_track_spec.rb index 24a408f..352561d 100644 --- a/spec/rack_attack_track_spec.rb +++ b/spec/rack_attack_track_spec.rb @@ -47,15 +47,15 @@ describe 'Rack::Attack.track' do describe "without limit and period options" do it "should assign the track filter to a Check instance" do - tracker = Rack::Attack.track("homepage") { |req| req.path == "/" } - tracker.filter.class.must_equal Rack::Attack::Check + track = Rack::Attack.track("homepage") { |req| req.path == "/" } + track.filter.class.must_equal Rack::Attack::Check end end describe "with limit and period options" do it "should assign the track filter to a Throttle instance" do - tracker = Rack::Attack.track("homepage", :limit => 10, :period => 10) { |req| req.path == "/" } - tracker.filter.class.must_equal Rack::Attack::Throttle + track = Rack::Attack.track("homepage", :limit => 10, :period => 10) { |req| req.path == "/" } + track.filter.class.must_equal Rack::Attack::Throttle end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3f19b8f..6b6f661 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,14 +36,14 @@ class MiniTest::Spec end def app - Rack::Builder.new { + Rack::Builder.new do # Use Rack::Lint to test that rack-attack is complying with the rack spec use Rack::Lint use Rack::Attack use Rack::Lint run lambda { |_env| [200, {}, ['Hello World']] } - }.to_app + end.to_app end def self.it_allows_ok_requests