wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
683 stars 213 forks source link

API REST Cache return HTML instead of JSON #2542

Open hippocampestudio opened 4 years ago

hippocampestudio commented 4 years ago

When API REST caching is activated (with add_filter( 'rocket_cache_reject_wp_rest_api', '__return_false' );), API REST endpoints return cached data with HTML header instead of JSON header.

GeekPress commented 4 years ago

Hi @hippocampestudio 🤚

To be able to work on this issue, we need first to replicate it.

To help us, thanks to provide us the exact steps to reproduce the issue by using this template: https://github.com/wp-media/wp-rocket/issues/new?template=bug_report.md

hippocampestudio commented 4 years ago

Describe the bug When API REST caching is activated (with add_filter( 'rocket_cache_reject_wp_rest_api', '__return_false' );), API REST endpoints return cached data with HTML header instead of JSON header.

To Reproduce Steps to reproduce the behavior:

  1. Desactivate WPRocket
  2. Add add_filter( 'rocket_cache_reject_wp_rest_api', '__return_false' ); to functions.php
  3. Re-activate WP-rocket
  4. Go to /wp-json/wp/v2/posts (version with no cache)
  5. Refresh /wp-json/wp/v2/posts (cached version)

Expected behavior The cached JSON should be a valid JSON but is HTML

Screenshots https://i.ibb.co/0VJmstK/Capture-d-cran-2020-04-17-15-41-08.png

Tabrisrp commented 4 years ago

Duplicate of #291

GeekPress commented 4 years ago

@Tabrisrp Thanks for the check. I'm going to close the #291 as this one as the steps to reproduce.

crystinutzaa commented 4 years ago

Reproduce the issue ✅ Identified the issue on localhost.

Identify the root cause ✅ The root cause is that the JSON is saved as .html and no JSON headers are provided.

Scope a solution ✅ @hellofromtonya , @Tabrisrp ideas? Basically this will affect the .htaccess rules and also the serve_cache_file() https://github.com/wp-media/wp-rocket/blob/57003cff362df710d36acb49085115ea4d09e0eb/inc/classes/Buffer/class-cache.php#L167

A solution can be to save the file as .json instead of .html Another solution is to provide the JSON headers. Both solutions should be applied IF the filepath contains: wp-json/wp/v2/

Effort ✅ I think this is an [M], but probably I am overestimating it 😄

hellofromtonya commented 4 years ago

@Tabrisrp What do you think?

Tabrisrp commented 4 years ago

Saving the file with the json extension seems the better choice to me.

I'm not sure we should bother too much with the htaccess rules, as the speed benefit is pretty low, and a JSON file is relatively small anyway.

What we need to take into account there is the following:

hippocampestudio commented 3 years ago

Hello, Is there any news about this bug?

hippocampestudio commented 3 years ago

This issue is nearly 8 months old and still no solution :(

nhterry commented 3 years ago

Any updates on this?

More and more websites are moving to use WordPress as a Headless CMS and caching these API endpoints is critical.

GeekPress commented 3 years ago

@nhterry Until you will see a milestone set up on this issue, it means we don't have any update.

We have currently 434 opened issues and we can't fix all of them in a week. We have to prioritize.

Thanks for your comprehension.

hippocampestudio commented 3 years ago

We have currently 434 opened issues and we can't fix all of them in a week. We have to prioritize.

@GeekPress : I understand the idea of prioritizing, but this issue is almost one year old (15 Apr 2020)... So this is not an issue from last week, and as @nhterry says, it becomes critical On my side I am forced to look for other competing solutions because I really need this :(

RoyKoeleman commented 2 years ago

This problem is even worse, as it also removes all custom headers used.

AntoineGuerra commented 1 year ago

Please, this problem will be fixed or not ?

AntoineGuerra commented 1 year ago

If you have CloudFlare you can overwrite the contentype by rules/transform-rules. It works on my side, i created a transform rule to update the "Content-Type" to "application/json" that match if the "URL Path" begin with /wp-json/ and it works

RoyKoeleman commented 1 year ago

You can, but I think the most common use case is to cache product filtering for webshops. As pagination is all done in the headers, that is totally broken when all headers are removed on the cached response.

RoyKoeleman commented 1 year ago

A solution can be to save the file as .json instead of .html Another solution is to provide the JSON headers. Both solutions should be applied IF the filepath contains: wp-json/wp/v2/

This will rule out all plugins or other custom routes, so just wp-json/ would be better in my opinion.

viobru commented 4 hours ago

Related HS ticket: https://secure.helpscout.net/conversation/2658485327/503823 Slack thread: https://wp-media.slack.com/archives/C08N8J6VC/p1721726550203169

In this case, it is the feed that looks broken in the browser when cached.