Code

Ticket #6447: ticket_6447.diff

File ticket_6447.diff, 7.4 KB (added by carljm, 4 years ago)

updated patch, applies to trunk

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@@ -46,6 +46,7 @@
37             self._cull_frequency = 3
38 
39     def get(self, key, default=None):
40+        self._validate_key(key)
41         db = router.db_for_read(self.cache_model_class)
42         table = connections[db].ops.quote_name(self._table)
43         cursor = connections[db].cursor()
44@@ -65,9 +66,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@@ -103,6 +106,7 @@
57             return True
58 
59     def delete(self, key):
60+        self._validate_key(key)
61         db = router.db_for_write(self.cache_model_class)
62         table = connections[db].ops.quote_name(self._table)
63         cursor = connections[db].cursor()
64@@ -111,6 +115,7 @@
65         transaction.commit_unless_managed(using=db)
66 
67     def has_key(self, key):
68+        self._validate_key(key)
69         db = router.db_for_read(self.cache_model_class)
70         table = connections[db].ops.quote_name(self._table)
71         cursor = connections[db].cursor()
72diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py
73--- a/django/core/cache/backends/dummy.py
74+++ b/django/core/cache/backends/dummy.py
75@@ -6,22 +6,25 @@
76     def __init__(self, *args, **kwargs):
77         pass
78 
79-    def add(self, *args, **kwargs):
80+    def add(self, key, *args, **kwargs):
81+        self._validate_key(key)
82         return True
83 
84     def get(self, key, default=None):
85+        self._validate_key(key)
86         return default
87 
88-    def set(self, *args, **kwargs):
89-        pass
90+    def set(self, key, *args, **kwargs):
91+        self._validate_key(key)
92 
93-    def delete(self, *args, **kwargs):
94-        pass
95+    def delete(self, key, *args, **kwargs):
96+        self._validate_key(key)
97 
98     def get_many(self, *args, **kwargs):
99         return {}
100 
101-    def has_key(self, *args, **kwargs):
102+    def has_key(self, key, *args, **kwargs):
103+        self._validate_key(key)
104         return False
105 
106     def set_many(self, *args, **kwargs):
107diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py
108--- a/django/core/cache/backends/filebased.py
109+++ b/django/core/cache/backends/filebased.py
110@@ -32,6 +32,7 @@
111             self._createdir()
112 
113     def add(self, key, value, timeout=None):
114+        self._validate_key(key)
115         if self.has_key(key):
116             return False
117 
118@@ -39,6 +40,7 @@
119         return True
120 
121     def get(self, key, default=None):
122+        self._validate_key(key)
123         fname = self._key_to_file(key)
124         try:
125             f = open(fname, 'rb')
126@@ -56,6 +58,7 @@
127         return default
128 
129     def set(self, key, value, timeout=None):
130+        self._validate_key(key)
131         fname = self._key_to_file(key)
132         dirname = os.path.dirname(fname)
133 
134@@ -79,6 +82,7 @@
135             pass
136 
137     def delete(self, key):
138+        self._validate_key(key)
139         try:
140             self._delete(self._key_to_file(key))
141         except (IOError, OSError):
142@@ -95,6 +99,7 @@
143             pass
144 
145     def has_key(self, key):
146+        self._validate_key(key)
147         fname = self._key_to_file(key)
148         try:
149             f = open(fname, 'rb')
150diff --git a/django/core/cache/backends/locmem.py b/django/core/cache/backends/locmem.py
151--- a/django/core/cache/backends/locmem.py
152+++ b/django/core/cache/backends/locmem.py
153@@ -30,6 +30,7 @@
154         self._lock = RWLock()
155 
156     def add(self, key, value, timeout=None):
157+        self._validate_key(key)
158         self._lock.writer_enters()
159         try:
160             exp = self._expire_info.get(key)
161@@ -44,6 +45,7 @@
162             self._lock.writer_leaves()
163 
164     def get(self, key, default=None):
165+        self._validate_key(key)
166         self._lock.reader_enters()
167         try:
168             exp = self._expire_info.get(key)
169@@ -76,6 +78,7 @@
170         self._expire_info[key] = time.time() + timeout
171 
172     def set(self, key, value, timeout=None):
173+        self._validate_key(key)
174         self._lock.writer_enters()
175         # Python 2.4 doesn't allow combined try-except-finally blocks.
176         try:
177@@ -87,6 +90,7 @@
178             self._lock.writer_leaves()
179 
180     def has_key(self, key):
181+        self._validate_key(key)
182         self._lock.reader_enters()
183         try:
184             exp = self._expire_info.get(key)
185@@ -127,6 +131,7 @@
186             pass
187 
188     def delete(self, key):
189+        self._validate_key(key)
190         self._lock.writer_enters()
191         try:
192             self._delete(key)
193diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt
194--- a/docs/topics/cache.txt
195+++ b/docs/topics/cache.txt
196@@ -537,6 +537,11 @@
197     >>> cache.get('my_key')
198     'hello, world!'
199 
200+.. note::
201+
202+   Cache keys may not be longer than 250 characters, and may not
203+   contain whitespace or control characters.
204+
205 The ``timeout`` argument is optional and defaults to the ``timeout``
206 argument in the ``CACHE_BACKEND`` setting (explained above). It's the number of
207 seconds the value should be stored in the cache.
208diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py
209--- a/tests/regressiontests/cache/tests.py
210+++ b/tests/regressiontests/cache/tests.py
211@@ -366,6 +366,21 @@
212                 count = count + 1
213         self.assertEqual(count, final_count)
214 
215+    def test_invalid_keys(self):
216+        """
217+        All the builtin backends should refuse keys that would be refused by
218+        memcached, so code using the cache API is portable. Refs #6447.
219+
220+        We assert only that a generic exception is raised because we don't
221+        want to encumber the memcached backend with key-checking code, and the
222+        specific exceptions raised from memcached will depend on the client
223+        library used.
224+       
225+        """
226+        self.assertRaises(Exception, self.cache.set, 'key with spaces', 'value')
227+        # memcached also limits key length to 250
228+        self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value')
229+
230 class DBCacheTests(unittest.TestCase, BaseCacheTests):
231     def setUp(self):
232         # Spaces are used in the table name to ensure quoting/escaping is working