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

Limit maximum amount of bytes to read, fixes #216 #217

Merged
merged 2 commits into from
Feb 20, 2020
Merged

Conversation

mikey179
Copy link
Member

@mikey179 mikey179 commented Feb 19, 2020

The result of rand() can be numbers of up to 2147483647 (PHP 7.3.14 on macOS). Before the actual file read PHP allocates the amount of memory specified by the second argument to fread() to ensure everything fits into memory. For unit tests that's bad, as it can mean allocating 2 GB of memory.

Therefore this fix limits the amount to 10000. For typical vfsStream usages that's more than sufficient. The usage of rand() is kept to verify that the functionality tested works independent of the amount of bytes to be read from the file.

The result of rand() can be numbers of up to 2147483647 (PHP 7.3.14 on macOS). Before the actual file read PHP allocates the amount of memory specified by the second argument to fread() to ensure everything fits into memory. For unit tests that's bad, as it can mean of allocating 2 GB of memory.

Therefore this fix limits the amount to 10000. For typical vfsStream usages that's more than sufficient. The usage of rand() is kept to verify that the functionality tested works independent of the amount of bytes to be read from the file.
@mikey179 mikey179 changed the title limit maximum amount of bytes to read, fixes #216 Limit maximum amount of bytes to read, fixes #216 Feb 19, 2020
@mikey179 mikey179 requested a review from a team February 19, 2020 16:30
Copy link
Contributor

@bizurkur bizurkur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice catch

@bizurkur bizurkur merged commit 890e754 into master Feb 20, 2020
@mikey179 mikey179 deleted the bugfix/issue_216 branch February 20, 2020 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants