summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--django/core/files/storage.py44
-rw-r--r--django/db/models/fields/files.py17
-rw-r--r--docs/howto/custom-file-storage.txt20
-rw-r--r--docs/internals/deprecation.txt4
-rw-r--r--docs/ref/files/storage.txt20
-rw-r--r--docs/releases/1.8.txt21
-rw-r--r--tests/file_storage/models.py19
-rw-r--r--tests/file_storage/tests.py70
8 files changed, 194 insertions, 21 deletions
diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 2d7c0c78e9..90ab1be27a 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -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):
diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py
index 8ca8532113..301244bbef 100644
--- a/django/db/models/fields/files.py
+++ b/django/db/models/fields/files.py
@@ -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
diff --git a/docs/howto/custom-file-storage.txt b/docs/howto/custom-file-storage.txt
index 9b217b8031..3d2c96e2bd 100644
--- a/docs/howto/custom-file-storage.txt
+++ b/docs/howto/custom-file-storage.txt
@@ -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.
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 345202545e..fe392cff97 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -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
diff --git a/docs/ref/files/storage.txt b/docs/ref/files/storage.txt
index a16b6b6c9d..88da416dcf 100644
--- a/docs/ref/files/storage.txt
+++ b/docs/ref/files/storage.txt
@@ -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``.
diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index c4e4e34894..122064971c 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -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``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/file_storage/models.py b/tests/file_storage/models.py
index 126cd202aa..2f58c53ddd 100644
--- a/tests/file_storage/models.py
+++ b/tests/file_storage/models.py
@@ -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',
+ )
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index e1938e9ea5..8e8b0e0a32 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -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'))
generated by cgit v1.3 (git 2.53.0) at 2026-07-06 11:41:50 +0000