keycdn / cache-enabler

A lightweight caching plugin for WordPress that makes your website faster by generating static HTML files.
https://wordpress.org/plugins/cache-enabler/
123 stars 46 forks source link

A modification to how you use `wp_mkdir_p` #193

Closed boogah closed 3 years ago

boogah commented 3 years ago

When using wp_mkdir_p, would you mind setting the directory permissions?

When proxying Apache to a server like Nginx, PHP creates the directory and the files within it (using your current implementation) to 0600. My proposed change is a simple one — and, honestly, this probably should be a pull request, but I thought it better to have a discussion about this first — all you should need to do is change this:

https://github.com/keycdn/cache-enabler/blob/72fc1a1300aa17b292e0fbe4a90cbef2b1c4cfb1/inc/cache_enabler_disk.class.php#L375

To this:

        if ( ! wp_mkdir_p( self::cache_file_dir_path(), 0644 ) ) {
            wp_die( 'Unable to create directory.' );
        }

And this:

https://github.com/keycdn/cache-enabler/blob/72fc1a1300aa17b292e0fbe4a90cbef2b1c4cfb1/inc/cache_enabler_disk.class.php#L486

To this:

        if ( ! wp_mkdir_p( dirname( $new_settings_file ), 0644 ) ) {
            wp_die( 'Unable to create directory.' );
        }

Both WP Super Cache and W3 Total Cache set permissions appropriately when used instead of Cache Enabler in the environment described above. But I'd rather not use either of those in the project that I'm working on. 😄

Anyhow, let me know what y'all think!

boogah commented 3 years ago

Alternately, you can use FS_CHMOD_DIR instead of 0644 so folks can set their permissions to whatever their environment requires:

https://wordpress.org/support/article/editing-wp-config-php/#override-of-default-file-permissions

rfxn commented 3 years ago

To be clear, this is not unique to Nginx or Apache, or combinations thereof. This is in relation to PHP suPHP, PHP FPM or similar environments where PHP writes out as a userspace user and Apache reads cache text files as the Apache user.

The mod_rewrite htaccess rules to access the cache contents is read by Apache and processed as the Apache user (nobody or apache). Given that the cache files are written out by PHP, running as the web site user (e.g rfxncom) with mode 0600, the Apache web server user can not read those files, which generates an error 403.

The fix here is as @boogah mentioned, cache path/files must be created as 0644 permissions.

coreykn commented 3 years ago

@boogah Setting the permissions would be a great addition to be made for the reasons you and @rfxn stated. A pull request for this change would be appreciated, but I do have one question. Does the wp_mkdir_p() function actually allow this? After a quick glance I don't see this in the documentation and noticed that @stevegrunwell created a new method to address this in nexcess/cache-enabler#1.