From ec4826ef5ba369dda989e1cee23cdb367cea6853 Mon Sep 17 00:00:00 2001 From: Julien Castiaux Date: Thu, 3 Nov 2022 13:59:37 +0000 Subject: [PATCH] [FIX] core: session logout after 16.0 migration Create a 15.0 database with website, access the home page via your browser. Stop the server and migrate the database to 16.0. Restart the server with a `--dbfilter` that rejects the database you created and refresh your browser. 500 Internal server error, attribute error: the `request` object as no `session`. An error could occurs after a migration to 16.0 due to the presence of the `geoip` key in the session. `request.session.geoip` has been made a deprecated alias to `request.geoip` between 15.0 and 16.0, see 04e9726. Because the session was created before 16.0, the session dict does contain a `geoip` key. Upon logging the session out, the session dict is cleared. The default implementation of `clear()`[^1] inside of `collections.abc.MutableMapping` can be summarized for our usecase to: for key in self: value = self[key] del self[key] There is an extra `__getitem__` call due to `value = self[key]`, in the case of the `geoip` key, it would access the alias. It is not possible to accessing that alias inside of the `_get_dbname_and_session` method of request as the session has not been set on `self` (the request) yet. Yet inside of that method, we do `session.logout()` which `clear()` the session which (wrongly) access the alias because `geoip` exists in the internal dict (`'geoip' in self.keys() # True`). The solution has been to implement the `clear()` function ourself instead of using the mixin of `MutableMapping`. [^1]: https://github.com/python/cpython/blob/b43496c01a554cf41ae654a0379efae18609ad39/Lib/_collections_abc.py#L925-L931 closes odoo/odoo#105763 X-original-commit: b66e1ffa8e348eedf2de735babbc398290a8bffb Signed-off-by: Julien Castiaux --- odoo/addons/test_http/tests/test_session.py | 17 +++++++++++++++++ odoo/http.py | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/odoo/addons/test_http/tests/test_session.py b/odoo/addons/test_http/tests/test_session.py index f67967afe59..2737ca33dbd 100644 --- a/odoo/addons/test_http/tests/test_session.py +++ b/odoo/addons/test_http/tests/test_session.py @@ -1,5 +1,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. +from urllib.parse import urlparse from unittest.mock import patch import odoo @@ -74,3 +75,19 @@ class TestHttpSession(TestHttpBase): res.raise_for_status() self.assertEqual(res.text, str(GEOIP_ODOO_FARM_2)) mock_resolve.assert_called_once() + + def test_session3_logout_15_0_geoip(self): + session = self.authenticate(None, None) + session['db'] = 'idontexist' + session['geoip'] = {} # Until saas-15.2 geoip was directly stored in the session + odoo.http.root.session_store.save(session) + + with self.assertLogs('odoo.http', level='WARNING') as (_, warnings): + res = self.multidb_url_open('/test_http/ensure_db', dblist=['db1', 'db2']) + + self.assertEqual(warnings, [ + "WARNING:odoo.http:Logged into database 'idontexist', but dbfilter rejects it; logging session out.", + ]) + self.assertFalse(session['db']) + self.assertEqual(res.status_code, 303) + self.assertEqual(urlparse(res.headers['Location']).path, '/web/database/selector') diff --git a/odoo/http.py b/odoo/http.py index a887b28f969..ea093266d46 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -915,6 +915,10 @@ class Session(collections.abc.MutableMapping): else: self[key] = val + def clear(self): + self.data.clear() + self.is_dirty = True + # # Session methods #