Skip to content

Commit

Permalink
MemCacheStore: always convert underlying values into an Entry
Browse files Browse the repository at this point in the history
There's a bug in the Mem Cache Store on 6.1, I haven't confirmed if it exists on `main` too (since #42025).

We came across the bug when upgrading an app from 6.0 to 6.1 that had recently switched from the `dalli_store` to the `mem_cache_store`. petergoldstein/dalli#771 seems similar, but in our case it's specifically to do with caching the value `false`.

To replicate:

- Open a Rails console with the `dalli_store` enabled.
- `Rails.cache.write "test", false`
- Open another Rails console, with the `mem_cache_store` enabled.
- Enable the local cache (starting a Rails server would also achieve this): `ActiveSupport::Cache::Strategy::LocalCache::LocalCacheRegistry.set_cache_for(Rails.cache.send(:local_cache_key), ActiveSupport::Cache::Strategy::LocalCache::LocalStore.new)`
- ` Rails.cache.fetch("test") { false }`

It will crash with `NoMethodError (undefined method `dup_value!' for false:FalseClass)`.

The fix is to convert any entry into an `Entry`, even if it's `nil` or `false`.
  • Loading branch information
ghiculescu committed Jun 22, 2021
1 parent e3e3a97 commit e60f3ff
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/cache/mem_cache_store.rb
Expand Up @@ -198,7 +198,7 @@ def normalize_key(key, options)

def deserialize_entry(payload)
entry = super
entry = Entry.new(entry, compress: false) if entry && !entry.is_a?(Entry)
entry = Entry.new(entry, compress: false) unless entry.nil? || entry.is_a?(Entry)
entry
end

Expand Down
32 changes: 32 additions & 0 deletions activesupport/test/cache/stores/mem_cache_store_test.rb
Expand Up @@ -211,6 +211,38 @@ def test_large_object_with_default_compression_settings
assert_compressed(LARGE_OBJECT)
end

def test_can_read_and_write_falsy_value
key = "test-with-false-value-through-public-api"

@cache.write(key, false)
assert_equal false, @cache.read(key)
end

def test_can_load_raw_values_from_dalli_store
key = "test-with-false-value-the-way-the-dalli-store-did"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), false, 0, compress: false) }
assert_equal false, @cache.read(key)
end

def test_can_read_and_write_falsy_value_with_local_cache
key = "test-with-false-value-through-public-api"

@cache.write(key, false)
@cache.with_local_cache do
assert_equal false, @cache.read(key)
end
end

def test_can_load_raw_values_from_dalli_store_with_local_cache
key = "test-with-false-value-the-way-the-dalli-store-did"

@cache.instance_variable_get(:@data).with { |c| c.set(@cache.send(:normalize_key, key, nil), false, 0, compress: false) }
@cache.with_local_cache do
assert_equal false, @cache.read(key)
end
end

private
def random_string(length)
(0...length).map { (65 + rand(26)).chr }.join
Expand Down

0 comments on commit e60f3ff

Please sign in to comment.