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

Bug fix: Ensure integer is passed to read() #2558

Merged
merged 2 commits into from Jan 15, 2019

Conversation

Projects
None yet
3 participants
@farpat
Copy link
Contributor

farpat commented Jan 3, 2019

No description provided.

@akrabat

This comment has been minimized.

Copy link
Member

akrabat commented Jan 3, 2019

Can you explain the problem you are having and why this fixes it?

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

Coverage remained the same at 97.819% when pulling 1bd7ef2 on farpat:patch-1 into a5a2da9 on slimphp:3.x.

@farpat

This comment has been minimized.

Copy link
Contributor Author

farpat commented Jan 3, 2019

Firstly sorry for my English, I'm French.

You use Zend\Diactoros\Stream@read where you send the parameter $length being a string (even if this one contains a number like "138") and when fread is called - a few lines later - a fatal error is triggered.

This is why I purpose this fix.

@akrabat

This comment has been minimized.

Copy link
Member

akrabat commented Jan 3, 2019

Which read() line causes the fatal error?

@farpat

This comment has been minimized.

Copy link
Contributor Author

farpat commented Jan 3, 2019

The fatal error is provoked in line ~277 of file " vendor/zendframework/zend-diactoros/src/Stream.php " => $result = fread($this->resource, $length);

$length must be an int not a string.

@akrabat

This comment has been minimized.

Copy link
Member

akrabat commented Jan 3, 2019

https://github.com/slimphp/Slim/blob/3.x/Slim/App.php#L277 is a comment just above the definition of group() ?

@farpat

This comment has been minimized.

Copy link
Contributor Author

farpat commented Jan 3, 2019

Sorry, I fix the the filename in my previous comment => https://github.com/zendframework/zend-diactoros/blob/master/src/Stream.php#L277

@akrabat

This comment has been minimized.

Copy link
Member

akrabat commented Jan 3, 2019

Ah!

Which read() call in Slim\App is causing the problem as there are two.

@farpat

This comment has been minimized.

Copy link
Contributor Author

farpat commented Jan 3, 2019

Sorry, I did not investigate enought to say it but I can search this if you want.

@akrabat

This comment has been minimized.

Copy link
Member

akrabat commented Jan 3, 2019

My main concern is that

$data = $body->read(min((int)$chunkSize, (int)$amountToRead));

already casts to int. However:

echo $body->read($chunkSize);

Could possibly be an int. Your fix doesn't change this variable, so maybe

$chunkSize      = $settings['responseChunkSize'];

needs the cast?

@farpat

This comment has been minimized.

Copy link
Contributor Author

farpat commented Jan 3, 2019

This concern surely the line $chunkSize = $settings['responseChunkSize']; as you talk.

Also, you'are right => My previous commit does not fix this but my second commit must be fix the issue normally.

Thanks for your patience.

@akrabat akrabat changed the title fix a fatal error on an other package Bug fix: Ensure integer is passed to read() Jan 3, 2019

@akrabat

akrabat approved these changes Jan 3, 2019

@akrabat akrabat added the Slim 3 label Jan 15, 2019

@akrabat akrabat added this to the 3.12 milestone Jan 15, 2019

@akrabat akrabat merged commit 1bd7ef2 into slimphp:3.x Jan 15, 2019

3 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 97.819%
Details

akrabat added a commit that referenced this pull request Jan 15, 2019

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