From 5004b04ac70e65399f622b7a1bdea8ede121ce65 Mon Sep 17 00:00:00 2001 From: Domenoth Date: Thu, 11 Jan 2018 17:42:17 -0800 Subject: [PATCH] Change object type yielded to ActiveSupport::Subscribers https://github.com/kickstarter/rack-attack/issues/255 Change the object type from instances of type Rack::Attack::Request to instances of type Hash. (`req` becomes `request: req`). --- README.md | 7 +++--- lib/rack/attack.rb | 2 +- spec/acceptance/track_spec.rb | 6 ++--- spec/acceptance/track_throttle_spec.rb | 6 ++--- spec/rack_attack_instrumentation_spec.rb | 31 ++++++++++++++++++++++++ 5 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 spec/rack_attack_instrumentation_spec.rb diff --git a/README.md b/README.md index d977edd..a31079f 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,8 @@ Rack::Attack.track("special_agent", limit: 6, period: 60) do |req| end # Track it using ActiveSupport::Notification -ActiveSupport::Notifications.subscribe("rack.attack") do |name, start, finish, request_id, req| +ActiveSupport::Notifications.subscribe("rack.attack") do |name, start, finish, request_id, payload| + req = payload[:request] if req.env['rack.attack.matched'] == "special_agent" && req.env['rack.attack.match_type'] == :track Rails.logger.info "special_agent: #{req.path}" STATSD.increment("special_agent") @@ -283,8 +284,8 @@ Rack::Attack uses the [ActiveSupport::Notifications](http://api.rubyonrails.org/ You can subscribe to 'rack.attack' events and log it, graph it, etc: ```ruby -ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, request_id, req| - puts req.inspect +ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, request_id, payload| + puts payload[:request].inspect end ``` diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 8c9df48..0b102d7 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -100,7 +100,7 @@ class Rack::Attack end def instrument(req) - notifier.instrument('rack.attack', req) if notifier + notifier.instrument('rack.attack', request: req) if notifier end def cache diff --git a/spec/acceptance/track_spec.rb b/spec/acceptance/track_spec.rb index 7f4b393..928cc31 100644 --- a/spec/acceptance/track_spec.rb +++ b/spec/acceptance/track_spec.rb @@ -9,9 +9,9 @@ describe "#track" do notification_matched = nil notification_type = nil - ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, request| - notification_matched = request.env["rack.attack.matched"] - notification_type = request.env["rack.attack.match_type"] + ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| + notification_matched = payload[:request].env["rack.attack.matched"] + notification_type = payload[:request].env["rack.attack.match_type"] end get "/", {}, "REMOTE_ADDR" => "5.6.7.8" diff --git a/spec/acceptance/track_throttle_spec.rb b/spec/acceptance/track_throttle_spec.rb index 7446ce4..f3f8a80 100644 --- a/spec/acceptance/track_throttle_spec.rb +++ b/spec/acceptance/track_throttle_spec.rb @@ -12,9 +12,9 @@ describe "#track with throttle-ish options" do notification_matched = nil notification_type = nil - ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, request| - notification_matched = request.env["rack.attack.matched"] - notification_type = request.env["rack.attack.match_type"] + ActiveSupport::Notifications.subscribe("rack.attack") do |_name, _start, _finish, _id, payload| + notification_matched = payload[:request].env["rack.attack.matched"] + notification_type = payload[:request].env["rack.attack.match_type"] end get "/", {}, "REMOTE_ADDR" => "1.2.3.4" diff --git a/spec/rack_attack_instrumentation_spec.rb b/spec/rack_attack_instrumentation_spec.rb new file mode 100644 index 0000000..88883d0 --- /dev/null +++ b/spec/rack_attack_instrumentation_spec.rb @@ -0,0 +1,31 @@ +# ActiveSupport::Subscribers added in ~> 4.0.2.0 +if ActiveSupport::VERSION::MAJOR > 3 + require_relative 'spec_helper' + require 'active_support/subscriber' + class CustomSubscriber < ActiveSupport::Subscriber + def rack(event) + # Do virtually (but not) nothing. + event.inspect + end + end + + describe 'Rack::Attack.instrument' do + before do + @period = 60 # Use a long period; failures due to cache key rotation less likely + Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new + Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip } + end + + describe "with throttling" do + before do + ActiveSupport::Notifications.stub(:notifier, ActiveSupport::Notifications::Fanout.new) do + CustomSubscriber.attach_to("attack") + 2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' } + end + end + it 'should instrument without error' do + last_response.status.must_equal 429 + end + end + end +end