Update RateLimiter.cfc, fix for CONTENTBOX-1512 #608
Conversation
| // the limiter data | ||
| variables.limitData = {}; | ||
| } | ||
| property name = "cachebox" inject = "Cachebox"; |
There was a problem hiding this comment.
use the cfformater please, so it uses the formatting rules.
|
|
||
| lock name="cb-ratelimiter-#hash( realIP )#" type="readonly" throwontimeout="true" timeout="5" { | ||
| var targetData = variables.limitData[ realIP ]; | ||
| var targetData = cache.get( cacheKey ); |
There was a problem hiding this comment.
best to use the getOrSet() for these type of situations.https://s3.amazonaws.com/apidocs.ortussolutions.com/cachebox/7.4.0/cachebox/system/cache/AbstractCacheBoxProvider.html#getOrSet()
There was a problem hiding this comment.
Like this? But how to handle, if cache key doesn't exist return default value (no further execution!
var targetData = cache.getOrSet( cacheKey, function(){
return { attempts = 0, lastAttempt = now() }
} );
There was a problem hiding this comment.
What this does is pretty much what you did manually. If the cacheKey exists, it returns the value of the cache. If it doesn't, it will seed it with the return value of the supplier function.
There was a problem hiding this comment.
yes, got it, but in case value is created I don't want to execute further code. Original code did this:
if( isNull( targetData ) ){ cache.set( cacheKey, { attempts = 1, lastAttempt = now() }); return this; }
There was a problem hiding this comment.
Ahh ok. Well, you would say
if( targetData.attempts == 1 ){
return this;
The usage of getOrSet() is preferred as it does internal locking
There was a problem hiding this comment.
I get the locking mechanism, to make it working I have to write something like this.
`var targetData = cache.getOrSet( cacheKey, function(){
return { attempts = 0, lastAttempt = now() }
} );
// on first visit no further processing
if( targetData.attempts == 0 ){
targetData.attempts++;
cache.set( cacheKey, targetData );
return this;
}`
|
Small changes and it's ready to roll! |
use cache.getOrSet, a little bit of formatting
Description
RateLimiter stores request data in variables scope. Entries are never purged, if site gets many request, can lead to memory/performance issues
This fix is not tested (mentioned to Luis), since we are on older version
Jira Issues
CONTENTBOX-1512
Type of change
Please delete options that are not relevant.
Checklist