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

[duanehutchins]

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).

[tschale]

Sounds like a good proposal to me.
@divio: Is there an "official" opinion about this?

[victor-yunenko]

Hi @duanehutchins, thank you for raising this! Divio handed over the implementation to django CMS Association and we're currently reviewing all pending tickets.

The proposal sounds reasonable to me, and a PR would be warmly welcomed!

Also might be worth checking the 4.X branch to confirm that the issue still persists there.

[duanehutchins]

Hi @victor-yunenko ! I wish they replied sooner. I haven't worked with django-cms for a couple years now.

However, I just looked over the 4.x branch code, and there have been some changes that might be resolved the issue already, at least in part, but I'm not sure. I'd have to test to confirm.

I'll try to get some time in the near future to test and maybe mock up a quick patch if it's not already resolved.

[victor-yunenko]

Thank you!

It also might be related to #6608? If yes then the proposed override there could help.

[duanehutchins]

Definitely related, but the proposed workaround only helps in specific cases.