Fix webob (#62)

* introduced some new tests; requiring session_id to be a string
* adding some new tests

CHANGES.md

@@ -1,10 +1,15 @@
 Changelog
 =========
 
-* 2023.06.08
+* 2023.06.??
+    * version 1.7.0dev
+    * Breaking Changes:
+      * The `session_id` will now always be serialized into a string.
+      * If a customer `id_generator` is used, it MUST return a string.
 	* code style changes
 	* dropping PY2
 	* initial typing support
+	* remove some webob dependencies
 
 * 2021.11.16
 	* version 1.6.3
@@ -27,7 +32,6 @@ Changelog
 			Add tests for old and new encoding args for RedisSessionFactory()
 	* add tests for incompatible kwargs (test_session_factory_incompatible_kwargs)
 
-
 * 2021.04.01
 	* version 1.6.1
 	* fix invalid `expires` default. thank you, @olemoign

README.md

@@ -33,6 +33,12 @@ and
 * Pyramid 2.0
 
 
+Breaking Changes
+----------------
+
+Starting in `1.7`, the NullSerializer will ensure data is serialized into a string.
+
+
 Prior Branches
 --------------
 

setup.cfg

@@ -30,5 +30,7 @@ per-file-ignores =
 	src/pyramid_session_redis/session.py: E501
 	src/pyramid_session_redis/util.py: E501
 	tests/test_factory.py: E501,E731
+	tests/test_serializers.py: E501
 	tests/test_session.py: E501,E731
+	tests/test_support.py: E501
 	tests/test_util.py: E731

setup.py

@@ -13,7 +13,7 @@
 # store version in the init.py
 with open(os.path.join(HERE, "src", "pyramid_session_redis", "__init__.py")) as v_file:
     package_version = (
-        re.compile(r'.*__VERSION__ = "(.*?)"', re.S).match(v_file.read()).group(1)
+        re.compile(r'.*__VERSION__ = "(.*?)"', re.S).match(v_file.read()).group(1)  # type: ignore[union-attr]
     )
 
 long_description = (

src/pyramid_session_redis/__init__.py

@@ -9,12 +9,12 @@
 from typing import Type
 from typing import TYPE_CHECKING
 
-
 # pypi
 from redis import VERSION as _redis_version  # since at least the 2.x branch
 from webob.cookies import SignedSerializer  # type: ignore[import]
 
 # local
+from . import compat  # noqa: F401 ; trigger compat routines
 from .connection import get_default_connection
 from .exceptions import InvalidSession
 from .exceptions import InvalidSession_NoSessionCookie
@@ -29,10 +29,11 @@
 from .util import LazyCreateSession
 from .util import NotSpecified
 from .util import TYPING_COOKIE_EXPIRES
+from .util import TYPING_SESSION_ID
 from .util import warn_future
 
 
-__VERSION__ = "1.7.0"
+__VERSION__ = "1.7.0dev"
 
 
 # typing
@@ -269,6 +270,7 @@ def RedisSessionFactory(
     to create a 40 character unique ID.
     A function to create a unique ID to be used as the session key when a
     session is first created.
+    As of `v1.7`, ``id_generator`` MUST return a str.
 
     ``set_redis_ttl``
     Boolean value;  Default `True`.
@@ -329,7 +331,7 @@ def RedisSessionFactory(
 
     ``cookie_signer``
     Default: ``None``
-    If specified,  ``secret`` must be ``None``.
+    If specified,  ``secret`` MUST be ``None``.
     An object with two methods, ``loads`` and ``dumps``.
     The ``loads`` method should accept bytes and return a Python object.
     The ``dumps`` method should accept a Python object and return bytes.
@@ -588,7 +590,7 @@ def RedisSessionFactory(
             secret,
             "pyramid_session_redis.",
             "sha512",
-            serializer=_NullSerializer(),
+            serializer=_NullSerializer(),  # convert to a string
         )
     else:
         _cookie_signer = cookie_signer
@@ -617,6 +619,7 @@ def factory(
             python_expires=python_expires,
         )
 
+        session_id: TYPING_SESSION_ID
         try:
             # attempt to retrieve a session_id from the cookie
             session_id = _get_session_id_from_cookie(
@@ -693,7 +696,7 @@ def factory(
 def _get_session_id_from_cookie(
     request: "Request",
     cookie_name: str,
-    cookie_signer: Type,  # has `.loads`, `.dumps`
+    cookie_signer: Type,  # has `.loads`, `.dumps`; MUST return a str
 ):
     """
     Attempts to retrieve and return a session ID from a session cookie in the
@@ -715,7 +718,7 @@ def _set_cookie(
     session,
     request: "Request",
     response: "Response",
-    cookie_signer: Type,  # has `.loads`, `.dumps`
+    cookie_signer: Type,  # has `.loads`, `.dumps`; MUST return a str
     cookie_name: str,
     **kwargs,
 ):

src/pyramid_session_redis/compat.py

@@ -7,23 +7,11 @@
 # stdlib
 import sys
 
-# !!!: MIGRATION. these move in webob 2.0
-try:
-    # webob 1.x
-    from webob.compat import bytes_ as webob_bytes_
-    from webob.compat import text_ as webob_text_
-except ImportError as exc:  # noqa: F841
-    # webob 2.x
-    from webob.util import bytes_ as webob_bytes_
-    from webob.util import text_ as webob_text_
-
-
 # This moved in py3.10
 if sys.version_info.major == 3:
     if sys.version_info.minor >= 10:
         import collections
 
         collections.Callable = collections.abc.Callable  # type: ignore[attr-defined]
 
-
 # ==============================================================================

src/pyramid_session_redis/exceptions.py

@@ -74,3 +74,15 @@ class RawDeserializationError(Exception):
     """
 
     pass
+
+
+class InvalidSessionId_Deserialization(Exception):
+    """Base class for Serialization Issues"""
+
+    pass
+
+
+class InvalidSessionId_Serialization(Exception):
+    """Base class for Serialization Issues"""
+
+    pass

src/pyramid_session_redis/util.py

@@ -21,8 +21,8 @@
 from redis.exceptions import WatchError
 
 # local
-from .compat import webob_bytes_
-from .compat import webob_text_
+from .exceptions import InvalidSessionId_Deserialization
+from .exceptions import InvalidSessionId_Serialization
 
 if TYPE_CHECKING:
     from .session import RedisSession
@@ -145,8 +145,8 @@ def prefixed_id(prefix: str = "session:") -> str:
     Adds a prefix to the unique session id, for cases where you want to
     visually distinguish keys in redis.
     """
-    session_id = _generate_session_id()
-    prefixed_id = prefix + session_id
+    session_id: str = _generate_session_id()
+    prefixed_id: str = prefix + session_id
     return prefixed_id
 
 
@@ -246,6 +246,9 @@ def _insert_session_id_if_unique(
 
     This will create an empty/null session and redis entry for the id.
 
+    The return value will be the input `session_id` upon success, or `None` upon
+    a failure.
+
     ``data_payload`` = payload to use
     ``new_payload_func`` = specify a fallback function to generate a payload
     if both are ``None``, then `empty_session_payload`
@@ -315,7 +318,7 @@ def create_unique_session_id(
     :returns:
     """
     while 1:
-        session_id = generator()
+        session_id: str = generator()
         # attempt will be the session_id
         attempt = _insert_session_id_if_unique(
             redis,
@@ -448,12 +451,31 @@ def wrapped_refresh(session: "RedisSession", *arg, **kw):
 
 class _NullSerializer(object):
     """
-    A fake serializer for compatibility with ``webob.cookies.SignedSerializer``.
-    Our usage is only signing the session_id, which is a string.
+    A cheap serializer for compatibility with ``webob.cookies.SignedSerializer``.
+    Our usage is only for encoding a signed session_id.
+
+    By default, webob uses json loads/dumps.  As this library only uses strings
+    for session, id, we can have a quick savings here.
+
+    The webob interface dictates:
+
+        https://github.com/Pylons/webob/blob/main/src/webob/cookies.py#L663
+
+        An object with two methods: `loads`` and ``dumps``.  The ``loads`` method
+        should accept bytes and return a Python object.  The ``dumps`` method
+        should accept a Python object and return bytes.  A ``ValueError`` should
+        be raised for malformed inputs.  Default: ``None`, which will use a
+        derivation of :func:`json.dumps` and ``json.loads``.
     """
 
-    def dumps(self, appstruct: str) -> bytes:
-        return webob_bytes_(appstruct, encoding="utf-8")
+    def dumps(self, s: str) -> bytes:
+        try:
+            return s.encode("utf-8", "strict")
+        except Exception as exc:
+            raise InvalidSessionId_Serialization(exc)
 
-    def loads(self, bstruct: bytes) -> str:
-        return webob_text_(bstruct, encoding="utf-8")
+    def loads(self, s: bytes) -> str:
+        try:
+            return str(s, "utf-8", "strict")
+        except Exception as exc:
+            raise InvalidSessionId_Deserialization(exc)

tests/test_serializers.py

@@ -8,6 +8,7 @@
 from webob.cookies import SignedSerializer
 
 # local
+from pyramid_session_redis.exceptions import InvalidSessionId_Serialization
 from pyramid_session_redis.legacy import GracefulCookieSerializer
 from pyramid_session_redis.legacy import LegacyCookieSerializer
 from pyramid_session_redis.util import _NullSerializer
@@ -17,17 +18,25 @@
 
 
 class TestNullSerializer(unittest.TestCase):
-    def test_roundtrip_string(self):
+    def test_string(self):
+        "this should be a roundtrip"
         serializer = _NullSerializer()
-        data = "foo"
-        _serialized = serializer.dumps(data)
-        self.assertEqual(data, serializer.loads(_serialized))
+        data_in = "foo"
+        data_out = data_in
+        _serialized = serializer.dumps(data_in)
+        self.assertEqual(data_out, serializer.loads(_serialized))
 
-    def test_roundtrip_int(self):
+    def test_int(self):
+        "this should fail; _NullSerializer requires a str"
         serializer = _NullSerializer()
-        data = 100
-        _serialized = serializer.dumps(data)
-        self.assertEqual(data, serializer.loads(_serialized))
+        data_in = 100
+        self.assertRaises(InvalidSessionId_Serialization, serializer.dumps, data_in)
+
+    def test_bytes(self):
+        "this should fail; _NullSerializer requires a str"
+        serializer = _NullSerializer()
+        data_in = b"foo"
+        self.assertRaises(InvalidSessionId_Serialization, serializer.dumps, data_in)
 
 
 class TestCookieSerialization(unittest.TestCase):
@@ -53,21 +62,21 @@ def _makeOne_graceful(self, secret, logging_hook=None):
 
     def test_roundtrip_default(self):
         secret = "foo"
-        session_id = "123"
+        session_id = "session_id:123"
         cookie_signer = self._makeOne_default(secret)
         _serialized = cookie_signer.dumps(session_id)
         self.assertEqual(session_id, cookie_signer.loads(_serialized))
 
     def test_roundtrip_legacy(self):
         secret = "foo"
-        session_id = "123"
+        session_id = "session_id:123"
         cookie_signer = self._makeOne_legacy(secret)
         _serialized = cookie_signer.dumps(session_id)
         self.assertEqual(session_id, cookie_signer.loads(_serialized))
 
     def test_incompatible(self):
         secret = "foo"
-        session_id = "123"
+        session_id = "session_id:123"
         cookie_signer_current = self._makeOne_default(secret)
         cookie_signer_legacy = self._makeOne_legacy(secret)
         _serialized_current = cookie_signer_current.dumps(session_id)
@@ -82,7 +91,7 @@ def test_incompatible(self):
 
     def test_graceful(self):
         secret = "foo"
-        session_id = "123"
+        session_id = "session_id:123"
         cookie_signer_current = self._makeOne_default(secret)
         cookie_signer_legacy = self._makeOne_legacy(secret)
         cookie_signer_graceful = self._makeOne_graceful(secret)
@@ -106,7 +115,7 @@ def test_graceful(self):
 
     def test_graceful_hooks(self):
         secret = "foo"
-        session_id = "123"
+        session_id = "session_id:123"
 
         class LoggingHook(object):
             def __init__(self):

tests/test_session.py

@@ -5,6 +5,7 @@
 import itertools
 import pickle
 import time
+from typing import Optional
 import unittest
 
 # local
@@ -755,13 +756,13 @@ def _deserialize_session(self, session, deserialize=pickle.loads):
 
 
 class _TestRedisSessionNew__MIXIN_A(object):
-    PYTHON_EXPIRES = None
-    set_redis_ttl = None
-    set_redis_ttl_readheavy = None
-    session_id = "session_id"
-    timeout = 3
-    timeout_trigger = 6
-    adjusted_timeout = 6
+    PYTHON_EXPIRES: Optional[bool] = None
+    set_redis_ttl: Optional[bool] = None
+    set_redis_ttl_readheavy: Optional[bool] = None
+    session_id: str = "session_id"
+    timeout: Optional[int] = 3
+    timeout_trigger: Optional[int] = 6
+    adjusted_timeout: Optional[int] = 6
 
     def _session_new(self):
         session = self._set_up_session_in_Redis_and_makeOne(