From e60f3ff22c398887295a3c54f5c46e809bb2db85 Mon Sep 17 00:00:00 2001
From: Alex Ghiculescu <alex@tanda.co>
Date: Tue, 22 Jun 2021 10:08:47 -0500
Subject: [PATCH] MemCacheStore: always convert underlying values into an
 `Entry`

There's a bug in the Mem Cache Store on 6.1, I haven't confirmed if it exists on `main` too (since https://github.com/rails/rails/pull/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`. https://github.com/petergoldstein/dalli/issues/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`.
---
 .../active_support/cache/mem_cache_store.rb   |  2 +-
 .../test/cache/stores/mem_cache_store_test.rb | 32 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb
index d783b92819183..bb04a94cbe91a 100644
--- a/activesupport/lib/active_support/cache/mem_cache_store.rb
+++ b/activesupport/lib/active_support/cache/mem_cache_store.rb
@@ -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
 
diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb
index c8967f94bd425..2710d58c247d1 100644
--- a/activesupport/test/cache/stores/mem_cache_store_test.rb
+++ b/activesupport/test/cache/stores/mem_cache_store_test.rb
@@ -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