CodeIgniter4: Bug: Throttler does not show correct token time until 1 token is available
PHP Version
8.0
CodeIgniter4 Version
4.1.5
CodeIgniter4 Installation Method
Composer (using codeigniter4/appstarter
)
Which operating systems have you tested for this bug?
Windows
Which server did you use?
cli-server (PHP built-in webserver)
Database
No response
What happened?
After some experiments with Throttler where I execute an action and get tokenTime returned after each action, I tackled some unexpected behaviour where I executed a successful action too early.
Example:
- Action
- token time: 50 seconds
- wait 5 seconds
- Action
- token time: 45 seconds
- wait 5 seconds
- Action
- Successful (even though there should be 40 seconds)
After some more experiments I realized that tokenTime is calculated using timestamp of last successful check instead of calculating when 1 token is available again.
Steps to Reproduce
Here is a unit test which uses a capacity of 2 and a refill timer of 200. This means
- 1 token every 100 seconds
- 0.01 tokens per second
It measures the token time after doing following steps:
- check
- sleep for 3 seconds
- check
- check (here it should expect false)
- Expect token time 100 - 3 seconds = 97 seconds
<?php
use CodeIgniter\Test\CIUnitTestCase;
use Config\Services;
class ThrottlerTest extends CIUnitTestCase
{
public function testThrottlerTokenTime()
{
$throttler = Services::throttler();
$seconds = 200;
$capacity = 2;
// refresh = 200 / 2 = 100 seconds
// refresh rate = 2 / 200 = 0.01 token per second
// token should be 2
$this->assertTrue($throttler->check('test', $capacity, $seconds));
// token should be 2 - 1 = 1
// do nothing for 3 seconds
sleep(3);
// token should be 1 + 3 * 0.01 = 1.03
$this->assertTrue($throttler->check('test', $capacity, $seconds));
// token should be 1.03 - 1 = 0.03
$this->assertFalse($throttler->check('test', $capacity, $seconds));
// token should still be 0.03 because check failed
// expect remaining time: (1 - 0.03) * 100 = 97
$this->assertEquals(97, $throttler->getTokenTime(), 'Wrong token time');
}
}
?>
Here is the test output
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.
Warning: XDEBUG_MODE=coverage or xdebug.mode=coverage has to be set
F 1 / 1 (100%)
Time: 00:03.125, Memory: 10.00 MB
There was 1 failure:
1) ThrottlerTest::testThrottlerTokenTime
Wrong token time
Failed asserting that 100 matches expected 97.
C:\Users\chris\Documents\projects\php\spotify-web-party\tests\unit\ThrottlerTest.php:31
FAILURES!
Tests: 1, Assertions: 4, Failures: 1.
Expected Output
Throttler::tokenTime
should return the remaining time until 1 token is available in case check failed.
Anything else?
Looking at your source code, I saw that you don’t use consistent time. Sometimes you use time()
(php function) and sometimes you use Throttler::time()
(which uses either test time or time()
from php). I tried above tests using $throttler-setTestTime(...)
, but got assertion errors not at tokenTime
, but at the 2nd check
.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 21 (21 by maintainers)
Commits related to this issue
- fix: does not show correct token time until 1 token is available Fixes #5458 — committed to kenjis/CodeIgniter4 by kenjis 3 years ago
- fix: does not show correct token time until 1 token is available Fixes #5458 — committed to kenjis/CodeIgniter4 by kenjis 3 years ago
- fix: does not show correct token time until 1 token is available Fixes #5458 — committed to kenjis/CodeIgniter4 by kenjis 3 years ago
@rumpfc You are correct. Changing from
int
to?int
is not a breaking change. https://3v4l.org/Ss8klI think 0 is enough for now. 0 means you should not have got it, or it is not calculated.
@rumpfc Thank you for quoting. According the CI manual, tokenTime can not be used (must be 0?) when
check()
returns true.