p15693087
/
django
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fixed #9893 -- Allowed using a field's max_length in the Storage.

This commit is contained in:
Pavel Shpilev 2014-10-15 18:42:06 +11:00 committed by Tim Graham
parent b5c1a85b50
commit a7c256cb54
8 changed files with 194 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
django/core/files/storage.py
View File

@ -1,8 +1,11 @@
import os
import errno
from datetime import datetime
import errno
from inspect import getargspec
import os
import warnings
from django.conf import settings
from django.core.exceptions import SuspiciousFileOperation
from django.core.files import locks, File
from django.core.files.move import file_move_safe
from django.utils.crypto import get_random_string
@ -13,6 +16,7 @@ from django.utils.six.moves.urllib.parse import urljoin
from django.utils.text import get_valid_filename
from django.utils._os import safe_join, abspathu
from django.utils.deconstruct import deconstructible
from django.utils.deprecation import RemovedInDjango20Warning
__all__ = ('Storage', 'FileSystemStorage', 'DefaultStorage', 'default_storage')
@ -33,7 +37,7 @@ class Storage(object):
"""
return self._open(name, mode)
def save(self, name, content):
def save(self, name, content, max_length=None):
"""
Saves new content to the file specified by name. The content should be
a proper File object or any python file-like object, ready to be read
@ -46,7 +50,18 @@ class Storage(object):
if not hasattr(content, 'chunks'):
content = File(content)
name = self.get_available_name(name)
args, varargs, varkw, defaults = getargspec(self.get_available_name)
if 'max_length' in args:
name = self.get_available_name(name, max_length=max_length)
else:
warnings.warn(
'Backwards compatibility for storage backends without '
'support for the `max_length` argument in '
'Storage.get_available_name() will be removed in Django 2.0.',
RemovedInDjango20Warning, stacklevel=2
)
name = self.get_available_name(name)
name = self._save(name, content)
# Store filenames with forward slashes, even on Windows
@ -61,7 +76,7 @@ class Storage(object):
"""
return get_valid_filename(name)
def get_available_name(self, name):
def get_available_name(self, name, max_length=None):
"""
Returns a filename that's free on the target storage system, and
available for new content to be written to.
@ -71,10 +86,25 @@ class Storage(object):
# If the filename already exists, add an underscore and a random 7
# character alphanumeric string (before the file extension, if one
# exists) to the filename until the generated filename doesn't exist.
while self.exists(name):
# Truncate original name if required, so the new filename does not
# exceed the max_length.
while self.exists(name) or (max_length and len(name) > max_length):
# file_ext includes the dot.
name = os.path.join(dir_name, "%s_%s%s" % (file_root, get_random_string(7), file_ext))
if max_length is None:
continue
# Truncate file_root if max_length exceeded.
truncation = len(name) - max_length
if truncation > 0:
file_root = file_root[:-truncation]
# Entire file_root was truncated in attempt to find an available filename.
if not file_root:
raise SuspiciousFileOperation(
'Storage can not find an available filename for "%s". '
'Please make sure that the corresponding file field '
'allows sufficient "max_length".' % name
)
name = os.path.join(dir_name, "%s_%s%s" % (file_root, get_random_string(7), file_ext))
return name
def path(self, name):

17
django/db/models/fields/files.py
View File

@ -1,5 +1,7 @@
import datetime
from inspect import getargspec
import os
import warnings
from django import forms
from django.db.models.fields import Field
@ -11,6 +13,7 @@ from django.db.models import signals
from django.utils.encoding import force_str, force_text
from django.utils import six
from django.utils.translation import ugettext_lazy as _
from django.utils.deprecation import RemovedInDjango20Warning
class FieldFile(File):
@ -85,7 +88,19 @@ class FieldFile(File):
def save(self, name, content, save=True):
name = self.field.generate_filename(self.instance, name)
self.name = self.storage.save(name, content)
args, varargs, varkw, defaults = getargspec(self.storage.save)
if 'max_length' in args:
self.name = self.storage.save(name, content, max_length=self.field.max_length)
else:
warnings.warn(
'Backwards compatibility for storage backends without '
'support for the `max_length` argument in '
'Storage.save() will be removed in Django 2.0.',
RemovedInDjango20Warning, stacklevel=2
)
self.name = self.storage.save(name, content)
setattr(self.instance, self.field.name, self.name)
# Update the filesize cache

20
docs/howto/custom-file-storage.txt
View File

@ -50,7 +50,7 @@ typically have to be overridden:
* :meth:`Storage.size`
* :meth:`Storage.url`
Note however that not all these methods are required and may be deliberately
Note however that not all these methods are required and may be deliberately
omitted. As it happens, it is possible to leave each method unimplemented and
still have a working Storage.
@ -87,7 +87,6 @@ instead).
.. method:: get_valid_name(name)
Returns a filename suitable for use with the underlying storage system. The
``name`` argument passed to this method is the original filename sent to the
server, after having any path information removed. Override this to customize
@ -96,21 +95,28 @@ how non-standard characters are converted to safe filenames.
The code provided on ``Storage`` retains only alpha-numeric characters, periods
and underscores from the original filename, removing everything else.
.. method:: get_available_name(name)
.. method:: get_available_name(name, max_length=None)
Returns a filename that is available in the storage mechanism, possibly taking
the provided filename into account. The ``name`` argument passed to this method
will have already cleaned to a filename valid for the storage system, according
to the ``get_valid_name()`` method described above.
.. versionchanged:: 1.7
The length of the filename will not exceed ``max_length``, if provided. If a
free unique filename cannot be found, a :exc:`SuspiciousFileOperation
<django.core.exceptions.SuspiciousOperation>` exception is raised.
If a file with ``name`` already exists, an underscore plus a random 7
character alphanumeric string is appended to the filename before the
extension.
If a file with ``name`` already exists, an underscore plus a random 7 character
alphanumeric string is appended to the filename before the extension.
.. versionchanged:: 1.7
Previously, an underscore followed by a number (e.g. ``"_1"``, ``"_2"``,
etc.) was appended to the filename until an available name in the destination
directory was found. A malicious user could exploit this deterministic
algorithm to create a denial-of-service attack. This change was also made
in Django 1.6.6, 1.5.9, and 1.4.14.
.. versionchanged:: 1.8
The ``max_length`` argument was added.

4
docs/internals/deprecation.txt
View File

@ -143,6 +143,10 @@ details on these changes.
* Support for the ``=`` comparison operator in the ``if`` template tag will be
removed.
* The backwards compatibility shims to allow ``Storage.get_available_name()``
and ``Storage.save()`` to be defined without a ``max_length`` argument will
be removed.
.. _deprecation-removed-in-1.9:
1.9

20
docs/ref/files/storage.txt
View File

@ -114,12 +114,17 @@ The Storage Class
in the storage system, or ``False`` if the name is available for a new
file.
.. method:: get_available_name(name)
.. method:: get_available_name(name, max_length=None)
Returns a filename based on the ``name`` parameter that's free and
available for new content to be written to on the target storage
system.
The length of the filename will not exceed ``max_length``, if provided.
If a free unique filename cannot be found, a
:exc:`SuspiciousFileOperation
<django.core.exceptions.SuspiciousOperation>` exception will be raised.
If a file with ``name`` already exists, an underscore plus a random
7 character alphanumeric string is appended to the filename before
the extension.
@ -133,6 +138,10 @@ The Storage Class
attack. This change was also made in Django 1.6.6, 1.5.9, and
1.4.14.
.. versionchanged:: 1.8
The ``max_length`` argument was added.
.. method:: get_valid_name(name)
Returns a filename based on the ``name`` parameter that's suitable
@ -165,17 +174,24 @@ The Storage Class
standard ``open()``. For storage systems that aren't accessible from
the local filesystem, this will raise ``NotImplementedError`` instead.
.. method:: save(name, content)
.. method:: save(name, content, max_length=None)
Saves a new file using the storage system, preferably with the name
specified. If there already exists a file with this name ``name``, the
storage system may modify the filename as necessary to get a unique
name. The actual name of the stored file will be returned.
The ``max_length`` argument is passed along to
:meth:`get_available_name`.
The ``content`` argument must be an instance of
:class:`django.core.files.File` or of a subclass of
:class:`~django.core.files.File`.
.. versionchanged:: 1.8
The ``max_length`` argument was added.
.. method:: size(name)
Returns the total size, in bytes, of the file referenced by ``name``.

21
docs/releases/1.8.txt
View File

@ -317,7 +317,15 @@ Email
File Storage
^^^^^^^^^^^^
* ...
* :meth:`Storage.get_available_name()
<django.core.files.storage.Storage.get_available_name>` and
:meth:`Storage.save() <django.core.files.storage.Storage.save>`
now take a ``max_length`` argument to implement storage-level maximum
filename length constraints. Filenames exceeding this argument will get
truncated. This prevents a database error when appending a unique suffix to a
long filename that already exists on the storage. See the :ref:`deprecation
note <storage-max-length-update>` about adding this argument to your custom
storage classes.
File Uploads
^^^^^^^^^^^^
@ -1432,6 +1440,17 @@ loader that inherits ``BaseLoader``, you must inherit ``Loader`` instead.
Private API ``django.test.utils.TestTemplateLoader`` is deprecated in favor of
``django.template.loaders.locmem.Loader``.
.. _storage-max-length-update:
Support for the ``max_length`` argument on custom ``Storage`` classes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``Storage`` subclasses should add ``max_length=None`` as a parameter to
:meth:`~django.core.files.storage.Storage.get_available_name` and/or
:meth:`~django.core.files.storage.Storage.save` if they override either method.
Support for storages that do not accept this argument will be removed in
Django 2.0.
``qn`` replaced by ``compiler``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

19
tests/file_storage/models.py
View File

@ -13,6 +13,19 @@ from django.db import models
from django.core.files.storage import FileSystemStorage
class OldStyleFSStorage(FileSystemStorage):
"""
Storage backend without support for the ``max_length`` argument in
``get_available_name()`` and ``save()``; for backward-compatibility and
deprecation testing.
"""
def get_available_name(self, name):
return name
def save(self, name, content):
return super(OldStyleFSStorage, self).save(name, content)
temp_storage_location = tempfile.mkdtemp(dir=os.environ['DJANGO_TEST_TEMP_DIR'])
temp_storage = FileSystemStorage(location=temp_storage_location)
@ -31,3 +44,9 @@ class Storage(models.Model):
random = models.FileField(storage=temp_storage, upload_to=random_upload_to)
default = models.FileField(storage=temp_storage, upload_to='tests', default='tests/default.txt')
empty = models.FileField(storage=temp_storage)
limited_length = models.FileField(storage=temp_storage, upload_to='tests', max_length=20)
extended_length = models.FileField(storage=temp_storage, upload_to='tests', max_length=300)
old_style = models.FileField(
storage=OldStyleFSStorage(location=temp_storage_location),
upload_to='tests',
)

70
tests/file_storage/tests.py
View File

@ -8,6 +8,7 @@ import sys
import tempfile
import time
import unittest
import warnings
from datetime import datetime, timedelta
try:
@ -16,7 +17,7 @@ except ImportError:
import dummy_threading as threading
from django.core.cache import cache
from django.core.exceptions import SuspiciousOperation
from django.core.exceptions import SuspiciousOperation, SuspiciousFileOperation
from django.core.files.base import File, ContentFile
from django.core.files.storage import FileSystemStorage, get_storage_class
from django.core.files.uploadedfile import (InMemoryUploadedFile, SimpleUploadedFile,
@ -407,7 +408,7 @@ class FileStorageTests(unittest.TestCase):
class CustomStorage(FileSystemStorage):
def get_available_name(self, name):
def get_available_name(self, name, max_length=None):
"""
Append numbers to duplicate files rather than underscores, like Trac.
"""
@ -433,7 +434,7 @@ class CustomStorageTests(FileStorageTests):
self.storage.delete(second)
class FileFieldStorageTests(unittest.TestCase):
class FileFieldStorageTests(SimpleTestCase):
def tearDown(self):
shutil.rmtree(temp_storage_location)
@ -503,6 +504,69 @@ class FileFieldStorageTests(unittest.TestCase):
for o in objs:
o.delete()
def test_file_truncation(self):
# Given the max_length is limited, when multiple files get uploaded
# under the same name, then the filename get truncated in order to fit
# in _(7 random chars). When most of the max_length is taken by
# dirname + extension and there are not enough characters in the
# filename to truncate, an exception should be raised.
objs = [Storage() for i in range(2)]
filename = 'filename.ext'
for o in objs:
o.limited_length.save(filename, ContentFile('Same Content'))
try:
# Testing truncation.
names = [o.limited_length.name for o in objs]
self.assertEqual(names[0], 'tests/%s' % filename)
six.assertRegex(self, names[1], 'tests/fi_%s.ext' % FILE_SUFFIX_REGEX)
# Testing exception is raised when filename is too short to truncate.
filename = 'short.longext'
objs[0].limited_length.save(filename, ContentFile('Same Content'))
self.assertRaisesMessage(
SuspiciousFileOperation, 'Storage can not find an available filename',
objs[1].limited_length.save, *(filename, ContentFile('Same Content'))
)
finally:
for o in objs:
o.delete()
def test_extended_length_storage(self):
# Testing FileField with max_length > 255. Most systems have filename
# length limitation of 255. Path takes extra chars.
filename = 251 * 'a' # 4 chars for extension.
obj = Storage()
obj.extended_length.save('%s.txt' % filename, ContentFile('Same Content'))
self.assertEqual(obj.extended_length.name, 'tests/%s.txt' % filename)
self.assertEqual(obj.extended_length.read(), b'Same Content')
obj.extended_length.close()
def test_old_style_storage(self):
# Testing backward-compatibility with old-style storage backends that
# don't take ``max_length`` parameter in ``get_available_name()``
# and save(). A deprecation warning should be raised.
obj = Storage()
with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')
obj.old_style.save('deprecated_storage_test.txt', ContentFile('Same Content'))
self.assertEqual(len(warns), 2)
self.assertEqual(
str(warns[0].message),
'Backwards compatibility for storage backends without support for '
'the `max_length` argument in Storage.save() will be removed in '
'Django 2.0.'
)
self.assertEqual(
str(warns[1].message),
'Backwards compatibility for storage backends without support for '
'the `max_length` argument in Storage.get_available_name() will '
'be removed in Django 2.0.'
)
self.assertEqual(obj.old_style.name, 'tests/deprecated_storage_test.txt')
self.assertEqual(obj.old_style.read(), b'Same Content')
obj.old_style.close()
def test_filefield_default(self):
# Default values allow an object to access a single file.
temp_storage.save('tests/default.txt', ContentFile('default content'))