WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 6 months ago

Last modified 6 months ago

#45136 closed defect (bug) (fixed)

wp-personal-data-export cron error if folder not exists - create folder

Reported by: DuckDagobert Owned by: desrosj
Milestone: 5.2 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch commit
Focuses: Cc:

Description

Fix the wp-personal-data-exports function that is hooked to cron that it will create the folder if it doesn't exist.

PHP Error: opendir(/mypath/wp-content/uploads/wp-personal-data-exports/): failed to open dir: No such file or directory
File: /mypath/wp-admin/includes/file.php on line 143
Error URL: https://myurl.com/wp-cron.php
Trace:
#1 /mypath/wp-admin/includes/file.php(143): opendir()
#2 /mypath/wp-includes/functions.php(6056): list_files(, 100, Array)
#3 /mypath/wp-includes/class-wp-hook.php(286): wp_privacy_delete_old_export_files()
#4 /mypath/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(, Array)
#5 /mypath/wp-includes/plugin.php(515): WP_Hook->do_action(Array)
#6 /mypath/wp-cron.php(126): do_action_ref_array(wp_privacy_dele..., Array)

PHP Error: closedir() expects parameter 1 to be resource, boolean given
File: /mypath/wp-admin/includes/file.php on line 168
Error URL: https://myurl.com/wp-cron.php
Trace:
#1 /mypath/wp-admin/includes/file.php(168): closedir(false)
#2 /mypath/wp-includes/functions.php(6056): list_files(, 100, Array)
#3 /mypath/wp-includes/class-wp-hook.php(286): wp_privacy_delete_old_export_files()
#4 /mypath/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(, Array)
#5 /mypath/wp-includes/plugin.php(515): WP_Hook->do_action(Array)
#6 /mypath/wp-cron.php(126): do_action_ref_array(wp_privacy_dele..., Array)

Attachments (3)

#45136.patch (544 bytes) - added by arena 9 months ago.
patch #45136 ... testing exports_dir
#45136_v2.patch (622 bytes) - added by arena 7 months ago.
patch as required
45136.2.diff (639 bytes) - added by garrett-eclipse 7 months ago.
Minor update to adhere to Coding Standards

Download all attachments as: .zip

Change History (18)

#1 @mukesh27
11 months ago

  • Component changed from General to Privacy
  • Focuses privacy added
  • Keywords needs-patch added

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


10 months ago

#3 @desrosj
10 months ago

  • Focuses privacy removed
  • Keywords reporter-feedback added; needs-patch removed
  • Severity changed from major to normal
  • Version changed from 4.9.8 to 4.9.6

Thanks for the ticket, @DuckDagobert!

Can you share any more information about your environment? I am currently unable to replicate this issue.

Both opendir() and closedir() are prefixed by @ on the lines in question, which should suppress the E_WARNING level error triggered if the directory does not exist.

It's possible you have a custom error handler defined using set_error_handler() that is ignoring the @.

#4 @DuckDagobert
10 months ago

Sorry for the late reply, somehow it didn't save my comment the first time round, so I resubmit it now (bc I just checked if anybody replied):

Yes I am using a custom error handler.

I wonder why it doesn't just create the folder if it doesn't exist, instead of suppressing the error with

@desrosj

#5 @arena
9 months ago

  • Keywords needs-patch added
  • Version 4.9.6 deleted

I confirm that this is a bug.

Apparently, the directory 'wp-personal-data-exports' is only created when the first personal data file is generated.

On the other side, the wp-cron job scans every hour the directory 'wp-personal-data-exports' and do not test if the folder exists. ( hook name : wp_privacy_delete_old_export_files )

My config :

  • Version de WordPress : 5.1-alpha-44356
  • Version de PHP/MySQL : 7.2.3 / 5.7.17

wp-cron called by :

/wordpress/wp-cron.php?doing_wp_cron=1545348420.1551721096038818359375

WARNINGS :

PHP [E_WARNING] 2 : opendir( X:\..\wordpress/wp-content/uploads/wp-personal-data-exports/,wordpress/wp-content/uploads/wp-personal-data-exports/): The system cannot find the file specified. (code: 2) in wordpress\wp-admin\includes\file.php at line 143

PHP [E_WARNING] 2 : opendir( X:\..\wordpress/wp-content/uploads/wp-personal-data-exports/): failed to open dir: No such file or directory in wordpress\wp-admin\includes\file.php at line 143

PHP [E_WARNING] 2 : closedir() expects parameter 1 to be resource, boolean given in wordpress\wp-admin\includes\file.php at line 168

@arena
9 months ago

patch #45136 ... testing exports_dir

#6 @arena
9 months ago

  • Keywords has-patch added; needs-patch removed

#7 @arena
9 months ago

Patch working like a charm !
No more warnings !

#8 @desrosj
7 months ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to desrosj
  • Status changed from new to reviewing

#9 @garrett-eclipse
7 months ago

  • Keywords needs-refresh added
  • Version set to 4.9.6

Thanks for the patch @arena as well as the report @DuckDagobert

@arena would you mind refreshing the patch with two minor things;

  1. Remove the initial empty line introduced after the function declaration and the first line of code.
  2. Generate the patch from the root directory, currently running grunt patch:45136 results in error as the patch was diff'd from the same directory as the file and not the root of the repository.
    Running "patch:45136" (patch) task
    (Stripping trailing CRs from patch.)
    can't find file to patch at input line 5
    Perhaps you used the wrong -p or --strip option?
    The text leading up to this was:
    --------------------------
    |Index: functions.php
    |===================================================================
    |--- functions.php	(revision 44186)
    |+++ functions.php	(working copy)
    --------------------------
    

Quoting - 'ensure that you are in the root directory created by your SVN checkout'
Reference - https://make.wordpress.org/core/handbook/tutorials/working-with-patches/#creating-a-patch

Thanks

@arena
7 months ago

patch as required

#10 @arena
7 months ago

  • Keywords needs-refresh removed

@garrett-eclipse
7 months ago

Minor update to adhere to Coding Standards

#11 @garrett-eclipse
7 months ago

  • Keywords needs-testing needs-unit-tests added
  • Milestone changed from Future Release to 5.2

Thanks @arena I appreciate the refresh. I made another minor refresh in 45136.2.diff to adhere to the coding standards for Space Usage and avoiding Clever Code.

Moving this into 5.2 for some additional testing and flagging for unit tests as it may warrant them but am unsure if required. Will review with the team on Monday.
Cheers

#12 @desrosj
6 months ago

  • Keywords commit added; needs-testing needs-unit-tests removed

#13 @desrosj
6 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 44910:

Privacy: Do not attempt to cleanup personal data export files when the directory does not exist.

Checking for the presence of the directory and returning early prevents PHP warnings when attempting to list files in a non-existent directory.

Props arena, garrett-eclipse.
Fixes #45136.

#14 @SergeyBiryukov
6 months ago

#46741 was marked as a duplicate.

#15 @SergeyBiryukov
6 months ago

#46740 was marked as a duplicate.

Note: See TracTickets for help on using tickets.