diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py --- a/django/core/cache/backends/base.py +++ b/django/core/cache/backends/base.py @@ -5,6 +5,12 @@ class InvalidCacheBackendError(ImproperlyConfigured): pass +class InvalidCacheKeyError(Exception): + pass + +# memcached does not accept keys longer than 250 characters +MAX_KEY_LENGTH = 250 + class BaseCache(object): def __init__(self, params): timeout = params.get('timeout', 300) @@ -116,3 +122,15 @@ def clear(self): """Remove *all* values from the cache at once.""" raise NotImplementedError + + def _validate_key(self, key): + """ + Enforce keys that would be valid on all backends, to keep cache code portable. + + """ + if len(key) > MAX_KEY_LENGTH: + raise InvalidCacheKeyError('Key is too long (max length %s)' % MAX_KEY_LENGTH) + for char in key: + if ord(char) < 33 or ord(char) == 127: + raise InvalidCacheKeyError('Key contains control character') + diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py --- a/django/core/cache/backends/db.py +++ b/django/core/cache/backends/db.py @@ -25,6 +25,7 @@ self._cull_frequency = 3 def get(self, key, default=None): + self._validate_key(key) cursor = connection.cursor() cursor.execute("SELECT cache_key, value, expires FROM %s WHERE cache_key = %%s" % self._table, [key]) row = cursor.fetchone() @@ -39,9 +40,11 @@ return pickle.loads(base64.decodestring(value)) def set(self, key, value, timeout=None): + self._validate_key(key) self._base_set('set', key, value, timeout) def add(self, key, value, timeout=None): + self._validate_key(key) return self._base_set('add', key, value, timeout) def _base_set(self, mode, key, value, timeout=None): @@ -74,11 +77,13 @@ return True def delete(self, key): + self._validate_key(key) cursor = connection.cursor() cursor.execute("DELETE FROM %s WHERE cache_key = %%s" % self._table, [key]) transaction.commit_unless_managed() def has_key(self, key): + self._validate_key(key) now = datetime.now().replace(microsecond=0) cursor = connection.cursor() cursor.execute("SELECT cache_key FROM %s WHERE cache_key = %%s and expires > %%s" % self._table, diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py --- a/django/core/cache/backends/dummy.py +++ b/django/core/cache/backends/dummy.py @@ -6,22 +6,25 @@ def __init__(self, *args, **kwargs): pass - def add(self, *args, **kwargs): + def add(self, key, *args, **kwargs): + self._validate_key(key) return True def get(self, key, default=None): + self._validate_key(key) return default - def set(self, *args, **kwargs): - pass + def set(self, key, *args, **kwargs): + self._validate_key(key) - def delete(self, *args, **kwargs): - pass + def delete(self, key, *args, **kwargs): + self._validate_key(key) def get_many(self, *args, **kwargs): return {} - def has_key(self, *args, **kwargs): + def has_key(self, key, *args, **kwargs): + self._validate_key(key) return False def set_many(self, *args, **kwargs): diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py --- a/django/core/cache/backends/filebased.py +++ b/django/core/cache/backends/filebased.py @@ -32,6 +32,7 @@ self._createdir() def add(self, key, value, timeout=None): + self._validate_key(key) if self.has_key(key): return False @@ -39,6 +40,7 @@ return True def get(self, key, default=None): + self._validate_key(key) fname = self._key_to_file(key) try: f = open(fname, 'rb') @@ -56,6 +58,7 @@ return default def set(self, key, value, timeout=None): + self._validate_key(key) fname = self._key_to_file(key) dirname = os.path.dirname(fname) @@ -79,6 +82,7 @@ pass def delete(self, key): + self._validate_key(key) try: self._delete(self._key_to_file(key)) except (IOError, OSError): @@ -95,6 +99,7 @@ pass def has_key(self, key): + self._validate_key(key) fname = self._key_to_file(key) try: f = open(fname, 'rb') diff --git a/django/core/cache/backends/locmem.py b/django/core/cache/backends/locmem.py --- a/django/core/cache/backends/locmem.py +++ b/django/core/cache/backends/locmem.py @@ -30,6 +30,7 @@ self._lock = RWLock() def add(self, key, value, timeout=None): + self._validate_key(key) self._lock.writer_enters() try: exp = self._expire_info.get(key) @@ -44,6 +45,7 @@ self._lock.writer_leaves() def get(self, key, default=None): + self._validate_key(key) self._lock.reader_enters() try: exp = self._expire_info.get(key) @@ -76,6 +78,7 @@ self._expire_info[key] = time.time() + timeout def set(self, key, value, timeout=None): + self._validate_key(key) self._lock.writer_enters() # Python 2.4 doesn't allow combined try-except-finally blocks. try: @@ -87,6 +90,7 @@ self._lock.writer_leaves() def has_key(self, key): + self._validate_key(key) self._lock.reader_enters() try: exp = self._expire_info.get(key) @@ -127,6 +131,7 @@ pass def delete(self, key): + self._validate_key(key) self._lock.writer_enters() try: self._delete(key) diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py --- a/tests/regressiontests/cache/tests.py +++ b/tests/regressiontests/cache/tests.py @@ -352,6 +352,21 @@ self.assertEqual(self.cache.get('key3'), 'sausage') self.assertEqual(self.cache.get('key4'), 'lobster bisque') + def test_invalid_keys(self): + """ + All the builtin backends should refuse keys that would be refused by + memcached, so code using the cache API is portable. Refs #6447. + + We assert only that a generic exception is raised because we don't + want to encumber the memcached backend with key-checking code, and the + specific exceptions raised from memcached will depend on the client + library used. + + """ + self.assertRaises(Exception, self.cache.set, 'key with spaces', 'value') + # memcached also limits key length to 250 + self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value') + class DBCacheTests(unittest.TestCase, BaseCacheTests): def setUp(self): # Spaces are used in the table name to ensure quoting/escaping is working