Support aliases to expires_in for cache stores

The `expires_in` option is easy to misremember or mistype as `expire_in`
or `expired_in`, with potentially harmful results. If a developer wants
to cache a value for only 2 seconds but mistakenly types
`expire_in: 2.seconds`, that value will instead be cached for the
default time, likely 6 hours, meaning that users of the site will see
the same data for much longer than they should, and the only recovery
(short of waiting for the 6 hours to elapse) is to manually expire all
relevant keys. This commit allows cache stores to recognize these common
typos as aliases by normalizing them before consuming the options.

In general, we should be careful about adding too many aliases for
options to the cache stores, since each option key used by the base
Cache::Store class is one fewer key that each cache implementation can
customize for itself. This case was approved because of the similarity
of the aliases to the root key and the potential damage caused by
mistaking them.

Fixes #39850.
This commit is contained in:
Nate Pinsky 2020-12-28 14:13:46 -08:00
parent 96446a9708
commit fd8c36707f
2 changed files with 56 additions and 3 deletions

View File

@ -22,7 +22,12 @@ module ActiveSupport
# These options mean something to all cache implementations. Individual cache
# implementations may support additional options.
UNIVERSAL_OPTIONS = [:namespace, :compress, :compress_threshold, :expires_in, :race_condition_ttl, :coder, :skip_nil]
UNIVERSAL_OPTIONS = [:namespace, :compress, :compress_threshold, :expires_in, :expire_in, :expired_in, :race_condition_ttl, :coder, :skip_nil]
# Mapping of canonical option names to aliases that a store will recognize.
OPTION_ALIASES = {
expires_in: [:expire_in, :expired_in]
}.freeze
module Strategy
autoload :LocalCache, "active_support/cache/strategy/local_cache"
@ -186,7 +191,7 @@ module ActiveSupport
# except for <tt>:namespace</tt> which can be used to set the global
# namespace for the cache.
def initialize(options = nil)
@options = options ? options.dup : {}
@options = options ? normalize_options(options) : {}
@coder = @options.delete(:coder) { self.class::DEFAULT_CODER } || NullCoder
end
@ -249,7 +254,9 @@ module ActiveSupport
# All caches support auto-expiring content after a specified number of
# seconds. This value can be specified as an option to the constructor
# (in which case all entries will be affected), or it can be supplied to
# the +fetch+ or +write+ method to effect just one entry.
# the +fetch+ or +write+ method to affect just one entry.
# <tt>:expire_in</tt> and <tt>:expired_in</tt> are aliases for
# <tt>:expires_in</tt>.
#
# cache = ActiveSupport::Cache::MemoryStore.new(expires_in: 5.minutes)
# cache.write(key, value, expires_in: 1.minute) # Set a lower value for one entry
@ -634,6 +641,7 @@ module ActiveSupport
# Merges the default options with ones specific to a method call.
def merged_options(call_options)
if call_options
call_options = normalize_options(call_options)
if options.empty?
call_options
else
@ -644,6 +652,17 @@ module ActiveSupport
end
end
# Normalize aliased options to their canonical form
def normalize_options(options)
options = options.dup
OPTION_ALIASES.each do |canonical_name, aliases|
alias_key = aliases.detect { |key| options.key?(key) }
options[canonical_name] ||= options[alias_key] if alias_key
options.except!(*aliases)
end
options
end
# Expands and namespaces the cache key. May be overridden by
# cache stores to do additional normalization.
def normalize_key(key, options = nil)

View File

@ -406,6 +406,40 @@ module CacheStoreBehavior
end
end
def test_expire_in_is_alias_for_expires_in
time = Time.local(2008, 4, 24)
Time.stub(:now, time) do
@cache.write("foo", "bar", expire_in: 20)
assert_equal "bar", @cache.read("foo")
end
Time.stub(:now, time + 10) do
assert_equal "bar", @cache.read("foo")
end
Time.stub(:now, time + 21) do
assert_nil @cache.read("foo")
end
end
def test_expired_in_is_alias_for_expires_in
time = Time.local(2008, 4, 24)
Time.stub(:now, time) do
@cache.write("foo", "bar", expired_in: 20)
assert_equal "bar", @cache.read("foo")
end
Time.stub(:now, time + 10) do
assert_equal "bar", @cache.read("foo")
end
Time.stub(:now, time + 21) do
assert_nil @cache.read("foo")
end
end
def test_race_condition_protection_skipped_if_not_defined
@cache.write("foo", "bar")
time = @cache.send(:read_entry, @cache.send(:normalize_key, "foo", {}), **{}).expires_at