emrikol / focus Goto Github PK

The $nonce and $action variables are always set, thus checking for isset() in https://github.com/emrikol/FOCUS/blob/master/focus-cache.php#L129 is redundant and actually does not do what you might think it's doing.

Strict comparison agains false might be better check in there.

Due to the way that the cache is stored on the filesystem, we don't need to have unique keys per install. Multisite is supported by default, and if more than one WordPress site is using the same installation directory, there's probably much more that will be broken than this cache.

(Props to WordPress.org user @bugnumber9)

When a persistent object cache is used, the existing transients in the database are left behind--unnecessarily bloating the database. Instructions should be added to the README for deleting the old transients.

(Props to WordPress.org user @bugnumber9)

The cache should be flushed on these instances:

1. Plugin updating
2. Plugin deactivation
3. Plugin activation
4. Object cache deactivation
5. Object cache activation

The cached files should not be forced to live in /wp-content/focus-object-cache.

(Props to WordPress.org user @bugnumber9)

Related to #13, a UI to purge database transients may be useful.

(Props to WordPress.org user @bugnumber9)

The plugin needs a standard WordPress readme.txt

Please consider documenting constants and filters found in the plugin.

I use the Constants: header but that is far away from documentation.
https://github.com/szepeviktor/tiny-cache/blob/master/tiny-nav-menu-cache.php#L6

Unit tests are needed for:

1. Enabling and disabling the drop-in
2. Enabling and disabling the plugin

https://github.com/emrikol/focus/blob/master/includes/object-cache.php#L409
The wp-content directory may have a custom location.

WP_CONTENT_DIR maybe?

If the dropin can't be added/updated, perhaps the user is in a managed WordPress host? Notify them.

opcache.validate_timestamps = 0 may prevent functioning properly
as PHP files are not reread.

The cache files should not be accessible at all via the web server. Adding a generic .htaccess file and/or instructions (nginx perhaps) to the README file will help users secure the cache files.

(Props to WordPress.org user @bugnumber9)

The FOCUS_Cache::render_admin_page is sanitising the $nonce variable using wp_sanitize_text_field and wp_unslash. By default, the nonce should not contain any character which would need any slashes or sanitisation (it should be md5 hash) and not even the WordPress core is sanitising the nonces, as they are invalid already, if they need some sanitisation.

See https://github.com/WordPress/WordPress/search?utf8=%E2%9C%93&q=wp_verify_nonce

There should be a cron job set up to clear out stale cron files, runs hourly by default but filterable.

Please add a repo link here on GitHub
https://wordpress.org/plugins/focus-object-cache/

Use rawurlencode for sanitising the user input used by add_query_arg function.

• https://github.com/emrikol/FOCUS/blob/ef67d048f4a5bf3f144609edf63fbbf72c11a267/focus-cache.php#L133

• https://github.com/emrikol/FOCUS/blob/ef67d048f4a5bf3f144609edf63fbbf72c11a267/focus-cache.php#L304

• https://github.com/emrikol/FOCUS/blob/ef67d048f4a5bf3f144609edf63fbbf72c11a267/focus-cache.php#L332

The code in FOCUS_Cache::render_admin_page is double sanitising the $action variable via sanitize_text_field.

First in https://github.com/emrikol/FOCUS/blob/master/focus-cache.php#L126 and for second time in https://github.com/emrikol/FOCUS/blob/master/focus-cache.php#L130

It's really needed only once.

Further, the double wp_unslash call may actually introduce some unwanted bugs. Eg.:

wp> addslashes( "O\'Reilly" );
string(11) "O\\\'Reilly"
wp> wp_unslash( "O\\\'Reilly" );
string(9) "O\'Reilly"
wp> wp_unslash( "O\'Reilly" );
string(8) "O'Reilly"