Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freeze time to fix flakey specs #640

Merged
merged 1 commit into from
Nov 21, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 49 additions & 28 deletions spec/rack_attack_throttle_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# frozen_string_literal: true

require_relative 'spec_helper'
require 'timecop'

describe 'Rack::Attack.throttle' do
before do
@period = 60 # Use a long period; failures due to cache key rotation less likely
@period = 60
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip }
end
Expand All @@ -14,14 +15,18 @@
it_allows_ok_requests

describe 'a single request' do
before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }

it 'should set the counter for one request' do
key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
Timecop.freeze do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
end
end

it 'should populate throttle data' do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

data = {
count: 1,
limit: 1,
Expand All @@ -36,7 +41,9 @@

describe "with 2 requests" do
before do
2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
Timecop.freeze do
2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
end
end

it 'should block the last request' do
Expand All @@ -62,22 +69,25 @@

describe 'Rack::Attack.throttle with limit as proc' do
before do
@period = 60 # Use a long period; failures due to cache key rotation less likely
@period = 60
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: @period) { |req| req.ip }
end

it_allows_ok_requests

describe 'a single request' do
before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }

it 'should set the counter for one request' do
key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
Timecop.freeze do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
end
end

it 'should populate throttle data' do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
data = {
count: 1,
limit: 1,
Expand All @@ -93,22 +103,26 @@

describe 'Rack::Attack.throttle with period as proc' do
before do
@period = 60 # Use a long period; failures due to cache key rotation less likely
@period = 60
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: lambda { |_req| @period }) { |req| req.ip }
end

it_allows_ok_requests

describe 'a single request' do
before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }

it 'should set the counter for one request' do
key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
Timecop.freeze do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
_(Rack::Attack.cache.store.read(key)).must_equal 1
end
end

it 'should populate throttle data' do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

data = {
count: 1,
limit: 1,
Expand All @@ -122,7 +136,7 @@
end
end

describe 'Rack::Attack.throttle with block retuning nil' do
describe 'Rack::Attack.throttle with block returning nil' do
before do
@period = 60
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
Expand All @@ -132,14 +146,17 @@
it_allows_ok_requests

describe 'a single request' do
before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }

it 'should not set the counter' do
key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
assert_nil Rack::Attack.cache.store.read(key)
Timecop.freeze do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'

key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
assert_nil Rack::Attack.cache.store.read(key)
end
end

it 'should not populate throttle data' do
get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
assert_nil last_request.env['rack.attack.throttle_data']
end
end
Expand All @@ -162,20 +179,24 @@
end

it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
post_logins
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:[email protected]"
_(Rack::Attack.cache.store.read(key)).must_equal 3
Timecop.freeze do
post_logins
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:[email protected]"
_(Rack::Attack.cache.store.read(key)).must_equal 3
end
end

it 'should differentiate requests when throttle_discriminator_normalizer is disabled' do
begin
prev = Rack::Attack.throttle_discriminator_normalizer
Rack::Attack.throttle_discriminator_normalizer = nil

post_logins
@emails.each do |email|
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
_(Rack::Attack.cache.store.read(key)).must_equal 1
Timecop.freeze do
post_logins
@emails.each do |email|
key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
_(Rack::Attack.cache.store.read(key)).must_equal 1
end
end
ensure
Rack::Attack.throttle_discriminator_normalizer = prev
Expand Down
Loading