WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 7 weeks ago

#46581 reviewing defect (bug)

$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 dev-feedback 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)

copy-dir-skip-list-bug-fix.diff (473 bytes) - added by codex-m 3 months ago.
Patch
debug.jpg (357.6 KB) - added by codex-m 3 months ago.
Debug screenshot

Download all attachments as: .zip

Change History (4)

@codex-m
3 months ago

Debug screenshot

#1 @SergeyBiryukov
7 weeks 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 @SergeyBiryukov
7 weeks 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.

Note: See TracTickets for help on using tickets.