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

The toolbar middleware triggers Vary: Cookie header on non-toolbar pages #6569

Open
duanehutchins opened this Issue Dec 4, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@duanehutchins

duanehutchins commented Dec 4, 2018

Summary

The toolbar middleware triggers Vary: Cookie header on non-toolbar pages because it checks the session unnecessarily. This breaks caching.

Expected behaviour

Do not check session variables if there is no toolbar on the page.

Actual behaviour

The toolbar middleware triggers Vary: Cookie HTTP header on non-toolbar pages, which breaks caching. This happens because cms.middleware.toolbar.ToolbarMiddleware checks for session variables, which then causes the session middleware to send the Vary: Cookie header.

Environment

  • Python version: 3.6.6
  • Django version: 1.11.15
  • django CMS version: 3.4.5

Possible solutions I can think of:

  • Lazy-load the toolbar.
    • Session is only checked if the toolbar is accessed/used.
    • Users will benefit from it without any changes to their existing code.
  • Add a new CMS setting which disables the toolbar on all pages unless ?edit is in the path.
    • Setting defaults to off, so it does not affect existing sites.
    • Session is only checked if ?edit is in the path.
    • Keeps Vary: Cookie off of the CMS pages while not in edit mode.
    • Can be used in conjunction with the above lazy-load solution.
  • Load the toolbar in middleware via process_template_response only if the template has the cms_toolbar placeholder.
    • However, I'm not sure if this will break existing app hooks or older versions of Django.

I'm willing and able to write the PR for this. Let's please discuss the preferred solution(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment