Skip to content

Fix #79423: copy command is limited to size of file it can copy #5319

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
cmb69 wants to merge 3 commits into from
Closed

Fix #79423: copy command is limited to size of file it can copy #5319

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95e001e
Fix #79423: copy command is limited to size of file it can copy
cmb69 Mar 29, 2020
4ec9b0d
Drop regression test
cmb69 Sep 8, 2020
16a5373
More closely match POSIX semantics of stat()
cmb69 Sep 9, 2020
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
Next Next commit
Fix #79423: copy command is limited to size of file it can copy 
Passing `NULL` as `lpFileSizeHigh` to `GetFileSize()` gives wrong
results for files larger than 0xFFFFFFFF bytes.  We fix this by using
`GetFileSizeEx()`, and proceed with `MIN(file_size, SIZE_MAX)`.
  • Loading branch information
@cmb69
cmb69 committed Sep 3, 2020
commit 95e001e27504e02d13dbcc1096daba640df34bcc
31 changes: 31 additions & 0 deletions ext/standard/tests/streams/bug79423.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Bug #79423 (copy command is limited to size of file it can copy)
--SKIPIF--
<?php
if (getenv('SKIP_SLOW_TESTS')) die('skip slow test');
?>
--FILE--
<?php
$orig = __DIR__ . '/bug79423_orig.txt';
$copy = __DIR__ . '/bug79423_copy.txt';

// create file of 0xFFFFFFFF bytes (SIZE_MAX on 32bit systems)
$text = str_repeat('*', 1024 * 1024);
$stream = fopen($orig, 'w');
for ($i = 0; $i < 4095; $i++) {
fwrite($stream, $text);
}
fwrite($stream, $text, strlen($text) - 1);
fclose($stream);

var_dump(copy($orig, $copy));
var_dump(filesize($orig) === filesize($copy));
?>
--CLEAN--
<?php
unlink(__DIR__ . '/bug79423_orig.txt');
unlink(__DIR__ . '/bug79423_copy.txt');
?>
--EXPECT--
bool(true)
bool(true)
15 changes: 14 additions & 1 deletion main/streams/plain_wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void
php_stream_mmap_range *range = (php_stream_mmap_range*)ptrparam;
HANDLE hfile = (HANDLE)_get_osfhandle(fd);
DWORD prot, acc, loffs = 0, delta = 0;
LARGE_INTEGER file_size;

switch (value) {
case PHP_STREAM_MMAP_SUPPORTED:
Expand Down Expand Up @@ -785,7 +786,19 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void
return PHP_STREAM_OPTION_RETURN_ERR;
}

size = GetFileSize(hfile, NULL);
if (!GetFileSizeEx(hfile, &file_size)) {
return PHP_STREAM_OPTION_RETURN_ERR;
}
# if defined(_WIN64)
size = file_size.QuadPart;
# else
if (file_size.HighPart) {
/* file is too large; process as much as possible */
size = SIZE_MAX;
Copy link
Member

@nikic nikic Mar 30, 2020

Choose a reason for hiding this comment

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

Will this still copy the rest as a separate chunk, or produce a partial copy?

Copy link
Member Author

@cmb69 cmb69 Mar 30, 2020

Choose a reason for hiding this comment

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

That would copy only SIZE_MAX bytes, but copy() returns false in that case.

Copy link
Member Author

@cmb69 cmb69 Sep 8, 2020

Choose a reason for hiding this comment

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

What would happen on a 32bit POSIX system? Would stat() fail for 4GiB files?

Copy link
Member Author

@cmb69 cmb69 Sep 9, 2020

Choose a reason for hiding this comment

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

At least according to POSIX it should fail (likely for >= 2 GiB already), so I matched the implementation; that doesn't make a real differrence in practise, though.

} else {
size = file_size.LowPart;
}
# endif
if (range->offset > size) {
range->offset = size;
}
Expand Down