Skip to content
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

SqlAlchemy: TypeError if SESSION_PERMANENT=False #67

Closed
Ardius opened this issue Apr 11, 2017 · 15 comments
Closed

SqlAlchemy: TypeError if SESSION_PERMANENT=False #67

Ardius opened this issue Apr 11, 2017 · 15 comments

Comments

@Ardius
Copy link

Ardius commented Apr 11, 2017

Using SqlAlchemy (both with mysql and sqlite) if I set SESSION_PERMANENT to False the system return a TypeError

File "/home/draio/project/venv/lib/python2.7/site-packages/flask_session/sessions.py", line 516, in open_session
     if saved_session and saved_session.expiry <= datetime.utcnow():
 TypeError: can't compare datetime.datetime to NoneType

Changing the SESSION_TYPE in filesystem the system works and the cookie will be deleted when the browser is closed.

On the DB table the session row has the expiry row empty.
Expiry comes from expires variable tha is set on row 547:
expires = self.get_expiration_time(app, session)

get_expiration_time is a flask fuction that returns the expiration date for the session:

 if session.permanent:
            return datetime.utcnow() + app.permanent_session_lifetime

That's why the expiry field on sessions table is empty, a cookie set as non-permanent doesn't have a expiration date. So it's not possible for mysql and sqlite to check a NULL date.

To populate the expiry field in the DB table I changed:
expires = self.get_expiration_time(app, session)
into

expires = self.get_expiration_time(app, session) \
           or datetime.utcnow() + app.permanent_session_lifetime

No more error but if i close the browser the ,session cookie is still there.
That's because the set_cookie function must have expires empty to delete the cookie when browser is closed.

So I changed:
expires = self.get_expiration_time(app, session)
into

expires = self.get_expiration_time(app, session) \
           or datetime.utcnow() + app.permanent_session_lifetime
expires_cookie=expires if self.permanent else ''

And on line 561 I changed:

response.set_cookie(app.session_cookie_name, session_id,
                    expires=expires, httponly=httponly,
                    domain=domain, path=path, secure=secure)

into

response.set_cookie(app.session_cookie_name, session_id,
                    expires=expires_cookie, httponly=httponly,
                    domain=domain, path=path, secure=secure)

I hope that this could by helpful and clear.


UPDATE

Another way, more elegant, to solve the problem is to check self.permanent before checking if saved_session.expire is outdated.

On line 516 change
if saved_session and saved_session.expiry <= datetime.utcnow():
into something like
if self.permanent and saved_session and saved_session.expiry <= datetime.utcnow():

Also this solution works but, if I'm not wrong, in this way the session will never be deleted from the DB table and, more important, it will never be tested. That's could be a security problem.

In the other solution the expiration date is written to the DB. In that case the problem may be that for a time a cookie remains active even if on the browser has already been deleted. I do not know if this could be considered a security problem.


UPDATE IE11 issue

On IE11 you cannot have a Expires=;, otherwise the cookie will never be stored.
For now I solved brutally with an if on

if not saved_session:
     if expires_cookie:
         response.set_cookie(app.session_cookie_name, session_id,
                                          expires=expires_cookie, httponly=httponly,
                                          domain=domain, path=path, secure=secure)
     else:
         response.set_cookie(app.session_cookie_name, session_id,
                                           httponly=httponly,
                                           domain=domain, path=path, secure=secure)
@NightBlues
Copy link

Same bug for mongodb backend.

@nagappan
Copy link

I made this fix for Mongo when testing with curl

    if document:
        expiration = document.get('expiration')
    else:
        expiration = None
    if document and expiration and expiration <= datetime.utcnow():

Worked for me

@zardilior
Copy link

@negappan could you pull request?

@macTracyHuang
Copy link

Same issue in PostgreSql

@DLAcoding
Copy link

Same here, solved with:

if saved_session and saved_session.expiry:
    if saved_session.expiry<= datetime.utcnow():

@manugarri
Copy link

is there any way to fix this without changing the app code? We are experiencing this bug on AWS Airflow 2.4.3 and we cant really change airflow's version there

@Danipiario
Copy link

I have the same problem with a fresh installation on AWS Airflow 2.4.3
I've substituted in mwwa requirements flask-session lib with Flask-Session2 (https://github.com/christopherpickering/flask-session2), the code was changed in release 1.2.0 (christopherpickering#4) due to this bug

@manugarri
Copy link

For what is worth, clearing browser data seems to 'fix' the issue

@Danipiario
Copy link

Yes, clearing the session data fix the access but temporarily until the session expires again. I've experienced the issue more times

@giom-l
Copy link

giom-l commented Jan 25, 2023

Hi @Danipiario ,
Did the update of flask-session lib in mwaa requirements fix the issue ?
Or are you still encountering it in mwaa 2.4.3 despite the update ?

@Danipiario
Copy link

So far the workaround works

@wilminator
Copy link

wilminator commented Mar 12, 2023

FYI I have found a fork of this project on Pypi flask-session2 that is actively maintained and does not have this problem. I was able to use it in my project as a drop-in replacement.

@Lxstr
Copy link
Contributor

Lxstr commented Feb 25, 2024

Fixed since 0.6.0. Please report any further issues

@Lxstr Lxstr closed this as completed Feb 25, 2024
@HammadAwan424
Copy link

It doesn't give error but now nothing interesting happens. After setting up session, it returns None if you try to retrieve session data.

@Lxstr
Copy link
Contributor

Lxstr commented Mar 16, 2024

@HammadAwan424 can you show me a minimum example? Also can you try 0.7.0rc2?

If you are upgrading from 0.5.0 while there are existing sessions, it will remove those sessions that had expiry None.

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

No branches or pull requests