500 error on every page with read-only file system
omarjackman opened this issue · 7 comments
Describe the bug
The plugin causes a 500 error on every page when the uploads directory is not writable
PHP version: 8.2
Expected behavior
The page should load
Current behavior
A 500 error is displayed to the user and the error log shows an out of memory error
Steps to reproduce
- Make the uploads directory read-only. It is important that
wp_upload_dir
return an error. - load any page with this plugin enabled
- notice that the page hangs
Additional context
After debugging I found that the error is caused by an infinite loop that starts at https://github.com/PopupMaker/Popup-Maker/blob/v1.18.2/classes/Utils/Logging.php#L82
Heres what happens:
- The logger class initializes and tries to get the upload dir within its constructor via
PUM_Helpers::get_upload_dir
. - The
PUM_Helpers::get_upload_dir
detects that there was an error when callingwp_upload_dir
so it then tries to log the error here. - Since the constructor never finished executing the first instance of the logger, a new instance is created and we start over at 1 again.
@omarjackman - First, why would the uploads directory ever be read only, that doesnt seem correct for the WP core specs Seems to indicate a deeper problem than just our plugin.
Will see if there is a simple fix to multiple instances being spawned, seems simple enough stop gap.
Does the version of PHP being 8.2 actually matter, or does it occur on others as well?
@omarjackman - First, why would the uploads directory ever be read only, that doesnt seem correct for the WP core specs Seems to indicate a deeper problem than just our plugin.
Will see if there is a simple fix to multiple instances being spawned, seems simple enough stop gap.
Thanks for looking into the issue. The "why" shouldn't matter, my use-case has the file system as read-only and enabling this plugin caused every request to the site to go into an infinite loop. If it helps, forget about my use-case and just imagine any other scenario where wp_upload_dir returns with an error.
Does the version of PHP being 8.2 actually matter, or does it occur on others as well?
It probably doesn't matter but the issue template asked for the version of PHP I was using
Saw you mention it originally in issue title, just wanted to confirm.
This could partially be fixed by a solution to #1043 which bypasses all filesystem checks if logging is disabled (define( 'PUM_DISABLE_LOGGING', true )
). One would presumably set that constant if the uploads directory is known to be read-only. It doesn't account for other temporary error conditions, however.