Opened 6 years ago
Last modified 3 days ago
#41855 new defect (bug)
copy_dir fails in coping some files in nested dirs
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | 4.8.1 |
Component: | Filesystem API | Keywords: | has-patch has-unit-tests has-testing-info |
Focuses: | Cc: |
Description
Tricky, subtle bug!
Let's suppose we do have a folder structure like this:
/folder1/folder2/
/folder1/folder2/file1.txt
/folder1/folder2/subfolder1/
/folder1/folder2/subfolder1/file2.txt
/folder3/
and that we want to move or copy the entire folder2 to folder3 so that the final structure is like this:
/folder1/folder2/
/folder1/folder2/file1.txt
/folder1/folder2/subfolder1/
/folder1/folder2/subfolder1/file1.txt
/folder3/folder2/
/folder3/folder2/file1.txt
/folder3/folder2/subfolder1/
/folder3/folder2/subfolder1/file1.txt
IMPORTANT: Please note that initially the path /folder3/folder2 doesn't exist.
The problem arise based on the order used internally by copy_dir to retieve the source elements.
Let's see it in detail.
command:
copy_dir('/folder1/folder2', '/folder3/folder2')
Result:
First iteration: /folder3/folder2 is created with mkdir.
Second iteration (if copy_dir retrieves first /folder1/folder2/file1.txt): copy(/folder1/folder2/file1.txt, /folder3/folder2/file1.txt) fails because /folder3/folder2 doesn't exist!
Second iteration (if copydir retrieves first /folder1/folder2): /folder3/folder2 is created. Now, by the next iteration, copy(/folder1/folder2/file1.txt, /folder3/folder2/file1.txt) will not fail.
I've discover this bug almost accidentally using the following (buggy) code:
<?php function copy_recursive($source, $dest, $merge = true) { if (is_file($dest) && is_dir($source)) { error_log(__("[PPC] - Error: Cannot copy a dir into a file.")); return; } if (!$merge) delete_recursive($dest); if (is_dir($source)) { $iterator = new RecursiveIteratorIterator( new RecursiveDirectoryIterator($source, RecursiveDirectoryIterator::SKIP_DOTS), RecursiveIteratorIterator::SELF_FIRST ); foreach ($iterator as $file) { if ($file->isDir()) { mkdir($dest.DIRECTORY_SEPARATOR.$iterator->getSubPathName(), 0777, true); } else { copy($file, $dest.DIRECTORY_SEPARATOR.$iterator->getSubPathName()); } } } else { copy($source, $dest); } }
I need a copy_dir (or copy_recursive) function for copying my custom plugin's data (a complex nested structure of file/folders) to wordpress' upload dir. The above code used to fail, apparently randomly, the copy of only some files.
Then I've tried to use wordpress' copy_dir function only to discover, with disappointment, that it used to fail exactly the same way.
The solution, at least for my (buggy) code, is to create the destination folder before any copy command, like this is:
<?php function copy_recursive($source, $dest, $merge = true) { if (is_file($dest) && is_dir($source)) { error_log(__("[PPC] - Error: Cannot copy a dir into a file.")); return; } if (!$merge) delete_recursive($dest); if (!file_exists($dest)) mkdir($dest, 0777, true); // <<<---- Bug Fix! if (is_dir($source)) { $iterator = new RecursiveIteratorIterator( new RecursiveDirectoryIterator($source, RecursiveDirectoryIterator::SKIP_DOTS), RecursiveIteratorIterator::SELF_FIRST ); foreach ($iterator as $file) { if ($file->isDir()) { mkdir($dest.DIRECTORY_SEPARATOR.$iterator->getSubPathName(), 0777, true); } else { copy($file, $dest.DIRECTORY_SEPARATOR.$iterator->getSubPathName()); } } } else { copy($source, $dest); } }
Attachments (1)
Change History (5)
This ticket was mentioned in PR #4221 on WordPress/wordpress-develop by @costdev.
3 months ago
#1
- Keywords has-patch has-unit-tests added
#2
@
3 months ago
- Milestone changed from Awaiting Review to 6.3
PR 4221 creates the destination directory if it does not exist. Includes a PHPUnit test which demonstrates the issue, proves the fix is successful, and protects backward compatibility going forward.
Milestoning for 6.3.
This ticket was mentioned in Slack in #core by oglekler. View the logs.
4 days ago
#4
@
3 days ago
- Keywords has-testing-info added
Testing Instructions
These steps define how to reproduce the issue, and indicate the expected behavior.
Steps to Reproduce or Test
- Download the attached testing plugin.
- Navigate to
Plugins > Add New
. - Click
Upload Plugin
, selecttest-41855.zip
and clickInstall Now
. - 🐞 Once installed, click
Activate
. - Apply the patch.
- ✅ Refresh the page.
Expected Results
When reproducing a bug:
- ❌ An admin notice should appear stating
Could not create directory.
. - ❌
wp-content/plugins/test-41855/test-result/test-directory
should not exist.
When testing a patch to validate it works as expected:
- ✅ An admin notice should appear stating
Success!
. - ✅
wp-content/plugins/test-41855/test-result/test-directory
should exist with the same contents aswp-content/plugins/test-41855/test-directory
.
Test Report Icons:
🐞 <= Indicates where issue ("bug") occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.
This creates the destination directory in
copy_dir()
if it does not exist.Trac ticket: https://core.trac.wordpress.org/ticket/41855