rack / rack-attack

Rack middleware for blocking & throttling
MIT License
5.58k stars 337 forks source link

Issue with reset! method and MemoryStore Cache #672

Open synth opened 1 month ago

synth commented 1 month ago

https://github.com/rack/rack-attack/blob/0d40ea6538c95fb3c6f70862356d2cb54c02b02e/lib/rack/attack/cache.rb#L56-L59

This code calls delete_matched and passes in a string. However, MemoryStore for Rails.cache requires it to be a Regex.

When calling the reset! method with Rails.cache set to MemoryStore, you will be greeted with this error:

     Failure/Error: # Rack::Attack.reset!

     NoMethodError:
       undefined method `source' for "rack::attack*":String

                   source = pattern.source
santib commented 3 weeks ago

Hi, what version of Rails/ActiveSupport and Rack::Attack are you using? I tried testing it with

      it 'should delete rack attack keys' do
        memory_store = ActiveSupport::Cache::MemoryStore.new
        memory_store.write('key', 'value')
        memory_store.write("#{Rack::Attack.cache.prefix}::key", 'value')
        Rack::Attack.cache.store = memory_store
        Rack::Attack.reset!

        _(memory_store.read('key')).must_equal 'value'
        _(memory_store.read("#{Rack::Attack.cache.prefix}::key")).must_be_nil
      end

but it worked just fine.

Looking https://github.com/rails/rails/blob/main/activesupport/lib/active_support/cache/memory_store.rb#L173 and https://github.com/rails/rails/blob/main/activesupport/lib/active_support/cache.rb#L779 I don't think pattern.source is invoked unless the namespace option is set, which doesn't seem to be our case.

Would you mind providing a reproduction script or more detailed steps to reproduce?

Thanks

synth commented 3 weeks ago

Thanks for looking. Indeed in our case namespace is set! Apologies for leaving out that detail.

santib commented 3 weeks ago

Cool. And how is that possible? Are you monkey-patching Rack::Attack or something? Because I don't see Rack::Attack supporting that option

synth commented 3 weeks ago

Namespace is a Rails cache store argument. The namespace is setting the scope of the cache basically.

santib commented 3 weeks ago

Namespace is a Rails cache store argument. The namespace is setting the scope of the cache basically.

Gotcha, thanks for the clarification. I opened https://github.com/rack/rack-attack/pull/673 to address this issue.