Repository: pallets/flask
Pull request: Make session serializer extensible
Continues #1452.

Refactors `flask.session.TaggedJSONSerializer` to a new `flask.json.tag` module. Creates a `JSONTag` interface and a `TaggedJSONSerializer.register` method to register new tags.

~~~python
from flask.json.tag import JSONTag

class TagOrderedDict(JSONTag):
    __slots__ = ('serializer',)
    key = ' od'

    def check(self, value):
        return isinstance(value, OrderedDict)

    def to_json(self, value):
        return [[k, self.serializer.tag(v)] for k, v in iteritems(value)]

    def to_python(self, value):
        return OrderedDict(value)

app.session_interface.serializer.register(TagOrderedDict, 0)
~~~

Since `register` takes an index, it can be used to override existing tags (although it would be slightly more efficient to subclass `TaggedJSONSerializer` instead). This can address #1908 by adding a higher-priority `dict` serializer that uses a different format to support non-string keys.

Closes #1438, #1511, #1908.

~Still needs:~

- [x] refactor to move the `serializer` argument to `self.serializer` instead. It was an attempt at optimization but didn't change anything.
- [x] refactor `TagDict` to split the logic between addressing #1511 and regular dict passthrough, as #1452 originally did. This was another optimization attempt that didn't help, it just made the code more confusing.
- [x] more tests
- [x] documentation

Problem statement:
Make TaggedJSONSerializer extensible
My suggestion would be to add a method to the class that lets you register custom types, e.g. like this:

```
session_json_serializer.register_type('md', MultiDict)
```

This would use the `' md'` prefix when encountering a `MultiDict` object (`type(x) is MultiDict` as subclasses probably shouldn't be handled). To serialize/deserialize I'd simply use `__getstate__` and `__setstate__`.

Hints from issue discussion:
This will probably be superseded by the easier solution in #1452. With the changes I've suggested there one could simply subclass and override `_tag` and `_untag` to handle custom types.

Commit message: pass serializer at tag init instead of to each method
split tagged dict and passthrough into separate cases
add docstrings

Patch to review:
diff --git a/CHANGES b/CHANGES
index dc39a95db4..a3504ad546 100644
--- a/CHANGES
+++ b/CHANGES
@@ -65,6 +65,8 @@ Major release, unreleased
 - ``TRAP_BAD_REQUEST_ERRORS`` is enabled by default in debug mode.
   ``BadRequestKeyError`` has a message with the bad key in debug mode instead
   of the generic bad request message. (`#2348`_)
+- Allow registering new tags with ``TaggedJSONSerializer`` to support
+  storing other types in the session cookie. (`#2352`_)
 
 .. _#1489: https://github.com/pallets/flask/pull/1489
 .. _#1621: https://github.com/pallets/flask/pull/1621
@@ -84,6 +86,7 @@ Major release, unreleased
 .. _#2319: https://github.com/pallets/flask/pull/2319
 .. _#2326: https://github.com/pallets/flask/pull/2326
 .. _#2348: https://github.com/pallets/flask/pull/2348
+.. _#2352: https://github.com/pallets/flask/pull/2352
 
 Version 0.12.2
 --------------
diff --git a/flask/json.py b/flask/json/__init__.py
similarity index 97%
rename from flask/json.py
rename to flask/json/__init__.py
index a029e73ae6..93e6fdc46c 100644
--- a/flask/json.py
+++ b/flask/json/__init__.py
@@ -1,18 +1,9 @@
 # -*- coding: utf-8 -*-
-"""
-    flask.json
-    ~~~~~~~~~~
-
-    Implementation helpers for the JSON support in Flask.
-
-    :copyright: (c) 2015 by Armin Ronacher.
-    :license: BSD, see LICENSE for more details.
-"""
 import io
 import uuid
 from datetime import date
-from .globals import current_app, request
-from ._compat import text_type, PY2
+from flask.globals import current_app, request
+from flask._compat import text_type, PY2
 
 from werkzeug.http import http_date
 from jinja2 import Markup
diff --git a/flask/json/tag.py b/flask/json/tag.py
new file mode 100644
index 0000000000..3cf32a0c09
--- /dev/null
+++ b/flask/json/tag.py
@@ -0,0 +1,253 @@
+from base64 import b64decode, b64encode
+from datetime import datetime
+from uuid import UUID
+
+from jinja2 import Markup
+from werkzeug.http import http_date, parse_date
+
+from flask._compat import iteritems, text_type
+from flask.json import dumps, loads
+
+
+class JSONTag(object):
+    """Base class for defining type tags for :class:`TaggedJSONSerializer`."""
+
+    __slots__ = ('serializer',)
+    key = None
+    """The tag to mark the serialized object with. If ``None``, this tag is
+    only used as an intermediate step during tagging."""
+
+    def __init__(self, serializer):
+        """Create a tagger for the given serializer."""
+
+        self.serializer = serializer
+
+    def check(self, value):
+        """Check if the given value should be tagged by this tag."""
+
+        raise NotImplementedError
+
+    def to_json(self, value):
+        """Convert the Python object to an object that is a valid JSON type.
+        The tag will be added later."""
+
+        raise NotImplementedError
+
+    def to_python(self, value):
+        """Convert the JSON representation back to the correct type. The tag
+        will already be removed."""
+
+        raise NotImplementedError
+
+    def tag(self, value):
+        """Convert the value to a valid JSON type and add the tag structure
+        around it."""
+
+        return {self.key: self.to_json(value)}
+
+
+class TagDict(JSONTag):
+    """Tag for 1-item dicts whose only key matches a registered tag.
+
+    Internally, the dict key is suffixed with `__`, and the suffix is removed
+    when deserializing.
+    """
+
+    __slots__ = ('serializer',)
+    key = ' di'
+
+    def check(self, value):
+        return (
+            isinstance(value, dict)
+            and len(value) == 1
+            and next(iter(value)) in self.serializer.tags
+        )
+
+    def to_json(self, value):
+        key = next(iter(value))
+        return {key + '__': self.serializer.tag(value[key])}
+
+    def to_python(self, value):
+        key = next(iter(value))
+        return {key[:-2]: value[key]}
+
+
+class PassDict(JSONTag):
+    __slots__ = ('serializer',)
+
+    def check(self, value):
+        return isinstance(value, dict)
+
+    def to_json(self, value):
+        # JSON objects may only have string keys, so don't bother tagging the
+        # key here.
+        return dict((k, self.serializer.tag(v)) for k, v in iteritems(value))
+
+    tag = to_json
+
+
+class TagTuple(JSONTag):
+    __slots__ = ('serializer',)
+    key = ' t'
+
+    def check(self, value):
+        return isinstance(value, tuple)
+
+    def to_json(self, value):
+        return [self.serializer.tag(item) for item in value]
+
+    def to_python(self, value):
+        return tuple(value)
+
+
+class PassList(JSONTag):
+    __slots__ = ('serializer',)
+
+    def check(self, value):
+        return isinstance(value, list)
+
+    def to_json(self, value):
+        return [self.serializer.tag(item) for item in value]
+
+    tag = to_json
+
+
+class TagBytes(JSONTag):
+    __slots__ = ('serializer',)
+    key = ' b'
+
+    def check(self, value):
+        return isinstance(value, bytes)
+
+    def to_json(self, value):
+        return b64encode(value).decode('ascii')
+
+    def to_python(self, value):
+        return b64decode(value)
+
+
+class TagMarkup(JSONTag):
+    """Serialize anything matching the :class:`~markupsafe.Markup` API by
+    having a ``__html__`` method to the result of that method. Always
+    deserializes to an instance of :class:`~markupsafe.Markup`."""
+
+    __slots__ = ('serializer',)
+    key = ' m'
+
+    def check(self, value):
+        return callable(getattr(value, '__html__', None))
+
+    def to_json(self, value):
+        return text_type(value.__html__())
+
+    def to_python(self, value):
+        return Markup(value)
+
+
+class TagUUID(JSONTag):
+    __slots__ = ('serializer',)
+    key = ' u'
+
+    def check(self, value):
+        return isinstance(value, UUID)
+
+    def to_json(self, value):
+        return value.hex
+
+    def to_python(self, value):
+        return UUID(value)
+
+
+class TagDateTime(JSONTag):
+    __slots__ = ('serializer',)
+    key = ' d'
+
+    def check(self, value):
+        return isinstance(value, datetime)
+
+    def to_json(self, value):
+        return http_date(value)
+
+    def to_python(self, value):
+        return parse_date(value)
+
+
+class TaggedJSONSerializer(object):
+    """Serializer that uses a tag system to compactly represent objects that
+    are not JSON types. Passed as the intermediate serializer to
+    :class:`itsdangerous.Serializer`."""
+
+    __slots__ = ('tags', 'order')
+    default_tags = [
+        TagDict, PassDict, TagTuple, PassList, TagBytes, TagMarkup, TagUUID,
+        TagDateTime,
+    ]
+    """Tag classes to bind when creating the serializer. Other tags can be
+    added later using :meth:`~register`."""
+
+    def __init__(self):
+        self.tags = {}
+        self.order = []
+
+        for cls in self.default_tags:
+            self.register(cls)
+
+    def register(self, tag_class, force=False, index=-1):
+        """Register a new tag with this serializer.
+
+        :param tag_class: tag class to register. Will be instantiated with this
+            serializer instance.
+        :param force: overwrite an existing tag. If false (default), a
+            :exc:`KeyError` is raised.
+        :param index: index to insert the new tag in the tag order. Useful when
+            the new tag is a special case of an existing tag. If -1 (default),
+            the tag is appended to the end of the order.
+
+        :raise KeyError: if the tag key is already registered and ``force`` is
+            not true.
+        """
+
+        tag = tag_class(self)
+        key = tag.key
+
+        if key is not None:
+            if not force and key in self.tags:
+                raise KeyError("Tag '{0}' is already registered.".format(key))
+
+            self.tags[key] = tag
+
+        if index == -1:
+            self.order.append(tag)
+        else:
+            self.order.insert(index, tag)
+
+    def tag(self, value):
+        """Convert a value to a tagged representation if necessary."""
+
+        for tag in self.order:
+            if tag.check(value):
+                return tag.tag(value)
+
+        return value
+
+    def untag(self, value):
+        """Convert a tagged representation back to the original type."""
+
+        if len(value) != 1:
+            return value
+
+        key = next(iter(value))
+
+        if key not in self.tags:
+            return value
+
+        return self.tags[key].to_python(value[key])
+
+    def dumps(self, value):
+        """Tag the value and dump it to a compact JSON string."""
+
+        return dumps(self.tag(value), separators=(',', ':'))
+
+    def loads(self, value):
+        """Load data from a JSON string and deserialized any tagged objects."""
+        return loads(value, object_hook=self.untag)
diff --git a/flask/sessions.py b/flask/sessions.py
index 3a5e2ac3e3..82b588bc9d 100644
--- a/flask/sessions.py
+++ b/flask/sessions.py
@@ -8,20 +8,15 @@
     :copyright: (c) 2015 by Armin Ronacher.
     :license: BSD, see LICENSE for more details.
 """
-
 import hashlib
-import uuid
 import warnings
-from base64 import b64decode, b64encode
 from datetime import datetime
 
 from itsdangerous import BadSignature, URLSafeTimedSerializer
 from werkzeug.datastructures import CallbackDict
-from werkzeug.http import http_date, parse_date
 
-from . import Markup, json
-from ._compat import iteritems, text_type
-from .helpers import is_ip, total_seconds
+from flask.helpers import is_ip, total_seconds
+from flask.json.tag import TaggedJSONSerializer
 
 
 class SessionMixin(object):
@@ -58,66 +53,6 @@ def _set_permanent(self, value):
     #: from being served the same cache.
     accessed = True
 
-def _tag(value):
-    if isinstance(value, tuple):
-        return {' t': [_tag(x) for x in value]}
-    elif isinstance(value, uuid.UUID):
-        return {' u': value.hex}
-    elif isinstance(value, bytes):
-        return {' b': b64encode(value).decode('ascii')}
-    elif callable(getattr(value, '__html__', None)):
-        return {' m': text_type(value.__html__())}
-    elif isinstance(value, list):
-        return [_tag(x) for x in value]
-    elif isinstance(value, datetime):
-        return {' d': http_date(value)}
-    elif isinstance(value, dict):
-        return dict((k, _tag(v)) for k, v in iteritems(value))
-    elif isinstance(value, str):
-        try:
-            return text_type(value)
-        except UnicodeError:
-            from flask.debughelpers import UnexpectedUnicodeError
-            raise UnexpectedUnicodeError(u'A byte string with '
-                u'non-ASCII data was passed to the session system '
-                u'which can only store unicode strings.  Consider '
-                u'base64 encoding your string (String was %r)' % value)
-    return value
-
-
-class TaggedJSONSerializer(object):
-    """A customized JSON serializer that supports a few extra types that
-    we take for granted when serializing (tuples, markup objects, datetime).
-    """
-
-    def dumps(self, value):
-        return json.dumps(_tag(value), separators=(',', ':'))
-
-    LOADS_MAP = {
-        ' t': tuple,
-        ' u': uuid.UUID,
-        ' b': b64decode,
-        ' m': Markup,
-        ' d': parse_date,
-    }
-
-    def loads(self, value):
-        def object_hook(obj):
-            if len(obj) != 1:
-                return obj
-            the_key, the_value = next(iteritems(obj))
-            # Check the key for a corresponding function
-            return_function = self.LOADS_MAP.get(the_key)
-            if return_function:
-                # Pass the value to the function
-                return return_function(the_value)
-            # Didn't find a function for this object
-            return obj
-        return json.loads(value, object_hook=object_hook)
-
-
-session_json_serializer = TaggedJSONSerializer()
-
 
 class SecureCookieSession(CallbackDict, SessionMixin):
     """Base class for sessions based on signed cookies."""
@@ -225,10 +160,10 @@ def is_null_session(self, obj):
 
     def get_cookie_domain(self, app):
         """Returns the domain that should be set for the session cookie.
-        
+
         Uses ``SESSION_COOKIE_DOMAIN`` if it is configured, otherwise
         falls back to detecting the domain based on ``SERVER_NAME``.
-        
+
         Once detected (or if not set at all), ``SESSION_COOKIE_DOMAIN`` is
         updated to avoid re-running the logic.
         """
@@ -318,7 +253,7 @@ def should_set_cookie(self, app, session):
         has been modified, the cookie is set. If the session is permanent and
         the ``SESSION_REFRESH_EACH_REQUEST`` config is true, the cookie is
         always set.
-        
+
         This check is usually skipped if the session was deleted.
 
         .. versionadded:: 0.11
@@ -345,6 +280,9 @@ def save_session(self, app, session, response):
         raise NotImplementedError()
 
 
+session_json_serializer = TaggedJSONSerializer()
+
+
 class SecureCookieSessionInterface(SessionInterface):
     """The default session interface that stores sessions in signed cookies
     through the :mod:`itsdangerous` module.
diff --git a/tests/test_basic.py b/tests/test_basic.py
index a38428b21a..e4f6883223 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -442,6 +442,9 @@ def modify_session(response):
         flask.session['dt'] = now
         flask.session['b'] = b'\xff'
         flask.session['t'] = (1, 2, 3)
+        flask.session['spacefirst'] = {' t': 'not-a-tuple'}
+        flask.session['withunderscores'] = {' t__': 'not-a-tuple'}
+        flask.session['notadict'] = {' di': 'not-a-dict'}
         return response
 
     @app.route('/')
@@ -457,6 +460,9 @@ def dump_session_contents():
     assert rv['b'] == b'\xff'
     assert type(rv['b']) == bytes
     assert rv['t'] == (1, 2, 3)
+    assert rv['spacefirst'] == {' t': 'not-a-tuple'}
+    assert rv['withunderscores'] == {' t__': 'not-a-tuple'}
+    assert rv['notadict'] == {' di': 'not-a-dict'}
 
 
 def test_session_cookie_setting(app):


Code review comments:
diff_hunk: @@ -0,0 +1,253 @@
+from base64 import b64decode, b64encode
+from datetime import datetime
+from uuid import UUID
+
+from jinja2 import Markup
+from werkzeug.http import http_date, parse_date
+
+from flask._compat import iteritems, text_type
+from flask.json import dumps, loads
+
+
+class JSONTag(object):
+    """Base class for defining type tags for :class:`TaggedJSONSerializer`."""
+
+    __slots__ = ('serializer',)
+    key = None
+    """The tag to mark the serialized object with. If ``None``, this tag is
path: flask/json/tag.py
text: @user1:
AFAIK the most common sphinx way of documenting a class-level variable is this:

```python
#: lorem ipsum
#: blah blah blah
foo = None
```

At least I find this "dangling" docstring quite ugly and I think some editors even highlight it as a no-op statement  and/or a docstring that's not where it should be/

@author:
Sphinx and PyCharm both recognize it. Not a big deal though, I can switch to `#:` and ignore PyCharm. Should probably just open a bug about it with PyCharm anyway.

@user1:
ah ok, never noticed pycharm handling it nicely. probably never opened a file using this notation ;)

I just checked and `#:` is quite widespread in the flask codebase so I'd go for that