File-based caching no-cache request headers bug

Hello @corytrevor,

Thank you for the provided feedback and the suggestions made. I have brought this to the attention of our plugin’s developers.

They will review the information and decide whether or not changes will be applied on the plugin’s functionality.

You may check the Changelog of the plugin in future releases for more details.

Best Regards,
Gergana Petrova

Excellent, thanks @gpetrova

Hi, I noticed that a fix for this wasn’t included in the recent update.

Here are the steps to recreate the bug.

Add this code to a child theme to set the ‘cache-control: no-cache’ response header:

add_filter( 'wp_headers', function( $headers ) {
   unset( $headers['cache-control'] );	
   $headers['cache-control'] = 'no-cache';
   return $headers;
} );

Enable file-based caching and purge cache.
Open the network tab of dev tools in an incognito window, make sure ‘disable cache’ is unchecked and visit a page on the site and note the ‘cache-control: no-cache’ response header for the HTML.
If hosted on SiteGround, purge the dynamic cache in Site Tools.
Refresh the page and note the ‘sg-f-cache: HIT’ response header.

So the page was cached by the file-based caching even though the ‘cache-control: no-cache’ header was set.

Here’s how to fix that. On line 225 in sg-cachepress/core/helper/File_Cacher_Trait.php replace apache_request_headers() with apache_response_headers()

Repeat the steps above, note how this time it has the ‘sg-f-cache: BYPASS’ response header as it is correctly bypassing the cache due to the presence of the ‘cache-control: no-cache’ response header.

All good so far, however if more cache-control parameters are added then it prevents the cache bypass due to the way it’s been coded to check if the header value is an exact match. To verify this, update the child theme code to:

add_filter( 'wp_headers', function( $headers ) {
  return array_merge( $headers, wp_get_nocache_headers() );
} );

This changes the cache-control header to ‘no-cache, must-revalidate, max-age=0’ but the plugin only checks if it’s an exact match for ‘no-cache’ so it caches it even though no-cache is set in the cache-control header.

This can be fixed by replacing the foreach loop beginning on line 225 with this code which uses str_contains() to check if ‘no-cache’ is part of the cache-control header value.

foreach ( apache_response_headers() as $header => $value ) {
	$lowercase_header = strtolower( $header );
	$header_value = trim( $value );
			
	if (array_key_exists( $lowercase_header, $ignore_headers )) {
		$header_ignore_value = $ignore_headers[ $lowercase_header ];
	}
			
	// Do not cache if any of the ignore headers exists and matches the ignore header value.
	if ( 
		isset($header_ignore_value) &&
		str_contains($header_value, $header_ignore_value)
	) {
		return true;
	}
}

Please can you make sure one of the senior plugin developers takes a look a this issue. Let me know if you have any questions.

Cheers

Hello @corytrevor,

Our support representatives have already reported the bug you have explained here to us.

Yesterday’s release included only discontinuation of Cloudflare support as we wanted to separate it from other changes in the plugin.

We have investigated the reported behaviour and found some complications, so the suggested fix will not be suitable for all cases. Still we have an internal bug opened for the reported case and we will address it in the future plugin releases.

Thank you for the report and cooperation on the matter!

Best Regards,
Elena

Hi Elena

Thanks for getting back to me.

Are you able to clarify if the intended functionality of the file-based caching is to ignore cache-control response headers?

Cheers

This is not intended functionality.

We can not just use apache responce headers as File Based cache is being applied on the optimized content of the site – while all optimization options already took place on the markup. Dynamic caching is one more thing we are considering as well. Thus most of the application headers are not applied in the responce.

We aim to cache the page after it is being optimized in order to serve the correct and fastest content to the visitors.

As soon as there is a working solution on the case we will include it in the plugin. You can use the URL exclude from cache option from the plugin interface in the meantime.

Best Regards,
Elena

• This reply was modified 2 years ago by Elena Chavdarova.

Hi @elenachavdarova)

Thanks for your reply.

I’m not sure if I’m following correctly. In my example, the response headers are being used to exclude a page from being cached by the file-based caching, which it does successfully.

If a page is excluded from file-based caching then there’s no issue with the response headers not being present in a cached hit because the page will not be cached in the first place so there will be no cached hits.

There is also no issue with the dynamic cache in this case as the dynamic caching respects cache-control: no-cache.

Cheers

Thank you for the responce!

We are taking your feedback into consideration in the cache generation and headers checks refactoring.

As we can not provide an ETA when a fix will be deployed, you can check the plugin changelog once a new version is released.

Best Regards,
Elena

Hi @elenachavdarova

Thanks for including a fix for this in the 7.2.7 update!

However, there is an issue with the implementation that means that the file-based caching still caches pages when the no-cache response header is present.

On line 231 of sg-cachepress/core/helper/File_Cacher_Trait.php it uses the stripos() function:

stripos(?trim(?$value?),?$ignore_headers[?$lowercase_header?]?)

In the context that it’s being used it needs to return a value of true for the if statement to work, but if the header string contains the ignore header then stripos() returns an integer – https://www.php.net/manual/en/function.strpos.php

An easy work around is just to use an is_int() check so on line 231 it would have:

is_int(stripos( trim( $value ), $ignore_headers[ $lowercase_header ] ))

I’ve tried it out with that modification and it works when I test it.

Cheers

Thank you for the tests and cooperation!

We have just released new version of the plugin and included the fix.

Best Regards,
Elena

You’re welcome. Perfect, thanks @elenachavdarova

Cheers