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

Update Sapi.php #61

Closed
wants to merge 1 commit into from
Closed

Update Sapi.php #61

wants to merge 1 commit into from

Conversation

chakphanu
Copy link

ensure $contentLength is integer value.

@evert
Copy link
Member

evert commented Apr 4, 2016

Our of curiosity.. what problem does this solve?

@chakphanu
Copy link
Author

from: owncloud/core#23788
have error log: "stream_copy_to_stream() expects parameter 3 to be integer, string given at /var/www/owncloud/3rdparty/sabre/http/lib/Sapi.php#78"

@evert
Copy link
Member

evert commented Apr 4, 2016

If the Sapi receives a string with a number in it, this will work just fine. PHP will cast it. The only case that error is triggered is when the received value is not a number at all, in which case intval will likely turn it to 0, which I think is undesirable.

What should happen is :

  1. sabre/http should throw a better error when the value is non-numeric.
  2. Find the root-cause of the issue. Why is it getting a non-numeric string?

@staabm
Copy link
Member

staabm commented Apr 4, 2016

@evert I guess php7 is used with strict types in the above warning/error case

@evert
Copy link
Member

evert commented Apr 4, 2016

@staabm , not possible. Strict types are set by the caller and strict is off per-file by default. Only if Sapi.php was explicitly set to strict, this could have resulted in that error.

@evert
Copy link
Member

evert commented Oct 13, 2016

It was never clear what the root cause was for this issue, however.. I've now made a change that fixes this as a side-effect of using PHP 7 strict_types everywhere.

@evert evert closed this Oct 13, 2016
@rikmeijer
Copy link

rikmeijer commented Apr 5, 2017

In nextcloud this problem arises when downloading files > 2 GB on 32 bit machines. In my comment I showed a possible fix, but it involves changing sabre/http

@derkostka
Copy link
Contributor

derkostka commented Apr 5, 2017

@evert please consider taking this change over. This effectively enables owncloud and nextcloud users to be able to transfer large files even on 32 Bit machines. Thanks @rikmeijer 👍

Patch is here: nextcloud/server#1707 (comment)

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.

5 participants