summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--django/core/cache/__init__.py2
-rw-r--r--django/core/cache/backends/base.py28
-rw-r--r--django/core/cache/backends/db.py5
-rw-r--r--django/core/cache/backends/dummy.py15
-rw-r--r--django/core/cache/backends/filebased.py5
-rw-r--r--django/core/cache/backends/locmem.py5
-rw-r--r--django/core/exceptions.py7
-rw-r--r--docs/topics/cache.txt39
-rw-r--r--tests/regressiontests/cache/liberal_backend.py9
-rw-r--r--tests/regressiontests/cache/tests.py62
10 files changed, 167 insertions, 10 deletions
diff --git a/django/core/cache/__init__.py b/django/core/cache/__init__.py
index 1b602908cb..680f724f94 100644
--- a/django/core/cache/__init__.py
+++ b/django/core/cache/__init__.py
@@ -18,7 +18,7 @@ See docs/cache.txt for information on the public API.
from cgi import parse_qsl
from django.conf import settings
from django.core import signals
-from django.core.cache.backends.base import InvalidCacheBackendError
+from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning
from django.utils import importlib
# Name for use in settings file --> name of module in "backends" directory.
diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py
index e58267a2e9..83dd461804 100644
--- a/django/core/cache/backends/base.py
+++ b/django/core/cache/backends/base.py
@@ -1,10 +1,18 @@
"Base Cache class."
-from django.core.exceptions import ImproperlyConfigured
+import warnings
+
+from django.core.exceptions import ImproperlyConfigured, DjangoRuntimeWarning
class InvalidCacheBackendError(ImproperlyConfigured):
pass
+class CacheKeyWarning(DjangoRuntimeWarning):
+ pass
+
+# Memcached does not accept keys longer than this.
+MEMCACHE_MAX_KEY_LENGTH = 250
+
class BaseCache(object):
def __init__(self, params):
timeout = params.get('timeout', 300)
@@ -116,3 +124,21 @@ class BaseCache(object):
def clear(self):
"""Remove *all* values from the cache at once."""
raise NotImplementedError
+
+ def validate_key(self, key):
+ """
+ Warn about keys that would not be portable to the memcached
+ backend. This encourages (but does not force) writing backend-portable
+ cache code.
+
+ """
+ if len(key) > MEMCACHE_MAX_KEY_LENGTH:
+ warnings.warn('Cache key will cause errors if used with memcached: '
+ '%s (longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH),
+ CacheKeyWarning)
+ for char in key:
+ if ord(char) < 33 or ord(char) == 127:
+ warnings.warn('Cache key contains characters that will cause '
+ 'errors if used with memcached: %r' % key,
+ CacheKeyWarning)
+
diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py
index 833fa749aa..c4429c80b3 100644
--- a/django/core/cache/backends/db.py
+++ b/django/core/cache/backends/db.py
@@ -46,6 +46,7 @@ class CacheClass(BaseCache):
self._cull_frequency = 3
def get(self, key, default=None):
+ self.validate_key(key)
db = router.db_for_read(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
@@ -65,9 +66,11 @@ class CacheClass(BaseCache):
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):
@@ -103,6 +106,7 @@ class CacheClass(BaseCache):
return True
def delete(self, key):
+ self.validate_key(key)
db = router.db_for_write(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
@@ -111,6 +115,7 @@ class CacheClass(BaseCache):
transaction.commit_unless_managed(using=db)
def has_key(self, key):
+ self.validate_key(key)
db = router.db_for_read(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py
index 4337484cb1..f73b7408bc 100644
--- a/django/core/cache/backends/dummy.py
+++ b/django/core/cache/backends/dummy.py
@@ -6,22 +6,25 @@ class CacheClass(BaseCache):
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
index 6057f11ef7..46e69f3091 100644
--- a/django/core/cache/backends/filebased.py
+++ b/django/core/cache/backends/filebased.py
@@ -32,6 +32,7 @@ class CacheClass(BaseCache):
self._createdir()
def add(self, key, value, timeout=None):
+ self.validate_key(key)
if self.has_key(key):
return False
@@ -39,6 +40,7 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
pass
def delete(self, key):
+ self.validate_key(key)
try:
self._delete(self._key_to_file(key))
except (IOError, OSError):
@@ -95,6 +99,7 @@ class CacheClass(BaseCache):
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
index eff1201b97..fe33d33307 100644
--- a/django/core/cache/backends/locmem.py
+++ b/django/core/cache/backends/locmem.py
@@ -30,6 +30,7 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
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 @@ class CacheClass(BaseCache):
pass
def delete(self, key):
+ self.validate_key(key)
self._lock.writer_enters()
try:
self._delete(key)
diff --git a/django/core/exceptions.py b/django/core/exceptions.py
index ee6d5fe37b..21be8702fa 100644
--- a/django/core/exceptions.py
+++ b/django/core/exceptions.py
@@ -1,4 +1,9 @@
-"Global Django exceptions"
+"""
+Global Django exception and warning classes.
+"""
+
+class DjangoRuntimeWarning(RuntimeWarning):
+ pass
class ObjectDoesNotExist(Exception):
"The requested object does not exist"
diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt
index e5ca526494..5797199411 100644
--- a/docs/topics/cache.txt
+++ b/docs/topics/cache.txt
@@ -641,6 +641,45 @@ nonexistent cache key.::
However, if the backend doesn't natively provide an increment/decrement
operation, it will be implemented using a two-step retrieve/update.
+Cache key warnings
+------------------
+
+.. versionadded:: 1.3
+
+Memcached, the most commonly-used production cache backend, does not allow
+cache keys longer than 250 characters or containing whitespace or control
+characters, and using such keys will cause an exception. To encourage
+cache-portable code and minimize unpleasant surprises, the other built-in cache
+backends issue a warning (``django.core.cache.backends.base.CacheKeyWarning``)
+if a key is used that would cause an error on memcached.
+
+If you are using a production backend that can accept a wider range of keys (a
+custom backend, or one of the non-memcached built-in backends), and want to use
+this wider range without warnings, you can silence ``CacheKeyWarning`` with
+this code in the ``management`` module of one of your
+:setting:`INSTALLED_APPS`::
+
+ import warnings
+
+ from django.core.cache import CacheKeyWarning
+
+ warnings.simplefilter("ignore", CacheKeyWarning)
+
+If you want to instead provide custom key validation logic for one of the
+built-in backends, you can subclass it, override just the ``validate_key``
+method, and follow the instructions for `using a custom cache backend`_. For
+instance, to do this for the ``locmem`` backend, put this code in a module::
+
+ from django.core.cache.backends.locmem import CacheClass as LocMemCacheClass
+
+ class CacheClass(LocMemCacheClass):
+ def validate_key(self, key):
+ """Custom validation, raising exceptions or warnings as needed."""
+ # ...
+
+...and use the dotted Python path to this module as the scheme portion of your
+:setting:`CACHE_BACKEND`.
+
Upstream caches
===============
diff --git a/tests/regressiontests/cache/liberal_backend.py b/tests/regressiontests/cache/liberal_backend.py
new file mode 100644
index 0000000000..5c7e312690
--- /dev/null
+++ b/tests/regressiontests/cache/liberal_backend.py
@@ -0,0 +1,9 @@
+from django.core.cache.backends.locmem import CacheClass as LocMemCacheClass
+
+class LiberalKeyValidationMixin(object):
+ def validate_key(self, key):
+ pass
+
+class CacheClass(LiberalKeyValidationMixin, LocMemCacheClass):
+ pass
+
diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py
index 0f9f5c9fff..1e0a4046bb 100644
--- a/tests/regressiontests/cache/tests.py
+++ b/tests/regressiontests/cache/tests.py
@@ -8,11 +8,12 @@ import shutil
import tempfile
import time
import unittest
+import warnings
from django.conf import settings
from django.core import management
from django.core.cache import get_cache
-from django.core.cache.backends.base import InvalidCacheBackendError
+from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning
from django.http import HttpResponse, HttpRequest
from django.middleware.cache import FetchFromCacheMiddleware, UpdateCacheMiddleware
from django.utils import translation
@@ -366,6 +367,33 @@ class BaseCacheTests(object):
count = count + 1
self.assertEqual(count, final_count)
+ def test_invalid_keys(self):
+ """
+ All the builtin backends (except memcached, see below) should warn on
+ keys that would be refused by memcached. This encourages portable
+ caching code without making it too difficult to use production backends
+ with more liberal key rules. Refs #6447.
+
+ """
+ # On Python 2.6+ we could use the catch_warnings context
+ # manager to test this warning nicely. Since we can't do that
+ # yet, the cleanest option is to temporarily ask for
+ # CacheKeyWarning to be raised as an exception.
+ warnings.simplefilter("error", CacheKeyWarning)
+
+ # memcached does not allow whitespace or control characters in keys
+ self.assertRaises(CacheKeyWarning, self.cache.set, 'key with spaces', 'value')
+ # memcached limits key length to 250
+ self.assertRaises(CacheKeyWarning, self.cache.set, 'a' * 251, 'value')
+
+ # The warnings module has no public API for getting the
+ # current list of warning filters, so we can't save that off
+ # and reset to the previous value, we have to globally reset
+ # it. The effect will be the same, as long as the Django test
+ # runner doesn't add any global warning filters (it currently
+ # does not).
+ warnings.resetwarnings()
+
class DBCacheTests(unittest.TestCase, BaseCacheTests):
def setUp(self):
# Spaces are used in the table name to ensure quoting/escaping is working
@@ -397,6 +425,22 @@ if settings.CACHE_BACKEND.startswith('memcached://'):
def setUp(self):
self.cache = get_cache(settings.CACHE_BACKEND)
+ def test_invalid_keys(self):
+ """
+ On memcached, we don't introduce a duplicate key validation
+ step (for speed reasons), we just let the memcached API
+ library raise its own exception on bad keys. Refs #6447.
+
+ In order to be memcached-API-library agnostic, we only assert
+ that a generic exception of some kind is raised.
+
+ """
+ # memcached does not allow whitespace or control characters in keys
+ self.assertRaises(Exception, self.cache.set, 'key with spaces', 'value')
+ # memcached limits key length to 250
+ self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value')
+
+
class FileBasedCacheTests(unittest.TestCase, BaseCacheTests):
"""
Specific test cases for the file-based cache.
@@ -429,6 +473,22 @@ class FileBasedCacheTests(unittest.TestCase, BaseCacheTests):
def test_cull(self):
self.perform_cull_test(50, 28)
+class CustomCacheKeyValidationTests(unittest.TestCase):
+ """
+ Tests for the ability to mixin a custom ``validate_key`` method to
+ a custom cache backend that otherwise inherits from a builtin
+ backend, and override the default key validation. Refs #6447.
+
+ """
+ def test_custom_key_validation(self):
+ cache = get_cache('regressiontests.cache.liberal_backend://')
+
+ # this key is both longer than 250 characters, and has spaces
+ key = 'some key with spaces' * 15
+ val = 'a value'
+ cache.set(key, val)
+ self.assertEqual(cache.get(key), val)
+
class CacheUtils(unittest.TestCase):
"""TestCase for django.utils.cache functions."""