mbr / flask-kvsession

A drop-in replacement for Flask's session handling using server-side sessions.
http://pythonhosted.org/Flask-KVSession/
MIT License
168 stars 53 forks source link

cookie/session getting lost when session not modified during a request #27

Closed whacked closed 9 years ago

whacked commented 9 years ago

When using this as a drop-in replacement for flask's default session object, the cookie can unexpectedly get lost if a request is made without a modification to the session.

When making requests via browser loads, the behavior is not reproducible, probably becuase browsers manage their own cookies across requests such that when an empty cookie is received, the old cookie is used.

The behavior is reproducible if you manage cookies manually on a per-request basis e.g. via requests or curl

Example:

server.py


from flask import Flask, session, request
from flask_kvsession import KVSessionExtension
from simplekv.memory.redisstore import RedisStore
import redis

app = Flask(__name__)
app.secret_key = 'foo'
# toggle for default session
KVSessionExtension(RedisStore(redis.StrictRedis()), app)
@app.route('/')
def index():
    if 'x' not in session:
        session['x'] = 0
    return str(session['x'])
@app.route('/test')
def test():
    session['x'] = session.get('x', 0) + 1
    return str(session['x'])
app.run(debug=True)

client.py

import requests
gcookie = {}
for i in range(10):
    res = requests.get('http://localhost:5000/' + (i%2==0 and 'test' or ''),
                       cookies = gcookie,)
    gcookie = res.cookies
    print res.content

Start server.py then client.py, comparing default sessions with kvsession

expected behavior (default sessions): client will print increasing numbers like 1 1 2 2 3 3 ...

observed behavior (kvsession): client will print 1 1 1 1 1 1...

This seems to happen when session.modified is False, in save_session the response won't contain the session cookie. One can either store the session cookie in the client and continue using it when one isn't returned, or by forcing a session.modified = True in e.g. before_request.

Would it be a good idea to add some carryover behavior for when the session is not modified, to maintain behavior parity with the default session? Or is it standard to just leave it to the client? Thanks!

justinfay commented 9 years ago

@whacked From a quick look at this I think you may not be setting something correctly in your request as using a requests.Session object the behaviour is as expected.

import requests
session = requests.Session()
for i in range(10):
    res = session.get('http://localhost:5000/' + (i % 2 == 0 and 'test' or ''))
    print res.content

Which prints the expected output when run in a terminal

% python client.py 
1
1
2
2
3
3
4
4
5
5
mbr commented 9 years ago

8e9e28a has a test that replicates justinfay's result and compares it against "stock behaviour". I'm sorry, but I cannot reproduce this issue at this moment.

whacked commented 9 years ago

@mbr @justinfay I was actually looking into this yesterday but didn't have time to look much deeper. In short, I agree with @justinfay that I should be using requests.Session instead and agree with you closing this issue. Thanks both for your responses.