From 9aeffae14f5eebca4006e2666d39be1505f867f3 Mon Sep 17 00:00:00 2001 From: Petrik Date: Mon, 28 Aug 2023 20:11:53 +0200 Subject: [PATCH] Use SecureRandom.alphanumeric for SecureRandom.base36 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruby 3.3 allows passing a list of characters to `SecureRandom.alphanumeric`. For `SecureRandom.base36` using `choose` is faster than the current implementation. For `SecureRandom.base58` it is a bit slower. I've also added a test to make sure passing nil as the length defaults the length to 16. _Benchmark__ ```ruby require "bundler/inline" gemfile(true) do source "https://rubygems.org" git_source(:github) { |repo| "https://github.com/#{repo}.git" } gem "rails", github: "rails/rails", branch: "main" gem "benchmark-ips" end require "active_support" require "active_support/core_ext/securerandom" module SecureRandom def self.fast_base36(n) alphanumeric(n, chars: BASE36_ALPHABET) end end [10, 100, 1000, 10000].each do |length| puts puts " #{length} ".center(80, "=") puts Benchmark.ips do |x| x.report("base36") { SecureRandom.base36(length) } x.report("fast_base36") { SecureRandom.fast_base36(length) } x.compare! end end ``` ``` ====================================== 10 ====================================== Warming up -------------------------------------- base36 20.513k i/100ms fast_base36 24.843k i/100ms Calculating ------------------------------------- base36 200.940k (±13.8%) i/s - 984.624k in 5.060203s fast_base36 235.531k (± 5.7%) i/s - 1.192M in 5.080574s Comparison: fast_base36: 235530.9 i/s base36: 200939.9 i/s - same-ish: difference falls within error ===================================== 100 ====================================== Warming up -------------------------------------- base36 2.746k i/100ms fast_base36 2.995k i/100ms Calculating ------------------------------------- base36 25.559k (± 8.5%) i/s - 129.062k in 5.087961s fast_base36 30.265k (± 6.6%) i/s - 152.745k in 5.070263s Comparison: fast_base36: 30264.7 i/s base36: 25558.8 i/s - 1.18x slower ===================================== 1000 ===================================== Warming up -------------------------------------- base36 278.000 i/100ms fast_base36 308.000 i/100ms Calculating ------------------------------------- base36 2.595k (±11.6%) i/s - 12.788k in 5.007921s fast_base36 3.133k (± 6.1%) i/s - 15.708k in 5.033310s Comparison: fast_base36: 3132.6 i/s base36: 2594.9 i/s - 1.21x slower ==================================== 10000 ===================================== Warming up -------------------------------------- base36 24.000 i/100ms fast_base36 34.000 i/100ms Calculating ------------------------------------- base36 256.601 (± 8.6%) i/s - 1.296k in 5.089604s fast_base36 322.119 (± 6.5%) i/s - 1.632k in 5.089614s Comparison: fast_base36: 322.1 i/s base36: 256.6 i/s - 1.26x slower ``` --- .../active_support/core_ext/securerandom.rb | 36 ++++++++++++------- .../test/core_ext/secure_random_test.rb | 22 ++++++++++++ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/securerandom.rb b/activesupport/lib/active_support/core_ext/securerandom.rb index fa6b68b101..6c1f2be919 100644 --- a/activesupport/lib/active_support/core_ext/securerandom.rb +++ b/activesupport/lib/active_support/core_ext/securerandom.rb @@ -16,12 +16,18 @@ module SecureRandom # # p SecureRandom.base58 # => "4kUgL2pdQMSCQtjE" # p SecureRandom.base58(24) # => "77TMHrHJFvFDwodq8w7Ev2m7" - def self.base58(n = 16) - SecureRandom.random_bytes(n).unpack("C*").map do |byte| - idx = byte % 64 - idx = SecureRandom.random_number(58) if idx >= 58 - BASE58_ALPHABET[idx] - end.join + if RUBY_VERSION >= "3.3" + def self.base58(n = 16) + SecureRandom.alphanumeric(n, chars: BASE58_ALPHABET) + end + else + def self.base58(n = 16) + SecureRandom.random_bytes(n).unpack("C*").map do |byte| + idx = byte % 64 + idx = SecureRandom.random_number(58) if idx >= 58 + BASE58_ALPHABET[idx] + end.join + end end # SecureRandom.base36 generates a random base36 string in lowercase. @@ -35,11 +41,17 @@ def self.base58(n = 16) # # p SecureRandom.base36 # => "4kugl2pdqmscqtje" # p SecureRandom.base36(24) # => "77tmhrhjfvfdwodq8w7ev2m7" - def self.base36(n = 16) - SecureRandom.random_bytes(n).unpack("C*").map do |byte| - idx = byte % 64 - idx = SecureRandom.random_number(36) if idx >= 36 - BASE36_ALPHABET[idx] - end.join + if RUBY_VERSION >= "3.3" + def self.base36(n = 16) + SecureRandom.alphanumeric(n, chars: BASE36_ALPHABET) + end + else + def self.base36(n = 16) + SecureRandom.random_bytes(n).unpack("C*").map do |byte| + idx = byte % 64 + idx = SecureRandom.random_number(36) if idx >= 36 + BASE36_ALPHABET[idx] + end.join + end end end diff --git a/activesupport/test/core_ext/secure_random_test.rb b/activesupport/test/core_ext/secure_random_test.rb index 5b9f26adc8..f2e1c2ba5c 100644 --- a/activesupport/test/core_ext/secure_random_test.rb +++ b/activesupport/test/core_ext/secure_random_test.rb @@ -28,6 +28,18 @@ def test_base58_with_length assert_match(/^[^0OIl]+$/, s2) end + def test_base58_with_nil + s1 = SecureRandom.base58(nil) + s2 = SecureRandom.base58(nil) + + assert_not_equal s1, s2 + assert_equal 16, s1.length + assert_match(/^[a-zA-Z0-9]+$/, s1) + assert_match(/^[a-zA-Z0-9]+$/, s2) + assert_match(/^[^0OIl]+$/, s1) + assert_match(/^[^0OIl]+$/, s2) + end + def test_base36 s1 = SecureRandom.base36 s2 = SecureRandom.base36 @@ -47,4 +59,14 @@ def test_base36_with_length assert_match(/^[a-z0-9]+$/, s1) assert_match(/^[a-z0-9]+$/, s2) end + + def test_base36_with_nil + s1 = SecureRandom.base36(nil) + s2 = SecureRandom.base36(nil) + + assert_not_equal s1, s2 + assert_equal 16, s1.length + assert_match(/^[a-z0-9]+$/, s1) + assert_match(/^[a-z0-9]+$/, s2) + end end