Code

Ticket #6447: 6447_r13405.diff

File 6447_r13405.diff, 7.0 KB (added by carljm, 4 years ago)

patch to enforce same key restrictions on all cache backends

Line 
1diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py
2--- a/django/core/cache/backends/base.py
3+++ b/django/core/cache/backends/base.py
4@@ -5,6 +5,12 @@
5 class InvalidCacheBackendError(ImproperlyConfigured):
6     pass
7 
8+class InvalidCacheKeyError(Exception):
9+    pass
10+
11+# memcached does not accept keys longer than 250 characters
12+MAX_KEY_LENGTH = 250
13+
14 class BaseCache(object):
15     def __init__(self, params):
16         timeout = params.get('timeout', 300)
17@@ -116,3 +122,15 @@
18     def clear(self):
19         """Remove *all* values from the cache at once."""
20         raise NotImplementedError
21+
22+    def _validate_key(self, key):
23+        """
24+        Enforce keys that would be valid on all backends, to keep cache code portable.
25+
26+        """
27+        if len(key) > MAX_KEY_LENGTH:
28+            raise InvalidCacheKeyError('Key is too long (max length %s)' % MAX_KEY_LENGTH)
29+        for char in key:
30+            if ord(char) < 33 or ord(char) == 127:
31+                raise InvalidCacheKeyError('Key contains control character')
32+       
33diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py
34--- a/django/core/cache/backends/db.py
35+++ b/django/core/cache/backends/db.py
36@@ -25,6 +25,7 @@
37             self._cull_frequency = 3
38 
39     def get(self, key, default=None):
40+        self._validate_key(key)
41         cursor = connection.cursor()
42         cursor.execute("SELECT cache_key, value, expires FROM %s WHERE cache_key = %%s" % self._table, [key])
43         row = cursor.fetchone()
44@@ -39,9 +40,11 @@
45         return pickle.loads(base64.decodestring(value))
46 
47     def set(self, key, value, timeout=None):
48+        self._validate_key(key)
49         self._base_set('set', key, value, timeout)
50 
51     def add(self, key, value, timeout=None):
52+        self._validate_key(key)
53         return self._base_set('add', key, value, timeout)
54 
55     def _base_set(self, mode, key, value, timeout=None):
56@@ -74,11 +77,13 @@
57             return True
58 
59     def delete(self, key):
60+        self._validate_key(key)
61         cursor = connection.cursor()
62         cursor.execute("DELETE FROM %s WHERE cache_key = %%s" % self._table, [key])
63         transaction.commit_unless_managed()
64 
65     def has_key(self, key):
66+        self._validate_key(key)
67         now = datetime.now().replace(microsecond=0)
68         cursor = connection.cursor()
69         cursor.execute("SELECT cache_key FROM %s WHERE cache_key = %%s and expires > %%s" % self._table,
70diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py
71--- a/django/core/cache/backends/dummy.py
72+++ b/django/core/cache/backends/dummy.py
73@@ -6,22 +6,25 @@
74     def __init__(self, *args, **kwargs):
75         pass
76 
77-    def add(self, *args, **kwargs):
78+    def add(self, key, *args, **kwargs):
79+        self._validate_key(key)
80         return True
81 
82     def get(self, key, default=None):
83+        self._validate_key(key)
84         return default
85 
86-    def set(self, *args, **kwargs):
87-        pass
88+    def set(self, key, *args, **kwargs):
89+        self._validate_key(key)
90 
91-    def delete(self, *args, **kwargs):
92-        pass
93+    def delete(self, key, *args, **kwargs):
94+        self._validate_key(key)
95 
96     def get_many(self, *args, **kwargs):
97         return {}
98 
99-    def has_key(self, *args, **kwargs):
100+    def has_key(self, key, *args, **kwargs):
101+        self._validate_key(key)
102         return False
103 
104     def set_many(self, *args, **kwargs):
105diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py
106--- a/django/core/cache/backends/filebased.py
107+++ b/django/core/cache/backends/filebased.py
108@@ -32,6 +32,7 @@
109             self._createdir()
110 
111     def add(self, key, value, timeout=None):
112+        self._validate_key(key)
113         if self.has_key(key):
114             return False
115 
116@@ -39,6 +40,7 @@
117         return True
118 
119     def get(self, key, default=None):
120+        self._validate_key(key)
121         fname = self._key_to_file(key)
122         try:
123             f = open(fname, 'rb')
124@@ -56,6 +58,7 @@
125         return default
126 
127     def set(self, key, value, timeout=None):
128+        self._validate_key(key)
129         fname = self._key_to_file(key)
130         dirname = os.path.dirname(fname)
131 
132@@ -79,6 +82,7 @@
133             pass
134 
135     def delete(self, key):
136+        self._validate_key(key)
137         try:
138             self._delete(self._key_to_file(key))
139         except (IOError, OSError):
140@@ -95,6 +99,7 @@
141             pass
142 
143     def has_key(self, key):
144+        self._validate_key(key)
145         fname = self._key_to_file(key)
146         try:
147             f = open(fname, 'rb')
148diff --git a/django/core/cache/backends/locmem.py b/django/core/cache/backends/locmem.py
149--- a/django/core/cache/backends/locmem.py
150+++ b/django/core/cache/backends/locmem.py
151@@ -30,6 +30,7 @@
152         self._lock = RWLock()
153 
154     def add(self, key, value, timeout=None):
155+        self._validate_key(key)
156         self._lock.writer_enters()
157         try:
158             exp = self._expire_info.get(key)
159@@ -44,6 +45,7 @@
160             self._lock.writer_leaves()
161 
162     def get(self, key, default=None):
163+        self._validate_key(key)
164         self._lock.reader_enters()
165         try:
166             exp = self._expire_info.get(key)
167@@ -76,6 +78,7 @@
168         self._expire_info[key] = time.time() + timeout
169 
170     def set(self, key, value, timeout=None):
171+        self._validate_key(key)
172         self._lock.writer_enters()
173         # Python 2.4 doesn't allow combined try-except-finally blocks.
174         try:
175@@ -87,6 +90,7 @@
176             self._lock.writer_leaves()
177 
178     def has_key(self, key):
179+        self._validate_key(key)
180         self._lock.reader_enters()
181         try:
182             exp = self._expire_info.get(key)
183@@ -127,6 +131,7 @@
184             pass
185 
186     def delete(self, key):
187+        self._validate_key(key)
188         self._lock.writer_enters()
189         try:
190             self._delete(key)
191diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py
192--- a/tests/regressiontests/cache/tests.py
193+++ b/tests/regressiontests/cache/tests.py
194@@ -352,6 +352,21 @@
195         self.assertEqual(self.cache.get('key3'), 'sausage')
196         self.assertEqual(self.cache.get('key4'), 'lobster bisque')
197 
198+    def test_invalid_keys(self):
199+        """
200+        All the builtin backends should refuse keys that would be refused by
201+        memcached, so code using the cache API is portable. Refs #6447.
202+
203+        We assert only that a generic exception is raised because we don't
204+        want to encumber the memcached backend with key-checking code, and the
205+        specific exceptions raised from memcached will depend on the client
206+        library used.
207+       
208+        """
209+        self.assertRaises(Exception, self.cache.set, 'key with spaces', 'value')
210+        # memcached also limits key length to 250
211+        self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value')
212+
213 class DBCacheTests(unittest.TestCase, BaseCacheTests):
214     def setUp(self):
215         # Spaces are used in the table name to ensure quoting/escaping is working