pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
507 stars 238 forks source link

Session is established and saved for each new request #19

Closed rmoorman closed 9 years ago

rmoorman commented 9 years ago

I just noticed this behavior while using the Filesystem sessions of Flask-Session. On each new request, a new session file is written with a new key.

Take the following example application

from flask import Flask
from flask.ext.session import Session

app = Flask(__name__)
app.config["SECRET_KEY"] = "__replacewithsomethingunique__"
app.config["SESSION_TYPE"] = "filesystem"

Session(app)

app.run(port=8001)

When I run this code and issue several curl requests, I receive a different session cookie header on each request.

$ curl -is localhost:8001 | grep Set-Cookie
Set-Cookie: session=2a2d9266-deaa-4412-82de-48193503534e; Expires=Mon, 12-Oct-2015 17:46:02 GMT; HttpOnly; Path=/

$ curl -is localhost:8001 | grep Set-Cookie
Set-Cookie: session=a98eebd4-909c-4c42-b68c-3cae7750e390; Expires=Mon, 12-Oct-2015 17:46:02 GMT; HttpOnly; Path=/

$ curl -is localhost:8001 | grep Set-Cookie
Set-Cookie: session=e547e8d2-54de-4359-a4a5-bcf206e88c1e; Expires=Mon, 12-Oct-2015 17:46:03 GMT; HttpOnly; Path=/

$ curl -is localhost:8001 | grep Set-Cookie
Set-Cookie: session=69e78241-5c2e-4bf4-936d-35931cb81e8c; Expires=Mon, 12-Oct-2015 17:46:03 GMT; HttpOnly; Path=/

After which the flask_session directory is filled with several session files

$ ls -1 flask_session/
113e38a76a00f2ebb4912fa547d75ec9
3184c223a385dd63ce728b9e8b778f5a
a62fce576b1734cc77fdd1302e815916
b242e7911524d66023836aaf9847b6fd

This differs from the default flask session behavior. By default flask does not send a Set-Cookie header if nothing happens to the session. Obviously this could cause some trouble.

If you have more question about this issue or if there is something else I can do to help solving it please let me know.

fengsp commented 9 years ago

Maybe you should try it in a real browser, and I believe Flask has the same behavior.

rmoorman commented 9 years ago

Hello @fengsp,

Flask does not expose this behavior by default. If I use a simple app like this (which is basically my first code example without the flask-session things):

from flask import Flask

app = Flask(__name__)
app.config["SECRET_KEY"] = "__replacewithsomethingunique__"

app.run(port=8001)

I don't receive any set-cookie header. I am using Werkzeug/0.10.4 Python/3.4.3 Flask/0.10.1 and Flask-Session/0.2.1 on Linux by the way.

A real browser (google chrome in my case) triggers the same behaviors.

This actually is a problem as sessions expire way too early this way as the session directory is filled up with session files which should not be there in the first place as far as I can tell.

fengsp commented 9 years ago

Show me the code, all of them.

rmoorman commented 9 years ago

Sure. Have a look at the code over here https://github.com/rmoorman/flask-session-example

I added the two examples which exhibit the behavior along with a requirements.txt which matches the versions of flask and flask-session I am currently using.

What's in there:

It's a matter of just starting the apps and using curl/a browser or the like to fire a few requests and inspect the headers sent back as well as the contents of the flask_session directory.

And I really hope you can confirm what is happening over here :-)

Best regards, Rico Moorman

fengsp commented 9 years ago

The problem here is that you are using Flask default session in non-permanent mode, which means once you close your browser, your session is gone.

If this is what you want, you should use Flask itself without extension.

If this is not what you want, then your Flask demo is wrong, because you didn't use a permanent session.

Currently, all sessions in Flask-Session are permanent, check http://pythonhosted.org/Flask-Session/ for more details, which notes you that All non-null sessions in Flask-Session are permanent.

rmoorman commented 9 years ago

Hello @fengsp,

Thank you for your quick reply! I actually read the documentation. When I read the note you are quoting though, it reads to me as if all sessions which are actually filled with some data (non-null) would then be permanent. But as far as I can see, it is not a non-null session we have here. It is a session which does not contain anything (at least I didn't do anything to set some data on it within the example code).

Flask itself saves a session only if there have been any additions to it. Flask-Session alters this behavior currently to always save sessions. Even though there haven't been any actions which would justify a session save IMHO.

I agree that a once saved server-side session should be permanent (in a configurable way though), I fail to see that starting and saving an empty session with each request is correct behavior.

Wouldn't it be better to save the session and make it permanent only if there actually is session data to save?

Best regards, Rico Moorman

fengsp commented 9 years ago

If you use Flask default session as a permanent session, it will set cookie for an empty session too. It needed to be fixed in Flask itself if necessary.

rmoorman commented 9 years ago

Hello @fengsp,

Thank you for your response. The point is that if we do so with default flask sessions, we choose to do it. I actually always set it to permanent once I want to save something to the session. If I forget to do that, the session is not permanent but then it would be due to my implementation.

With flask-session enabled on the other hand, it is beyond our control when the session will be saved. It is always saved, even though we didn't add anything to the session.

I don't know how all the other session storage backends behave (besides redis, see below), but as for the filesystem sessions, the flask_session directory is quickly filled up which triggers the threshold value for cleanup of the underlying filesystem cache and the "permanent" sessions are suddenly not that permanent anymore.

I also tested the redis session backend and running curl within a loop (while true; do curl -i localhost:8000; done;) against the app causes redis to fill up too, but then without cleanup as it seems, which could cause other problems too I guess.

Furthermore I took a quick look at the internals of the session interface and implementation and changed the ServerSideSession a bit over here, but I could only make it work partially right now and would rather have the author of this library to have a look how this could be solved correctly.

Could you please have a look at how this behavior could be adjusted?

Best regards, Rico Moorman

fengsp commented 9 years ago

You could clear the session manually.

rmoorman commented 9 years ago

That wouldn't be a good way to move forward I suppose. It just doesn't feel right to randomly remove sessions which shouldn't be there in the first place, don't you agree?

Do you see any technical problems (in werkzeug or flask-session for that matter) which make it difficult to just set the session to non-permanent initially and set it permanent/save it in case something is added? Do you have any pointers or advice that could help in case I would like to fix it myself?

fengsp commented 9 years ago

I just released Flask-Session 0.2.2 and added support for non-permanent session.

rmoorman commented 9 years ago

Hello @fengsp,

I just tried the new version and the new setting SESSION_PERMANENT works as expected over here. Thank you for that and keep up the good work!

Best regards, Rico Moorman