Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with reset! method and MemoryStore Cache #672

Open
synth opened this issue Oct 25, 2024 · 5 comments · May be fixed by #673
Open

Issue with reset! method and MemoryStore Cache #672

synth opened this issue Oct 25, 2024 · 5 comments · May be fixed by #673

Comments

@synth
Copy link

synth commented Oct 25, 2024

def reset!
if store.respond_to?(:delete_matched)
store.delete_matched("#{prefix}*")
else

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
Copy link
Collaborator

santib commented Oct 31, 2024

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
Copy link
Author

synth commented Nov 1, 2024

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

@santib
Copy link
Collaborator

santib commented Nov 1, 2024

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
Copy link
Author

synth commented Nov 2, 2024

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

@santib santib linked a pull request Nov 4, 2024 that will close this issue
@santib
Copy link
Collaborator

santib commented Nov 4, 2024

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

Gotcha, thanks for the clarification. I opened #673 to address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants