Skip to content
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

Feature Request: If-Modified-Since HTTP Header #255

Open
bridgeport opened this issue Aug 2, 2014 · 7 comments
Open

Feature Request: If-Modified-Since HTTP Header #255

bridgeport opened this issue Aug 2, 2014 · 7 comments

Comments

@bridgeport
Copy link

Quick Cache delivers a tremendous improvement on page load speeds out the box, but I've been curious about one aspect of its implementation: status code and headers.

As far as I can tell, Quick Cached delivered pages always respond with a 200 (OK) status code and does not deliver a "If-Modified-Since" header.

Would it be more advantageous to deliver a 304 status code and "If-Modified-Since" header in order to save bandwidth (e.g., crawlers, googlebot) and provide faster performance, from a client's browser perspective?

When examining some other caching implementations, I've noticed their response headers implemented 304 status codes and the "If-Modified-Since" header.

That got me to thinking about the HTML comments Quick Cache appends to outputted pages. The last comment line is always dynamically regenerated for every page request:

<!-- Quick Cache fully functional :-) Cache file served for (http://www.example.com/) in 0.00232 seconds, on: Aug 2nd, 2014 @ 3:28 am UTC. -->

While this last line is an interesting addition (especially for debugging and benchmarking purposes), would this be the reason 304 status code can't be implemented? Because the page is actually "changing" content for every request, even though 99.999% of its source is unchanged?

Unless there's a performance rationale behind this implementation, would the following approach be worth considering and incorporating, changing Quick Cache's implementation:

  1. Return 304 status code and If-Modified-Since header when appropriate.
  2. Remove the always-changing millisecond timestamp comment so that cached pages delivered to all visitors is truly identical on a binary level. Or just Insert the millisecond comment line only with a admin debug setting enabled, but otherwise off by default.

I think it would make sense for Quick Cache pages to be identical. If I set the cache to 7 days, I think it would be logical for the "content" delivered to googlebot on day 1 be the same as the content delivered to a visitor on day 5. That would require all the Quick Cache comments appended to the end of the page's HTML to always be static.

Thanks.

@raamdev
Copy link
Contributor

raamdev commented Aug 3, 2014

the page is actually "changing" content for every request, even though 99.999% of its source is unchanged?

Quick Cache isn't actually changing the content of the page (or more correctly, the cache file) but rather just appending those additional notes to the output before serving the request. You can easily disable this output in the Quick Cache options (Dashboard → Quick Cache → Plugin Options → Enable/Disable and set the box at the bottom to "No, I don't want my source code to contain any of these notes.").

Regarding the 304 Status Code and the If-Modified-Since header: That actually sounds like a really good idea to me.

@jaswsinc do you have any input here?

@bridgeport
Copy link
Author

@raamdev I think the source code comment that Quick Cache adds is very helpful. It's a good way to see its internal logic of what it caches, when it expires, and when it specifies why a page hasn't been cached (e.g., exclusion URI). The millisecond snippet is the one thing I think could be sacrificed (if necessary) to achieve some other benefit.

The reason I originally brought it up was in regards to the 304 status code and modified since header... If you were to implement those responses while still appending the always changing millisecond snippet for every page view, would those headers be considered valid? Even if only 0.0001% of the page is changing every page view, but the response headers are telling the browser or googlebot that it hasn't changed, is that an invalid implementation of 304 and If-Modified-Since? Does the page actually have to be completely static (on a binary level) for it to "conform"?

Or, another way to think of it, if an MD5/CRC hash was taken of a cached Quick Cage page, it would always change (per page view) due to the millisecond snippet.

Either way, thanks for taking this issue into consideration.

Thanks.

@jaswrks
Copy link

jaswrks commented Aug 3, 2014

@jaswsinc do you have any input here?

I think the explanation I wrote here awhile back is closely related to this issue. @bridgeport For an explanation of why Quick Cache intentionally does not use 304 or If-Modified-Since, please see: #134 (comment)

Also, there is further information specifically on the topic of If-Modified-Since being unset with PHP's header_remove() function here. This was brought up in TRAC awhile back and the WP lead developer chimed in there also.

WordPress itself (QC follows this standard), will remove the If-Modified-Since header to help to prevent a browser from caching the resulting HTML output. Again, this is intentional for the reasons I gave in the explanation awhile back.


Additional Thoughts...

While I wouldn't use it myself (on any site), I don't think it's necessarily a bad idea to provide this option for site owners running Quick Cache; i.e. to allow a site owner to choose to allow browser caching if they so desire. This is in fact an option available in QC already. @bridgeport If you want to allow a browser to cache the cache, you can change your QC config and tell Quick Cache NOT to send nocache_headers() to the browser.

@raamdev I think what @bridgeport would like to see, and I agree that it's something we could work toward, is an option that would allow a site owner to use Quick Cache more like a static site generator. That is, once the cache is created, future hits to those cached HTML files could be served not via PHP, but via rewrite rules that would bypass PHP altogether.

With that in place (since the files are being served without PHP at all this case), sending an ETag, If-Modified-Since and 304 responses would be a much better match — in that type of configuration. WP Super Cache and W3 Total Cache both offer similar options, so if you take a look at how they work from the inside will help to better clarify this approach with greater detail.


Should we Do It?

From a broader perspective, I think it would be a good idea to consider whether or not the end result of such efforts (to make this a more powerful/enhanced option in QC) would really be worthwhile or not.

I personally would never attempt to run a WordPress site (even when it's being cached) as all static content. It is the fact that Quick Cache retains some control over the browser cache that makes it such a practical/powerful solution — one that is very simple to implement and for novice site owners to have confidence in. Is there a performance hit because of this? Yes, but I would argue that it's very small given the way in which it's been implemented.

Serving cached content as static HTML...

This functionality already exists in other plugins like WP Super Cache and W3 Total Cache. My own opinion is that Quick Cache need not deal with this. I just don't see it being practical for the majority of WP installations out there. That's just me though (one opinion). If Raam decides to add this in, maybe we just set a priority that matches what we feel is appropriate. Again, I don't see anything wrong with making it an option, but giving this a higher priority than say, CDN integration or a graphical UI for cache analysis would be a mistake in my view.

@raamdev
Copy link
Contributor

raamdev commented Jun 18, 2015

@jaswsinc writes...

Comet Cache removes Last-Modified headers because WordPress itself removes Last-Modified to avoid compatibility issues when the desire is to prevent browser caching. See: https://developer.wordpress.org/reference/functions/nocache_headers/
Discussion: https://core.trac.wordpress.org/ticket/22258

The other reason Comet Cache currently removes Last-Modified headers is because Comet Cache currently does not support ETags or If-Modified-Since checks, which would necessarily need to occur in PHP code at this point. There is really no good reason for us not to support this though, and as soon as we do, that will only help speed Comet Cache up. With that in place, we will want to include the Last-Modified header instead of excluding it: https://github.com/websharks/zencache-pro/blob/150605-RC/src/includes/closures/Ac/ObUtils.php#L162

In short, this is currently unsupported, because Comet Cache lacks a sub-routine that would deal with a browser's If-Modified-Since request, which would necessarily need to occur in PHP, not in Apache. Why? Because the request is for a dynamic file generated by WordPress, not for a static file that Apache knows about and can run stat checks on.


I'm updating this GitHub issue to a Feature Request to add support for ETags and If-Modified-Since.

@raamdev raamdev changed the title Should Quick Cache return 304 status code and "If-Modified-Since" header? Feature Request: ETags and If-Modified-Since Jun 18, 2015
@raamdev
Copy link
Contributor

raamdev commented Jun 18, 2015

Related: #371 #134

@skipthedrive
Copy link

I'd love to see this feature implemented (if-modified-since). Thanks!

@raamdev raamdev changed the title Feature Request: ETags and If-Modified-Since Feature Request: If-Modified-Since HTTP Header May 26, 2017
@raamdev
Copy link
Contributor

raamdev commented May 26, 2017

This issue was opened several years ago as a feature request for ETags and If-Modified-Since. This issue now contains lots of outdated information (e.g., ETags is now supported by Comet Cache via the new Leverage Browser Caching option in the Apache Optimizations panel, and WordPress Core no longer removes the If-Modified-Since header but instead includes support for it).

Here's the latest information about the state of Comet Cache and WordPress Core with regards to the If-Modified-Since header:


@jaswrks writes...

Regarding If-Modified-Since. This header is not supported by Comet Cache at this time. However, it is now supported by WordPress core. So we have some work to do in this dept. in order for us to catch up with recent improvements to the WP core. See: https://github.com/WordPress/WordPress/blob/4.7.5/wp-includes/class-wp.php#L447-L470

Having said that, what Comet Cache does support is If-Modified-Since and/or ETag headers for static assets. For example, if you enable Leverage Browser Caching in the Apache Optimizations panel of Comet Cache, that will configure Apache in a way that allows for these headers to be supported and reduce the number of connections needed to serve static assets like images, CSS, JS, etc. You can do the same thing following the suggestions we have for Nginx.

@raamdev raamdev added this to the Future Release milestone Jul 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants