Allow to configure maximum cache key sizes

Author: fatkodima

activesupport/CHANGELOG.md

@@ -1,3 +1,17 @@
+*   Allow to configure maximum cache key sizes
+
+    When the key exceeds the configured limit (250 bytes by default), it will be truncated and
+    the digest of the rest of the key appended to it.
+
+    Note that previously `ActiveSupport::Cache::RedisCacheStore` allowed up to 1kb cache keys before
+    truncation, which is now reduced to 250 bytes.
+
+    ```ruby
+    config.cache_store = :redis_cache_store, { max_key_size: 64 }
+    ```
+
+    *fatkodima*
+
 *   Use `UNLINK` command instead of `DEL` in `ActiveSupport::Cache::RedisCacheStore` for non-blocking deletion.
 
     *Aron Roh*

activesupport/lib/active_support/cache.rb

@@ -36,6 +36,7 @@ module Cache
       :serializer,
       :skip_nil,
       :raw,
+      :max_key_size,
     ]
 
     # Mapping of canonical option names to aliases that a store will recognize.
@@ -190,6 +191,9 @@ class Store
       # Default +ConnectionPool+ options
       DEFAULT_POOL_OPTIONS = { size: 5, timeout: 5 }.freeze
 
+      # Keys are truncated with the Active Support digest if they exceed the limit.
+      MAX_KEY_SIZE = 250
+
       cattr_accessor :logger, instance_writer: true
       cattr_accessor :raise_on_invalid_cache_expiration_time, default: false
 
@@ -299,6 +303,9 @@ def initialize(options = nil)
         @options[:compress] = true unless @options.key?(:compress)
         @options[:compress_threshold] ||= DEFAULT_COMPRESS_LIMIT
 
+        @max_key_size = @options.delete(:max_key_size)
+        @max_key_size = MAX_KEY_SIZE if @max_key_size.nil? # allow 'false' as a value
+
         @coder = @options.delete(:coder) do
           legacy_serializer = Cache.format_version < 7.1 && !@options[:serializer]
           serializer = @options.delete(:serializer) || default_serializer
@@ -955,16 +962,33 @@ def validate_options(options)
           options
         end
 
-        # Expands and namespaces the cache key.
+        # Expands, namespaces and truncates the cache key.
         # Raises an exception when the key is +nil+ or an empty string.
         # May be overridden by cache stores to do additional normalization.
         def normalize_key(key, options = nil)
+          key = expand_and_namespace_key(key, options)
+          truncate_key(key)
+        end
+
+        def expand_and_namespace_key(key, options = nil)
           str_key = expanded_key(key)
           raise(ArgumentError, "key cannot be blank") if !str_key || str_key.empty?
 
           namespace_key str_key, options
         end
 
+        def truncate_key(key)
+          if key && @max_key_size && key.bytesize > @max_key_size
+            suffix = ":hash:#{ActiveSupport::Digest.hexdigest(key)}"
+            truncate_at = @max_key_size - suffix.bytesize
+            key = key.byteslice(0, truncate_at)
+            key.scrub!("")
+            "#{key}#{suffix}"
+          else
+            key
+          end
+        end
+
         # Prefix the key with a namespace string:
         #
         #   namespace_key 'foo', namespace: 'cache'

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -41,7 +41,6 @@ def self.supports_cache_versioning?
 
       prepend Strategy::LocalCache
 
-      KEY_MAX_SIZE = 250
       ESCAPE_KEY_CHARS = /[\x00-\x20%\x7F-\xFF]/n
 
       # Creates a new Dalli::Client instance with specified addresses and options.
@@ -80,6 +79,7 @@ def initialize(*addresses)
         if options.key?(:cache_nils)
           options[:skip_nil] = !options.delete(:cache_nils)
         end
+        options[:max_key_size] ||= MAX_KEY_SIZE
         super(options)
 
         unless [String, Dalli::Client, NilClass].include?(addresses.first.class)
@@ -258,19 +258,12 @@ def serialize_entry(entry, raw: false, **options)
         # before applying the regular expression to ensure we are escaping all
         # characters properly.
         def normalize_key(key, options)
-          key = super
+          key = expand_and_namespace_key(key, options)
           if key
             key = key.dup.force_encoding(Encoding::ASCII_8BIT)
             key = key.gsub(ESCAPE_KEY_CHARS) { |match| "%#{match.getbyte(0).to_s(16).upcase}" }
-
-            if key.size > KEY_MAX_SIZE
-              key_separator = ":hash:"
-              key_hash = ActiveSupport::Digest.hexdigest(key)
-              key_trim_size = KEY_MAX_SIZE - key_separator.size - key_hash.size
-              key = "#{key[0, key_trim_size]}#{key_separator}#{key_hash}"
-            end
           end
-          key
+          truncate_key(key)
         end
 
         def deserialize_entry(payload, raw: false, **)

activesupport/lib/active_support/cache/redis_cache_store.rb

@@ -35,9 +35,6 @@ module Cache
     #   +Redis::Distributed+ 4.0.1+ for distributed mget support.
     # * +delete_matched+ support for Redis KEYS globs.
     class RedisCacheStore < Store
-      # Keys are truncated with the Active Support digest if they exceed 1kB
-      MAX_KEY_BYTESIZE = 1024
-
       DEFAULT_REDIS_OPTIONS = {
         connect_timeout:    1,
         read_timeout:       1,
@@ -106,7 +103,6 @@ def build_redis_client(**redis_options)
           end
       end
 
-      attr_reader :max_key_bytesize
       attr_reader :redis
 
       # Creates a new Redis cache store.
@@ -169,7 +165,6 @@ def initialize(error_handler: DEFAULT_ERROR_HANDLER, **redis_options)
           @redis = self.class.build_redis(**redis_options)
         end
 
-        @max_key_bytesize = MAX_KEY_BYTESIZE
         @error_handler = error_handler
 
         super(universal_options)
@@ -436,21 +431,6 @@ def write_multi_entries(entries, **options)
           end
         end
 
-        # Truncate keys that exceed 1kB.
-        def normalize_key(key, options)
-          truncate_key super&.b
-        end
-
-        def truncate_key(key)
-          if key && key.bytesize > max_key_bytesize
-            suffix = ":hash:#{ActiveSupport::Digest.hexdigest(key)}"
-            truncate_at = max_key_bytesize - suffix.bytesize
-            "#{key.byteslice(0, truncate_at)}#{suffix}"
-          else
-            key
-          end
-        end
-
         def deserialize_entry(payload, raw: false, **)
           if raw && !payload.nil?
             Entry.new(payload)

activesupport/test/cache/behaviors/cache_store_behavior.rb

@@ -748,6 +748,20 @@ def test_configuring_store_with_raw
     assert_equal "bar", cache.read("foo")
   end
 
+  def test_max_key_size
+    cache = lookup_store(max_key_size: 64)
+    key = "foobar" * 20
+    cache.write(key, "bar")
+    assert_equal "bar", cache.read(key)
+  end
+
+  def test_max_key_size_disabled
+    cache = lookup_store(max_key_size: false)
+    key = "a" * 1000
+    cache.write(key, "bar")
+    assert_equal "bar", cache.read(key)
+  end
+
   private
     def with_raise_on_invalid_cache_expiration_time(new_value, &block)
       old_value = ActiveSupport::Cache::Store.raise_on_invalid_cache_expiration_time

activesupport/test/cache/stores/memory_store_test.rb

@@ -4,15 +4,13 @@
 require "active_support/cache"
 require_relative "../behaviors"
 
-class MemoryStoreTest < ActiveSupport::TestCase
-  def setup
-    @cache = lookup_store(expires_in: 60)
-  end
-
+class StoreTest < ActiveSupport::TestCase
   def lookup_store(options = {})
     ActiveSupport::Cache.lookup_store(:memory_store, options)
   end
+end
 
+class MemoryStoreTest < StoreTest
   include CacheStoreBehavior
   include CacheStoreVersionBehavior
   include CacheStoreCoderBehavior
@@ -23,6 +21,10 @@ def lookup_store(options = {})
   include CacheInstrumentationBehavior
   include CacheLoggingBehavior
 
+  def setup
+    @cache = lookup_store(expires_in: 60)
+  end
+
   def test_increment_preserves_expiry
     @cache = lookup_store
     @cache.write("counter", 1, raw: true, expires_in: 30.seconds)
@@ -92,7 +94,7 @@ def compression_always_disabled_by_default?
     end
 end
 
-class MemoryStorePruningTest < ActiveSupport::TestCase
+class MemoryStorePruningTest < StoreTest
   def setup
     @record_size = ActiveSupport::Cache.lookup_store(:memory_store).send(:cached_size, 1, ActiveSupport::Cache::Entry.new("aaaaaaaaaa"))
     @cache = ActiveSupport::Cache.lookup_store(:memory_store, expires_in: 60, size: @record_size * 10 + 1)
@@ -158,7 +160,7 @@ def test_prune_size_on_write_based_on_key_length
     assert @cache.exist?(8)
     assert @cache.exist?(7)
     assert @cache.exist?(6)
-    assert_not @cache.exist?(5), "no entry"
+    assert @cache.exist?(5)
     assert_not @cache.exist?(4), "no entry"
     assert_not @cache.exist?(3), "no entry"
     assert_not @cache.exist?(2), "no entry"