From 8fc91c0769d436b45ef8c6e176cbf38d257f9f60 Mon Sep 17 00:00:00 2001 From: Josh Schneier Date: Mon, 11 Oct 2021 14:06:56 -0400 Subject: [PATCH] [s3] Fix saving files with S3ManifestStaticStorage (#1069) Resolves #1068 --- storages/backends/s3boto3.py | 77 +++++++++++++++++------------------- tests/test_s3boto3.py | 33 +++++++++++++--- 2 files changed, 64 insertions(+), 46 deletions(-) diff --git a/storages/backends/s3boto3.py b/storages/backends/s3boto3.py index 81b12bd32..4e9aa9d97 100644 --- a/storages/backends/s3boto3.py +++ b/storages/backends/s3boto3.py @@ -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 @@ -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) @@ -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 @@ -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): @@ -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.""" @@ -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 @@ -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): @@ -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 diff --git a/tests/test_s3boto3.py b/tests/test_s3boto3.py index ea2efcba8..8600a12d8 100644 --- a/tests/test_s3boto3.py +++ b/tests/test_s3boto3.py @@ -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): @@ -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): """ @@ -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'))