New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comet Cache intervening with WordPress trailing slashes in Permalinks #554

Closed
raamdev opened this Issue Aug 19, 2015 · 18 comments

Comments

Projects
None yet
3 participants
@raamdev
Member

raamdev commented Aug 19, 2015

We're investigating a report of ZenCache preventing a WordPress site from redirecting example.com/test to example.com/test/ (with the slash).

I'm having varying success reproducing this issue. I can reproduce it on my live test site, but I cannot reproduce it on my local test site. At first I thought this had only to do with the .htaccess rewrite rules, but then I remembered seeing code in WordPress Core somewhere that looks at the permalink settings and redirects to the /-version if the trailing slash exists in the Permalink settings.

@jaswsinc writes...

Right. There is the user_trailingslashit() function that handles this in permalink generation. https://codex.wordpress.org/Function_Reference/user_trailingslashit
See also: https://github.com/WordPress/WordPress/blob/ce557062f4123d8513378cf415b4e8b612c33ccc/wp-includes/canonical.php#L41
See also: http://httpd.apache.org/docs/current/mod/mod_dir.html#directoryslash

That does sound like it could be a bug. I will run tests against this later and see what I can do.


Support tickets referencing this issue

@jaswrks

This comment has been minimized.

Member

jaswrks commented Aug 21, 2015

My Thoughts

There's no reason for ZenCache to differentiate a trailing slash from no trailing slash, in terms of how we generate cache paths. It's really the exact same location. The only difference is in preference; i.e., what a site owner prefers on the end of their permalinks.

I do agree it's a bug though, because whatever you configure in your WordPress Permalink structure should be adhered to. If you configure your Permalink structure with a trailing /, WordPress generates all permalinks that way. And, by default, the redirect_canonical() function in WordPress enforces that behavior.

The issue here is that redirect_canonical() is bypassed whenever caching is enabled; i.e., whenever ZenCache is running. The cache file is detected early in the request, then served, long before redirect_canonical() gets a chance to run. No way around this either. In fact, that's intentional. So to resolve this we need to get creative.


Suggested Next Actions

I would say this issue is blocked by the fact that we don't yet have a system in place that deals with .htaccess tweaks. Once work on this outline I posted a few days ago is underway, I think that a solution will present itself here.

What I suggest is that we add a couple of lines into the /.htaccess file whenever ZenCache is enabled that take care of this issue; based on the configured permalink structure. WordPress itself relies upon redirect_canonical(), but that's not feasible w/ caching enabled. Thus, we need to deal with this at the Apache level via .htaccess in my view.

@raamdev

This comment has been minimized.

Member

raamdev commented Aug 21, 2015

I would say this issue is blocked by the fact that we don't yet have a system in place that deals with .htaccess tweaks. Once work on this outline I posted a few days ago is underway, I think that a solution will present itself here.

Got it. Thanks for the update!

Blocked by #427

@raamdev raamdev added the blocked label Aug 21, 2015

@raamdev

This comment has been minimized.

Member

raamdev commented Oct 16, 2015

@jaswsinc I'm updating this GitHub issue to note that it's no longer blocked by #427. Do you have any examples for what .htaccess rule(s) will need to be added to fix this issue?

Also, I'm a bit worried that such rules would have such a wide-reaching effect on a site that it might be cause for trouble. For example, if a site owner has their own custom rewrite rules (in addition to those created by WordPress), the rules we add to fix this GitHub issue might create an unexpected conflict. Thoughts on this?

@raamdev raamdev removed the blocked label Oct 16, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Oct 16, 2015

Force a trailing slash:

# BEGIN Force Trailing Slash
<IfModule rewrite_module>
  RewriteEngine On
  RewriteBase /

  # If not a real file or directory.
  RewriteCond %{REQUEST_FILENAME} !-f
  RewriteCond %{REQUEST_FILENAME} !-d

  # Not a part of the WP admin area.
  RewriteCond %{REQUEST_URI} !(?:^|/)wp\-admin(?:/|$)

  # If there is no trailing slash.
  RewriteCond %{REQUEST_URI} !(?:.*)/$

  # Force a trailing slash on all virtual requests.
  RewriteRule ^(.*)$ /$1/ [QSA,L,R=301]
</IfModule>
# END Force Trailing Slash

Force NO trailing slash:

# BEGIN Force NO Trailing Slash
<IfModule rewrite_module>
  RewriteEngine On
  RewriteBase /

  # If not a real file or directory.
  RewriteCond %{REQUEST_FILENAME} !-f
  RewriteCond %{REQUEST_FILENAME} !-d

  # Not a part of the WP admin area.
  RewriteCond %{REQUEST_URI} !(?:^|/)wp\-admin(?:/|$)

  # If there is a trailing slash.
  RewriteCond %{REQUEST_URI} (?:.*)/$

  # Force NO trailing slash on all virtual requests.
  RewriteRule ^(.*)/$ /$1 [QSA,L,R=301]
</IfModule>
# END Force NO Trailing Slash

Also, I'm a bit worried that such rules would have such a wide-reaching effect on a site that it might be cause for trouble. For example, if a site owner has their own custom rewrite rules (in addition to those created by WordPress), the rules we add to fix this GitHub issue might create an unexpected conflict. Thoughts on this?

I think if we detect permalink settings that either require or do not require a trailing slash and add the appropriate snippet to the very top of the .htaccess file there is very little chance of there being an issue. Really, one of these snippets just needs to come somewhere before the WordPress section.

However, since Permalink settings in WordPress can be changed, and because of the unknowns that you mentioned, I would suggest that this is not injected automatically, but presented as an option for those who would like to implement it. Upon enabling the option, only then do we automatically inject it.

@raamdev

This comment has been minimized.

Member

raamdev commented Oct 18, 2015

I would suggest that this is not injected automatically, but presented as an option for those who would like to implement it. Upon enabling the option, only then do we automatically inject it.

Great idea! Where do you think such an option should go? I'm thinking maybe a new .htaccess Tweaks options panel. We could then use that new panel in the future for any other .htaccess tweaks like this that we might want to make easy for a site owner to add/remove from their .htaccess file.

@raamdev raamdev added this to the Next Release (Pro) milestone Oct 18, 2015

@jaswrks

This comment has been minimized.

Member

jaswrks commented Oct 21, 2015

Where do you think such an option should go? I'm thinking maybe a new .htaccess Tweaks

After having worked with some additional tweaks as a part of the work in this PR, I think a good title for this would be, "Apache .htaccess Tuner". Like you said, we can nest all of the tweaks that we are going to introduce in this panel. The one mentioned in this issue, and the others that I'm working on in that PR too.

@raamdev

This comment has been minimized.

Member

raamdev commented Oct 22, 2015

"Apache .htaccess Tuner"

That means we'd probably want to get rid of the GZIP Compression menu option and move that into the new Apache Optimization (?) panel, correct? That probably makes sense considering the whole GZIP Compression panel won't be applicable to web servers not running Apache. (Sure, we could give advice for enabling GZIP Compression on other web servers inside the GZIP Compression options panel, but I feel like that's not an organized way to approach this now that we're adding several more Apache-related optimizations.)

If we go the route of condensing Apache-related stuff into a single Apache Optimization panel, then we could eventually have other panels for other web servers, e.g., NGINX Optimization, where we give advice for optimizing other web servers (assuming ZenCache cannot optimize them itself). Thoughts?

@jaswrks

This comment has been minimized.

Member

jaswrks commented Oct 22, 2015

Apache Optimization

Sounds perfect. I wanted to go with "Apache Performance Tuning", but that seems a little misleading. Apache Optimization is better.

If we go the route of condensing Apache-related stuff into a single Apache Optimization panel, then we could eventually have other panels for other web servers, e.g., NGINX Optimization, where we give advice for optimizing other web servers (assuming ZenCache cannot optimize them itself). Thoughts?

I think that's a great idea!

@raamdev

This comment has been minimized.

Member

raamdev commented Oct 22, 2015

I think that's a great idea!

Cool! If you don't want to work the consolidation and creation of a new Apache Optimization panel into websharks/comet-cache-pro#165, let me know when you're ready for me to dive in and I can take care of that.

@jaswrks

This comment has been minimized.

Member

jaswrks commented Oct 22, 2015

Go for it! I still have some work to do in other areas before I get to the UI, so if you'd like to work on that panel then I can merge that into feature/sonic-boom when I'm ready to start testing. That would make this go faster I think.

@raamdev

This comment has been minimized.

Member

raamdev commented Oct 22, 2015

then I can merge that into feature/sonic-boom when I'm ready to start testing. That would make this go faster I think.

Roger that.

@raamdev raamdev modified the milestones: Next Release (Pro), Future Release (Pro) Oct 29, 2015

@raamdev

This comment has been minimized.

Member

raamdev commented Dec 2, 2015

Punting this to a Future Release. We need to build an Apache Optimizations panel so that we have a place to put the option required to close this issue. The Apache Optimizations panel itself depends on work discussed in websharks/team#36.

@raamdev raamdev modified the milestones: Future Release (Pro), Next Release (Pro) Dec 2, 2015

@raamdev raamdev modified the milestones: Next Release (Pro), Future Release (Pro) Dec 28, 2015

@raamdev raamdev added this to the Future Release (Pro) milestone Feb 13, 2016

@renzms renzms self-assigned this Jun 17, 2016

@raamdev raamdev changed the title from ZenCache intervening with WordPress trailing slashes in Permalinks to Comet Cache intervening with WordPress trailing slashes in Permalinks Jun 17, 2016

@raamdev

This comment has been minimized.

Member

raamdev commented Jun 17, 2016

Noting that this feature should be disabled by default. We should not be updating the .htaccess unless the site owner tells Comet Cache it should be doing so.

Apache Optimization should be a Pro-only feature, yes.

Otherwise the outline looks good to me.

@raamdev

This comment has been minimized.

Member

raamdev commented Jun 17, 2016

@raamdev writes in #554 (comment)...

That means we'd probably want to get rid of the GZIP Compression menu option and move that into the new Apache Optimization (?) panel, correct? That probably makes sense considering the whole GZIP Compression panel won't be applicable to web servers not running Apache. (Sure, we could give advice for enabling GZIP Compression on other web servers inside the GZIP Compression options panel, but I feel like that's not an organized way to approach this now that we're adding several more Apache-related optimizations.)

@renzms As I mentioned on Slack, let's go ahead and move GZIP Compression into the Apache Optimizations panel. Also, we'll make the Apache Optimizations panel available in the Lite version (so that Lite users still gain access to GZIP Compression), but all options, except for the GZIP Compression, should be a Pro-only features.

With the Apache Optimizations panel available in the Lite version, we could add an extra Pro Preview teaser right inside that panel, so that site owners using the Lite version and looking at the Apache Optimizations panel see that there are additional optimizations available in the Pro version. (We'll save that work for a later date and a separate GitHub issue—I just wanted to make note of it for future reference.)

renzms added a commit to websharks/comet-cache-pro that referenced this issue Jun 17, 2016

@raamdev raamdev added Epic and removed Epic labels Jun 20, 2016

@raamdev raamdev referenced this issue Jun 20, 2016

Closed

Feature/sonic boom #165

0 of 4 tasks complete

raamdev added a commit to websharks/comet-cache-pro that referenced this issue Jun 27, 2016

@raamdev

This comment has been minimized.

Member

raamdev commented Jun 29, 2016

Next Release Changelog:

  • Bug Fix (Pro): Fixed a bug where Comet Cache would appear to prevent WordPress from redirecting Permalinks that don't include a trailing slash, to the URL that does include a trailing slash. This was due to the fact that Comet Cache loads very early on (for caching purposes) and as a result the WordPress redirect_canonical() function never gets run. This was fixed by adding an option to the new Apache Optimizations panel that allows you to Enforce Canonical URLs. Props @renzms, @jaswsinc. See Issue #554.
  • New Pro Feature! A new "Enforce Canonical URLs" option has been added to the new Apache Optimizations panel. This options adds the appropriate .htaccess code to enforce the correct canonical URLs according to your WordPress Permalink settings (Comet Cache detects if the Permalink Structure ends with a trailing slash, or without a trailing slash). Props @renzms, @jaswsinc. See Issue #554.

@raamdev raamdev closed this Jun 29, 2016

@jaswrks jaswrks referenced this issue Jun 30, 2016

Closed

Comet Cache v160629-RC (Release Candidate) #792

2 of 2 tasks complete

raamdev added a commit that referenced this issue Jul 7, 2016

Phing release of v160706 with the following changes:
- **New Feature! Apache Optimizations.** This release includes a completely new option panel for Apache Performance Tuning. Current options for Apache tuning include GZIP Compression, Leverage Browser Caching, Enforce Canonical URLs, and Send Access-Control-Allow-Origin Header (for Static CDN Filters). These options automatically add or remove from your `.htaccess` file the appropriate configuration based on the options you enable or disable (all options are disabled by default, so your `.htaccess` file is not modified unless you say so). If you prefer to update your `.htaccess` file manually, the necessary configuration can be viewed beneath each option. Props @jaswsinc, @renzms. See [Issue #789](#789).
- **New Feature!** A new "Enable GZIP Compression" option has been added to the new Apache Optimizations panel. This option will automatically add the appropriate configuration to your `.htaccess` file to enable GZIP compression. This option is disabled by default. The old "GZIP Compression" panel has been removed in favor of the new option inside Apache Optimizations. Props @renzms, @jaswsinc. See [Issue #764](#764).
- **New Feature!** Multisite Host Exclusion Patterns. It's now possible to exclude entire sites from the cache in a Multisite Network environment. Domain mapping is also supported! See _Comet Cache → Plugin Options → Host Exclusion Patterns_. If you're running a Multisite Network with Sub-Directories, you can exclude sites using the existing URI Exclusion Patterns feature. Props @kristineds. See [Issue #754](#754).
- **New Feature (Pro)!** A new "Leverage Browser Caching" option has been added to the new Apache Optimizations panel. This option will automatically add the appropriate configuration to your `.htaccess` file to enable Browser Caching. This option is disabled by default. Props @renzms, @jaswsinc. See [Issue #764](#764).
- **New Feature (Pro)!** A new "Enforce Canonical URLs" option has been added to the new Apache Optimizations panel. This options adds the appropriate `.htaccess` code to enforce the correct canonical URLs according to your WordPress Permalink settings (Comet Cache detects if the Permalink Structure ends with a trailing slash, or without a trailing slash). Props @renzms, @jaswsinc. See [Issue #554](#554).
- **Bug Fix**: In some scenarios the Cron Event that cleans up expired cache files (`_cron_comet_cache_cleanup`) would never run, or the Next Run time would constantly reset to 1 minute away from running every time a page was reloaded. We suspect this is a race condition and in attempt to work around this issue we now skip all of our Cron-related checks if Cron is currently in the middle of running a process. Props @xberg and @lkraav for help reporting. See [Issue #653](#653).
- **Bug Fix**: If your site uses aliased domains, Comet Cache now properly considers all possible domain variations when it clears the cache on WP Standard installations. Props @kristineds, @jaswsinc, @yoffe, and @VR51. See [Issue #608](#608).
- **Bug Fix** (Pro): Fixed a bug where Comet Cache would appear to prevent WordPress from redirecting Permalinks that don't include a trailing slash, to the URL that does include a trailing slash. This was due to the fact that Comet Cache loads very early on (for caching purposes) and as a result the WordPress `redirect_canonical()` function never gets run. This was fixed by adding an option to the new Apache Optimizations panel that allows you to Enforce Canonical URLs. Props @renzms, @jaswsinc. See [Issue #554](#554).
- **UX Bug Fix** (Pro): If you had your WordPress Dashboard login details saved by your browser, the browser autofill would automatically fill in the Pro Plugin Updater fields with those details, which then needed to be replaced with your actual Pro license details. The browser autofill has been disabled for those fields (tested in Chrome, Firefox, and Safari). Props @renzms. See [Issue #741](#741).
- **Enhancement**: Added links the Options Page for the Comet Cache [Twitter](http://twitter.com/cometcache) and [Facebook](http://facebook.com/cometcache) accounts. Props @renzms. [Issue #771](#771).
- **Enhancement:** Added full support for UTF-8 (multibyte strings). This release adds full support for UTF-8 throughout the Comet Cache codebase, greatly enhancing Comet Cache's ability to deal with file paths and URLs that may contain UTF-8 characters. Props @jaswsinc. [Issue #703](#703).
- **UI Enhancements**: Improved the Logged-In Users and the Client-Side Caching options panels to dim additional options when the feature is disabled. Additionally, the "Enable HTML Compression for Logged-In Users?" option has been relocated from the HTML Compressor option panel to the more appropriate Logged-In Users option panel. See [Issue #768](#768).
- **UX Enhancement**: Improved the inline docs for Auto-Clear List of Custom URLs to clarify that full URLs must be provided. Props @renzms. See [Issue #781](#781).
- **Enhancement** (Pro): The Pro Plugin Updater has been improved to allow for better compatibility with hosting platforms that use Apache's ModSecurity. In some cases, site owners were seeing a 404 error when attempting to update the Pro version using the Pro Plugin updater because certain ModSecurity rules were blocking the Pro Updater requests. The Pro Plugin Updater now uses WP Transients to store the necessary metadata, which works around the issue with ModSecurity. Props to @seozones for reporting and @jaswsinc for help fixing this. [Issue #416](#416).
- **Enhancement** (Pro): When Static CDN Filters are enabled, it's now possible to disable the automatic insertion of rules into your `.htaccess` file that are designed to prevent issues with [CORS](https://cometcache.com/kb-article/what-are-cross-origin-request-blocked-errors/). See _Apache Optimizations → Send Access-Control-Allow-Origin Header?_ See [Issue #787](#787).
- **Enhancement** (Pro): The HTML Notes added to the bottom of a cached page now specify if the page was cached as the result of an HTTP Request or if it was cached by the Auto-Cache Engine. Props @kristineds. See [Issue #292](#292).
- **Enhancement** (Pro): The Auto-Cache Engine now supports a fallback to cURL using the WP HTTP API. If your PHP configuration has `allow_fopen_url=0`, the Auto-Cache Engine will use the fallback to download the XML Sitemap and parse it from a temporary file. If you want to force the use of this fallback even when `allow_fopen_url=1`, you can use [a filter](#440 (comment)). See [Issue #440](#440).
- **UI Enhancement** (Pro): A second button has been added to the bottom of the Pro Plugin Updater page that allows you to "Save and Update Comet Cache Pro" in one step. Props @renzms. See [Issue #741](#741).
- **UI Enhancement** (Pro): The "Cache Stats" button in Admin Bar is now linked to the Cache Stats page. Instead of hovering over the button and then clicking "More Info" inside the popup panel, you can now just click the "Cache Stats" button to go directly to the Cache Stats page. Props @Presskopp, @renzms. See [Issue #780](#780).
- **Comment Mail Compatibility:** Improved compatibility with the Comment Mail plugin by automatically clearing the cache whenever Comment Mail options are changed. Many of the Comment Mail options affect front-end portions of the site, so it's important that the cache is cleared whenever Comment Mail options change. See [Comment Mail Issue #278](websharks/comment-mail#278 (comment)).
- **PHP Compatibility:** Improved compatibility back to PHP 5.2 (the lowest version allowed by WordPress). Comet Cache still requires PHP 5.4+, but if you install Comet Cache on a site running PHP 5.2, it will now fail gracefully with a Dashboard notice indicating PHP 5.4+ is required, instead of producing a fatal error. See [Issue #784](#784).
- **WP-CLI Compatibility**: Fixed a bug with deactivating Comet Cache using WP-CLI. Doing so was producing a "Invalid argument; host token empty!" error message. This has been resolved. Props @MarioKnight @jaswsinc @renzms. See [Issue #728](#728).
- Renamed `COMET_CACHE_ALLOW_BROWSER_CACHE` constant to `COMET_CACHE_ALLOW_CLIENT_SIDE_CACHE`. Backwards compatibility has been maintained.
- Renamed `allow_browser_cache` plugin option to `allow_client_side_cache`.
@raamdev

This comment has been minimized.

Member

raamdev commented Jul 7, 2016

Comet Cache v160706 has been released and includes changes from this GitHub Issue. See the v160706 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#554).

@websharks websharks locked and limited conversation to collaborators Jul 7, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.