From 5f5c8f6084feba48e8dfd26276f8f4075837ac73 Mon Sep 17 00:00:00 2001
From: John Bergvall <john@mividas.com>
Date: Thu, 26 Sep 2024 10:04:18 +0200
Subject: [PATCH 1/3] Use context manager when writing files to make sure fd is
closed at once
---
binary_database_files/storage.py | 3 ++-
binary_database_files/utils.py | 8 ++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/binary_database_files/storage.py b/binary_database_files/storage.py
index 9615140..27a8ae9 100644
--- a/binary_database_files/storage.py
+++ b/binary_database_files/storage.py
@@ -54,7 +54,8 @@ def _open(self, name, mode="rb"):
fqfn = self.path(name)
if os.path.isfile(fqfn):
# print('Loading file into database.')
- self._save(name, open(fqfn, mode))
+ with open(fqfn, mode) as fd:
+ self._save(name, fd)
fh = super(DatabaseStorage, self)._open(name, mode)
content = fh.read()
size = fh.size
diff --git a/binary_database_files/utils.py b/binary_database_files/utils.py
index 2f04428..16d4627 100644
--- a/binary_database_files/utils.py
+++ b/binary_database_files/utils.py
@@ -60,7 +60,9 @@ def write_file(name, content, overwrite=False):
fqfn_parts = os.path.split(fqfn)
if not os.path.isdir(fqfn_parts[0]):
os.makedirs(fqfn_parts[0])
- open(fqfn, "wb").write(content)
+
+ with open(fqfn, "wb") as fd:
+ fd.write(content)
# Cache hash.
hash_value = get_file_hash(fqfn)
@@ -69,7 +71,9 @@ def write_file(name, content, overwrite=False):
value = bytes(hash_value, "utf-8")
except TypeError:
value = hash_value
- open(hash_fn, "wb").write(value)
+
+ with open(hash_fn, "wb") as fd:
+ fd.write(value)
# Set ownership and permissions.
uname = getattr(settings, "DATABASE_FILES_USER", None)
From bb2cfa3d336df49ad8abbdc35dd5618e46516ed7 Mon Sep 17 00:00:00 2001
From: John Bergvall <john@mividas.com>
Date: Thu, 26 Sep 2024 10:05:58 +0200
Subject: [PATCH 2/3] Use regular Django file and directory
permissions-settings as fallback when saving files
---
binary_database_files/utils.py | 35 ++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/binary_database_files/utils.py b/binary_database_files/utils.py
index 16d4627..f1bf486 100644
--- a/binary_database_files/utils.py
+++ b/binary_database_files/utils.py
@@ -41,14 +41,38 @@ def get_hash_fn(name):
fqfn = os.path.join(settings.MEDIA_ROOT, name)
fqfn = os.path.normpath(fqfn)
fqfn_parts = os.path.split(fqfn)
- if not os.path.isdir(fqfn_parts[0]):
- os.makedirs(fqfn_parts[0])
+
+ create_directory_for_file(fqfn)
+
hash_fn = os.path.join(
fqfn_parts[0], _settings.DB_FILES_DEFAULT_HASH_FN_TEMPLATE % fqfn_parts[1]
)
return hash_fn
+def create_directory_for_file(full_path):
+
+ directory = os.path.dirname(full_path)
+ directory_permissions_mode = settings.FILE_UPLOAD_DIRECTORY_PERMISSIONS
+
+ if os.path.isdir(directory):
+ return
+
+ try:
+ if directory_permissions_mode is not None:
+ # Set the umask because os.makedirs() doesn't apply the "mode"
+ # argument to intermediate-level directories.
+ old_umask = os.umask(0o777 & ~directory_permissions_mode)
+ try:
+ os.makedirs(directory, directory_permissions_mode, exist_ok=True)
+ finally:
+ os.umask(old_umask)
+ else:
+ os.makedirs(directory, exist_ok=True)
+ except FileExistsError:
+ raise FileExistsError('%s exists and is not a directory.' % directory)
+
+
def write_file(name, content, overwrite=False):
"""
Writes the given content to the relative filename under the MEDIA_ROOT.
@@ -57,9 +81,8 @@ def write_file(name, content, overwrite=False):
fqfn = os.path.normpath(fqfn)
if os.path.isfile(fqfn) and not overwrite:
return
- fqfn_parts = os.path.split(fqfn)
- if not os.path.isdir(fqfn_parts[0]):
- os.makedirs(fqfn_parts[0])
+
+ create_directory_for_file(fqfn)
with open(fqfn, "wb") as fd:
fd.write(content)
@@ -84,7 +107,7 @@ def write_file(name, content, overwrite=False):
os.system('chown -RL %s%s "%s"' % (uname, gname, fqfn_parts[0])) # noqa: S605
# Set permissions.
- perms = getattr(settings, "DATABASE_FILES_PERMS", None)
+ perms = getattr(settings, "DATABASE_FILES_PERMS", settings.FILE_UPLOAD_PERMISSIONS)
if perms:
os.system('chmod -R %s "%s"' % (perms, fqfn_parts[0])) # noqa: S605
From be96e96aef6fd2690a22bea355fe7d5552d87557 Mon Sep 17 00:00:00 2001
From: John Bergvall <john@mividas.com>
Date: Thu, 26 Sep 2024 10:06:50 +0200
Subject: [PATCH 3/3] Use shutil.chown() and os.chmod() instead of shell
commands to change file permissions and owner.
Only change one file at the time instead of recursive change, due to that directory permissions and file permissions usually don't mix
---
binary_database_files/utils.py | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/binary_database_files/utils.py b/binary_database_files/utils.py
index f1bf486..e93cf0a 100644
--- a/binary_database_files/utils.py
+++ b/binary_database_files/utils.py
@@ -1,10 +1,14 @@
import hashlib
+import logging
import os
+import shutil
from django.conf import settings
from binary_database_files import settings as _settings
+logger = logging.getLogger(__name__)
+
def is_fresh(name, content_hash):
"""
@@ -101,15 +105,21 @@ def write_file(name, content, overwrite=False):
# Set ownership and permissions.
uname = getattr(settings, "DATABASE_FILES_USER", None)
gname = getattr(settings, "DATABASE_FILES_GROUP", None)
- if gname:
- gname = ":" + gname
- if uname:
- os.system('chown -RL %s%s "%s"' % (uname, gname, fqfn_parts[0])) # noqa: S605
+ if uname is not None or gname is not None:
+ try:
+ shutil.chown(fqfn, user=uname, group=gname)
+ shutil.chown(hash_fn, user=uname, group=gname)
+ except OSError as e:
+ logger.warning('Could not chown file %s: %s', fqfn, e)
# Set permissions.
perms = getattr(settings, "DATABASE_FILES_PERMS", settings.FILE_UPLOAD_PERMISSIONS)
if perms:
- os.system('chmod -R %s "%s"' % (perms, fqfn_parts[0])) # noqa: S605
+ try:
+ os.chmod(fqfn, perms)
+ os.chmod(hash_fn, perms)
+ except OSError as e:
+ logger.warning('Could not chmod file %s: %s', fqfn, e)
def get_file_hash(fin, force_encoding=None, encoding=None, errors=None, chunk_size=128):