Regression: line 125 of classes/AssetCache.php has broken caching in recent versions
robsheldon opened this issue · 2 comments
Hi,
Commit dda3314 introduced a bug on line 125 of classes/AssetCache.php:
if ( true !== get_option( 'pum_files_writeable', true ) ) {
If a strict comparison is required here, then the line needs to be changed to:
if ( '1' !== get_option( 'pum_files_writeable', true ) ) {
See also https://developer.wordpress.org/reference/functions/get_option/, which states that boolean true
values get stored as a string(1) "1"
value in wp_options
, and "1"
is what get_option()
returns after that.
I've manually confirmed this behavior.
This bug is causing recent versions of Popup Maker to incorrectly mark its cache as not writeable.
Thanks!
Looks like line 663 of the same file may have the same bug too:
Popup-Maker/classes/AssetCache.php
Line 663 in b006732
@robsheldon Thanks for the report, marking this one closed as it has already been patched in the develop
branch.
Your correct, and the final solution was to coerce the get_option into a boolean for the strict comparison.
https://github.com/PopupMaker/Popup-Maker/blob/develop/classes/AssetCache.php#L715-L717
We are pushing PHPCS cleanup, and this one got by us. We have a review going over all the other changes to strict comparisons looking for anything comparing boolean to possibly non-boolean values, where similar fixes will be employed.