Skip to content

Commit 9e5c7cf

Browse files
Detach Messages::Rotator from SecureCompareRotator
Prior to this commit, `ActiveSupport::SecureCompareRotator` used `ActiveSupport::Messages::Rotator` for part of its rotation logic, even though `SecureCompareRotator` is entirely unrelated to messages. This made it precarious to alter `Messages::Rotator`, especially because `Messages::Rotator` was `prepend`ed to `SecureCompareRotator` rather than `include`d. This commit reimplements `SecureCompareRotator` without `Messages::Rotator`, which simplifies the logic and, as a bonus, improves performance: ```ruby # frozen_string_literal: true require "benchmark/ips" require "active_support/all" comparer = ActiveSupport::SecureCompareRotator.new("new secret") comparer.rotate("old secret") Benchmark.ips do |x| x.report("compare old") do comparer.secure_compare!("old secret") end end ``` __Before__ ``` Warming up -------------------------------------- compare old 72.073k i/100ms Calculating ------------------------------------- compare old 719.844k (± 1.0%) i/s - 3.604M in 5.006682s ``` __After__ ``` Warming up -------------------------------------- compare old 147.486k i/100ms Calculating ------------------------------------- compare old 1.473M (± 0.9%) i/s - 7.374M in 5.006655s ```
1 parent 10fcbee commit 9e5c7cf

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

activesupport/lib/active_support/secure_compare_rotator.rb

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,28 @@ module ActiveSupport
2929
# end
3030
class SecureCompareRotator
3131
include SecurityUtils
32-
prepend Messages::Rotator
3332

3433
InvalidMatch = Class.new(StandardError)
3534

36-
def initialize(value, **_options)
35+
def initialize(value, on_rotation: nil)
3736
@value = value
37+
@rotate_values = []
38+
@on_rotation = on_rotation
3839
end
3940

40-
def secure_compare!(other_value, on_rotation: @on_rotation)
41-
secure_compare(@value, other_value) ||
42-
run_rotations(on_rotation) { |wrapper| wrapper.secure_compare!(other_value) } ||
43-
raise(InvalidMatch)
41+
def rotate(previous_value)
42+
@rotate_values << previous_value
4443
end
4544

46-
private
47-
def build_rotation(previous_value, **_options)
48-
self.class.new(previous_value)
45+
def secure_compare!(other_value, on_rotation: @on_rotation)
46+
if secure_compare(@value, other_value)
47+
true
48+
elsif @rotate_values.any? { |value| secure_compare(value, other_value) }
49+
on_rotation&.call
50+
true
51+
else
52+
raise InvalidMatch
4953
end
54+
end
5055
end
5156
end

0 commit comments

Comments
 (0)