Unhandled Exception in save_session() for Redis.

[b-i-l-l]

https://github.com/pallets-eco/flask-session/blob/main/src/flask_session/sessions.py#L135

The following statements cause a traceback when session is None.

if not session:
    if session.modified:

I have not traced through the code, but there needs to be a test for None. The boolean evaluation of session is insufficient.

[christopherpickering]

What python version are you using?

Check this out: https://www.online-python.com/xuPalfqrUY

[b-i-l-l]

3.11

I see your point about the boolean evaluation, however there still seems to be a glitch. If session is None, then reading None.modified raises an AttributeError. Either session is erroneously set to None or a check for None is needed prior to reading an attribute.

[christopherpickering]

Well the logic is backwards right?, it should probably be

if session:
   if session.modified

?

that line is built to break :)

[ThiefMaster]

not session checks for an empty session. So it can be reasonable to check if the session is empty but has just been emptied ("empty and modified").

And in fact in save_session this is exactly what the code is checking for, because in this situation you want to delete the session from the storage backend.

Also: Is save_session ever called with a None value for session? I see lots of discussion here but not a single traceback where this ACTUALLY happens. According to the type annotation in Flask this does not happen:

    def save_session(
        self, app: Flask, session: SessionMixin, response: Response
    ) -> None:

[b-i-l-l]

Type annotations are not enforced at runtime. Tools like mypy do the actual checking. Here is a traceback.

ConnectionResetError: [Errno 104] Connection reset by peer
  File "redis/connection.py", line 812, in read_response
    response = self._parser.read_response(disable_decoding=disable_decoding)
  File "redis/connection.py", line 318, in read_response
    raw = self._buffer.readline()
  File "redis/connection.py", line 249, in readline
    self._read_from_socket()
  File "redis/connection.py", line 192, in _read_from_socket
    data = self._sock.recv(socket_read_size)
ConnectionError: Error while reading from indexing-b4bee79.7vb13t.0001.use2.cache.amazonaws.com:6379 : (104, 'Connection reset by peer')
  File "flask/app.py", line 2524, in wsgi_app
    ctx.push()
  File "flask/ctx.py", line 375, in push
    self.session = session_interface.open_session(self.app, self.request)
  File "flask_session/sessions.py", line 133, in open_session
    val = self.redis.get(self.key_prefix + sid)
  File "redis/commands/core.py", line 1728, in get
    return self.execute_command("GET", name)
  File "redis/client.py", line 1258, in execute_command
    return conn.retry.call_with_retry(
  File "redis/retry.py", line 49, in call_with_retry
    fail(error)
  File "redis/client.py", line 1262, in <lambda>
    lambda error: self._disconnect_raise(conn, error),
  File "redis/client.py", line 1248, in _disconnect_raise
    raise error
  File "redis/retry.py", line 46, in call_with_retry
    return do()
  File "redis/client.py", line 1259, in <lambda>
    lambda: self._send_command_parse_response(
  File "redis/client.py", line 1235, in _send_command_parse_response
    return self.parse_response(conn, command_name, **options)
  File "redis/client.py", line 1275, in parse_response
    response = connection.read_response()
  File "redis/connection.py", line 818, in read_response
    raise ConnectionError(f"Error while reading from {hosterr}" f" : {e.args}")
AttributeError: 'NoneType' object has no attribute 'modified'
  File "flask/app.py", line 1844, in finalize_request
    response = self.process_response(response)
  File "flask/app.py", line 2340, in process_response
    self.session_interface.save_session(self, ctx.session, response)
  File "flask_session/sessions.py", line 146, in save_session
    if session.modified:

[ThiefMaster]

Sure, my point was that according to the type annotations it's not supposed to be None. I don't think the session should be None when reading data from redis fails. And that would be an unrelated bug in here.

[christopherpickering]

@b-i-l-l do you have example code that can reproduce it?

[b-i-l-l]

Unfortunately I do not have code that can reproduce this behavior. The traceback was taken from Sentry. It occurs sporadically. That's the best I can do.