Proposed Bugfix: Prevent Login Redirect From Browser Cache Rules

  • Resolved KZeni

    (@kzeni)


    4 years, 8 months ago

    I have a scenario where I’m using a Private Website via Profile Builder and then also have W3 Total Cache set to have browser caching being optimized for pages that are public (which then doesn’t really allow for individual page exceptions for the browser caching rules). I imagine this also applies to Restricted Content that’s set to redirect if not logged in as a suitable user.

    The problem I had was that a user could try to go to a page that’s protected while not being logged in as a sufficient user so then they’re redirected to the login page (expected behavior), but then it appears the page result with the redirect would actually get cached by the web browser even though it’s the default 302 redirect from wp_redirect and wp_safe_redirect. That meant that trying to view that page after logging in would still have the redirect occur that sends them back to the login page. Using a browser’s Network inspector within the Inspect tool showed that the redirect was being loaded from the browser cache and not actually checking with the site again.

    I came across https://github.com/qTranslate-Team/qtranslate-x/issues/306 where another plugin encountered this same problem where they had a redirect that really should be 100% temporary (not cached in any way), but then browsers were still caching the page result that included the 302 redirect so it was sticking around for longer than needed/wanted.

    – The Fix –
    The fix was to patch Profile Builder to use what it appears other plugins have found to be a reliable resolution for the issue and actually utilizes a function that’s native to WordPress’ core.

    In short, I edited profile-builder/features/functions.php and profile-builder/features/content-restriction/content-restriction-functions.php so that any & all instances of wp_redirect and wp_safe_redirect have nocache_headers(); // Prevent browser caching of page with the redirect header included as the line immediately before it. I found 4 instances of this in these 2 files, but it might be worthwhile to see if there are additional spots for this to be included as well (these just seemed to be the ones related to this particular issue.)

    This prevents the problem(s) that result from the possibility of the various temporary redirects from being cached by the web browser in an official, low-overhead, and graceful way.

    I’d love to see this included in the next version of Profile Builder, and let me know if you have any questions or anything.

Viewing 10 replies - 1 through 10 (of 10 total)
  • Thread Starter KZeni

    (@kzeni)

    FYI, I have proposed a patch to WordPress’ core at https://core.trac.www.ads-software.com/ticket/50422, but that’s still awaiting testing, approval, etc. so it really would be beneficial to update Profile Builder with this fix for until/if WordPress core chooses to address this. Also, this fix won’t break things if/when WordPress core implements this (it can be removed since it’d be redundant, but it’d also still fix it for those on older versions of WordPress while not hurting anything for those up-to-date.)

    Paul

    (@paulplapsa)

    Hello @kzeni,

    Thank you for for suggesting a solution for this issue!

    At first glance our development team agrees that this looks like a good fix so we will seriously consider including it in the next plugin update.

    Best Regards,
    Paul

    Thread Starter KZeni

    (@kzeni)

    Great to hear. I appreciate the update!

    Thread Starter KZeni

    (@kzeni)

    Ah, looks like the update around 2 weeks ago didn’t have it. Maybe the next one will?

    Thread Starter KZeni

    (@kzeni)

    The update from a day ago (3.2.1) looks to still not include it…?

    This really does prevent an issue on a site where a visitor’s logged-in status doesn’t always behave properly so I’d still love to see this officially implemented so I don’t need to keep re-patching the plugin as versions come out that still don’t have it (re-introduces the problem after updating).

    Is there any reason this is being held off on?

    Thread Starter KZeni

    (@kzeni)

    Now I see version 3.2.2 was released a week ago where this still hasn’t been implemented/formally addressed.

    Why hasn’t this been implemented?

    Also, why was this topic marked as resolved? Is it going to be in an upcoming update?

    Again, this is addressing an actual issue being experienced by users of an active site so I’ve been needing to re-implement my nocache_headers(); fix (as detailed above) for each version to then stay up-to-date while having the problem not re-introduce itself.

    An update on the issue would be appreciated. Thanks!

    Thread Starter KZeni

    (@kzeni)

    *Accidental double-post due to odd cache thing I experienced on my end. Disregard this one post.*

    • This reply was modified 4 years, 6 months ago by KZeni. Reason: Removed accidental double-post content. Noted as such since full deletion isn't an option, unfortunately
    Thread Starter KZeni

    (@kzeni)

    This plugin is now version 3.2.5 as of a day ago, and this quick fix (simply utilize nocache_headers(); when a redirect is done to prevent a browser remembering the redirect [ex. it should show the private content after logging in rather than re-showing the login page at that same URL after logging in]) is still not implemented. What? Why not? Why no update on this since saying it was being looked into months ago & marking this topic as resolved (I don’t recall marking it as resolved) without actually seeming to resolve this or following up to say it was resolved in a different way?

    Plugin Author madalin.ungureanu

    (@madalinungureanu)

    Hello @kzeni

    You are right and we apologize for this!
    The explanation ( not excuse, because it should have been don by now ), is that we had some compatibility issues with WordPress 5.5 that had a higher priority.
    I am sorry for this, and I will be patching this today. If you need it earlier than our release cycle please write us at https://www.cozmoslabs.com/support/open-ticket/

    It was marked as Resolved by our support team because they were told it would be fixed.

    Best Regards,
    Madalin

    Thread Starter KZeni

    (@kzeni)

    Understood. I wasn’t sure if I should seek out sending a support ticket for the free version of the plugin when it was already noted here (also have a paid version of the plugin, as an aside), but I’ll keep that in mind in the future after seeing the free version of the plugin is listed in the support ticket form’s dropdown.

    Thanks for the info & the update on the matter, @madalinungureanu!

Viewing 10 replies - 1 through 10 (of 10 total)
  • The topic ‘Proposed Bugfix: Prevent Login Redirect From Browser Cache Rules’ is closed to new replies.