Skip to content

Fix big file download for 32-bit servers #33197

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

Closed
fwsmit wants to merge 1 commit into from
Closed

Fix big file download for 32-bit servers #33197

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions lib/private/Streamer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ class Streamer {
* Streamer constructor.
*
* @param IRequest $request
* @param int $size The size of the files in bytes
* @param $size The size of the files in bytes. The type hint is left
* out. Size may be a float if the size is more than PHP_INT_MAX
* (notably on 32-bit systems, see #12422). Note that when size
* is converted to a float, it might not be accurate to the byte.
* @param int $numberOfFiles The number of files (and directories) that will
* be included in the streamed file
*/
public function __construct(IRequest $request, int $size, int $numberOfFiles) {
public function __construct(IRequest $request, $size, int $numberOfFiles) {
Copy link
Contributor

@szaimen szaimen Jul 13, 2022

Choose a reason for hiding this comment

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

this is not acceptable

Copy link
Contributor

@szaimen szaimen Jul 13, 2022

Choose a reason for hiding this comment

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

See #16636

Copy link
Author

@fwsmit fwsmit Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I've read that issue. When size is bigger than a 32 bit int can handle, the size overflows to a float. This will not be as accurate, but there is an error margin.
As for 64 bit systems you are only losing the safety that the type hint provides. It's that unacceptable? If so, is there another way of fixing this that would be accepted? For example, checking if 32 bit and calling another function all together?

EDIT: I could also overload the streamer class to provide a 32 bit variant


/**
* zip32 constraints for a basic (without compression, volumes nor
Expand All @@ -72,21 +75,29 @@ public function __construct(IRequest $request, int $size, int $numberOfFiles) {
*
* Due to all that, zip32 is used if the size is below 4GB and there are
* less than 65536 files; the margin between 4*1000^3 and 4*1024^3
* should give enough room for the extra zip metadata. Technically, it
* would still be possible to create an invalid zip32 file (for example,
* a zip file from files smaller than 4GB with a central directory
* larger than 4GiB), but it should not happen in the real world.
* should give enough room for the extra zip metadata and
* tolerance for rounding when size is a float. Technically, it
* would still be possible to create an invalid zip32 file (for
* example, a zip file from files smaller than 4GB with a
* central directory larger than 4GiB), but it should not
* happen in the real world.
*
* We also have to check for a size above 0. As negative sizes could be
* from not fully scanned external storage. And then things fall apart
* if somebody tries to package to much.
*/
if ($size > 0 && $size < 4 * 1000 * 1000 * 1000 && $numberOfFiles < 65536) {
$this->streamerInstance = new ZipStreamer(['zip64' => false]);
} elseif ($request->isUserAgent($this->preferTarFor)) {
} elseif ($request->isUserAgent($this->preferTarFor) || PHP_INT_SIZE < 8) {
/**
* On 32-bit servers generating a 64-bit zip will
* result in broken zip files. Some clients might also
* have bugs in their zip64 decompression. For these
* cases, generate a tar file.
*/
$this->streamerInstance = new TarStreamer();
} else {
$this->streamerInstance = new ZipStreamer(['zip64' => PHP_INT_SIZE !== 4]);
$this->streamerInstance = new ZipStreamer(['zip64' => true]);
}
}

Expand Down