Make WordPress Core

#61942 closed defect (bug) (fixed)

Add "no-store" to Cache-Control header to prevent unexpected cache behavior

Reported by: kkmuffme's profile kkmuffme Owned by: johnbillion's profile johnbillion
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

https://core.trac.wordpress.org/ticket/21938

Added no-store, private to Cache-Control in WP 6.1 for logged in users.
However, since this ticket was more than a decade old and created in an age before widespread reverse-proxying (CDNs), this is a problem: since those can and will store responses that have no-cache (but not no-store): https://developers.cloudflare.com/cache/concepts/cache-control/
Either by default or depending on the configuration.

Practically, not all actions are for logged in users - e.g. you have a cart/checkout/thankyou page, which will end up in a proxy-cache bc of this bug and could end up being served from cache incorrectly.

The no-store, private should be added for non-logged in users too/the user logged in condition removed

Change History (7)

This ticket was mentioned in PR #7257 on WordPress/wordpress-develop by @devansh2002.


5 months ago
#1

  • Keywords has-patch added

Remove logged-in check for no-store, private Cache-Control
Trac ticket: https://core.trac.wordpress.org/ticket/61942

#2 @ayeshrajans
5 months ago

  • Keywords has-patch removed

I agree with what the ticket proposes, but we are already doing this for logged in users.

PR 7257 seems wrong, it removes the is-logged-in check for some reason, but it's not what we should be doing because the existing code seems correct to me.

#3 @johnbillion
2 months ago

  • Keywords has-patch needs-testing reporter-feedback added

@kkmuffme What do you think about the comment above from @ayeshrajans ?

#4 @kkmuffme
8 weeks ago

Sorry, but @ayeshrajans makes no sense to me

it removes the is-logged-in check for some reason

The reason is exactly the issue described by me in the OP

the existing code seems correct to me

Again: did you read the original bug I reported above? Why does it seem correct to you?

#5 @johnbillion
7 days ago

  • Milestone changed from Awaiting Review to 6.8
  • Owner set to johnbillion
  • Status changed from new to reviewing

#6 @johnbillion
69 minutes ago

  • Keywords has-unit-tests added; needs-testing reporter-feedback removed

#7 @johnbillion
27 minutes ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 59724:

Security: Always include the no-store and private directives in the Cache-Control header when setting headers that prevent caching.

The intention of these headers is to prevent any form of caching, whether that's in the browser or in an intermediate cache such as a proxy server. These directives instruct an intermediate cache to not store the response in their cache for any user – not just for logged-in users.

This does not affect the caching behaviour of assets within a page such as images, CSS, and JavaScript files.

Props kkmuffme, devansh2002, johnbillion.

Fixes #61942

Note: See TracTickets for help on using tickets.