https://bugzilla.wikimedia.org/show_bug.cgi?id=20595
Summary: Rate limiter blocks never expires when not using
memcached
Product: MediaWiki
Version: 1.16-svn
Platform: All
OS/Version: All
Status: NEW
Severity: major
Priority: Normal
Component: Blocking
AssignedTo: [email protected]
ReportedBy: [email protected]
Setting, for example:
$wgRateLimits['edit']['ip'] = array( 10, 600 );
Anonymous users are blocked after 10 hits, but the block never expires, even
hours and hours later. This only happens when using any memory cache other
than memcached (for example: eAccelerator, Turck MMCache, etc.). Memcached
presents the correct expected behavior.
The problem seems to be in the implementation of incr() in BagOStuff class. The
increment operation is implemented as:
if( ( $n = $this->get( $key ) ) !== false ) {
$n += $value;
$this->set( $key, $n ); // exptime?
}
Note that when the new value is set, no expire time is passed, and this makes
the new replaced key to last forever in the cache. Whoever implemented it
already noticed this case and left that comment in the code (it should have
also noted that it is a bug).
Also, related: In User::pingLimiter(), the limiter is implemented as:
if( $count ) {
/* ...code to check the limit... */
} else {
wfDebug( __METHOD__.": adding record for $key $summary\n" );
$wgMemc->add( $key, 1, intval( $period ) );
}
$wgMemc->incr( $key );
The key is added with the correct expire time, but right after that it is
incremented using incr(), which kills the expire time. In fact, in this code,
if $count==0, the user starts with 2 hits instead of one (another bug).
Probably the incr() call should be inside the 'if' clause.
What would be the correct fix for this bug? I think of two possibilities:
1. Modify BagOStuff to store an aggregate like `array( $value, $exptime )` in
order to be able to have access to the original exptime and properly reset it
when incrementing/decrementing the value;
2. Or deprecate incr() and decr(), consider that they can't be reliably
implemented out of memcached and reformat User::pingLimiter() in order to do
its own expiry control, independent of the cache TTL?
--
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
