Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Unnecessary Cookie Checks #592

Closed
jaswrks opened this issue Oct 21, 2015 · 7 comments
Closed

Bug: Unnecessary Cookie Checks #592

jaswrks opened this issue Oct 21, 2015 · 7 comments
Assignees
Labels
bug
Milestone

Comments

@jaswrks
Copy link

@jaswrks jaswrks commented Oct 21, 2015

Overview

While working on some .htaccess tweaks in other parts of ZenCache, I noticed that we have two unnecessary cookie checks in ZenCache. This line in particular is causing several pages not to be cached whenever there are stray cookies, even when the user is not logged in.

Next Actions

  • New feature branch in the websharks/zencache-pro repo.
  • Comment these two lines out; i.e., leave them there, but comment them out please.
  • Submit PR.
@raamdev
Copy link
Contributor

@raamdev raamdev commented Oct 29, 2015

Next Pro Release Changelog:

  • Bug Fix: Fixed a bug where in some scenarios a page might not be cached due to a stray AUTH_COOKIE or SECURE_AUTH_COOKIE cookie, even when the user is not logged in. Props @jaswsinc @renzms. See Issue #592.
@lkraav
Copy link

@lkraav lkraav commented Oct 29, 2015

I can confirm, I have seen caching being incorrectly disabled with the "user is logged in" message. In our case it happened on staging sites, where admin bar is always visible and https://github.com/Rarst/toolbar-theme-switcher is used for hopping between themes.

@raamdev
Copy link
Contributor

@raamdev raamdev commented Oct 29, 2015

@lkraav Thanks for the update. :-) If you still experience this issue after the next Pro release, please let us know and we can reopen this GitHub issue.

@jaswrks
Copy link
Author

@jaswrks jaswrks commented Oct 29, 2015

It looks like we can also optimize this routine a bit further, now that we no longer have to consider cookie names that begin with wordpress_. I haven't looked carefully at this, but at first glance it looks like there is still room for some improvement in these lines.

i.e., that perhaps we can remove those and simplify this routine overall now. That would help, because that routine is called at least once on every page load.

@jaswrks jaswrks reopened this Oct 29, 2015
jaswrks pushed a commit to wpsharks/comet-cache-pro that referenced this issue Oct 30, 2015
jaswrks pushed a commit to wpsharks/comet-cache-pro that referenced this issue Oct 30, 2015
@raamdev
Copy link
Contributor

@raamdev raamdev commented Oct 30, 2015

@jaswsinc I assume this can be closed now that wpsharks/comet-cache-pro#168 has been merged, correct?

@jaswrks
Copy link
Author

@jaswrks jaswrks commented Oct 30, 2015

Yes. Thanks!

@raamdev raamdev closed this Oct 30, 2015
@raamdev
Copy link
Contributor

@raamdev raamdev commented Nov 14, 2015

ZenCache v151114 has been released and includes changes from this GitHub Issue. See the v151114 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#592).

@wpsharks wpsharks locked and limited conversation to collaborators Nov 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants