# HG changeset patch # User Sylvain Thénault # Date 1280327492 -7200 # Node ID 09db6f4619c97c042da5736fd31fa6269f6a2f10 # Parent 52f9a43d8e084ea5148e8ce0ef09b6f582dee21b# Parent 4b721e739f530ab35c29b81f88d6a53951e10db5 backport stable diff -r 4b721e739f53 -r 09db6f4619c9 dbapi.py --- a/dbapi.py Wed Jul 28 16:27:57 2010 +0200 +++ b/dbapi.py Wed Jul 28 16:31:32 2010 +0200 @@ -313,19 +313,17 @@ # low level session data management ####################################### - def get_shared_data(self, key, default=None, pop=False): - """return value associated to `key` in shared data""" - return self.cnx.get_shared_data(key, default, pop) - - def set_shared_data(self, key, value, querydata=False): - """set value associated to `key` in shared data + def get_shared_data(self, key, default=None, pop=False, txdata=False): + """see :meth:`Connection.get_shared_data`""" + return self.cnx.get_shared_data(key, default, pop, txdata) - if `querydata` is true, the value will be added to the repository - session's query data which are cleared on commit/rollback of the current - transaction, and won't be available through the connexion, only on the - repository side. - """ - return self.cnx.set_shared_data(key, value, querydata) + def set_shared_data(self, key, value, txdata=False, querydata=None): + """see :meth:`Connection.set_shared_data`""" + if querydata is not None: + txdata = querydata + warn('[3.10] querydata argument has been renamed to txdata', + DeprecationWarning, stacklevel=2) + return self.cnx.set_shared_data(key, value, txdata) # server session compat layer ############################################# @@ -507,10 +505,12 @@ return DBAPIRequest(self.vreg, DBAPISession(self)) def check(self): - """raise `BadConnectionId` if the connection is no more valid""" + """raise `BadConnectionId` if the connection is no more valid, else + return its latest activity timestamp. + """ if self._closed is not None: raise ProgrammingError('Closed connection') - self._repo.check_session(self.sessionid) + return self._repo.check_session(self.sessionid) def set_session_props(self, **props): """raise `BadConnectionId` if the connection is no more valid""" @@ -518,23 +518,29 @@ raise ProgrammingError('Closed connection') self._repo.set_session_props(self.sessionid, props) - def get_shared_data(self, key, default=None, pop=False): - """return value associated to `key` in shared data""" - if self._closed is not None: - raise ProgrammingError('Closed connection') - return self._repo.get_shared_data(self.sessionid, key, default, pop) + def get_shared_data(self, key, default=None, pop=False, txdata=False): + """return value associated to key in the session's data dictionary or + session's transaction's data if `txdata` is true. - def set_shared_data(self, key, value, querydata=False): - """set value associated to `key` in shared data + If pop is True, value will be removed from the dictionnary. - if `querydata` is true, the value will be added to the repository - session's query data which are cleared on commit/rollback of the current - transaction, and won't be available through the connexion, only on the - repository side. + If key isn't defined in the dictionnary, value specified by the + `default` argument will be returned. """ if self._closed is not None: raise ProgrammingError('Closed connection') - return self._repo.set_shared_data(self.sessionid, key, value, querydata) + return self._repo.get_shared_data(self.sessionid, key, default, pop, txdata) + + def set_shared_data(self, key, value, txdata=False): + """set value associated to `key` in shared data + + if `txdata` is true, the value will be added to the repository session's + transaction's data which are cleared on commit/rollback of the current + transaction. + """ + if self._closed is not None: + raise ProgrammingError('Closed connection') + return self._repo.set_shared_data(self.sessionid, key, value, txdata) def get_schema(self): """Return the schema currently used by the repository. diff -r 4b721e739f53 -r 09db6f4619c9 etwist/twconfig.py --- a/etwist/twconfig.py Wed Jul 28 16:27:57 2010 +0200 +++ b/etwist/twconfig.py Wed Jul 28 16:31:32 2010 +0200 @@ -76,12 +76,6 @@ the repository rather than the user running the command', 'group': 'main', 'level': WebConfiguration.mode == 'system' }), - ('session-time', - {'type' : 'time', - 'default': '30min', - 'help': 'session expiration time, default to 30 minutes', - 'group': 'main', 'level': 1, - }), ('pyro-server', {'type' : 'yn', # pyro is only a recommends by default, so don't activate it here diff -r 4b721e739f53 -r 09db6f4619c9 misc/migration/3.10.0_common.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/misc/migration/3.10.0_common.py Wed Jul 28 16:31:32 2010 +0200 @@ -0,0 +1,1 @@ +option_group_changed('cleanup-session-time', 'web', 'main') diff -r 4b721e739f53 -r 09db6f4619c9 server/repository.py --- a/server/repository.py Wed Jul 28 16:27:57 2010 +0200 +++ b/server/repository.py Wed Jul 28 16:31:32 2010 +0200 @@ -267,7 +267,10 @@ # call instance level initialisation hooks self.hm.call_hooks('server_startup', repo=self) # register a task to cleanup expired session - self.looping_task(self.config['session-time']/3., self.clean_sessions) + self.cleanup_session_time = self.config['cleanup-session-time'] or 60 * 60 * 24 + assert self.cleanup_session_time > 0 + cleanup_session_interval = min(60*60, self.cleanup_session_time / 3) + self.looping_task(cleanup_session_interval, self.clean_sessions) assert isinstance(self._looping_tasks, list), 'already started' for i, (interval, func, args) in enumerate(self._looping_tasks): self._looping_tasks[i] = task = utils.LoopTask(interval, func, args) @@ -622,24 +625,32 @@ session.reset_pool() def check_session(self, sessionid): - """raise `BadConnectionId` if the connection is no more valid""" - self._get_session(sessionid, setpool=False) + """raise `BadConnectionId` if the connection is no more valid, else + return its latest activity timestamp. + """ + return self._get_session(sessionid, setpool=False).timestamp + + def get_shared_data(self, sessionid, key, default=None, pop=False, txdata=False): + """return value associated to key in the session's data dictionary or + session's transaction's data if `txdata` is true. - def get_shared_data(self, sessionid, key, default=None, pop=False): - """return the session's data dictionary""" + If pop is True, value will be removed from the dictionnary. + + If key isn't defined in the dictionnary, value specified by the + `default` argument will be returned. + """ session = self._get_session(sessionid, setpool=False) - return session.get_shared_data(key, default, pop) + return session.get_shared_data(key, default, pop, txdata) - def set_shared_data(self, sessionid, key, value, querydata=False): + def set_shared_data(self, sessionid, key, value, txdata=False): """set value associated to `key` in shared data - if `querydata` is true, the value will be added to the repository - session's query data which are cleared on commit/rollback of the current - transaction, and won't be available through the connexion, only on the - repository side. + if `txdata` is true, the value will be added to the repository session's + transaction's data which are cleared on commit/rollback of the current + transaction. """ session = self._get_session(sessionid, setpool=False) - session.set_shared_data(key, value, querydata) + session.set_shared_data(key, value, txdata) def commit(self, sessionid, txid=None): """commit transaction for the session with the given id""" @@ -771,7 +782,7 @@ """close sessions not used since an amount of time specified in the configuration """ - mintime = time() - self.config['session-time'] + mintime = time() - self.cleanup_session_time self.debug('cleaning session unused since %s', strftime('%T', localtime(mintime))) nbclosed = 0 diff -r 4b721e739f53 -r 09db6f4619c9 server/serverconfig.py --- a/server/serverconfig.py Wed Jul 28 16:27:57 2010 +0200 +++ b/server/serverconfig.py Wed Jul 28 16:31:32 2010 +0200 @@ -120,10 +120,16 @@ the repository rather than the user running the command', 'group': 'main', 'level': (CubicWebConfiguration.mode == 'installed') and 0 or 1, }), - ('session-time', + ('cleanup-session-time', {'type' : 'time', - 'default': '30min', - 'help': 'session expiration time, default to 30 minutes', + 'default': '24h', + 'help': 'duration of inactivity after which a session ' + 'will be closed, to limit memory consumption (avoid sessions that ' + 'never expire and cause memory leak when http-session-time is 0, or ' + 'because of bad client that never closes their connection). ' + 'So notice that even if http-session-time is 0 and the user don\'t ' + 'close his browser, he will have to reauthenticate after this time ' + 'of inactivity. Default to 24h.', 'group': 'main', 'level': 3, }), ('connections-pool-size', diff -r 4b721e739f53 -r 09db6f4619c9 server/session.py --- a/server/session.py Wed Jul 28 16:27:57 2010 +0200 +++ b/server/session.py Wed Jul 28 16:31:32 2010 +0200 @@ -618,16 +618,20 @@ # shared data handling ################################################### - def get_shared_data(self, key, default=None, pop=False): + def get_shared_data(self, key, default=None, pop=False, txdata=False): """return value associated to `key` in session data""" - if pop: - return self.data.pop(key, default) + if txdata: + data = self.transaction_data else: - return self.data.get(key, default) + data = self.data + if pop: + return data.pop(key, default) + else: + return data.get(key, default) - def set_shared_data(self, key, value, querydata=False): + def set_shared_data(self, key, value, txdata=False): """set value associated to `key` in session data""" - if querydata: + if txdata: self.transaction_data[key] = value else: self.data[key] = value diff -r 4b721e739f53 -r 09db6f4619c9 server/test/unittest_repository.py --- a/server/test/unittest_repository.py Wed Jul 28 16:27:57 2010 +0200 +++ b/server/test/unittest_repository.py Wed Jul 28 16:31:32 2010 +0200 @@ -161,7 +161,7 @@ def test_check_session(self): repo = self.repo cnxid = repo.connect(self.admlogin, password=self.admpassword) - self.assertEquals(repo.check_session(cnxid), None) + self.assertIsInstance(repo.check_session(cnxid), float) repo.close(cnxid) self.assertRaises(BadConnectionId, repo.check_session, cnxid) diff -r 4b721e739f53 -r 09db6f4619c9 web/application.py --- a/web/application.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/application.py Wed Jul 28 16:31:32 2010 +0200 @@ -31,7 +31,7 @@ from cubicweb import set_log_methods, cwvreg from cubicweb import ( ValidationError, Unauthorized, AuthenticationError, NoSelectableObject, - RepositoryError, CW_EVENT_MANAGER) + RepositoryError, BadConnectionId, CW_EVENT_MANAGER) from cubicweb.dbapi import DBAPISession from cubicweb.web import LOGGER, component from cubicweb.web import ( @@ -48,48 +48,43 @@ def __init__(self, vreg): self.session_time = vreg.config['http-session-time'] or None - if self.session_time is not None: - assert self.session_time > 0 - self.cleanup_session_time = self.session_time - else: - self.cleanup_session_time = vreg.config['cleanup-session-time'] or 1440 * 60 - assert self.cleanup_session_time > 0 - self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60 - assert self.cleanup_anon_session_time > 0 self.authmanager = vreg['components'].select('authmanager', vreg=vreg) + interval = (self.session_time or 0) / 2. if vreg.config.anonymous_user() is not None: - self.clean_sessions_interval = max( - 5 * 60, min(self.cleanup_session_time / 2., - self.cleanup_anon_session_time / 2.)) - else: - self.clean_sessions_interval = max( - 5 * 60, - self.cleanup_session_time / 2.) + self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60 + assert self.cleanup_anon_session_time > 0 + if self.session_time is not None: + self.cleanup_anon_session_time = min(self.session_time, + self.cleanup_anon_session_time) + interval = self.cleanup_anon_session_time / 2. + # we don't want to check session more than once every 5 minutes + self.clean_sessions_interval = max(5 * 60, interval) def clean_sessions(self): """cleanup sessions which has not been unused since a given amount of time. Return the number of sessions which have been closed. """ self.debug('cleaning http sessions') + session_time = self.session_time closed, total = 0, 0 for session in self.current_sessions(): - no_use_time = (time() - session.last_usage_time) total += 1 - if session.anonymous_session: - if no_use_time >= self.cleanup_anon_session_time: + try: + last_usage_time = session.cnx.check() + except BadConnectionId: + self.close_session(session) + closed += 1 + else: + no_use_time = (time() - last_usage_time) + if session.anonymous_session: + if no_use_time >= self.cleanup_anon_session_time: + self.close_session(session) + closed += 1 + elif session_time is not None and no_use_time >= session_time: self.close_session(session) closed += 1 - elif no_use_time >= self.cleanup_session_time: - self.close_session(session) - closed += 1 return closed, total - closed - def has_expired(self, session): - """return True if the web session associated to the session is expired - """ - return not (self.session_time is None or - time() < session.last_usage_time + self.session_time) - def current_sessions(self): """return currently open sessions""" raise NotImplementedError() @@ -213,8 +208,6 @@ except AuthenticationError: req.remove_cookie(cookie, self.SESSION_VAR) raise - # remember last usage time for web session tracking - session.last_usage_time = time() def get_session(self, req, sessionid): return self.session_manager.get_session(req, sessionid) @@ -224,8 +217,6 @@ cookie = req.get_cookie() cookie[self.SESSION_VAR] = session.sessionid req.set_cookie(cookie, self.SESSION_VAR, maxage=None) - # remember last usage time for web session tracking - session.last_usage_time = time() if not session.anonymous_session: self._postlogin(req) return session diff -r 4b721e739f53 -r 09db6f4619c9 web/test/unittest_session.py --- a/web/test/unittest_session.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/test/unittest_session.py Wed Jul 28 16:31:32 2010 +0200 @@ -7,10 +7,11 @@ :license: GNU Lesser General Public License, v2.1 - http://www.gnu.org/licenses """ from cubicweb.devtools.testlib import CubicWebTC +from cubicweb.web import InvalidSession class SessionTC(CubicWebTC): - def test_auto_reconnection(self): + def test_session_expiration(self): sm = self.app.session_handler.session_manager # make is if the web session has been opened by the session manager sm._sessions[self.cnx.sessionid] = self.websession @@ -23,11 +24,8 @@ # fake an incoming http query with sessionid in session cookie # don't use self.request() which try to call req.set_session req = self.requestcls(self.vreg) - websession = sm.get_session(req, sessionid) - self.assertEquals(len(sm._sessions), 1) - self.assertIs(websession, self.websession) - self.assertEquals(websession.sessionid, sessionid) - self.assertNotEquals(websession.sessionid, websession.cnx.sessionid) + self.assertRaises(InvalidSession, sm.get_session, req, sessionid) + self.assertEquals(len(sm._sessions), 0) finally: # avoid error in tearDown by telling this connection is closed... self.cnx._closed = True diff -r 4b721e739f53 -r 09db6f4619c9 web/views/authentication.py --- a/web/views/authentication.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/views/authentication.py Wed Jul 28 16:31:32 2010 +0200 @@ -74,7 +74,7 @@ self.repo = vreg.config.repository(vreg) self.log_queries = vreg.config['query-log-file'] self.authinforetreivers = sorted(vreg['webauth'].possible_objects(vreg), - key=lambda x: x.order) + key=lambda x: x.order) # 2-uple login / password, login is None when no anonymous access # configured self.anoninfo = vreg.config.anonymous_user() @@ -98,25 +98,11 @@ if login and session.login != login: raise InvalidSession('login mismatch') try: - lock = session.reconnection_lock - except AttributeError: - lock = session.reconnection_lock = Lock() - # need to be locked two avoid duplicated reconnections on concurrent - # requests - with lock: - cnx = session.cnx - try: - # calling cnx.user() check connection validity, raise - # BadConnectionId on failure - user = cnx.user(req) - except BadConnectionId: - # check if a connection should be automatically restablished - if (login is None or login == session.login): - cnx = self._authenticate(session.login, session.authinfo) - user = cnx.user(req) - session.cnx = cnx - else: - raise InvalidSession('bad connection id') + # calling cnx.user() check connection validity, raise + # BadConnectionId on failure + user = session.cnx.user(req) + except BadConnectionId: + raise InvalidSession('bad connection id') return user def authenticate(self, req): diff -r 4b721e739f53 -r 09db6f4619c9 web/views/debug.py --- a/web/views/debug.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/views/debug.py Wed Jul 28 16:31:32 2010 +0200 @@ -119,10 +119,15 @@ if sessions: w(u'') diff -r 4b721e739f53 -r 09db6f4619c9 web/views/editcontroller.py --- a/web/views/editcontroller.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/views/editcontroller.py Wed Jul 28 16:31:32 2010 +0200 @@ -115,7 +115,7 @@ form = req.form # so we're able to know the main entity from the repository side if '__maineid' in form: - req.set_shared_data('__maineid', form['__maineid'], querydata=True) + req.set_shared_data('__maineid', form['__maineid'], txdata=True) # no specific action, generic edition self._to_create = req.data['eidmap'] = {} self._pending_fields = req.data['pendingfields'] = set() diff -r 4b721e739f53 -r 09db6f4619c9 web/views/sessions.py --- a/web/views/sessions.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/views/sessions.py Wed Jul 28 16:31:32 2010 +0200 @@ -17,8 +17,8 @@ # with CubicWeb. If not, see . """web session component: by dfault the session is actually the db connection object :/ +""" -""" __docformat__ = "restructuredtext en" from cubicweb.web import InvalidSession @@ -51,9 +51,6 @@ if not sessionid in self._sessions: raise InvalidSession() session = self._sessions[sessionid] - if self.has_expired(session): - self.close_session(session) - raise InvalidSession() try: user = self.authmanager.validate_session(req, session) except InvalidSession: diff -r 4b721e739f53 -r 09db6f4619c9 web/webconfig.py --- a/web/webconfig.py Wed Jul 28 16:27:57 2010 +0200 +++ b/web/webconfig.py Wed Jul 28 16:31:32 2010 +0200 @@ -135,17 +135,6 @@ "Should be 0 or greater than repository\'s session-time.", 'group': 'web', 'level': 2, }), - ('cleanup-session-time', - {'type' : 'time', - 'default': '24h', - 'help': 'duration of inactivity after which a connection ' - 'will be closed, to limit memory consumption (avoid sessions that ' - 'never expire and cause memory leak when http-session-time is 0). ' - 'So even if http-session-time is 0 and the user don\'t close his ' - 'browser, he will have to reauthenticate after this time of ' - 'inactivity. Default to 24h.', - 'group': 'web', 'level': 3, - }), ('cleanup-anonymous-session-time', {'type' : 'time', 'default': '5min',