Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31140 closed Bug (invalid)

Caching of dict containing SafeText objects fails with bmemcached.

Reported by: Hugo Rodger-Brown Owned by: nobody
Component: Core (Cache system) Version: 2.2
Severity: Normal Keywords: cache bmemcached
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Hugo Rodger-Brown)

Not sure if this is a Django issue per se. but we have recently started having an issue where cache.set is failing where we have certain SafeText objects inside a dict. (Issue has already

The underlying exception is raised within bmemcached, but posting here in case someone else has had a similar issue, and any insight.


Expected behaviour - object can be cached:

>>> from django.utils.safestring import SafeString
>>> from django.core.cache import cache
>>> cache.set("test", SafeString("foo"))
>>> cache.set("test", SafeString("£"))

Actual outcome - cache.set fails with a recursion error when the value being stored is a dict that contains a SafeText value.

>>> from django.utils.safestring import SafeString
>>> from django.core.cache import cache
>>> cache.set("test", {"foo": SafeString("£")})
Traceback (most recent call last):
  File "/python3.7/site-packages/django/core/cache/backends/memcached.py", line 78, in set
    if not self._cache.set(key, value, self.get_backend_timeout(timeout)):
  File "/python3.7/site-packages/bmemcached/client/replicating.py", line 112, in set
    returns.append(server.set(key, value, time, compress_level=compress_level))
  File "/python3.7/site-packages/bmemcached/protocol.py", line 604, in set
    return self._set_add_replace('set', key, value, time, compress_level=compress_level)
  File "/python3.7/site-packages/bmemcached/protocol.py", line 561, in _set_add_replace
    flags, value = self.serialize(value, compress_level=compress_level)
  File "/python3.7/site-packages/bmemcached/protocol.py", line 347, in serialize
    pickler.dump(value)
  File "/python3.7/copyreg.py", line 66, in _reduce_ex
    state = base(self)
RecursionError: maximum recursion depth exceeded while getting the str of an object

Running on Python 3.7, Django 2.2

Configuration:

CACHES = {
    "default": {
        "BACKEND": "django_bmemcached.memcached.BMemcached",
        "BINARY": True,
        "OPTIONS": {
            "no_block": True,
            "tcp_nodelay": True,
            "tcp_keepalive": True,
            "remove_failed": 4,
            "retry_timeout": 2,
            "dead_timeout": 10,
            "_poll_timeout": 2000,
        },
    },
}

Change History (6)

comment:1 by Hugo Rodger-Brown, 5 years ago

Description: modified (diff)

Can't be sure, but I think it's the internal implementation of SafeText and how bmemcached is serializing that is causing the problem: https://github.com/django/django/blob/master/django/utils/safestring.py#L36-L37

comment:2 by Hugo Rodger-Brown, 5 years ago

I've dug into it, and it looks like SafeText's `str function returning self causes the bmemcached serialize method to tie itself in knots. Not a Django issue, or necessarily a bmemcached issue. We have fixed our usage to force SafeText objects to str (by appending and empty str) wherever they are being cached.

https://github.com/django/django/commit/ccfd1295f986cdf628d774937d0b38a14584721f

comment:3 by Mariusz Felisiak, 5 years ago

Component: UtilitiesCore (Cache system)
Resolution: invalid
Status: newclosed
Summary: Caching of dict containing django.utils.safestring.SafeText objects fails with bmemcached.Caching of dict containing SafeText objects fails with bmemcached.
Type: UncategorizedBug

Yes, that's not an issue in Django.

comment:4 by Jayson Reis, 5 years ago

I think that this should be considered a django bug as SafeString is not serializable with pickle protocol 0 and 1.
But after I monkey patch SafeString.__str__ to use str.__str__ it works.

Python 3.6.6 (default, Nov 15 2018, 12:50:13) 
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.10.44.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pickle import dumps, loads
>>> from django.utils.safestring import SafeString
>>> dumps(SafeString('test'))
b'\x80\x03cdjango.utils.safestring\nSafeText\nq\x00X\x04\x00\x00\x00testq\x01\x85q\x02\x81q\x03.'
>>> dumps(SafeString('test'), protocol=0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jayson/.pyenv/versions/3.6.6/lib/python3.6/copyreg.py", line 66, in _reduce_ex
    state = base(self)
RecursionError: maximum recursion depth exceeded while calling a Python object
>>> SafeString.__str__
<function SafeText.__str__ at 0x10c7fcf28>
>>> SafeString.__str__ = str.__str__
>>> dumps(SafeString('test'))
b'\x80\x03cdjango.utils.safestring\nSafeText\nq\x00X\x04\x00\x00\x00testq\x01\x85q\x02\x81q\x03.'
>>> dumps(SafeString('test'), protocol=0)
b'ccopy_reg\n_reconstructor\np0\n(cdjango.utils.safestring\nSafeText\np1\nc__builtin__\nunicode\np2\nVtest\np3\ntp4\nRp5\n.'
>>> loads(dumps(SafeString('test'), protocol=0))
'test'
>>> 

Last edited 5 years ago by Jayson Reis (previous) (diff)

comment:5 by Mariusz Felisiak, 5 years ago

pickle.dumps() for SafeString works with protocol=2, 3, 4, and 5. protocol=0 and 1 are quite old and unfortunately we cannot change StringText.__str__() implementation, because we will lose safe status on str() (see ccfd1295f986cdf628d774937d0b38a14584721f).

comment:6 by Hugo Rodger-Brown, 5 years ago

Yup - this ticket has resulted in a fix for django-bmemcached to allow the pickle_protocol to be set in Django config.

(For reference, PR - https://github.com/jaysonsantos/django-bmemcached/pull/14)

# django settings.py
CACHES = {
    "default": {
        "BACKEND": "django_bmemcached.memcached.BMemcached",
        "BINARY": True,
        "OPTIONS": {
            "pickle_protocol": 4,
        }
    }
}
Note: See TracTickets for help on using tickets.
Back to Top