From 9d90610bbe6368beb3aaedf2b657e37c5d689631 Mon Sep 17 00:00:00 2001 From: Aaron Suggs Date: Tue, 16 Feb 2016 16:59:24 -0500 Subject: [PATCH 1/2] Refactor StoreProxy to avoid autoloading MemCacheStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In v4.4.0, checking `defined?(ActiveSupport::Cache::MemCacheStore)` could trigger an error loading dalli, which isn’t needed. This fixes that bug, and prevents similar bugs by checking `store.class.to_s` rather than `defined?(klass) && store.is_a?(klass)`. Writing an automated test to ensure that dalli is truly optional is difficult, but I was able to recreate the dalli load error in v4.4.0 by running: gem uninstall dalli ruby -Ilib -ractive_support/all -ractive_support/cache/redis_store -rrack/attack -e 'p Rack::Attack::StoreProxy.build(Redis::Store.new)' Fixes #163 --- lib/rack/attack/store_proxy.rb | 36 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/rack/attack/store_proxy.rb b/lib/rack/attack/store_proxy.rb index d88ad674..4d698538 100644 --- a/lib/rack/attack/store_proxy.rb +++ b/lib/rack/attack/store_proxy.rb @@ -3,26 +3,30 @@ class Attack module StoreProxy PROXIES = [DalliProxy, MemCacheProxy, RedisStoreProxy] + ACTIVE_SUPPORT_WRAPPER_CLASSES = Set.new(['ActiveSupport::Cache::MemCacheStore', 'ActiveSupport::Cache::RedisStore']).freeze + ACTIVE_SUPPORT_CLIENTS = Set.new(['Redis::Store', 'Dalli::Client', 'MemCache']).freeze + def self.build(store) - # RedisStore#increment needs different behavior, so detect that - # (method has an arity of 2; must call #expire separately - if (defined?(::ActiveSupport::Cache::RedisStore) && store.is_a?(::ActiveSupport::Cache::RedisStore)) || - (defined?(::ActiveSupport::Cache::MemCacheStore) && store.is_a?(::ActiveSupport::Cache::MemCacheStore)) + client = unwrap_active_support_stores(store) + klass = PROXIES.find { |proxy| proxy.handle?(client) } + klass ? klass.new(client) : client + end + - # ActiveSupport::Cache::RedisStore doesn't expose any way to set an expiry, - # so use the raw Redis::Store instead. - # We also want to use the underlying Dalli client instead of ::ActiveSupport::Cache::MemCacheStore, - # and the MemCache client if using Rails 3.x - client = store.instance_variable_get(:@data) - if (defined?(::Redis::Store) && client.is_a?(Redis::Store)) || - (defined?(Dalli::Client) && client.is_a?(Dalli::Client)) || (defined?(MemCache) && client.is_a?(MemCache)) - store = store.instance_variable_get(:@data) - end + private + def self.unwrap_active_support_stores(store) + # ActiveSupport::Cache::RedisStore doesn't expose any way to set an expiry, + # so use the raw Redis::Store instead. + # We also want to use the underlying Dalli client instead of ::ActiveSupport::Cache::MemCacheStore, + # and the MemCache client if using Rails 3.x + + client = store.instance_variable_get(:@data) + if ACTIVE_SUPPORT_WRAPPER_CLASSES.include?(store.class.to_s) && ACTIVE_SUPPORT_CLIENTS.include?(client.class.to_s) + client + else + store end - klass = PROXIES.find { |proxy| proxy.handle?(store) } - klass ? klass.new(store) : store end - end end end From cf89457cedf682f12cb6e3ce5d4c56d3fa1f4998 Mon Sep 17 00:00:00 2001 From: Aaron Suggs Date: Tue, 16 Feb 2016 17:11:11 -0500 Subject: [PATCH 2/2] bump v4.4.1 --- CHANGELOG.md | 3 +++ lib/rack/attack/version.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 679bc00b..158546cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## master (unreleased) + - Fix an error affecting apps using Redis::Store saying that dalli was a + required dependency. Learn about ActiveSupport autoloading. (#165) + ## v4.4.0 - 10 Feb 2016 - New: support for MemCacheStore (#153). Thanks @elhu. diff --git a/lib/rack/attack/version.rb b/lib/rack/attack/version.rb index 6d44e99a..2c31f649 100644 --- a/lib/rack/attack/version.rb +++ b/lib/rack/attack/version.rb @@ -1,5 +1,5 @@ module Rack class Attack - VERSION = '4.4.0' + VERSION = '4.4.1' end end