diff --git a/common/djangoapps/util/memcache.py b/common/djangoapps/util/memcache.py index 540cf96539..ee450e68cb 100644 --- a/common/djangoapps/util/memcache.py +++ b/common/djangoapps/util/memcache.py @@ -8,15 +8,42 @@ import urllib def fasthash(string): - m = hashlib.new("md4") - m.update(string) - return m.hexdigest() + """ + Hashes `string` into a string representation of a 128-bit digest. + """ + md4 = hashlib.new("md4") + md4.update(string) + return md4.hexdigest() + + +def cleaned_string(val): + """ + Converts `val` to unicode and URL-encodes special characters + (including quotes and spaces) + """ + return urllib.quote_plus(smart_str(val)) def safe_key(key, key_prefix, version): - safe_key = urllib.quote_plus(smart_str(key)) + """ + Given a `key`, `key_prefix`, and `version`, + return a key that is safe to use with memcache. - if len(safe_key) > 250: - safe_key = fasthash(safe_key) + `key`, `key_prefix`, and `version` can be numbers, strings, or unicode. + """ - return ":".join([key_prefix, str(version), safe_key]) + # Clean for whitespace and control characters, which + # cause memcache to raise an exception + key = cleaned_string(key) + key_prefix = cleaned_string(key_prefix) + version = cleaned_string(version) + + # Attempt to combine the prefix, version, and key + combined = ":".join([key_prefix, version, key]) + + # If the total length is too long for memcache, hash it + if len(combined) > 250: + combined = fasthash(combined) + + # Return the result + return combined diff --git a/common/djangoapps/util/tests/__init__.py b/common/djangoapps/util/tests/__init__.py new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/common/djangoapps/util/tests/__init__.py @@ -0,0 +1 @@ + diff --git a/common/djangoapps/util/tests/test_memcache.py b/common/djangoapps/util/tests/test_memcache.py new file mode 100644 index 0000000000..de8d352c38 --- /dev/null +++ b/common/djangoapps/util/tests/test_memcache.py @@ -0,0 +1,124 @@ +""" +Tests for memcache in util app +""" + +from django.test import TestCase +from django.core.cache import get_cache +from django.conf import settings +from util.memcache import safe_key + + +class MemcacheTest(TestCase): + """ + Test memcache key cleanup + """ + + # Test whitespace, control characters, and some non-ASCII UTF-16 + UNICODE_CHAR_CODES = ([c for c in range(0, 30)] + [127] + + [129, 500, 2 ** 8 - 1, 2 ** 8 + 1, 2 ** 16 - 1]) + + def setUp(self): + self.cache = get_cache('default') + + def test_safe_key(self): + key = safe_key('test', 'prefix', 'version') + self.assertEqual(key, 'prefix:version:test') + + def test_numeric_inputs(self): + + # Numeric key + self.assertEqual(safe_key(1, 'prefix', 'version'), 'prefix:version:1') + + # Numeric prefix + self.assertEqual(safe_key('test', 5, 'version'), '5:version:test') + + # Numeric version + self.assertEqual(safe_key('test', 'prefix', 5), 'prefix:5:test') + + def test_safe_key_long(self): + + # Choose lengths close to memcached's cutoff (250) + for length in [248, 249, 250, 251, 252]: + + # Generate a key of that length + key = 'a' * length + + # Make the key safe + key = safe_key(key, '', '') + + # The key should now be valid + self.assertTrue(self._is_valid_key(key), + msg="Failed for key length {0}".format(length)) + + def test_long_key_prefix_version(self): + + # Long key + key = safe_key('a' * 300, 'prefix', 'version') + self.assertTrue(self._is_valid_key(key)) + + # Long prefix + key = safe_key('key', 'a' * 300, 'version') + self.assertTrue(self._is_valid_key(key)) + + # Long version + key = safe_key('key', 'prefix', 'a' * 300) + self.assertTrue(self._is_valid_key(key)) + + def test_safe_key_unicode(self): + + for unicode_char in self.UNICODE_CHAR_CODES: + + # Generate a key with that character + key = unichr(unicode_char) + + # Make the key safe + key = safe_key(key, '', '') + + # The key should now be valid + self.assertTrue(self._is_valid_key(key), + msg="Failed for unicode character {0}".format(unicode_char)) + + def test_safe_key_prefix_unicode(self): + + for unicode_char in self.UNICODE_CHAR_CODES: + + # Generate a prefix with that character + prefix = unichr(unicode_char) + + # Make the key safe + key = safe_key('test', prefix, '') + + # The key should now be valid + self.assertTrue(self._is_valid_key(key), + msg="Failed for unicode character {0}".format(unicode_char)) + + def test_safe_key_version_unicode(self): + + for unicode_char in self.UNICODE_CHAR_CODES: + + # Generate a version with that character + version = unichr(unicode_char) + + # Make the key safe + key = safe_key('test', '', version) + + # The key should now be valid + self.assertTrue(self._is_valid_key(key), + msg="Failed for unicode character {0}".format(unicode_char)) + + def _is_valid_key(self, key): + """ + Test that a key is memcache-compatible. + Based on Django's validator in core.cache.backends.base + """ + + # Check the length + if len(key) > 250: + return False + + # Check that there are no spaces or control characters + for char in key: + if ord(char) < 33 or ord(char) == 127: + return False + + return True diff --git a/common/djangoapps/util/tests.py b/common/djangoapps/util/tests/test_zendesk.py similarity index 99% rename from common/djangoapps/util/tests.py rename to common/djangoapps/util/tests/test_zendesk.py index d829676eaf..51d06a92ed 100644 --- a/common/djangoapps/util/tests.py +++ b/common/djangoapps/util/tests/test_zendesk.py @@ -1,4 +1,4 @@ -"""Tests for the util package""" +"""Tests for the Zendesk""" from django.conf import settings from django.contrib.auth.models import AnonymousUser