From 66f4f8ec68daaac767c013d6b1a30cf26a7ac1ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Freitag?= Date: Thu, 18 Jun 2020 14:43:10 +0200 Subject: [PATCH] [s3] Properly exclude empty folders during 'listdir' Users can create empty folders on S3, by creating an object which name ends with a forward slash, for example `folder/`. Using the `listdir` operation on an empty folder returns `[], ["."]`. This is unexpected, there are no file named `.` in the folder. The reason for its presence is AWS S3 returning the following content when listing the content of the directory: ``` { 'Contents': [ {... 'Key': 'folder/', ...} ], ... } ``` Then, `posixpath.relpath("folder/", "folder/")` returns ".". --- AUTHORS | 1 + storages/backends/s3boto3.py | 4 +++- tests/test_s3boto3.py | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index e995f5dd7..54db99c5f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -46,6 +46,7 @@ By order of apparition, thanks: * Zoe Liao (S3 docs) * Jonathan Ehwald * Dan Hook + * François Freitag (S3) Extra thanks to Marty for adding this in Django, diff --git a/storages/backends/s3boto3.py b/storages/backends/s3boto3.py index cd061e77a..02ac29a1a 100644 --- a/storages/backends/s3boto3.py +++ b/storages/backends/s3boto3.py @@ -502,7 +502,9 @@ def listdir(self, name): for entry in page.get('CommonPrefixes', ()): directories.append(posixpath.relpath(entry['Prefix'], path)) for entry in page.get('Contents', ()): - files.append(posixpath.relpath(entry['Key'], path)) + key = entry['Key'] + if key != path: + files.append(posixpath.relpath(key, path)) return directories, files def size(self, name): diff --git a/tests/test_s3boto3.py b/tests/test_s3boto3.py index 3fd3ab922..4c82df00b 100644 --- a/tests/test_s3boto3.py +++ b/tests/test_s3boto3.py @@ -509,6 +509,27 @@ def test_storage_listdir_subdir(self): self.assertEqual(dirs, ['path']) self.assertEqual(files, ['2.txt']) + def test_storage_listdir_empty(self): + # Files: + # dir/ + pages = [ + { + 'Contents': [ + {'Key': 'dir/'}, + ], + }, + ] + + paginator = mock.MagicMock() + paginator.paginate.return_value = pages + self.storage._connections.connection.meta.client.get_paginator.return_value = paginator + + dirs, files = self.storage.listdir('dir/') + paginator.paginate.assert_called_with(Bucket=None, Delimiter='/', Prefix='dir/') + + self.assertEqual(dirs, []) + self.assertEqual(files, []) + def test_storage_size(self): obj = self.storage.bucket.Object.return_value obj.content_length = 4098