[wp-trac] [WordPress Trac] #50422: Prevent Browser Caching From Getting Involved With wp_redirect and wp_safe_redirect (Leaving the Browser to Purely Honor the Redirect Code Used)

[WordPress Trac]

#50422: Prevent Browser Caching From Getting Involved With wp_redirect and
wp_safe_redirect (Leaving the Browser to Purely Honor the Redirect Code
Used)
-------------------------+-------------------------------------
 Reporter:  KZeni        |      Owner:  (none)
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  General      |    Version:  5.4.2
 Severity:  normal       |   Keywords:  needs-testing has-patch
  Focuses:               |
-------------------------+-------------------------------------
 Currently, {{{wp_redirect}}} (and therefore {{{wp_safe_redirect}}} as well
 since that uses {{{wp_redirect}}}) can be cached by web browsers depending
 on a site's browser caching rules. For example, W3 Total Cache has browser
 caching that one would certainly want to utilize for improved site
 performance, but then doesn't really have the means to prevent this
 potential problem where a redirect then gets cached by the browser (even
 if it's a 302 redirect). W3TC is just one example, but browser caching can
 be implemented in any number of ways which this then can be a problem.

 I came across this with a plugin and proposed a patch to the plugin per
 https://wordpress.org/support/topic/proposed-bugfix-prevent-login-
 redirect-from-browser-cache-rules/, but I'm wondering why the fix for this
 isn't part of WordPress' core so that plugins & developers in general
 don't need to specifically add this to prevent an issue that can come
 about given the right circumstances (with the fix being rather simple &
 straightforward, from what I can tell.)

 In short, having {{{nocache_headers();}}} before the redirect happens has
 the redirect still honored as it was set while removing the potential
 undesired effect of it being cached by the web browser (ex. trying to view
 a page that needs you logged in to view it has a plugin redirect to the
 login page, but then that 302 redirect was cached by the browser so the
 user is still redirected to the login when trying to visit that page even
 after they've already logged in, and it can really happen with any
 redirect that should be honored as being temporary that was then cached as
 the page's result by the browser when it shouldn't be [translation plugins
 have also encountered this in the past as well when switching between
 languages for pages via a redirect, and I'm sure there's more instances of
 this.])

 ----

 Yes, currently developers can prevent this themselves by implementing
 {{{nocache_headers();}}} before their {{{wp_redirect}}} and/or
 {{{wp_safe_redirect}}} (I've made sure to document this as such at
 https://developer.wordpress.org/reference/functions/wp_redirect/#comment-3973
 and
 https://developer.wordpress.org/reference/functions/wp_safe_redirect/#comment-3974,
 respectively [which I imagine both of these notes should be removed
 if/when this gets implemented natively]), but I'm wondering why that
 should be necessary. Would there really be any downside of having
 {{{wp_redirect}}} trigger {{{nocache_headers();}}} as part of that
 function natively so it just works more reliably without added code and/or
 developers potentially not knowing to do this which then results in
 problematic behavior?

 At that point, things like the {{{auth_redirect}}} function (and possibly
 others) can be cleaned up to not need to include {{{nocache_headers();}}}
 before their redirect. Meanwhile, {{{wp_get_nocache_headers();}}} (used by
 {{{nocache_headers();}}}) checks to see if headers have already been sent
 or not & also honors the existing headers while just enforcing specific
 ones so those that have this applied twice (ex. after this is included
 natively as part of {{{wp_redirect}}} while they still have it in their
 own implementation before this was done) shouldn't have any negative
 effects.

 ----

 **- The Proposed Patch -**
 This simply has {{{nocache_headers(); // Prevent browser caching of page
 with the redirect header (browser should still honor the redirect status
 code)}}} before the {{{header( "Location: $location", true, $status );}}}
 snippet in the {{{wp_redirect}}} function within ''wp-
 includes/pluggable.php'', and then has a minor optimization of removing
 {{{nocache_headers();}}} from the {{{auth_redirect}}} function in that
 same file since that's no longer needed (the {{{wp_redirect}}} later in
 the function now takes care of this. I've attached a patched version of
 pluggable.php from WP 5.4.2 with these 2 changes.

 I filed this as an enhancement since it's not really a bug within the code
 being fixed as much as it is preventing bugs made by plugin & other
 developers (so it still is fixing bugs within the WordPress ecosystem
 [just going to the core of the problem rather than having to fix the issue
 with each instance where this came up as a result of the core code not
 taking care of this itself yet.])

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50422>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform