Skip to content

Commit

Permalink
Fix get_available_overwrite_name when len(name) == max_length
Browse files Browse the repository at this point in the history
Closes #588
  • Loading branch information
jschneier committed Sep 6, 2018
1 parent e0a00fb commit 96d4268
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 12 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
django-storages CHANGELOG
=========================

1.7.1 (2018-09-XX)
******************

- Fix off-by-1 error in ``get_available_name`` whenever ``file_overwrite`` or ``overwrite_files`` is ``True`` (`#588`_, `#589`_)

.. _#588: https://github.com/jschneier/django-storages/issues/588
.. _#589: https://github.com/jschneier/django-storages/pull/589

1.7 (2018-09-03)
****************

Expand Down
2 changes: 1 addition & 1 deletion storages/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def lookup_env(names):


def get_available_overwrite_name(name, max_length):
if max_length is None or len(name) < max_length:
if max_length is None or len(name) <= max_length:
return name

# Adapted from Django
Expand Down
12 changes: 1 addition & 11 deletions tests/test_gcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
import mimetypes
from datetime import datetime, timedelta

from django.core.exceptions import (
ImproperlyConfigured, SuspiciousFileOperation,
)
from django.core.exceptions import ImproperlyConfigured
from django.core.files.base import ContentFile
from django.test import TestCase
from django.utils import timezone
Expand Down Expand Up @@ -376,14 +374,6 @@ def test_get_available_name_unicode(self):
filename = 'ủⓝï℅ⅆℇ.txt'
self.assertEqual(self.storage.get_available_name(filename), filename)

def test_get_available_name_overwrite_maxlength(self):
self.storage.file_overwrite = True

self.assertEqual(self.storage.get_available_name('test/foo.txt', 11), 'test/fo.txt')
self.assertEqual(self.storage.get_available_name('test_a/foobar.txt', None), 'test_a/foobar.txt')
with self.assertRaises(SuspiciousFileOperation):
self.storage.get_available_name('test_a/foobar.txt', 10)

def test_cache_control(self):
data = 'This is some test content.'
filename = 'cache_control_file.txt'
Expand Down
25 changes: 25 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import datetime

from django.conf import settings
from django.core.exceptions import SuspiciousFileOperation
from django.test import TestCase

from storages import utils
from storages.utils import get_available_overwrite_name as gaon


class SettingTest(TestCase):
Expand Down Expand Up @@ -108,3 +110,26 @@ def test_join_nothing(self):
def test_with_base_url_join_nothing(self):
path = utils.safe_join('base_url')
self.assertEqual(path, 'base_url/')


class TestGetAvailableOverwriteName(TestCase):
def test_maxlength_is_none(self):
name = 'superlong/file/with/path.txt'
self.assertEqual(gaon(name, None), name)

def test_maxlength_equals_name(self):
name = 'parent/child.txt'
self.assertEqual(gaon(name, len(name)), name)

def test_maxlength_is_greater_than_name(self):
name = 'parent/child.txt'
self.assertEqual(gaon(name, len(name) + 1), name)

def test_maxlength_less_than_name(self):
name = 'parent/child.txt'
self.assertEqual(gaon(name, len(name) - 1), 'parent/chil.txt')

def test_truncates_away_filename_raises(self):
name = 'parent/child.txt'
with self.assertRaises(SuspiciousFileOperation):
gaon(name, len(name) - 5)

0 comments on commit 96d4268

Please sign in to comment.