From fc58d6e23a42e2668122d11bf2070aee130d54f7 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Fri, 24 Dec 2021 15:59:42 -0600
Subject: [PATCH 01/13] test: private method replaces Time.now

---
 spec/allow2ban_spec.rb            |  8 ++++----
 spec/fail2ban_spec.rb             | 10 +++++-----
 spec/rack_attack_throttle_spec.rb | 12 ++++++------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/spec/allow2ban_spec.rb b/spec/allow2ban_spec.rb
index 105e0ad0..9f6533d1 100644
--- a/spec/allow2ban_spec.rb
+++ b/spec/allow2ban_spec.rb
@@ -34,7 +34,7 @@
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
+          key, _ = Rack::Attack.cache.send(:key_and_expiry, "allow2ban:count:1.2.3.4", @findtime)
 
           _(@cache.store.read(key)).must_equal 1
         end
@@ -57,7 +57,7 @@
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
+          key, _ = Rack::Attack.cache.send(:key_and_expiry, "allow2ban:count:1.2.3.4", @findtime)
           _(@cache.store.read(key)).must_equal 2
         end
 
@@ -94,7 +94,7 @@
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "allow2ban:count:1.2.3.4", @findtime)
         _(@cache.store.read(key)).must_equal 2
       end
 
@@ -114,7 +114,7 @@
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i / @findtime}:allow2ban:count:1.2.3.4"
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "allow2ban:count:1.2.3.4", @findtime)
         _(@cache.store.read(key)).must_equal 2
       end
 
diff --git a/spec/fail2ban_spec.rb b/spec/fail2ban_spec.rb
index 6a9d9bcf..42a0d2b5 100644
--- a/spec/fail2ban_spec.rb
+++ b/spec/fail2ban_spec.rb
@@ -33,7 +33,7 @@
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
+          key, _ = Rack::Attack.cache.send(:key_and_expiry, "fail2ban:count:1.2.3.4", @findtime)
           _(@cache.store.read(key)).must_equal 1
         end
 
@@ -55,7 +55,7 @@
         end
 
         it 'increases fail count' do
-          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
+          key, _ = Rack::Attack.cache.send(:key_and_expiry, "fail2ban:count:1.2.3.4", @findtime)
           _(@cache.store.read(key)).must_equal 2
         end
 
@@ -77,7 +77,7 @@
         end
 
         it 'resets fail count' do
-          key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
+          key, _ = Rack::Attack.cache.send(:key_and_expiry, "fail2ban:count:1.2.3.4", @findtime)
           assert_nil @cache.store.read(key)
         end
 
@@ -113,7 +113,7 @@
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "fail2ban:count:1.2.3.4", @findtime)
         _(@cache.store.read(key)).must_equal 2
       end
 
@@ -133,7 +133,7 @@
       end
 
       it 'does not increase fail count' do
-        key = "rack::attack:#{Time.now.to_i / @findtime}:fail2ban:count:1.2.3.4"
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "fail2ban:count:1.2.3.4", @findtime)
         _(@cache.store.read(key)).must_equal 2
       end
 
diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb
index 0b0d68ac..d7c2ae6e 100644
--- a/spec/rack_attack_throttle_spec.rb
+++ b/spec/rack_attack_throttle_spec.rb
@@ -17,7 +17,7 @@
     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"
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -73,7 +73,7 @@
     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"
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -104,7 +104,7 @@
     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"
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -135,7 +135,7 @@
     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"
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
       assert_nil Rack::Attack.cache.store.read(key)
     end
 
@@ -163,7 +163,7 @@
 
   it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
     post_logins
-    key = "rack::attack:#{Time.now.to_i / @period}:logins/email:person@example.com"
+    key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:person@example.com", @period)
     _(Rack::Attack.cache.store.read(key)).must_equal 3
   end
 
@@ -174,7 +174,7 @@
 
       post_logins
       @emails.each do |email|
-        key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:#{email}", @period)
         _(Rack::Attack.cache.store.read(key)).must_equal 1
       end
     ensure

From 146eeea77b69672ab3fb5692def006b68e1069e4 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Mon, 27 Dec 2021 16:56:32 -0600
Subject: [PATCH 02/13] refactor: retry_after in match_data

---
 README.md                         | 12 +++++-------
 lib/rack/attack/cache.rb          |  6 +++++-
 lib/rack/attack/configuration.rb  |  3 +--
 lib/rack/attack/throttle.rb       |  3 ++-
 spec/rack_attack_throttle_spec.rb |  4 ++++
 5 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/README.md b/README.md
index 6ed8d573..d212840a 100644
--- a/README.md
+++ b/README.md
@@ -339,33 +339,31 @@ end
 While Rack::Attack's primary focus is minimizing harm from abusive clients, it
 can also be used to return rate limit data that's helpful for well-behaved clients.
 
-If you want to return to user how many seconds to wait until they can start sending requests again, this can be done through enabling `Retry-After` header:
+If you want to report to the client how many seconds to wait until they can start sending requests again, per RFCs 6585 and 7231, this can be done through enabling the `Retry-After` header:
 ```ruby
 Rack::Attack.throttled_response_retry_after_header = true
 ```
 
-Here's an example response that includes conventional `RateLimit-*` headers:
+If you prefer to emit one of the RateLimit-style standards, you might write your own lambda like this (this example uses the [IETF WG standard](https://github.com/ietf-wg-httpapi/ratelimit-headers)):
 
 ```ruby
 Rack::Attack.throttled_response = lambda do |env|
   match_data = env['rack.attack.match_data']
-  now = match_data[:epoch_time]
 
   headers = {
     'RateLimit-Limit' => match_data[:limit].to_s,
     'RateLimit-Remaining' => '0',
-    'RateLimit-Reset' => (now + (match_data[:period] - now % match_data[:period])).to_s
+    'RateLimit-Reset' => (match_data[:retry_after] - match_data[:epoch_time]).to_s
   }
 
   [ 429, headers, ["Throttled\n"]]
 end
 ```
 
-
-For responses that did not exceed a throttle limit, Rack::Attack annotates the env with match data:
+For responses that exceeded a throttle limit, Rack::Attack annotates the env with match data:
 
 ```ruby
-request.env['rack.attack.throttle_data'][name] # => { discriminator: d, count: n, period: p, limit: l, epoch_time: t }
+request.env['rack.attack.throttle_data'][name] # => { discriminator: d, count: n, period: p, limit: l, epoch_time: t, retry_after: r }
 ```
 
 ## Logging & Instrumentation
diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb
index 0e1f606e..810869c4 100644
--- a/lib/rack/attack/cache.rb
+++ b/lib/rack/attack/cache.rb
@@ -5,6 +5,7 @@ class Attack
     class Cache
       attr_accessor :prefix
       attr_reader :last_epoch_time
+      attr_reader :last_retry_after_time
 
       def initialize
         self.store = ::Rails.cache if defined?(::Rails.cache)
@@ -62,8 +63,11 @@ def reset!
 
       def key_and_expiry(unprefixed_key, period)
         @last_epoch_time = Time.now.to_i
+        time_into_period = @last_epoch_time % period
+        period_remainder = period - time_into_period
+        @last_retry_after_time = @last_epoch_time + period_remainder
         # Add 1 to expires_in to avoid timing error: https://git.io/i1PHXA
-        expires_in = (period - (@last_epoch_time % period) + 1).to_i
+        expires_in = period_remainder + 1
         ["#{prefix}:#{(@last_epoch_time / period).to_i}:#{unprefixed_key}", expires_in]
       end
 
diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index c6093106..94d96168 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -10,8 +10,7 @@ class Configuration
       DEFAULT_THROTTLED_CALLBACK = lambda do |req|
         if Rack::Attack.configuration.throttled_response_retry_after_header
           match_data = req.env['rack.attack.match_data']
-          now = match_data[:epoch_time]
-          retry_after = match_data[:period] - (now % match_data[:period])
+          retry_after = match_data[:retry_after] - match_data[:epoch_time]
 
           [429, { 'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s }, ["Retry later\n"]]
         else
diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb
index 69923393..7cf237aa 100644
--- a/lib/rack/attack/throttle.rb
+++ b/lib/rack/attack/throttle.rb
@@ -35,7 +35,8 @@ def matched_by?(request)
           count: count,
           period: current_period,
           limit: current_limit,
-          epoch_time: cache.last_epoch_time
+          epoch_time: cache.last_epoch_time,
+          retry_after: cache.last_retry_after_time
         }
 
         (count > current_limit).tap do |throttled|
diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb
index d7c2ae6e..700c8a2f 100644
--- a/spec/rack_attack_throttle_spec.rb
+++ b/spec/rack_attack_throttle_spec.rb
@@ -27,6 +27,7 @@
         limit: 1,
         period: @period,
         epoch_time: Rack::Attack.cache.last_epoch_time.to_i,
+        retry_after: Rack::Attack.cache.last_retry_after_time.to_i,
         discriminator: "1.2.3.4"
       }
 
@@ -52,6 +53,7 @@
         limit: 1,
         period: @period,
         epoch_time: Rack::Attack.cache.last_epoch_time.to_i,
+        retry_after: Rack::Attack.cache.last_retry_after_time.to_i,
         discriminator: "1.2.3.4"
       )
 
@@ -83,6 +85,7 @@
         limit: 1,
         period: @period,
         epoch_time: Rack::Attack.cache.last_epoch_time.to_i,
+        retry_after: Rack::Attack.cache.last_retry_after_time.to_i,
         discriminator: "1.2.3.4"
       }
 
@@ -114,6 +117,7 @@
         limit: 1,
         period: @period,
         epoch_time: Rack::Attack.cache.last_epoch_time.to_i,
+        retry_after: Rack::Attack.cache.last_retry_after_time.to_i,
         discriminator: "1.2.3.4"
       }
 

From 6bbb75802eb72f1f354883ca72d4f956c0bbe4d3 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Fri, 24 Dec 2021 17:47:23 -0600
Subject: [PATCH 03/13] refactor: offset_for, offset into period, at 0

---
 lib/rack/attack/cache.rb | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb
index 810869c4..539d0f68 100644
--- a/lib/rack/attack/cache.rb
+++ b/lib/rack/attack/cache.rb
@@ -63,12 +63,21 @@ def reset!
 
       def key_and_expiry(unprefixed_key, period)
         @last_epoch_time = Time.now.to_i
-        time_into_period = @last_epoch_time % period
+        offset = offset_for(unprefixed_key, period)
+        period_number, time_into_period = period_number_and_time_into(period, offset)
         period_remainder = period - time_into_period
         @last_retry_after_time = @last_epoch_time + period_remainder
         # Add 1 to expires_in to avoid timing error: https://git.io/i1PHXA
         expires_in = period_remainder + 1
-        ["#{prefix}:#{(@last_epoch_time / period).to_i}:#{unprefixed_key}", expires_in]
+        ["#{prefix}:#{period_number}:#{unprefixed_key}", expires_in]
+      end
+
+      def offset_for(unprefixed_key, period)
+        0
+      end
+
+      def period_number_and_time_into(period, offset)
+        [((@last_epoch_time + offset) / period).to_i, (@last_epoch_time + offset) % period]
       end
 
       def do_count(key, expires_in)

From c96112b8ef08d0d9bdbff40f232c700e0dce02bd Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Wed, 29 Dec 2021 20:09:45 -0600
Subject: [PATCH 04/13] refactor: new use_offset argument

---
 lib/rack/attack/cache.rb         | 10 +++++-----
 lib/rack/attack/configuration.rb |  3 ++-
 lib/rack/attack/throttle.rb      |  4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb
index 539d0f68..ee01757b 100644
--- a/lib/rack/attack/cache.rb
+++ b/lib/rack/attack/cache.rb
@@ -23,8 +23,8 @@ def store=(store)
           end
       end
 
-      def count(unprefixed_key, period)
-        key, expires_in = key_and_expiry(unprefixed_key, period)
+      def count(unprefixed_key, period, use_offset = false)
+        key, expires_in = key_and_expiry(unprefixed_key, period, use_offset)
         do_count(key, expires_in)
       end
 
@@ -61,9 +61,9 @@ def reset!
 
       private
 
-      def key_and_expiry(unprefixed_key, period)
+      def key_and_expiry(unprefixed_key, period, use_offset = false)
         @last_epoch_time = Time.now.to_i
-        offset = offset_for(unprefixed_key, period)
+        offset = offset_for(unprefixed_key, period, use_offset)
         period_number, time_into_period = period_number_and_time_into(period, offset)
         period_remainder = period - time_into_period
         @last_retry_after_time = @last_epoch_time + period_remainder
@@ -72,7 +72,7 @@ def key_and_expiry(unprefixed_key, period)
         ["#{prefix}:#{period_number}:#{unprefixed_key}", expires_in]
       end
 
-      def offset_for(unprefixed_key, period)
+      def offset_for(unprefixed_key, period, use_offset)
         0
       end
 
diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index 94d96168..1d651d2e 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -88,8 +88,9 @@ def blocklisted?(request)
       end
 
       def throttled?(request)
+        use_offset = false
         @throttles.any? do |_name, throttle|
-          throttle.matched_by?(request)
+          throttle.matched_by?(request, use_offset)
         end
       end
 
diff --git a/lib/rack/attack/throttle.rb b/lib/rack/attack/throttle.rb
index 7cf237aa..3a8644ca 100644
--- a/lib/rack/attack/throttle.rb
+++ b/lib/rack/attack/throttle.rb
@@ -22,13 +22,13 @@ def cache
         Rack::Attack.cache
       end
 
-      def matched_by?(request)
+      def matched_by?(request, use_offset = false)
         discriminator = discriminator_for(request)
         return false unless discriminator
 
         current_period  = period_for(request)
         current_limit   = limit_for(request)
-        count           = cache.count("#{name}:#{discriminator}", current_period)
+        count           = cache.count("#{name}:#{discriminator}", current_period, use_offset)
 
         data = {
           discriminator: discriminator,

From 86deb35a67ad7d1dfbe8a0c62e7c905072e679e8 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Sun, 26 Dec 2021 13:37:23 -0600
Subject: [PATCH 05/13] test: add offset tests

---
 spec/acceptance/key_and_expiry_spec.rb | 47 ++++++++++++++++++++++++++
 spec/acceptance/random_offset_spec.rb  | 42 +++++++++++++++++++++++
 spec/rack_attack_throttle_spec.rb      | 12 +++----
 3 files changed, 95 insertions(+), 6 deletions(-)
 create mode 100644 spec/acceptance/key_and_expiry_spec.rb
 create mode 100644 spec/acceptance/random_offset_spec.rb

diff --git a/spec/acceptance/key_and_expiry_spec.rb b/spec/acceptance/key_and_expiry_spec.rb
new file mode 100644
index 00000000..748f8867
--- /dev/null
+++ b/spec/acceptance/key_and_expiry_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require_relative "../spec_helper"
+require "timecop"
+
+describe "Behavior of key_and_expiry" do
+  it "forms keys and expirations without offset as expected" do
+    unprefixed_key = "abc789"
+    period = 1000
+    time = Time.at(1_000_000_000)
+
+    Timecop.freeze(time) do
+      key, expiry = Rack::Attack.cache.send(:key_and_expiry, unprefixed_key, period, false)
+      assert_equal "rack::attack:1000000:abc789", key
+      assert_equal 1001, expiry
+    end
+  end
+
+  it "forms keys and expirations with offset as expected" do
+    unprefixed_key = "abc789"
+    period = 1000
+    time = Time.at(1_000_000_000)
+
+    Timecop.freeze(time) do
+      Rack::Attack.cache.stub :offset_for, 0 do
+        key, expiry = Rack::Attack.cache.send(:key_and_expiry, unprefixed_key, period, true)
+        assert_equal "rack::attack:1000000:abc789", key
+        assert_equal 1001, expiry
+      end
+
+      Rack::Attack.cache.stub :offset_for, 123 do
+        key, expiry = Rack::Attack.cache.send(:key_and_expiry, unprefixed_key, period, true)
+        assert_equal "rack::attack:1000000:abc789", key
+        assert_equal 1001 - 123, expiry
+      end
+    end
+  end
+
+  it "expires correctly when period is 1 second" do
+    Timecop.freeze do
+      current_epoch = Time.now.to_i
+      key, expiry = Rack::Attack.cache.send(:key_and_expiry, "abc789", 1)
+      assert_equal "rack::attack:#{current_epoch}:abc789", key
+      assert_equal 2, expiry
+    end
+  end
+end
diff --git a/spec/acceptance/random_offset_spec.rb b/spec/acceptance/random_offset_spec.rb
new file mode 100644
index 00000000..fbf416f4
--- /dev/null
+++ b/spec/acceptance/random_offset_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require_relative "../spec_helper"
+require "timecop"
+
+describe "#random offset" do
+  before do
+    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
+  end
+
+  it "expires predictably" do
+    Rack::Attack.throttle("by ip", limit: 1, period: 6) do |request|
+      request.ip
+    end
+
+    addresses = (1..100).map { |i| "1.2.3.#{i}" }
+
+    start_time = Time.gm('2020-01-01 00:00:00')
+    Timecop.freeze(start_time) do
+      addresses.each do |ip|
+        get "/", {}, "REMOTE_ADDR" => ip
+        assert_equal 200, last_response.status
+      end
+
+      get "/", {}, "REMOTE_ADDR" => "1.2.3.45"
+      assert_equal 429, last_response.status
+
+      responses200 = 0
+      responses429 = 0
+      Timecop.travel(start_time + 1) do
+        addresses.each do |ip|
+          get "/", {}, "REMOTE_ADDR" => ip
+          responses200 += 1 if last_response.status == 200
+          responses429 += 1 if last_response.status == 429
+        end
+      end
+
+      assert_equal 0, responses200
+      assert_equal 100, responses429
+    end
+  end
+end
diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb
index 700c8a2f..e01fafa0 100644
--- a/spec/rack_attack_throttle_spec.rb
+++ b/spec/rack_attack_throttle_spec.rb
@@ -17,7 +17,7 @@
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period, true)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -75,7 +75,7 @@
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period, true)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -107,7 +107,7 @@
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should set the counter for one request' do
-      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period, true)
       _(Rack::Attack.cache.store.read(key)).must_equal 1
     end
 
@@ -139,7 +139,7 @@
     before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
 
     it 'should not set the counter' do
-      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period)
+      key, _ = Rack::Attack.cache.send(:key_and_expiry, "ip/sec:1.2.3.4", @period, true)
       assert_nil Rack::Attack.cache.store.read(key)
     end
 
@@ -167,7 +167,7 @@
 
   it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
     post_logins
-    key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:person@example.com", @period)
+    key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:person@example.com", @period, true)
     _(Rack::Attack.cache.store.read(key)).must_equal 3
   end
 
@@ -178,7 +178,7 @@
 
       post_logins
       @emails.each do |email|
-        key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:#{email}", @period)
+        key, _ = Rack::Attack.cache.send(:key_and_expiry, "logins/email:#{email}", @period, true)
         _(Rack::Attack.cache.store.read(key)).must_equal 1
       end
     ensure

From 2a25360355b3db68509517aeb45dba1ef20a33dc Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Mon, 27 Dec 2021 17:54:25 -0600
Subject: [PATCH 06/13] refactor: change offset_for to pseudo-random

---
 lib/rack/attack/cache.rb               |  4 +++-
 lib/rack/attack/configuration.rb       |  2 +-
 spec/acceptance/key_and_expiry_spec.rb |  6 ++++++
 spec/acceptance/random_offset_spec.rb  | 10 +++++++---
 spec/acceptance/throttling_spec.rb     | 14 +++++++++++++-
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb
index ee01757b..41a09f77 100644
--- a/lib/rack/attack/cache.rb
+++ b/lib/rack/attack/cache.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'digest'
+
 module Rack
   class Attack
     class Cache
@@ -73,7 +75,7 @@ def key_and_expiry(unprefixed_key, period, use_offset = false)
       end
 
       def offset_for(unprefixed_key, period, use_offset)
-        0
+        use_offset ? Digest::MD5.hexdigest(unprefixed_key).hex % period : 0
       end
 
       def period_number_and_time_into(period, offset)
diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index 1d651d2e..070dcac4 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -88,7 +88,7 @@ def blocklisted?(request)
       end
 
       def throttled?(request)
-        use_offset = false
+        use_offset = true
         @throttles.any? do |_name, throttle|
           throttle.matched_by?(request, use_offset)
         end
diff --git a/spec/acceptance/key_and_expiry_spec.rb b/spec/acceptance/key_and_expiry_spec.rb
index 748f8867..f9890b7a 100644
--- a/spec/acceptance/key_and_expiry_spec.rb
+++ b/spec/acceptance/key_and_expiry_spec.rb
@@ -33,6 +33,12 @@
         assert_equal "rack::attack:1000000:abc789", key
         assert_equal 1001 - 123, expiry
       end
+
+      Digest::MD5.stub :hexdigest, "123" do
+        key, expiry = Rack::Attack.cache.send(:key_and_expiry, unprefixed_key, period, true)
+        assert_equal "rack::attack:1000000:abc789", key
+        assert_equal 1001 - "123".hex, expiry
+      end
     end
   end
 
diff --git a/spec/acceptance/random_offset_spec.rb b/spec/acceptance/random_offset_spec.rb
index fbf416f4..4b6286c5 100644
--- a/spec/acceptance/random_offset_spec.rb
+++ b/spec/acceptance/random_offset_spec.rb
@@ -8,7 +8,7 @@
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
   end
 
-  it "expires predictably" do
+  it "expires randomly by discriminator" do
     Rack::Attack.throttle("by ip", limit: 1, period: 6) do |request|
       request.ip
     end
@@ -35,8 +35,12 @@
         end
       end
 
-      assert_equal 0, responses200
-      assert_equal 100, responses429
+      # We would expect about one in six throttles to expire in the
+      # first second. The offset_for MD5 hash happens to come out to
+      # 15 out of 100. If anything about the algorithm changes, or
+      # the addresses or start_time, these values probably would too.
+      assert_equal 15, responses200
+      assert_equal 85, responses429
     end
   end
 end
diff --git a/spec/acceptance/throttling_spec.rb b/spec/acceptance/throttling_spec.rb
index 0db89dd6..0b54d0b2 100644
--- a/spec/acceptance/throttling_spec.rb
+++ b/spec/acceptance/throttling_spec.rb
@@ -48,7 +48,19 @@
 
     Timecop.freeze(Time.at(25)) do
       get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
-      assert_equal "35", last_response.headers["Retry-After"]
+      assert_equal 429, last_response.status
+      assert_equal "19", last_response.headers["Retry-After"]
+    end
+
+    Timecop.freeze(Time.at(42)) do
+      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+      assert_equal 429, last_response.status
+      assert_equal "2", last_response.headers["Retry-After"]
+    end
+
+    Timecop.freeze(Time.at(44)) do
+      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+      assert_equal 200, last_response.status
     end
   end
 

From d45351433237d4467be714fb180c76613013cb3f Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Wed, 29 Dec 2021 20:23:02 -0600
Subject: [PATCH 07/13] refactor: throttled_callback_is_offset_aware

---
 README.md                        | 2 ++
 lib/rack/attack/configuration.rb | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index d212840a..0568237e 100644
--- a/README.md
+++ b/README.md
@@ -332,6 +332,7 @@ Rack::Attack.throttled_callback = lambda do |request|
   # DOSed the site. Rack::Attack returns 429 for throttling by default
   [ 503, {}, ["Server Error\n"]]
 end
+Rack::Attack.configuration.throttled_callback_is_offset_aware = true
 ```
 
 ### RateLimit headers for well-behaved clients
@@ -358,6 +359,7 @@ Rack::Attack.throttled_response = lambda do |env|
 
   [ 429, headers, ["Throttled\n"]]
 end
+Rack::Attack.configuration.throttled_callback_is_offset_aware = true
 ```
 
 For responses that exceeded a throttle limit, Rack::Attack annotates the env with match data:
diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index 070dcac4..cfe156c0 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -19,7 +19,8 @@ class Configuration
       end
 
       attr_reader :safelists, :blocklists, :throttles, :anonymous_blocklists, :anonymous_safelists
-      attr_accessor :blocklisted_callback, :throttled_callback, :throttled_response_retry_after_header
+      attr_accessor :blocklisted_callback, :throttled_callback, :throttled_response_retry_after_header,
+                    :throttled_callback_is_offset_aware
 
       attr_reader :blocklisted_response, :throttled_response # Keeping these for backwards compatibility
 
@@ -88,7 +89,8 @@ def blocklisted?(request)
       end
 
       def throttled?(request)
-        use_offset = true
+        use_offset = throttled_callback_is_offset_aware ||
+                     ( !throttled_response && throttled_callback == DEFAULT_THROTTLED_CALLBACK )
         @throttles.any? do |_name, throttle|
           throttle.matched_by?(request, use_offset)
         end
@@ -117,6 +119,7 @@ def set_defaults
 
         @blocklisted_callback = DEFAULT_BLOCKLISTED_CALLBACK
         @throttled_callback = DEFAULT_THROTTLED_CALLBACK
+        @throttled_callback_is_offset_aware = false
 
         # Deprecated: Keeping these for backwards compatibility
         @blocklisted_response = nil

From 06787b5dc218a9c837c621377771e5f2867d0676 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Thu, 30 Dec 2021 08:03:55 -0600
Subject: [PATCH 08/13] docs: update changelog

---
 CHANGELOG.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e7fbfbf3..a952c8f5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 All notable changes to this project will be documented in this file.
 
+## [6.x.x] = 2022-xx-xx
+
+### Added
+
+- Added pseudo-random time offsets to throttling. If your application uses a custom throttle lambda to emit RateLimit-style headers, see the README for updated sample code.
+
 ## [6.5.0] - 2021-02-07
 
 ### Added

From 71198e7b46de2e54967b099a1df5fbb56816b381 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Sun, 13 Mar 2022 22:09:50 -0500
Subject: [PATCH 09/13] update: throttled_callback is now throttled_responder

---
 README.md                        | 4 ++--
 lib/rack/attack/configuration.rb | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index aa9f35b8..683b5bcc 100644
--- a/README.md
+++ b/README.md
@@ -332,7 +332,7 @@ Rack::Attack.throttled_responder = lambda do |request|
   # DOSed the site. Rack::Attack returns 429 for throttling by default
   [ 503, {}, ["Server Error\n"]]
 end
-Rack::Attack.configuration.throttled_callback_is_offset_aware = true
+Rack::Attack.configuration.throttled_responder_is_offset_aware = true
 ```
 
 ### RateLimit headers for well-behaved clients
@@ -359,7 +359,7 @@ Rack::Attack.throttled_response = lambda do |env|
 
   [ 429, headers, ["Throttled\n"]]
 end
-Rack::Attack.configuration.throttled_callback_is_offset_aware = true
+Rack::Attack.configuration.throttled_responder_is_offset_aware = true
 ```
 
 For responses that exceeded a throttle limit, Rack::Attack annotates the env with match data:
diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index 01ea077d..93513e2c 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -20,7 +20,7 @@ class Configuration
 
       attr_reader :safelists, :blocklists, :throttles, :anonymous_blocklists, :anonymous_safelists
       attr_accessor :blocklisted_responder, :throttled_responder, :throttled_response_retry_after_header,
-                    :throttled_callback_is_offset_aware
+                    :throttled_responder_is_offset_aware
 
       attr_reader :blocklisted_response, :throttled_response # Keeping these for backwards compatibility
 
@@ -87,8 +87,8 @@ def blocklisted?(request)
       end
 
       def throttled?(request)
-        use_offset = throttled_callback_is_offset_aware ||
-                     ( !throttled_response && throttled_callback == DEFAULT_THROTTLED_CALLBACK )
+        use_offset = throttled_responder_is_offset_aware ||
+                     ( !throttled_response && throttled_responder == DEFAULT_THROTTLED_RESPONDER )
         @throttles.any? do |_name, throttle|
           throttle.matched_by?(request, use_offset)
         end
@@ -117,7 +117,7 @@ def set_defaults
 
         @blocklisted_responder = DEFAULT_BLOCKLISTED_RESPONDER
         @throttled_responder = DEFAULT_THROTTLED_RESPONDER
-        @throttled_callback_is_offset_aware = false
+        @throttled_responder_is_offset_aware = false
 
         # Deprecated: Keeping these for backwards compatibility
         @blocklisted_response = nil

From ca42333094173aa61a053464f35231abee7ec505 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Mon, 21 Mar 2022 07:39:01 -0500
Subject: [PATCH 10/13] test: changing the throttled responder can change the
 offset algorithm

---
 .../customizing_throttled_affects_offset.rb   | 72 +++++++++++++++++++
 .../customizing_throttled_response_spec.rb    |  2 +
 spec/acceptance/random_offset_spec.rb         | 46 ------------
 3 files changed, 74 insertions(+), 46 deletions(-)
 create mode 100644 spec/acceptance/customizing_throttled_affects_offset.rb
 delete mode 100644 spec/acceptance/random_offset_spec.rb

diff --git a/spec/acceptance/customizing_throttled_affects_offset.rb b/spec/acceptance/customizing_throttled_affects_offset.rb
new file mode 100644
index 00000000..44310867
--- /dev/null
+++ b/spec/acceptance/customizing_throttled_affects_offset.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require_relative "../spec_helper"
+require "timecop"
+
+describe "Customizing throttled response" do
+  before do
+    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
+
+    Rack::Attack.throttle("by ip", limit: 1, period: 6) do |request|
+      request.ip
+    end
+  end
+
+  it "uses offset if responder is default" do
+    assert Rack::Attack.cache.store.is_a? ActiveSupport::Cache::MemoryStore
+    assert_equal 85, count_429_responses
+  end
+
+  it "does not use offset if responder is not default and aware is not set" do
+    assert_equal false, Rack::Attack.configuration.throttled_responder_is_offset_aware
+    Rack::Attack.throttled_responder = lambda do |req|
+      [429, {}, ["Throttled"]]
+    end
+    assert_equal 100, count_429_responses
+  end
+
+  it "uses offset if responder is not default and aware is set" do
+    Rack::Attack.throttled_responder = lambda do |req|
+      [429, {}, ["Throttled"]]
+    end
+    Rack::Attack.configuration.throttled_responder_is_offset_aware = true
+    assert_equal 85, count_429_responses
+  end
+
+  # Count the number of responses with 429 status, out of 100 requests,
+  # when the clock advances by one second.
+  #
+  # When this is invoked with a throttle with period 6 active, using
+  # a random offset, we would expect about one in six to expire in the
+  # first second. For the fixed start_time we're using, the offset_for
+  # MD5 hash happens to come out to 15 expired, 85 throttled, out of 100.
+  # (If anything about the algorithm changes, that count probably would
+  # too.)
+  #
+  # When using an old-style period, the start_time is at the beginning of
+  # the period, since 2020-01-01 00:00:00 == 1577836800 == 262972800*6,
+  # and after 1 second we would expect 0 expires, thus 100 of 100 requests
+  # to be throttled.
+
+  def count_429_responses
+    addresses = (1..100).map { |i| "1.2.3.#{i}" }
+    start_time = Time.gm('2020-01-01 00:00:00')
+    Timecop.freeze(start_time) do
+      initial_200_response_count = 0
+      addresses.each do |ip|
+        get "/", {}, "REMOTE_ADDR" => ip
+        initial_200_response_count += 1 if last_response.status == 200
+      end
+      assert_equal 100, initial_200_response_count
+
+      final_429_response_count = 0
+      Timecop.travel(start_time + 1) do
+        addresses.each do |ip|
+          get "/", {}, "REMOTE_ADDR" => ip
+          final_429_response_count += 1 if last_response.status == 429
+        end
+      end
+      final_429_response_count
+    end
+  end
+end
diff --git a/spec/acceptance/customizing_throttled_response_spec.rb b/spec/acceptance/customizing_throttled_response_spec.rb
index 0990975e..b512801c 100644
--- a/spec/acceptance/customizing_throttled_response_spec.rb
+++ b/spec/acceptance/customizing_throttled_response_spec.rb
@@ -24,6 +24,7 @@
       [503, {}, ["Throttled"]]
     end
 
+    get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
     get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
 
     assert_equal 503, last_response.status
@@ -74,6 +75,7 @@
       end
     end
 
+    get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
     get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
 
     assert_equal 503, last_response.status
diff --git a/spec/acceptance/random_offset_spec.rb b/spec/acceptance/random_offset_spec.rb
deleted file mode 100644
index 4b6286c5..00000000
--- a/spec/acceptance/random_offset_spec.rb
+++ /dev/null
@@ -1,46 +0,0 @@
-# frozen_string_literal: true
-
-require_relative "../spec_helper"
-require "timecop"
-
-describe "#random offset" do
-  before do
-    Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
-  end
-
-  it "expires randomly by discriminator" do
-    Rack::Attack.throttle("by ip", limit: 1, period: 6) do |request|
-      request.ip
-    end
-
-    addresses = (1..100).map { |i| "1.2.3.#{i}" }
-
-    start_time = Time.gm('2020-01-01 00:00:00')
-    Timecop.freeze(start_time) do
-      addresses.each do |ip|
-        get "/", {}, "REMOTE_ADDR" => ip
-        assert_equal 200, last_response.status
-      end
-
-      get "/", {}, "REMOTE_ADDR" => "1.2.3.45"
-      assert_equal 429, last_response.status
-
-      responses200 = 0
-      responses429 = 0
-      Timecop.travel(start_time + 1) do
-        addresses.each do |ip|
-          get "/", {}, "REMOTE_ADDR" => ip
-          responses200 += 1 if last_response.status == 200
-          responses429 += 1 if last_response.status == 429
-        end
-      end
-
-      # We would expect about one in six throttles to expire in the
-      # first second. The offset_for MD5 hash happens to come out to
-      # 15 out of 100. If anything about the algorithm changes, or
-      # the addresses or start_time, these values probably would too.
-      assert_equal 15, responses200
-      assert_equal 85, responses429
-    end
-  end
-end

From 24ae6b274f1aa7ee79164ec39524b68221d7bbd4 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Mon, 21 Mar 2022 08:17:12 -0500
Subject: [PATCH 11/13] readme: resolve lambda interface change

---
 README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 683b5bcc..cb5aca95 100644
--- a/README.md
+++ b/README.md
@@ -348,8 +348,8 @@ Rack::Attack.throttled_response_retry_after_header = true
 If you prefer to emit one of the RateLimit-style standards, you might write your own lambda like this (this example uses the [IETF WG standard](https://github.com/ietf-wg-httpapi/ratelimit-headers)):
 
 ```ruby
-Rack::Attack.throttled_response = lambda do |env|
-  match_data = env['rack.attack.match_data']
+Rack::Attack.throttled_response = lambda do |request|
+  match_data = request.env['rack.attack.match_data']
 
   headers = {
     'RateLimit-Limit' => match_data[:limit].to_s,

From dc18167d24a5c88c98e0076c90f869c3fded5788 Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Mon, 21 Mar 2022 08:19:08 -0500
Subject: [PATCH 12/13] readme: fix typo

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index cb5aca95..24130d6c 100644
--- a/README.md
+++ b/README.md
@@ -348,7 +348,7 @@ Rack::Attack.throttled_response_retry_after_header = true
 If you prefer to emit one of the RateLimit-style standards, you might write your own lambda like this (this example uses the [IETF WG standard](https://github.com/ietf-wg-httpapi/ratelimit-headers)):
 
 ```ruby
-Rack::Attack.throttled_response = lambda do |request|
+Rack::Attack.throttled_responder = lambda do |request|
   match_data = request.env['rack.attack.match_data']
 
   headers = {

From 0f5a8327a6a47c96420a846dbbd1066b797f605d Mon Sep 17 00:00:00 2001
From: Jamie McCarthy <jamie@mccarthy.vg>
Date: Tue, 12 Apr 2022 21:13:04 -0500
Subject: [PATCH 13/13] Rubocop auto-corrections

---
 lib/rack/attack/configuration.rb                        | 2 +-
 spec/acceptance/customizing_throttled_affects_offset.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index 2732612e..bcbafc49 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -88,7 +88,7 @@ def blocklisted?(request)
 
       def throttled?(request)
         use_offset = throttled_responder_is_offset_aware ||
-                     ( !throttled_response && throttled_responder == DEFAULT_THROTTLED_RESPONDER )
+                     (!throttled_response && throttled_responder == DEFAULT_THROTTLED_RESPONDER)
         @throttles.any? do |_name, throttle|
           throttle.matched_by?(request, use_offset)
         end
diff --git a/spec/acceptance/customizing_throttled_affects_offset.rb b/spec/acceptance/customizing_throttled_affects_offset.rb
index 44310867..4e122422 100644
--- a/spec/acceptance/customizing_throttled_affects_offset.rb
+++ b/spec/acceptance/customizing_throttled_affects_offset.rb
@@ -19,14 +19,14 @@
 
   it "does not use offset if responder is not default and aware is not set" do
     assert_equal false, Rack::Attack.configuration.throttled_responder_is_offset_aware
-    Rack::Attack.throttled_responder = lambda do |req|
+    Rack::Attack.throttled_responder = lambda do |_req|
       [429, {}, ["Throttled"]]
     end
     assert_equal 100, count_429_responses
   end
 
   it "uses offset if responder is not default and aware is set" do
-    Rack::Attack.throttled_responder = lambda do |req|
+    Rack::Attack.throttled_responder = lambda do |_req|
       [429, {}, ["Throttled"]]
     end
     Rack::Attack.configuration.throttled_responder_is_offset_aware = true