Opened 5 years ago
Last modified 3 years ago
#48265 new defect (bug)
The privacy export files cleanup can run unlink on directories throwing an error.
Reported by: | garrett-eclipse | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.9.6 |
Component: | Privacy | Keywords: | dev-feedback |
Focuses: | Cc: |
Description
Looking into some test failures on VVV flagged in Slack here;
https://wordpress.slack.com/archives/C02RQBWTW/p1570445063460400
It was found that the wp_privacy_delete_old_export_files
function runs unlink
on all files and subdirectories older than wp_privacy_export_expiration
, this becomes an issue as directories can't be removed via unlink
and will throw an Operation is not permitted error.
This occurs as the $export_files
list is collected from a list_files
call which has a level of 100 resulting in subdirectories being included.
So the question for me is should the export cleanup only do files in the current $export_dir
, or should it recurse into subdirectories and if so should the directories also be cleaned up if older than wp_privacy_export_expiration
. If we do go to the extent of removing subdirectories should the list_files
call be updated with an exclude filter so plugins can have custom directories in that location which would be avoided during the cleanup?
Note: If subdirectories are to be removed as well we'll have to recursively traverse them and remove their contents so rmdir
will work.
Along with addressing the issue in wp_privacy_delete_old_export_files
the cause of the original VVV test failures should also be addressed. What lies in the privacy unit test that creates the test_contents folder but doesn't clean it up here;
https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/privacy/wpPrivacyGeneratePersonalDataExportFile.php#L244
- simply removing this directory at the end of the test should suffice.
Change History (4)
#3
@
4 years ago
Relavent from the #51111 duplicate;
When running the test suite on Windows, occasionally I get some errors:
There were 4 errors: 1) Tests_Privacy_WpPrivacyDeleteOldExportFiles::test_expired_files_should_be_deleted unlink(S:\home\wordpress.test\develop/build/wp-content/uploads/wp-personal-data-exports/test_contents/): Is a directory S:\home\wordpress.test\develop\build\wp-includes\functions.php:7324 S:\home\wordpress.test\develop\tests\phpunit\tests\privacy\wpPrivacyDeleteOldExportFiles.php:120 2) Tests_Privacy_WpPrivacyDeleteOldExportFiles::test_unexpired_files_should_not_be_deleted unlink(S:\home\wordpress.test\develop/build/wp-content/uploads/wp-personal-data-exports/test_contents/): Is a directory S:\home\wordpress.test\develop\build\wp-includes\functions.php:7324 S:\home\wordpress.test\develop\tests\phpunit\tests\privacy\wpPrivacyDeleteOldExportFiles.php:131 3) Tests_Privacy_WpPrivacyDeleteOldExportFiles::test_index_file_should_never_be_deleted unlink(S:\home\wordpress.test\develop/build/wp-content/uploads/wp-personal-data-exports/test_contents/): Is a directory S:\home\wordpress.test\develop\build\wp-includes\functions.php:7324 S:\home\wordpress.test\develop\tests\phpunit\tests\privacy\wpPrivacyDeleteOldExportFiles.php:142 4) Tests_Privacy_WpPrivacyDeleteOldExportFiles::test_filtered_expiration_time unlink(S:\home\wordpress.test\develop/build/wp-content/uploads/wp-personal-data-exports/test_contents/): Is a directory S:\home\wordpress.test\develop\build\wp-includes\functions.php:7324 S:\home\wordpress.test\develop\tests\phpunit\tests\privacy\wpPrivacyDeleteOldExportFiles.php:155
Related - #44204