Opened 12 years ago

Closed 12 years ago

#17861 closed Bug (fixed)

Static files cache_key should handle spaces in filenames

Reported by: milosu Owned by: nobody
Component: contrib.staticfiles Version: 1.4-beta-1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

trying migrate my project to the Django 1.4c1, I get the following errors when running the storagefiles_tests test case:

======================================================================
ERROR: test_cache_invalidation (regressiontests.staticfiles_tests.tests.TestCollectionCachedStorage)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python26\Lib\site-packages\django_versions\Django-1.4c1\tests\regressiontests\staticfiles_tests\tests.py", lin
e 118, in setUp
    self.run_collectstatic()
  File "C:\Python26\Lib\site-packages\django_versions\Django-1.4c1\tests\regressiontests\staticfiles_tests\tests.py", lin
e 129, in run_collectstatic
    ignore_patterns=['*.ignoreme'], **kwargs)
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\core\management\__init__.py", line 150, in call
_command
    return klass.execute(*args, **defaults)
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\core\management\base.py", line 232, in execute
    output = self.handle(*args, **options)
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\core\management\base.py", line 371, in handle
    return self.handle_noargs(**options)
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\contrib\staticfiles\management\commands\collect
static.py", line 163, in handle_noargs
    collected = self.collect()
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\contrib\staticfiles\management\commands\collect
static.py", line 120, in collect
    for original_path, processed_path, processed in processor:
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\contrib\staticfiles\storage.py", line 192, in p
ost_process
    self.cache.delete_many([self.cache_key(path) for path in paths])
  File "C:\Python26\lib\site-packages\django_versions\Django-1.4c1\django\core\cache\backends\memcached.py", line 123, in delete_many
    self._cache.delete_multi(map(l, keys))
  File "build\bdist.win32\egg\memcache.py", line 336, in delete_multi
    server_keys, prefixed_to_orig_key = self._map_and_prefix_keys(keys, key_prefix)
  File "build\bdist.win32\egg\memcache.py", line 585, in _map_and_prefix_keys
    self.check_key(str_orig_key, key_extra_len=key_extra_len)
  File "build\bdist.win32\egg\memcache.py", line 958, in check_key
    "Control characters not allowed")
MemcachedKeyCharacterError: Control characters not allowed

After a bit of digging, the root cause is a file which filename contains spaces,

e.g. in my case there is a file:

django/contrib/admin/static/admin/filebrowser/uploadify/Uploadify v2.1.0 Manual.pdf

The above test case is failing while trying to use the following cache key:

u'staticfiles:cache:admin\\filebrowser\\uploadify\\Uploadify v2.1.0 Manual.pdf'

This cache key is invalid under Memcached that I'm running.

Patch attached.

Attachments (2)

staticfiles.diff (447 bytes ) - added by milosu 12 years ago.
err (29.5 KB ) - added by milosu 12 years ago.
python runtests .py --settings=settings staticfiles_tests

Download all attachments as: .zip

Change History (11)

by milosu, 12 years ago

Attachment: staticfiles.diff added

comment:1 by Thomas Güttler, 12 years ago

Triage Stage: UnreviewedAccepted

I tried to reproduce it, but I only get a warning.

mkdir django/contrib/admin/static/admin/filebrowser/uploadify/
export DJANGO_SETTINGS_MODULE=test_sqlite
vi 'django/contrib/admin/static/admin/filebrowser/uploadify/Uploadify v2.1.0 Manual.txt' # enter some text (empty file with "touch" is not enough)
./tests/runtests.py staticfiles_tests
Creating test database for alias 'default'...
Creating test database for alias 'other'...
........../home/modwork_eins_d/_django-1_4_x/django/core/cache/backends/base.py:202: CacheKeyWarning: Cache key contains characters that will cause errors if used with memcached: ':1:staticfiles:cache:admin/filebrowser/uploadify/Uploadify v2.1.0 Manual.txt'
  CacheKeyWarning)
.............................................................
----------------------------------------------------------------------
Ran 71 tests in 3.417s

comment:2 by milosu, 12 years ago

I'm missing warning due to this settings in settings.py:

CACHE_BACKEND = 'johnny.backends.memcached://127.0.0.1:11211/'

relative old app johnny cache with its own cache backend

comment:3 by Ramiro Morales, 12 years ago

So, is this a problem with Johnny cache?. Can we close this ticket?

comment:4 by Claude Paroz, 12 years ago

Memcached does not allow spaces in cache keys, so I think the bug is clearly confirmed.

comment:5 by Claude Paroz, 12 years ago

Type: UncategorizedBug

comment:6 by milosu, 12 years ago

I've changed my settings to:

CACHES = {
    'default': {
        'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache',
        'LOCATION': '127.0.0.1:11211',
    }
}

I'm still getting "MemcachedKeyCharacterError: Control characters not allowed" errors. See another attachment.

by milosu, 12 years ago

Attachment: err added

python runtests .py --settings=settings staticfiles_tests

comment:7 by Preston Holmes, 12 years ago

If this key is to be sanitized, should it be something that passes all requirements of django.core.cache.backends.base.BaseCache.validate_key ?

comment:8 by Preston Holmes, 12 years ago

Has patch: set

After talking with jezdez at the sprints - determined most straightforward fix is the md5 that path

https://github.com/django/django/pull/123

comment:9 by Jannis Leidel, 12 years ago

Resolution: fixed
Status: newclosed

In [17688]:

Fixed #17861 -- Took care of special characters when creating the staticfiles storage cache keys. Many thanks to Preston Holmes.

Note: See TracTickets for help on using tickets.
Back to Top