8000 ExtUvLoop silently allows timers which cause integer overflow · Issue #194 · reactphp/event-loop · GitHub
[go: up one dir, main page]
Skip to content

ExtUvLoop silently allows timers which cause integer overflow #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ghost opened this issue May 3, 2019 · 3 comments · Fixed by #196
Closed

ExtUvLoop silently allows timers which cause integer overflow #194

ghost opened this issue May 3, 2019 · 3 comments · Fixed by #196
Assignees
@WyriHaximus
Labels
bug easy pick help wanted

Comments

@ghost
Copy link
ghost commented May 3, 2019

ExtUvLoop allows timers with an time or interval value that cause the internal conversion to cause an integer overflow to 0.

event-loop/src/ExtUvLoop.php

Lines 128 to 133 in a0ecac9

\uv_timer_start(
$event,
(int) ($interval * 1000) + 1,
0,
$callback
);

Using \PHP_INT_MAX silently causes an integer overflow to 0, and results in an intermediate firing of the timer callback.

The loop implementation should instead throw an exception if the conversion could cause an integer overflow.
@WyriHaximus WyriHaximus self-assigned this May 3, 2019
@clue
Copy link
Member
clue commented May 12, 2019

@CharlotteDunois Thanks for spotting and reporting, this is interesting!

On 32 bit systems this means that the maximum time interval is ~25 days (2^31 milliseconds). On 64 bit systems this is ~300 million years (2^63 milliseconds), so this is unlikely to be an issue for most common use cases.

Is this something anybody would want to file as a PR with a simple test case?

@PabloKowalczyk
Copy link
Contributor

@clue could I work on this issue?

@WyriHaximus
Copy link
Member

@PabloKowalczyk go for it, AFAIK neither @clue or me are working on this at the moment. (It is on my/our list but not at the top.)

@PabloKowalczyk PabloKowalczyk mentioned this issue May 20, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@WyriHaximus WyriHaximus

Labels
bug easy pick help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Prevent "interval" overflow in ExtUvLoop PabloKowalczyk/event-loop
3 participants
@WyriHaximus @clue @PabloKowalczyk
0