Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[s3] Fix saving files with S3ManifestStaticStorage #1069

Merged
merged 2 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 36 additions & 41 deletions storages/backends/s3boto3.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ def _get_file(self):
if self._file is None:
self._file = SpooledTemporaryFile(
max_size=self._storage.max_memory_size,
suffix=".S3Boto3StorageFile",
dir=setting("FILE_UPLOAD_TEMP_DIR")
suffix='.S3Boto3StorageFile',
dir=setting('FILE_UPLOAD_TEMP_DIR')
)
if 'r' in self._mode:
self._is_dirty = False
Expand Down Expand Up @@ -178,9 +178,6 @@ def _buffer_file_size(self):
return length

def _flush_write_buffer(self):
"""
Flushes the write buffer.
"""
if self._buffer_file_size:
self._write_counter += 1
self.file.seek(0)
Expand All @@ -201,19 +198,19 @@ def _create_empty_on_close(self):
This behavior is meant to mimic the behavior of Django's builtin FileSystemStorage,
where files are always created after they are opened in write mode:

f = storage.open("file.txt", mode="w")
f = storage.open('file.txt', mode='w')
f.close()
"""
assert "w" in self._mode
assert 'w' in self._mode
assert self._raw_bytes_written == 0

try:
# Check if the object exists on the server; if so, don't do anything
self.obj.load()
except ClientError as err:
if err.response["ResponseMetadata"]["HTTPStatusCode"] == 404:
if err.response['ResponseMetadata']['HTTPStatusCode'] == 404:
self.obj.put(
Body=b"", **self._storage._get_write_parameters(self.obj.key)
Body=b'', **self._storage._get_write_parameters(self.obj.key)
)
else:
raise
Expand Down Expand Up @@ -303,37 +300,37 @@ def get_default_settings(self):
)

return {
"access_key": setting('AWS_S3_ACCESS_KEY_ID', setting('AWS_ACCESS_KEY_ID')),
"secret_key": setting('AWS_S3_SECRET_ACCESS_KEY', setting('AWS_SECRET_ACCESS_KEY')),
"session_profile": setting('AWS_S3_SESSION_PROFILE'),
"file_overwrite": setting('AWS_S3_FILE_OVERWRITE', True),
"object_parameters": setting('AWS_S3_OBJECT_PARAMETERS', {}),
"bucket_name": setting('AWS_STORAGE_BUCKET_NAME'),
"querystring_auth": setting('AWS_QUERYSTRING_AUTH', True),
"querystring_expire": setting('AWS_QUERYSTRING_EXPIRE', 3600),
"signature_version": setting('AWS_S3_SIGNATURE_VERSION'),
"location": setting('AWS_LOCATION', ''),
"custom_domain": setting('AWS_S3_CUSTOM_DOMAIN'),
"cloudfront_signer": cloudfront_signer,
"addressing_style": setting('AWS_S3_ADDRESSING_STYLE'),
"secure_urls": setting('AWS_S3_SECURE_URLS', True),
"file_name_charset": setting('AWS_S3_FILE_NAME_CHARSET', 'utf-8'),
"gzip": setting('AWS_IS_GZIPPED', False),
"gzip_content_types": setting('GZIP_CONTENT_TYPES', (
'access_key': setting('AWS_S3_ACCESS_KEY_ID', setting('AWS_ACCESS_KEY_ID')),
'secret_key': setting('AWS_S3_SECRET_ACCESS_KEY', setting('AWS_SECRET_ACCESS_KEY')),
'session_profile': setting('AWS_S3_SESSION_PROFILE'),
'file_overwrite': setting('AWS_S3_FILE_OVERWRITE', True),
'object_parameters': setting('AWS_S3_OBJECT_PARAMETERS', {}),
'bucket_name': setting('AWS_STORAGE_BUCKET_NAME'),
'querystring_auth': setting('AWS_QUERYSTRING_AUTH', True),
'querystring_expire': setting('AWS_QUERYSTRING_EXPIRE', 3600),
'signature_version': setting('AWS_S3_SIGNATURE_VERSION'),
'location': setting('AWS_LOCATION', ''),
'custom_domain': setting('AWS_S3_CUSTOM_DOMAIN'),
'cloudfront_signer': cloudfront_signer,
'addressing_style': setting('AWS_S3_ADDRESSING_STYLE'),
'secure_urls': setting('AWS_S3_SECURE_URLS', True),
'file_name_charset': setting('AWS_S3_FILE_NAME_CHARSET', 'utf-8'),
'gzip': setting('AWS_IS_GZIPPED', False),
'gzip_content_types': setting('GZIP_CONTENT_TYPES', (
'text/css',
'text/javascript',
'application/javascript',
'application/x-javascript',
'image/svg+xml',
)),
"url_protocol": setting('AWS_S3_URL_PROTOCOL', 'http:'),
"endpoint_url": setting('AWS_S3_ENDPOINT_URL'),
"proxies": setting('AWS_S3_PROXIES'),
"region_name": setting('AWS_S3_REGION_NAME'),
"use_ssl": setting('AWS_S3_USE_SSL', True),
"verify": setting('AWS_S3_VERIFY', None),
"max_memory_size": setting('AWS_S3_MAX_MEMORY_SIZE', 0),
"default_acl": setting('AWS_DEFAULT_ACL', None),
'url_protocol': setting('AWS_S3_URL_PROTOCOL', 'http:'),
'endpoint_url': setting('AWS_S3_ENDPOINT_URL'),
'proxies': setting('AWS_S3_PROXIES'),
'region_name': setting('AWS_S3_REGION_NAME'),
'use_ssl': setting('AWS_S3_USE_SSL', True),
'verify': setting('AWS_S3_VERIFY', None),
'max_memory_size': setting('AWS_S3_MAX_MEMORY_SIZE', 0),
'default_acl': setting('AWS_DEFAULT_ACL', None),
}

def __getstate__(self):
Expand Down Expand Up @@ -429,8 +426,7 @@ def _normalize_name(self, name):
try:
return safe_join(self.location, name)
except ValueError:
raise SuspiciousOperation("Attempted access to '%s' denied." %
name)
raise SuspiciousOperation("Attempted access to '%s' denied." % name)

def _compress_content(self, content):
"""Gzip a given string content."""
Expand All @@ -451,7 +447,7 @@ def _save(self, name, content):
name = self._normalize_name(cleaned_name)
params = self._get_write_parameters(name, content)

if content.seekable():
if not hasattr(content, 'seekable') or content.seekable():
content.seek(0, os.SEEK_SET)
if (self.gzip and
params['ContentType'] in self.gzip_content_types and
Expand Down Expand Up @@ -572,7 +568,7 @@ def _strip_signing_parameters(self, url):
# Note: Parameters that did not have a value in the original query string will have
# an '=' sign appended to it, e.g ?foo&bar becomes ?foo=&bar=
joined_qs = ('='.join(keyval) for keyval in filtered_qs)
split_url = split_url._replace(query="&".join(joined_qs))
split_url = split_url._replace(query='&'.join(joined_qs))
return split_url.geturl()

def url(self, name, parameters=None, expire=None, http_method=None):
Expand All @@ -583,16 +579,15 @@ def url(self, name, parameters=None, expire=None, http_method=None):
expire = self.querystring_expire

if self.custom_domain:
url = "{}//{}/{}{}".format(
url = '{}//{}/{}{}'.format(
self.url_protocol,
self.custom_domain,
filepath_to_uri(name),
"?{}".format(urlencode(params)) if params else "",
'?{}'.format(urlencode(params)) if params else '',
)

if self.querystring_auth and self.cloudfront_signer:
expiration = datetime.utcnow() + timedelta(seconds=expire)

return self.cloudfront_signer.generate_presigned_url(url, date_less_than=expiration)

return url
Expand Down
33 changes: 28 additions & 5 deletions tests/test_s3boto3.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@
from storages.backends import s3boto3


class S3Boto3TestCase(TestCase):
def setUp(self):
self.storage = s3boto3.S3Boto3Storage()
self.storage._connections.connection = mock.MagicMock()
class S3ManifestStaticStorageTestStorage(s3boto3.S3ManifestStaticStorage):
def read_manifest(self):
return None


class NonSeekableContentFile(ContentFile):
Expand All @@ -34,7 +33,10 @@ def seek(self, pos, whence=0):
raise AttributeError()


class S3Boto3StorageTests(S3Boto3TestCase):
class S3Boto3StorageTests(TestCase):
def setUp(self):
self.storage = s3boto3.S3Boto3Storage()
self.storage._connections.connection = mock.MagicMock()

def test_clean_name(self):
"""
Expand Down Expand Up @@ -764,3 +766,24 @@ def test_override_init_argument(self):
self.assertEqual(storage.location, 'foo1')
storage = s3boto3.S3Boto3Storage(location='foo2')
self.assertEqual(storage.location, 'foo2')


class S3StaticStorageTests(TestCase):
def setUp(self):
self.storage = s3boto3.S3StaticStorage()
self.storage._connections.connection = mock.MagicMock()

def test_querystring_auth(self):
self.assertFalse(self.storage.querystring_auth)


class S3ManifestStaticStorageTests(TestCase):
def setUp(self):
self.storage = S3ManifestStaticStorageTestStorage()
self.storage._connections.connection = mock.MagicMock()

def test_querystring_auth(self):
self.assertFalse(self.storage.querystring_auth)

def test_save(self):
self.storage.save('x.txt', ContentFile(b'abc'))