Permalink
Browse files

Fixes bug where Memcached connections get corrupted when an invalid e…

…xpire is passed in [#2854 state:resolved]
  • Loading branch information...
1 parent 92bff2e commit ee7d4c47e7abf1e5ec8f4d4a1e937cdee5bb8d9c Yehuda Katz + Carl Lerche committed Jul 1, 2009
Showing with 30 additions and 2 deletions.
  1. +11 −2 activesupport/lib/active_support/cache.rb
  2. +19 −0 activesupport/test/caching_test.rb
@@ -99,11 +99,16 @@ def self.expand_cache_key(key, namespace = nil)
class Store
cattr_accessor :logger
+ attr_reader :silence, :logger_off
+
def silence!
@silence = true
self
end
+ alias silence? silence
+ alias logger_off? logger_off
+
# Fetches data from the cache, using the given key. If there is data in
# the cache with the given key, then that data is returned.
#
@@ -233,11 +238,15 @@ def decrement(key, amount = 1)
private
def expires_in(options)
- (options && options[:expires_in]) || 0
+ expires_in = options && options[:expires_in]
+
+ raise ":expires_in must be a number" if expires_in && !expires_in.is_a?(Numeric)
+
+ expires_in || 0
end
def log(operation, key, options)
- logger.debug("Cache #{operation}: #{key}#{options ? " (#{options.inspect})" : ""}") if logger && !@silence && !@logger_off
+ logger.debug("Cache #{operation}: #{key}#{options ? " (#{options.inspect})" : ""}") if logger && !silence? && !logger_off?
end
end
end
@@ -1,3 +1,4 @@
+require 'logger'
require 'abstract_unit'
require 'active_support/cache'
@@ -190,6 +191,8 @@ def setup
@cache = ActiveSupport::Cache.lookup_store(:mem_cache_store)
@data = @cache.instance_variable_get(:@data)
@cache.clear
+ @cache.silence!
+ @cache.logger = Logger.new("/dev/null")
end
include CacheStoreBehavior
@@ -312,6 +315,22 @@ def test_middleware
app = @cache.middleware.new(app)
app.call({})
end
+
+ def test_expires_in
+ result = @cache.write('foo', 'bar', :expires_in => 1)
+ assert_equal 'bar', @cache.read('foo')
+ sleep 2
+ assert_equal nil, @cache.read('foo')
+ end
+
+ def test_expires_in_with_invalid_value
+ @cache.write('baz', 'bat')
+ assert_raise(RuntimeError) do
+ @cache.write('foo', 'bar', :expires_in => 'Mon Jun 29 13:10:40 -0700 2150')
+ end
+ assert_equal 'bat', @cache.read('baz')
+ assert_equal nil, @cache.read('foo')
+ end
end
class CompressedMemCacheStore < ActiveSupport::TestCase

4 comments on commit ee7d4c4

Contributor

alloy commented on ee7d4c4 Jul 2, 2009

There are also the Kernel#Float and Kernel#Integer methods which might help in drying up the expires in code. Granted, the message will be slightly less informative (no mention of :expires_in), but it will gain any Ruby duck-typing possibilities. Eg: #to_int, #to_i.

Contributor

alloy replied Jul 2, 2009

Something like:

def expires_in(options)
(expires_in = options && options[:expires_in]) ? Integer(expires_in) : 0
end

Contributor

alloy replied Jul 2, 2009

Ugh, formatting fail… Anyways :)

Contributor

carllerche replied Jul 3, 2009

If you can, could you submit a patch to lighthouse? It would be much appreciated.

Please sign in to comment.