If connection pooling is used with AS::Cache::MemCacheStore,
unwrap_active_support_stores wouldn't return the underlying dalli instance(s),
and so Rack::Attack.store would be the bare unproxied MemCacheStore instance.
Calling write then increment would silently fail because :raw wasn't used.
With this commit, we no longer try to unwrap AS::Cache::MemCacheStore instances.
* attempt to improve semantics for legibility
* Attempt to improve legibility by simplifying
* Make it more clear that we're calling procs/blocks here
* Enable rubocop Style/BlockDelimiters cop
* Prefer 'request' over 'req' abbreviation for legibility/clarity
* Instances of Track named 'track' not 'tracker'
Rack::Attack::PathNormalizer and Rack::Attack::Request are both
used in #call method, which is going to be used by every rack-attack
user as long as they insert the middleware in their app.
While a cache-store proxy exists for the redis-store gem, no such proxy
existed for using the redis gem itself. This fills that gap by adding
such a proxy.
Resolveskickstarter/rack-attack#190
The issue
---
When using rack-attack with a rails app, developers expect the request
path to be normalized. In particular, trailing slashes are stripped so
a request path "/login/" becomes "/login" by the time you're in
ActionController.
Since Rack::Attack runs before ActionDispatch, the request path is not
yet normalized. This can cause throttles and blacklists to not work as
expected.
E.g., a throttle:
throttle('logins', ...) {|req| req.path == "/login" }
would not match a request to '/login/', though Rails would route
'/login/' to the same '/login' action.
The solution
---
This patch looks if ActionDispatch's request normalization is loaded,
and if so, uses it to normalize the path before processing throttles,
blacklists, etc.
If it's not loaded, the request path is not modified.
Credit
---
Thanks to Andres Riancho at Include Security for reporting this issue.
I have a response which is a class. While I can still have my class
implement `#[]`, it does look a bit off. On the other side, having
objects, responding to #call, that are not procs is pretty common.
So I propose to invoke the responses with `#call` to let users override
it with response objects, that respond to `#call` instead of `#[]`.
It has a couple of cons:
1. If we slip a typo in the whole line, we won't easily catch it. Can
you guys spot the problem problem in the following line? Chasing such
issues is quite tricky.
```ruby
retry_after = evn['rack.attack.match_data'][:period] rescue nil
```
2. Throwing and catching an exception is quite slower than a new hash
allocation, so there is a speed benefit too.
We are guaranteed from Rack that env is a `Hash`, so we can even use
`Hash#fetch`.
```ruby
retry_after = env.fetch('rack.attack.match_data', {})[:period]
```
This reads better, but always allocates the default value hash, when the
other version allocates it only when needed. If you prefer `Hash#fetch`,
I'm fine with that, as long as we avoid `rescue nil`.
