Skip to content

Commit

Permalink
Fix S3Boto3: Double localtime call Exception on modified_time when US…
Browse files Browse the repository at this point in the history
…E_TZ=False jschneier#234
  • Loading branch information
piglei committed Dec 4, 2016
1 parent c366cbf commit d9d5dd5
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ By order of apparition, thanks:
* Anthony Monthe (Dropbox)
* EunPyo (Andrew) Hong (Azure)
* Michael Barrientos (S3 with Boto3)
* piglei (patches)

Extra thanks to Marty for adding this in Django,
you can buy his very interesting book (Pro Django).
9 changes: 7 additions & 2 deletions storages/backends/s3boto3.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from django.utils.encoding import force_text, smart_str, filepath_to_uri, force_bytes
from django.utils.six.moves.urllib import parse as urlparse
from django.utils.six import BytesIO
from django.utils.timezone import localtime
from django.utils.timezone import localtime, is_naive

try:
from boto3 import resource
Expand Down Expand Up @@ -534,7 +534,12 @@ def get_modified_time(self, name):

def modified_time(self, name):
"""Returns a naive datetime object containing the last modified time."""
return localtime(self.get_modified_time(name)).replace(tzinfo=None)
# If get_modified_time already returns a naive DateTime object, which happens
# when USE_TZ=False, return it directly instead of transfer it.
mtime = self.get_modified_time(name)
if is_naive(mtime):
return mtime
return localtime(mtime).replace(tzinfo=None)

def _strip_signing_parameters(self, url):
# Boto3 does not currently support generating URLs that are unsigned. Instead we
Expand Down
15 changes: 13 additions & 2 deletions tests/test_s3boto3.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import mock

from django.test import TestCase
from django.conf import settings
from django.core.files.base import ContentFile
from django.utils.six.moves.urllib import parse as urlparse
from django.utils.timezone import is_aware, utc
Expand Down Expand Up @@ -274,6 +275,12 @@ def test_storage_size(self):
self.assertEqual(self.storage.size(name), obj.content_length)

def test_storage_mtime(self):
# Test both USE_TZ cases
for use_tz in (True, False):
with self.settings(USE_TZ=use_tz):
self._test_storage_mtime(use_tz)

def _test_storage_mtime(self, use_tz):
obj = self.storage.bucket.Object.return_value
obj.last_modified = datetime.now(utc)

Expand All @@ -283,9 +290,13 @@ def test_storage_mtime(self):
'Naive datetime object expected from modified_time()'
)

self.assertTrue(
self.assertIs(
settings.USE_TZ,
is_aware(self.storage.get_modified_time(name)),
'Aware datetime object expected from get_modified_time()'
'%s datetime object expected from get_modified_time() when USE_TZ=%s' % (
('Naive', 'Aware')[settings.USE_TZ],
settings.USE_TZ
)
)

def test_storage_url(self):
Expand Down

0 comments on commit d9d5dd5

Please sign in to comment.