[Django] #29203: Cached Session may cause losing a session, when it failed to connect backend cache

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29203: Cached Session may cause losing a session, when it failed to connect backend cache

Django
#29203: Cached Session may cause losing a session, when it failed to connect
backend cache
-------------------------------------+-------------------------------------
               Reporter:  Kenial     |          Owner:  nobody
  Sookyum Lee                        |
                   Type:             |         Status:  new
  Uncategorized                      |
              Component:             |        Version:  master
  contrib.sessions                   |       Keywords:  session cookie,
               Severity:  Normal     |  cached session
           Triage Stage:             |      Has patch:  1
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 Some cache backends, Memcached and Redis, has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:22 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at that code
 without exception, even if cache nodes are unavailable. The reason is, I
 already explained above, they just work that way even if the cache backend
 failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.

--
Ticket URL: <https://code.djangoproject.com/ticket/29203>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/049.edb9502c640d38b5ea4de14d8f82dcfe%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect backend cache

Django
#29203: Cached Session may cause losing a session, when it failed to connect
backend cache
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Uncategorized        |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Kenial Sookyum Lee):

 * Attachment "patch_cached_session_deleting_session_cookie.diff" added.

 workaround patch

--
Ticket URL: <https://code.djangoproject.com/ticket/29203>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.c7fd4d7d1f045dc3e9b2bc99dc5569c3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect backend cache

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect
backend cache
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Kenial Sookyum Lee):

 * type:  Uncategorized => Bug


--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.41559f1190aced14d9e0f5cc22f9c0f3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect backend cache

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect
backend cache
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Kenial Sookyum Lee:

Old description:

> Some cache backends, Memcached and Redis, has a feature to ignore
> connection timeout exception, in order to ensure Django application
> working even if cache has failed. This can lead a subtle bug, deleting a
> session cookie. Following steps are reproduce this bug:
>
> - Set cached session for Django session (refer to
> https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
> sessions)
> - On Django app, try to log in or set a language in order to make a
> session cookie. At this step, cache nodes should be available.
> - To set cache nodes **unavailable**, by changing to wrong node name or
> turning the nodes off.
> - Reload a page on Django app. Django app responds back with this HTTP
> header, which deletes a session cookie:
> {{{
> Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
> Age=0; Path=/
> }}}
>
> The problematic codes are at django/contrib/sessions/middleware.py:22 :
> {{{
>  def process_response(self, request, response):
>         """
>         If request.session was modified, or if the configuration is to
> save the
>         session every time, save the changes and set a session cookie or
> delete
>         the session cookie if the session has been emptied.
>         """
>         try:
>             accessed = request.session.accessed
>             modified = request.session.modified
>             empty = request.session.is_empty()
>         except AttributeError:
>             pass
>         else:
>             # First check if we need to delete this cookie.
>             # The session should be deleted only if the session is
> entirely empty
>             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
>                 response.delete_cookie(
>                     settings.SESSION_COOKIE_NAME,
>                     path=settings.SESSION_COOKIE_PATH,
>                     domain=settings.SESSION_COOKIE_DOMAIN,
>                 )
> ...
> }}}
>
> I guess the initial intention was to delete a session if there is no
> values in it. However, it happens that code execution reaches at that
> code without exception, even if cache nodes are unavailable. The reason
> is, I already explained above, they just work that way even if the cache
> backend failed.
>
> I first met this bug while I was testing failover of AWS Elasticache
> (Redis). I was in testing of failover scenario, but Django application
> got me logged out repeatedly, even though session data itself is
> remaining in the cache replica. (it should, because it was doing
> failover, not reboot)
>
> IMHO, before checking ''empty'' of session data, there should be a logic
> to check cache backend is actually available. I found out
> ''request.session.cache_key'' can do that function, but it looks less
> explicit. Please show be a better way to do this, if you have one.
New description:

 Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:22 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at that code
 without exception, even if cache nodes are unavailable. The reason is, I
 already explained above, they just work that way even if the cache backend
 failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.4b36ae76ca55cdafe8160aa52fffae08%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend (was: Cached Session may cause losing a session, when it failed to connect backend cache)

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.04d1c428c55fd643c11d1496c5343add%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Kenial Sookyum Lee:

Old description:

> Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
> connection timeout exception, in order to ensure Django application
> working even if cache has failed. This can lead a subtle bug, deleting a
> session cookie. Following steps are reproduce this bug:
>
> - Set cached session for Django session (refer to
> https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
> sessions)
> - On Django app, try to log in or set a language in order to make a
> session cookie. At this step, cache nodes should be available.
> - To set cache nodes **unavailable**, by changing to wrong node name or
> turning the nodes off.
> - Reload a page on Django app. Django app responds back with this HTTP
> header, which deletes a session cookie:
> {{{
> Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
> Age=0; Path=/
> }}}
>
> The problematic codes are at django/contrib/sessions/middleware.py:22 :
> {{{
>  def process_response(self, request, response):
>         """
>         If request.session was modified, or if the configuration is to
> save the
>         session every time, save the changes and set a session cookie or
> delete
>         the session cookie if the session has been emptied.
>         """
>         try:
>             accessed = request.session.accessed
>             modified = request.session.modified
>             empty = request.session.is_empty()
>         except AttributeError:
>             pass
>         else:
>             # First check if we need to delete this cookie.
>             # The session should be deleted only if the session is
> entirely empty
>             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
>                 response.delete_cookie(
>                     settings.SESSION_COOKIE_NAME,
>                     path=settings.SESSION_COOKIE_PATH,
>                     domain=settings.SESSION_COOKIE_DOMAIN,
>                 )
> ...
> }}}
>
> I guess the initial intention was to delete a session if there is no
> values in it. However, it happens that code execution reaches at that
> code without exception, even if cache nodes are unavailable. The reason
> is, I already explained above, they just work that way even if the cache
> backend failed.
>
> I first met this bug while I was testing failover of AWS Elasticache
> (Redis). I was in testing of failover scenario, but Django application
> got me logged out repeatedly, even though session data itself is
> remaining in the cache replica. (it should, because it was doing
> failover, not reboot)
>
> IMHO, before checking ''empty'' of session data, there should be a logic
> to check cache backend is actually available. I found out
> ''request.session.cache_key'' can do that function, but it looks less
> explicit. Please show be a better way to do this, if you have one.
New description:

 Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:37 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at that code
 without exception, even if cache nodes are unavailable. The reason is, I
 already explained above, they just work that way even if the cache backend
 failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.58f7682a76aae99cd02e63d812b74993%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Kenial Sookyum Lee:

Old description:

> Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
> connection timeout exception, in order to ensure Django application
> working even if cache has failed. This can lead a subtle bug, deleting a
> session cookie. Following steps are reproduce this bug:
>
> - Set cached session for Django session (refer to
> https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
> sessions)
> - On Django app, try to log in or set a language in order to make a
> session cookie. At this step, cache nodes should be available.
> - To set cache nodes **unavailable**, by changing to wrong node name or
> turning the nodes off.
> - Reload a page on Django app. Django app responds back with this HTTP
> header, which deletes a session cookie:
> {{{
> Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
> Age=0; Path=/
> }}}
>
> The problematic codes are at django/contrib/sessions/middleware.py:37 :
> {{{
>  def process_response(self, request, response):
>         """
>         If request.session was modified, or if the configuration is to
> save the
>         session every time, save the changes and set a session cookie or
> delete
>         the session cookie if the session has been emptied.
>         """
>         try:
>             accessed = request.session.accessed
>             modified = request.session.modified
>             empty = request.session.is_empty()
>         except AttributeError:
>             pass
>         else:
>             # First check if we need to delete this cookie.
>             # The session should be deleted only if the session is
> entirely empty
>             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
>                 response.delete_cookie(
>                     settings.SESSION_COOKIE_NAME,
>                     path=settings.SESSION_COOKIE_PATH,
>                     domain=settings.SESSION_COOKIE_DOMAIN,
>                 )
> ...
> }}}
>
> I guess the initial intention was to delete a session if there is no
> values in it. However, it happens that code execution reaches at that
> code without exception, even if cache nodes are unavailable. The reason
> is, I already explained above, they just work that way even if the cache
> backend failed.
>
> I first met this bug while I was testing failover of AWS Elasticache
> (Redis). I was in testing of failover scenario, but Django application
> got me logged out repeatedly, even though session data itself is
> remaining in the cache replica. (it should, because it was doing
> failover, not reboot)
>
> IMHO, before checking ''empty'' of session data, there should be a logic
> to check cache backend is actually available. I found out
> ''request.session.cache_key'' can do that function, but it looks less
> explicit. Please show be a better way to do this, if you have one.
New description:

 Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:37 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
 # NEED TO UPDATE!
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at :37 that
 code without exception, even if cache nodes are unavailable. The reason
 is, I already explained above, they just work that way even if the cache
 backend failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.2e8d361a32324dc47cf8cbc179143de3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Kenial Sookyum Lee):

 When I tested the settings with SESSION_ENGINE =
 "django.contrib.sessions.backends.cached_db" (ie. 'write-through cache'),
 I didn't get through this bug. Configuration is: Django 1.11, MySQL
 5.6.35, mysqlclient 1.3.12, Redis 3.2.7 (x64), django-redis 4.9.0.

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.b0e7436ac91997ea76ee583eef3526b4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Kenial Sookyum Lee):

 * version:  master => 1.11


Old description:

> Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
> connection timeout exception, in order to ensure Django application
> working even if cache has failed. This can lead a subtle bug, deleting a
> session cookie. Following steps are reproduce this bug:
>
> - Set cached session for Django session (refer to
> https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
> sessions)
> - On Django app, try to log in or set a language in order to make a
> session cookie. At this step, cache nodes should be available.
> - To set cache nodes **unavailable**, by changing to wrong node name or
> turning the nodes off.
> - Reload a page on Django app. Django app responds back with this HTTP
> header, which deletes a session cookie:
> {{{
> Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
> Age=0; Path=/
> }}}
>
> The problematic codes are at django/contrib/sessions/middleware.py:37 :
> {{{
>  def process_response(self, request, response):
>         """
>         If request.session was modified, or if the configuration is to
> save the
>         session every time, save the changes and set a session cookie or
> delete
>         the session cookie if the session has been emptied.
>         """
>         try:
>             accessed = request.session.accessed
>             modified = request.session.modified
>             empty = request.session.is_empty()
>         except AttributeError:
>             pass
>         else:
>             # First check if we need to delete this cookie.
>             # The session should be deleted only if the session is
> entirely empty
>             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
> # NEED TO UPDATE!
>                 response.delete_cookie(
>                     settings.SESSION_COOKIE_NAME,
>                     path=settings.SESSION_COOKIE_PATH,
>                     domain=settings.SESSION_COOKIE_DOMAIN,
>                 )
> ...
> }}}
>
> I guess the initial intention was to delete a session if there is no
> values in it. However, it happens that code execution reaches at :37 that
> code without exception, even if cache nodes are unavailable. The reason
> is, I already explained above, they just work that way even if the cache
> backend failed.
>
> I first met this bug while I was testing failover of AWS Elasticache
> (Redis). I was in testing of failover scenario, but Django application
> got me logged out repeatedly, even though session data itself is
> remaining in the cache replica. (it should, because it was doing
> failover, not reboot)
>
> IMHO, before checking ''empty'' of session data, there should be a logic
> to check cache backend is actually available. I found out
> ''request.session.cache_key'' can do that function, but it looks less
> explicit. Please show be a better way to do this, if you have one.
New description:

 Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:37 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
 # NEED TO UPDATE!
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at :37 that
 code without exception, even if cache nodes are unavailable. The reason
 is, I already explained above, they just work that way even if the cache
 backend failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.


 fyi. Configuration is: Django 1.11, MySQL 5.6.35, mysqlclient 1.3.12,
 Redis 3.2.7 (x64), and django-redis 4.9.0. I found out this bug on Django
 1.11, but it has been changed since. So I believe this bug is applied to
 Django 2.x as well.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.66350ee9925e87e8d0f8b5d432bc4962%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Kenial Sookyum Lee:

Old description:

> Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
> connection timeout exception, in order to ensure Django application
> working even if cache has failed. This can lead a subtle bug, deleting a
> session cookie. Following steps are reproduce this bug:
>
> - Set cached session for Django session (refer to
> https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
> sessions)
> - On Django app, try to log in or set a language in order to make a
> session cookie. At this step, cache nodes should be available.
> - To set cache nodes **unavailable**, by changing to wrong node name or
> turning the nodes off.
> - Reload a page on Django app. Django app responds back with this HTTP
> header, which deletes a session cookie:
> {{{
> Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
> Age=0; Path=/
> }}}
>
> The problematic codes are at django/contrib/sessions/middleware.py:37 :
> {{{
>  def process_response(self, request, response):
>         """
>         If request.session was modified, or if the configuration is to
> save the
>         session every time, save the changes and set a session cookie or
> delete
>         the session cookie if the session has been emptied.
>         """
>         try:
>             accessed = request.session.accessed
>             modified = request.session.modified
>             empty = request.session.is_empty()
>         except AttributeError:
>             pass
>         else:
>             # First check if we need to delete this cookie.
>             # The session should be deleted only if the session is
> entirely empty
>             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
> # NEED TO UPDATE!
>                 response.delete_cookie(
>                     settings.SESSION_COOKIE_NAME,
>                     path=settings.SESSION_COOKIE_PATH,
>                     domain=settings.SESSION_COOKIE_DOMAIN,
>                 )
> ...
> }}}
>
> I guess the initial intention was to delete a session if there is no
> values in it. However, it happens that code execution reaches at :37 that
> code without exception, even if cache nodes are unavailable. The reason
> is, I already explained above, they just work that way even if the cache
> backend failed.
>
> I first met this bug while I was testing failover of AWS Elasticache
> (Redis). I was in testing of failover scenario, but Django application
> got me logged out repeatedly, even though session data itself is
> remaining in the cache replica. (it should, because it was doing
> failover, not reboot)
>
> IMHO, before checking ''empty'' of session data, there should be a logic
> to check cache backend is actually available. I found out
> ''request.session.cache_key'' can do that function, but it looks less
> explicit. Please show be a better way to do this, if you have one.
>

> fyi. Configuration is: Django 1.11, MySQL 5.6.35, mysqlclient 1.3.12,
> Redis 3.2.7 (x64), and django-redis 4.9.0. I found out this bug on Django
> 1.11, but it has been changed since. So I believe this bug is applied to
> Django 2.x as well.

New description:

 Some cache backends (AFAIK Memcached and Redis) has a feature to ignore
 connection timeout exception, in order to ensure Django application
 working even if cache has failed. This can lead a subtle bug, deleting a
 session cookie. Following steps are reproduce this bug:

 - Set cached session for Django session (refer to
 https://docs.djangoproject.com/en/2.0/topics/http/sessions/#using-cached-
 sessions)
 - On Django app, try to log in or set a language in order to make a
 session cookie. At this step, cache nodes should be available.
 - To set cache nodes **unavailable**, by changing to wrong node name or
 turning the nodes off.
 - Reload a page on Django app. Django app responds back with this HTTP
 header, which deletes a session cookie:
 {{{
 Set-Cookie:  sessionid=""; expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-
 Age=0; Path=/
 }}}

 The problematic codes are at django/contrib/sessions/middleware.py:37 :
 {{{
  def process_response(self, request, response):
         """
         If request.session was modified, or if the configuration is to
 save the
         session every time, save the changes and set a session cookie or
 delete
         the session cookie if the session has been emptied.
         """
         try:
             accessed = request.session.accessed
             modified = request.session.modified
             empty = request.session.is_empty()
         except AttributeError:
             pass
         else:
             # First check if we need to delete this cookie.
             # The session should be deleted only if the session is
 entirely empty
             if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
 # NEED TO UPDATE!
                 response.delete_cookie(
                     settings.SESSION_COOKIE_NAME,
                     path=settings.SESSION_COOKIE_PATH,
                     domain=settings.SESSION_COOKIE_DOMAIN,
                 )
 ...
 }}}

 I guess the initial intention was to delete a session if there is no
 values in it. However, it happens that code execution reaches at :37 that
 code without exception, even if cache nodes are unavailable. The reason
 is, I already explained above, they just work that way even if the cache
 backend failed.

 I first met this bug while I was testing failover of AWS Elasticache
 (Redis). I was in testing of failover scenario, but Django application got
 me logged out repeatedly, even though session data itself is remaining in
 the cache replica. (it should, because it was doing failover, not reboot)

 IMHO, before checking ''empty'' of session data, there should be a logic
 to check cache backend is actually available. I found out
 ''request.session.cache_key'' can do that function, but it looks less
 explicit. Please show be a better way to do this, if you have one.


 fyi. Configuration is: Django 1.11, MySQL 5.6.35, mysqlclient 1.3.12,
 Redis 3.2.7 (x64), and django-redis 4.9.0. I found out this bug on Django
 1.11 though, but it has not been changed since, so this bug must happen on
 Django 2.x as well.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.3213116137dabfb30b24454d7e4c4fd3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * has_patch:  1 => 0


Comment:

 I'm not sure if this is something that Django should implement. The patch
 doesn't look elegant because it's adding an implementation detail
 (`cache_key`) of a couple of the backends to code that's designed for all
 backends.

 As far as I know, Django generally [https://github.com/jazzband/django-
 constance/issues/236#issuecomment-346974084) requires the cache to be
 available]. It might be that we should document this requirement and
 discourage use of the "ignore connection timeout exception" option that
 you mentioned.

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.f28e253961d043fc9bb2d804de62ee31%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:
  cached session                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Kenial Sookyum Lee):

 Replying to [comment:9 Tim Graham]:
 > I'm not sure if this is something that Django should implement. The
 patch doesn't look elegant because it's adding an implementation detail
 (`cache_key`) of a couple of the backends to code that's designed for all
 backends.

 As long as there is another way to check if cache backend is available,
 it's okay. Any idea?

 > As far as I know, Django generally [https://github.com/jazzband/django-
 constance/issues/236#issuecomment-346974084) requires the cache to be
 available]. It might be that we should document this requirement and
 discourage use of the "ignore connection timeout exception" option that
 you mentioned.

 Agreed. But, the thing is, cache downtime itself is inevitable. As I told
 you, I found out this bug when I test failover AWS Elasticache - it means,
 this can happen during any kind of cache backend failover scenario. If
 "CAUTION: failover of cache backend might cause termination of Django
 sessions unintentionally" is in documentation, well... It sounds awkward.

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.e90d5a0e04a1a88e750347508608468f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29203: Cached Session may cause losing a session, when it failed to connect to cache backend

Django
In reply to this post by Django
#29203: Cached Session may cause losing a session, when it failed to connect to
cache backend
-------------------------------------+-------------------------------------
     Reporter:  Kenial Sookyum Lee   |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  contrib.sessions     |                  Version:  1.11
     Severity:  Normal               |               Resolution:
     Keywords:  session cookie,      |             Triage Stage:  Accepted
  cached session                     |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


Comment:

 I'm not sure what to do, but your report seems credible.

--
Ticket URL: <https://code.djangoproject.com/ticket/29203#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/064.e4d47eba841434dc1fbb92f2dc12c2ec%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.