launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-pgsession-datetime-compatibility into launchpad:master.

Commit message:
Handle unpickling of Python 2 datetime objects on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/399133

Some of the pickled objects in the session database are `datetime.datetime` objects (`logintime` and `last_write`).  There are particular difficulties with unpickling these objects on various Python 3 versions, as described in https://bugs.python.org/issue22005.  Work around these using a customized unpickler.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-pgsession-datetime-compatibility into launchpad:master.

diff --git a/lib/lp/services/webapp/pgsession.py b/lib/lp/services/webapp/pgsession.py
index 3f035d3..0a5c664 100644
--- a/lib/lp/services/webapp/pgsession.py
+++ b/lib/lp/services/webapp/pgsession.py
@@ -6,7 +6,9 @@
 __metaclass__ = type
 
 from collections import MutableMapping
+from datetime import datetime
 import hashlib
+import io
 import time
 
 from lazr.restful.utils import get_current_browser_request
@@ -30,6 +32,35 @@ HOURS = 60 * MINUTES
 DAYS = 24 * HOURS
 
 
+if six.PY3:
+    class Python2FriendlyUnpickler(pickle._Unpickler):
+        """An unpickler that handles Python 2 datetime objects.
+
+        Python 3 versions before 3.6 fail to unpickle Python 2 datetime
+        objects (https://bugs.python.org/issue22005); even in Python >= 3.6
+        they require passing a different encoding to pickle.loads, which may
+        have undesirable effects on other objects being unpickled.  Work
+        around this by instead patching in a different encoding just for the
+        argument to datetime.datetime.
+        """
+
+        def find_class(self, module, name):
+            if module == 'datetime' and name == 'datetime':
+                original_encoding = self.encoding
+                self.encoding = 'bytes'
+
+                def datetime_factory(pickle_data):
+                    self.encoding = original_encoding
+                    return datetime(pickle_data)
+
+                return datetime_factory
+            else:
+                return super(Python2FriendlyUnpickler, self).find_class(
+                    module, name)
+else:
+    Python2FriendlyUnpickler = pickle.Unpickler
+
+
 class PGSessionBase:
     store_name = 'session'
 
@@ -186,7 +217,8 @@ class PGSessionPkgData(MutableMapping, PGSessionBase):
         result = self.store.execute(
             query, (self.session_data.hashed_client_id, self.product_id))
         for key, pickled_value in result:
-            value = pickle.loads(bytes(pickled_value))
+            value = Python2FriendlyUnpickler(
+                io.BytesIO(bytes(pickled_value))).load()
             self._data_cache[key] = value
 
     def __getitem__(self, key):
diff --git a/lib/lp/services/webapp/tests/test_pgsession.py b/lib/lp/services/webapp/tests/test_pgsession.py
index 971ad4c..4d033fd 100644
--- a/lib/lp/services/webapp/tests/test_pgsession.py
+++ b/lib/lp/services/webapp/tests/test_pgsession.py
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+from datetime import datetime
 import hashlib
 
 from zope.publisher.browser import TestRequest
@@ -167,3 +168,43 @@ class TestPgSession(TestCase):
 
         # also see the page test xx-no-anonymous-session-cookies for tests of
         # the cookie behaviour.
+
+    def test_datetime_compatibility(self):
+        # datetime objects serialized by either Python 2 or 3 can be
+        # unserialized as part of the session.
+        client_id = u'Client Id #1'
+        product_id = u'Product Id'
+        expected_datetime = datetime(2021, 3, 4, 0, 50, 1, 300000)
+
+        session = self.sdc[client_id]
+        session._ensureClientId()
+
+        # These are returned by the following code in Python 2.7 and 3.5
+        # respectively:
+        #
+        #     pickle.dumps(expected_datetime, protocol=2)
+        python_2_pickle = (
+            b'\x80\x02cdatetime\ndatetime\nq\x00'
+            b'U\n\x07\xe5\x03\x04\x002\x01\x04\x93\xe0q\x01\x85q\x02Rq\x03.')
+        python_3_pickle = (
+            b'\x80\x02cdatetime\ndatetime\nq\x00'
+            b'c_codecs\nencode\nq\x01'
+            b'X\r\x00\x00\x00\x07\xc3\xa5\x03\x04\x002\x01\x04\xc2\x93\xc3\xa0'
+            b'q\x02X\x06\x00\x00\x00latin1q\x03\x86q\x04Rq\x05\x85q\x06R'
+            b'q\x07.')
+
+        store = self.sdc.store
+        store.execute(
+            "SELECT set_session_pkg_data(?, ?, ?, ?)",
+            (session.hashed_client_id, product_id, u'logintime',
+             python_2_pickle),
+            noresult=True)
+        store.execute(
+            "SELECT set_session_pkg_data(?, ?, ?, ?)",
+            (session.hashed_client_id, product_id, u'last_write',
+             python_3_pickle),
+            noresult=True)
+
+        pkgdata = session[product_id]
+        self.assertEqual(expected_datetime, pkgdata['logintime'])
+        self.assertEqual(expected_datetime, pkgdata['last_write'])