pantheon-systems / wp-redis

WordPress Object Cache using Redis.
https://wordpress.org/plugins/wp-redis/
GNU General Public License v2.0
225 stars 67 forks source link

esc_html is not safe to use (exception loop) #418

Closed yangm97 closed 1 year ago

yangm97 commented 1 year ago

Not sure which wordpress version introduced this behavior but I'm using wordpress 6.2 and esc_html() calls wp_check_invalid_utf8() which calls get_option() that ends up making a wp_cache_get() call.

Not only does this bring the whole site down but it also ends up obfuscating whatever the real issue is.

You should be able to reproduce this with bad redis credentials/server down/whatever.

Stack trace:
#0 /var/www/html/web/wp/wp-includes/option.php(165): wp_cache_get('notoptions', 'options')
#1 /var/www/html/web/wp/wp-includes/formatting.php(1107): get_option('blog_charset')
#2 /var/www/html/web/wp/wp-includes/formatting.php(4577): wp_check_invalid_utf8('WP Redis: ERR A...')
#3 /var/www/html/web/app/plugins/wp-redis/object-cache.php(1471): esc_html('WP Redis: ERR A...')
#4 /var/www/html/web/app/plugins/wp-redis/object-cache.php(1205): WP_Object_Cache->_exception_handler(Object(Exception))
#5 /var/www/html/web/app/plugins/wp-redis/object-cache.php(1510): WP_Object_Cache->_connect_redis()
#6 /var/www/html/web/app/plugins/wp-redis/object-cache.php(215): WP_Object_Cache->__construct()
#7 /var/www/html/web/wp/wp-includes/load.php(750): wp_cache_init()
#8 /var/www/html/web/wp/wp-settings.php(131): wp_start_object_cache()
#9 /var/www/html/web/wp-config.php(9): require_once('/var/www/h in /var/www/html/web/app/plugins/wp-redis/object-cache.php on line 139"
pwtyler commented 1 year ago

Confirming, is the fatal error before the stack trace PHP Fatal error: Uncaught Error: Call to a member function get() on null in /app/wp-content/plugins/wp-redis/object-cache.php:139, or a different fatal error? This seems related, if not the same as #410.

yangm97 commented 1 year ago

After removing esc_html from the exception handler I could see I got an authentication error because the password was blank (just like #359 but I'm using PHP 7.4.12 instead), so I worked around the issue by changing the redis configuration to require a password.

410 seems like an issue on its own but the stack trace got messed up because of the exception loop reported here.

pwtyler commented 1 year ago

This is fixed in 1.4.2, now live on .org