Merge pull request #49067 from p8/performance/securerandom-choose

Use SecureRandom.alphanumeric for SecureRandom.base36/base58
rails · Sep 1, 2023 · 946550b · 946550b
2 parents 7459392 + 9aeffae
commit 946550b
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 12 deletions.
diff --git 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
@@ -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