Skip to content
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.

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

Division by zero in src/Server.php line 175 #360

Closed
s-bronstein opened this issue May 29, 2020 · 2 comments · Fixed by #365
Closed

Division by zero in src/Server.php line 175 #360

s-bronstein opened this issue May 29, 2020 · 2 comments · Fixed by #365

Comments

@s-bronstein
Copy link

$concurrentRequests = \ceil($availableMemory / IniUtil::iniSizeToBytes(\ini_get('post_max_size')));

The problem is, in Server.php at line 175 it is assumed that post_max_size will always be a set limit.

However, according to PHP manual:
Allow unlimited post size by setting post_max_size to 0.
https://www.php.net/manual/en/ini.core.php#ini.post-max-size

reactphp/http v0.8.6
PHP 7.1.33

@clue clue added this to the v0.8.7 milestone Jun 5, 2020
@clue
Copy link
Member

clue commented Jun 5, 2020

@smscr Thanks for reporting, I can indeed reproduce this with post_max_size=0. Does anybody feel like filing a PR to address this? 👍

In the meantime, you can work around this by setting post_max_size to a large value, this should have the same effect.

@s-bronstein
Copy link
Author

@clue I've thought about a fix, and I see there is an intention to calculate an amount of request that can be handled concurrently, based on the memory limit and post_max_size.

I'm not sure what amount of request we should estimate to be possible the the post_max_size is unlimited, so that is why I'm not suggesting a PR to address this.

There could be several approaches.

  • To ignore the estimation and return self::MAXIMUM_CONCURRENT_REQUESTS? But what if the concurrent requests will exceed the memory then?
  • To limit to one concurrent request? But wouldn't that decrease the performance?
  • To change the estimation formula to use something other than post_max_size, but what in particular?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants