From 10be72f635a38507711a1d354ae535e06dc58c12 Mon Sep 17 00:00:00 2001 From: Josh Schneier Date: Fri, 29 Oct 2021 22:07:34 -0400 Subject: [PATCH 1/4] [s3] Re-raise non-404 errors in .exists() (#1085) --- storages/backends/s3boto3.py | 11 ++++------- tests/test_s3boto3.py | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/storages/backends/s3boto3.py b/storages/backends/s3boto3.py index 4e9aa9d97..dc402e344 100644 --- a/storages/backends/s3boto3.py +++ b/storages/backends/s3boto3.py @@ -469,18 +469,15 @@ def exists(self, name): self.connection.meta.client.head_object(Bucket=self.bucket_name, Key=name) return True except ClientError as error: - if error.response.get('Error', {}).get('Code') == '404': + if error.response['ResponseMetadata']['HTTPStatusCode'] == 404: return False - # Some other error was encountered. As `get_available_name` calls this, - # we have to assume the filename is unavailable. If we return true due to some - # other error, we'd overwrite a file. - return True + # Some other error was encountered. Re-raise it. + raise def listdir(self, name): path = self._normalize_name(self._clean_name(name)) - # The path needs to end with a slash, but if the root is empty, leave - # it. + # The path needs to end with a slash, but if the root is empty, leave it. if path and not path.endswith('/'): path += '/' diff --git a/tests/test_s3boto3.py b/tests/test_s3boto3.py index 8600a12d8..b389ef2c7 100644 --- a/tests/test_s3boto3.py +++ b/tests/test_s3boto3.py @@ -488,15 +488,25 @@ def test_storage_exists(self): def test_storage_exists_false(self): self.storage.connection.meta.client.head_object.side_effect = ClientError( - {'Error': {'Code': '404', 'Message': 'Not Found'}}, + {'Error': {}, 'ResponseMetadata': {'HTTPStatusCode': 404}}, 'HeadObject', ) - self.assertFalse(self.storage.exists("file.txt")) + self.assertFalse(self.storage.exists('file.txt')) self.storage.connection.meta.client.head_object.assert_called_with( Bucket=self.storage.bucket_name, Key='file.txt', ) + def test_storage_exists_other_error_reraise(self): + self.storage.connection.meta.client.head_object.side_effect = ClientError( + {'Error': {}, 'ResponseMetadata': {'HTTPStatusCode': 403}}, + 'HeadObject', + ) + with self.assertRaises(ClientError) as cm: + self.storage.exists('file.txt') + + self.assertEqual(cm.exception.response['ResponseMetadata']['HTTPStatusCode'], 403) + def test_storage_delete(self): self.storage.delete("path/to/file.txt") self.storage.bucket.Object.assert_called_with('path/to/file.txt') From 903f5b14888d72f63587e527739216c24b2277f3 Mon Sep 17 00:00:00 2001 From: Josh Schneier Date: Fri, 29 Oct 2021 22:16:22 -0400 Subject: [PATCH 2/4] [sftp] Catch FileNotFoundError rather than OSError in .exists() (#1087) This was covering up socket.timeout exceptions. --- storages/backends/sftpstorage.py | 2 +- tests/test_sftp.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/storages/backends/sftpstorage.py b/storages/backends/sftpstorage.py index 529daf278..643685c88 100644 --- a/storages/backends/sftpstorage.py +++ b/storages/backends/sftpstorage.py @@ -150,7 +150,7 @@ def exists(self, name): try: self.sftp.stat(self._remote_path(name)) return True - except OSError: + except FileNotFoundError: return False def _isdir_attr(self, item): diff --git a/tests/test_sftp.py b/tests/test_sftp.py index 094c4447f..448358f29 100644 --- a/tests/test_sftp.py +++ b/tests/test_sftp.py @@ -1,5 +1,6 @@ import io import os +import socket import stat from datetime import datetime from unittest.mock import MagicMock, patch @@ -56,7 +57,7 @@ def test_mkdir(self, mock_sftp): self.assertEqual(mock_sftp.mkdir.call_args[0], ('foo',)) @patch('storages.backends.sftpstorage.SFTPStorage.sftp', **{ - 'stat.side_effect': (IOError(), True) + 'stat.side_effect': (FileNotFoundError(), True) }) def test_mkdir_parent(self, mock_sftp): self.storage._mkdir('bar/foo') @@ -69,7 +70,7 @@ def test_save(self, mock_sftp): self.assertTrue(mock_sftp.open.return_value.write.called) @patch('storages.backends.sftpstorage.SFTPStorage.sftp', **{ - 'stat.side_effect': (IOError(), True) + 'stat.side_effect': (FileNotFoundError(), True) }) def test_save_in_subdir(self, mock_sftp): self.storage._save('bar/foo', File(io.BytesIO(b'foo'), 'foo')) @@ -86,11 +87,18 @@ def test_exists(self, mock_sftp): self.assertTrue(self.storage.exists('foo')) @patch('storages.backends.sftpstorage.SFTPStorage.sftp', **{ - 'stat.side_effect': IOError() + 'stat.side_effect': FileNotFoundError() }) def test_not_exists(self, mock_sftp): self.assertFalse(self.storage.exists('foo')) + @patch('storages.backends.sftpstorage.SFTPStorage.sftp', **{ + 'stat.side_effect': socket.timeout() + }) + def test_not_exists_timeout(self, mock_sftp): + with self.assertRaises(socket.timeout): + self.storage.exists('foo') + @patch('storages.backends.sftpstorage.SFTPStorage.sftp', **{ 'listdir_attr.return_value': [MagicMock(filename='foo', st_mode=stat.S_IFDIR), From bb01e14d73efb6dbeb424ed134ccaacdc20bb3ee Mon Sep 17 00:00:00 2001 From: David Hotham Date: Sat, 30 Oct 2021 03:16:58 +0100 Subject: [PATCH 3/4] [azure] fix use of custom domain with account key (#1083) --- CHANGELOG.rst | 11 +++++++++++ storages/backends/azure_storage.py | 5 ++++- tests/test_azure.py | 2 +- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 10ccb2f47..37b4dee6a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,17 @@ django-storages CHANGELOG ========================= +Unreleased +********** + +Azure +----- + +- Fix using ``AZURE_CUSTOM_DOMAIN`` with an account key credential (`#1082`_, `#1083`_) + +.. _#1082: https://github.com/jschneier/django-storages/issues/1082 +.. _#1083: https://github.com/jschneier/django-storages/pull/1083 + 1.12.2 (2021-10-16) ******************* diff --git a/storages/backends/azure_storage.py b/storages/backends/azure_storage.py index 23509a569..f8e242a48 100644 --- a/storages/backends/azure_storage.py +++ b/storages/backends/azure_storage.py @@ -158,7 +158,10 @@ def _get_service_client(self): credential = None if self.account_key: - credential = self.account_key + credential = { + "account_name": self.account_name, + "account_key": self.account_key, + } elif self.sas_token: credential = self.sas_token elif self.token_credential: diff --git a/tests/test_azure.py b/tests/test_azure.py index fa276848b..0306572fc 100644 --- a/tests/test_azure.py +++ b/tests/test_azure.py @@ -232,7 +232,7 @@ def test_container_client_params_account_key(self): self.assertEqual(storage.client, client_mock) bsc_mocked.assert_called_once_with( 'https://foo_domain', - credential='foo_key') + credential={'account_name': 'foo_name', 'account_key': 'foo_key'}) def test_container_client_params_sas_token(self): storage = azure_storage.AzureStorage() From a3c3f99a96ed96866bc6ac21e641a42cd5619bc2 Mon Sep 17 00:00:00 2001 From: Josh Schneier Date: Fri, 29 Oct 2021 22:34:00 -0400 Subject: [PATCH 4/4] Release version 1.12.3 (#1088) --- CHANGELOG.rst | 28 +++++++++++++++++++++++++--- storages/__init__.py | 2 +- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 37b4dee6a..48c644966 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,16 +1,37 @@ django-storages CHANGELOG ========================= -Unreleased -********** +1.12.3 (2021-10-29) +******************* + +General +------- + +- Add support for Python 3.10 (`#1078`_) + +S3 +-- + +- Re-raise non-404 errors in ``.exists()`` (`#1084`_, `#1085`_) Azure ----- - Fix using ``AZURE_CUSTOM_DOMAIN`` with an account key credential (`#1082`_, `#1083`_) +SFTP +---- + +- Catch ``FileNotFoundError`` instead of ``OSerror`` in ``.exists()`` to prevent swallowing ``socket.timeout`` exceptions (`#1064`_, `#1087`_) + + +.. _#1078: https://github.com/jschneier/django-storages/pull/1078 +.. _#1084: https://github.com/jschneier/django-storages/issues/1084 +.. _#1085: https://github.com/jschneier/django-storages/pull/1085 .. _#1082: https://github.com/jschneier/django-storages/issues/1082 .. _#1083: https://github.com/jschneier/django-storages/pull/1083 +.. _#1064: https://github.com/jschneier/django-storages/issues/1064 +.. _#1087: https://github.com/jschneier/django-storages/pull/1087 1.12.2 (2021-10-16) ******************* @@ -42,10 +63,11 @@ S3 1.12 (2021-10-06) ***************** +General +------- - Add support for Django 3.2 (`#1046`_, `#1042`_, `#1005`_) - Replace Travis CI with GitHub actions (`#1051`_) - S3 -- diff --git a/storages/__init__.py b/storages/__init__.py index 96ddfeb78..3081afbab 100644 --- a/storages/__init__.py +++ b/storages/__init__.py @@ -1 +1 @@ -__version__ = '1.12.2' +__version__ = '1.12.3'