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:

  1. Action
  2. token time: 50 seconds
  3. wait 5 seconds
  4. Action
  5. token time: 45 seconds
  6. wait 5 seconds
  7. Action
  8. 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:

  1. check
  2. sleep for 3 seconds
  3. check
  4. check (here it should expect false)
  5. 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

Most upvoted comments

@rumpfc You are correct. Changing from int to ?int is not a breaking change. https://3v4l.org/Ss8kl

I 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.