#46581 closed defect (bug) (fixed)
$skip_list in copy_dir function is not working as it should
Reported by: | codex-m | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | major | Version: | 3.7 |
Component: | Filesystem API | Keywords: | has-patch needs-unit-tests |
Focuses: | Cc: |
Description
Background: $skip_list
which is the third parameter of copy_dir()
is used to skip some folders and files from being copied with copy_dir
. The purpose is to reduce the total size of the copied folder.
I found this bug while attempting to write a code that uses copy_dir()
with $skip_list` set.
Documentation: https://developer.wordpress.org/reference/functions/copy_dir/
Issue:
Supposing I have the following $skip_list
in copy_dir()
:
$skip_list = ['2019/03/large-folder-test'];
When I call copy_dir()
with this exclusion list. The entire /2019/
is excluded instead of the targeted 2019/03/large-folder-test
.
I debugged and its because in the copy_dir()
, its not using strict type check in the in_array
call. Thus when the filename is 2009
, it returns true
with a $skip_list
of:
$skip_list = ['2019/03/large-folder-test'];
The debug screenshot says it all (attached on this ticket.)
This issue can easily be reproduced by putting large directory to exclude deep within the uploads directory just like the screenshot. Then write a custom code with $skip_list
set.
Fix:
The fix is simply to add true
to the third parameter of in_array
. This will ensure it will do an exact match check. I tested this and it now only excludes the folder in the skip list array.
I put the severity of this bug to major because if someone uses the third parameter of copy_dir()
, it will cause it to skip some crucial folders that needs to be copied.
Please review. Thank you.
Some of my testing environment just for information:
PHP version: 7.2.16
WordPress version where issue is found: trunk and latest WordPress 5.1.1
OS: Ubuntu 16.04
Attachments (2)
Change History (9)
#1
@
6 years ago
- Keywords needs-unit-tests added
- Milestone changed from Awaiting Review to 5.3
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#2
@
6 years ago
- Version changed from trunk to 3.7
Introduced in [25540].
A similar issue would need to be corrected in _copy_dir()
in wp-admin/includes/update-core.php
.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
#5
@
5 years ago
- Keywords needs-refresh added
- Milestone changed from 5.3 to Future Release
This ticket was discussed during today's bug scrub. It still needs a more inclusive patch, so it's being moved to Future Release. If anyone can take ownership during the 5.4 cycle, feel free to move up the milestone.
Patch