Merge pull request #3524 from jvanasco/fix-deprecate_PickleSerializer

deprecate PickleSerializer

4 files changed, 65 insertions, 19 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 987d5c3d4..537ca4671 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,11 @@ unreleased
 Features
 --------
 
+- Deprecated ``pyramid.session.PickleSerializer``.
+  See https://github.com/pylons/pyramid/issues/2709
+  and https://github.com/pylons/pyramid/pull/3353
+  and https://github.com/pylons/pyramid/pull/3413
+
 - Changed the default ``serializer`` on
   ``pyramid.session.SignedCookieSessionFactory`` to use
   ``pyramid.session.JSONSerializer`` instead of
diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst
index c2cc60de8..2da524d4c 100644
--- a/docs/narr/sessions.rst
+++ b/docs/narr/sessions.rst
@@ -85,32 +85,49 @@ This is a stricter contract than the previous requirement that all objects be pi
 This is a backward-incompatible change.
 Previously, if a client-side session implementation was compromised, it left the application vulnerable to remote code execution attacks using specially-crafted sessions that execute code when deserialized.
 
+Please reference the following tickets if detailed information on these changes is needed:
+
+* `2.0 feature request: Require that sessions are JSON serializable #2709 <https://github.com/pylons/pyramid/issues/2709>`_.
+* `deprecate pickleable sessions, recommend json #3353 <https://github.com/pylons/pyramid/pull/3353>`_.
+* `change to use JSONSerializer for SignedCookieSessionFactory #3413 <https://github.com/pylons/pyramid/pull/3413>`_.
+
 For users with compatibility concerns, it's possible to craft a serializer that can handle both formats until you are satisfied that clients have had time to reasonably upgrade.
 Remember that sessions should be short-lived and thus the number of clients affected should be small (no longer than an auth token, at a maximum). An example serializer:
 
 .. code-block:: python
     :linenos:
 
+    import pickle
     from pyramid.session import JSONSerializer
-    from pyramid.session import PickleSerializer
     from pyramid.session import SignedCookieSessionFactory
 
+
     class JSONSerializerWithPickleFallback(object):
         def __init__(self):
             self.json = JSONSerializer()
-            self.pickle = PickleSerializer()
 
-        def dumps(self, value):
-            # maybe catch serialization errors here and keep using pickle
-            # while finding spots in your app that are not storing
-            # JSON-serializable objects, falling back to pickle
-            return self.json.dumps(value)
+        def dumps(self, appstruct):
+            """
+            Accept a Python object and return bytes.
+
+            During a migration, you may want to catch serialization errors here,
+            and keep using pickle while finding spots in your app that are not
+            storing JSON-serializable objects. You may also want to integrate
+            a fall-back to pickle serialization here as well.
+            """
+            return self.json.dumps(appstruct)
 
-        def loads(self, value):
+        def loads(self, bstruct):
+            """Accept bytes and return a Python object."""
             try:
-                return self.json.loads(value)
+                return self.json.loads(bstruct)
             except ValueError:
-                return self.pickle.loads(value)
+                try:
+                    return pickle.loads(bstruct)
+                except Exception:
+                    # this block should catch at least:
+                    # ValueError, AttributeError, ImportError; but more to be safe
+                    raise ValueError
 
     # somewhere in your configuration code
     serializer = JSONSerializerWithPickleFallback()
diff --git a/src/pyramid/session.py b/src/pyramid/session.py
index 70ac4f55f..10e1ea313 100644
--- a/src/pyramid/session.py
+++ b/src/pyramid/session.py
@@ -44,14 +44,25 @@ def manage_changed(wrapped):
 
 
 class PickleSerializer(object):
-    """ A serializer that uses the pickle protocol to dump Python
-    data to bytes.
+    """
+    .. deprecated:: 2.0
+
+    .. warning::
+
+        In :app:`Pyramid` 2.0 the default ``serializer`` option changed to
+        use :class:`pyramid.session.JSONSerializer`, and ``PickleSerializer``
+        has been been removed from active Pyramid code.
+
+        Pyramid will require JSON-serializable objects in :app:`Pyramid` 2.0.
+
+        Please see :ref:`pickle_session_deprecation`.
 
-    This is the default serializer used by Pyramid.
+    A serializer that uses the pickle protocol to dump Python data to bytes.
+
+    This was the default serializer used by Pyramid, but has been deprecated.
 
     ``protocol`` may be specified to control the version of pickle used.
     Defaults to :attr:`pickle.HIGHEST_PROTOCOL`.
-
     """
 
     def __init__(self, protocol=pickle.HIGHEST_PROTOCOL):
@@ -61,8 +72,9 @@ class PickleSerializer(object):
         """Accept bytes and return a Python object."""
         try:
             return pickle.loads(bstruct)
-        # at least ValueError, AttributeError, ImportError but more to be safe
         except Exception:
+            # this block should catch at least:
+            # ValueError, AttributeError, ImportError; but more to be safe
             raise ValueError
 
     def dumps(self, appstruct):
@@ -70,6 +82,14 @@ class PickleSerializer(object):
         return pickle.dumps(appstruct, self.protocol)
 
 
+deprecated(
+    'PickleSerializer',
+    'pyramid.session.PickleSerializer is deprecated as of Pyramid 2.0 for '
+    'security concerns. Use pyramid.session.JSONSerializer or reference the '
+    'narrative documentation for information on building a migration tool.',
+)
+
+
 JSONSerializer = JSONSerializer  # api
 
 
@@ -438,10 +458,10 @@ def SignedCookieSessionFactory(
 
     .. warning::
 
-       In :app:`Pyramid` 2.0 the default ``serializer`` option changed to
-       use :class:`pyramid.session.JSONSerializer`. See
-       :ref:`pickle_session_deprecation` for more information about why this
-       change was made.
+        In :app:`Pyramid` 2.0 the default ``serializer`` option changed to
+        use :class:`pyramid.session.JSONSerializer`. See
+        :ref:`pickle_session_deprecation` for more information about why this
+        change was made.
 
     .. versionadded: 1.5a3
 
diff --git a/tests/test_session.py b/tests/test_session.py
index 8e5e82bb2..582a7ed4a 100644
--- a/tests/test_session.py
+++ b/tests/test_session.py
@@ -564,6 +564,10 @@ class Test_manage_changed(unittest.TestCase):
 
 
 class TestPickleSerializer(unittest.TestCase):
+    """
+    .. deprecated:: 2.0
+    """
+
     def _makeOne(self):
         from pyramid.session import PickleSerializer