Sessions setting vary cookie without providing a cookie

[Jd Collins]

While reviewing process_response in django/contrib/sessions/middleware.py I believe the code is trying to accomplish the following things:

1. Determine if the session has been accessed.
2. Determine if the session has been modified.
3. Determine if the session is empty.
4. If the session cookie is in the cookies but is empty remove the session cookie.
5. Else if not 4. then if the session has been accessed then patch the vary header to add Cookie.
6. If the session has been modified or settings dictate that we always save the session and the session is not empty then we setup some cookie attributes, save the session, and set the cookie in the response.
7. Return the response.

Possible issues in the current behavior:

1. As reported in 29471, step 4 in the current behavior is responding in a way that sets the cookie to a blank value and expired but not setting the vary on cookie so this response can be cached for a requests causing authenticated sessions to be logged out.
2. Step 5 in the current behavior, accessed is always True when using auth as a result of: django/contrib/auth/middleware.py in get_user. I am not sure how accessing the session alone is enough reason to vary on cookie regardless of why it was accessed?
3. Step 5 in the current behavior, adds the vary cookie header even if the session is empty and will not be setting a cookie. This is problematic for caching since you are requiring a unique session based cache even if the session is empty.

Proposed behavior:

1. Determine if the session has been accessed. (Not sure if this is providing much value for a vary header perspective but leaving it).
2. Determine if the session has been modified.
3. Determine if the session is empty.
4. If the session cookie is in the cookies but is empty remove the session cookie. Also set the vary cookie to address potential issue #1.
5. Else if the session has been modified or settings dictate that we always save the session and the session is not empty then we setup some cookie attributes, save the session, and set the cookie in the response. Also set the vary cookie.
6. Else if the session is not empty the set the vary cookie.
7. Return response.

Related Ticket(s)
https://code.djangoproject.com/ticket/29471

[Tim Graham]

It looks like you intend to try a patch. You might find answers to your questions if there are test failures with your approach. You can either send a pull request or close this ticket depending on the outcome.

[Jd Collins]

Yes I did plan to try a patch, and did create one. After creating the patch I ran the tests and found one failing test. Before continuing on at this point I decided to think about the importance of the test and the impact my solution could have before writing tests that fit my approach. I came to the conclusion that it may be logical to always provide Vary: Cookie when using sessions and auth even if the session is not authenticated and empty. The problem with trying to not provide this header if the session is anonymous and empty is the cached response would also then apply to authenticated and non empty sessions which is likely worse and less manageable result. Although Vary: Cookie has limitless potential to diminish the effectiveness of caching since the cookie can be unique for so many reasons beyond Django's use of sessions/cookies. It seems a better approach would be to do some normalization at the cache layer or maybe ​https://github.com/rory/django-dont-vary-on. I have decided to close this ticket and provided my patch for completeness or anything it might offer to ticket #29471.