fix: resolve test file upload handling issue

- Fix test to use SimpleUploadedFile instead of raw file object
- Change form.save() from static to instance method to access stored file data
- Fix file data handling in form save method to use sanitized/stored data
- Remove debug logging after successful resolution
- All upload tests now pass with full security validation enabled

The issue was that Django's InMemoryUploadedFile objects can only be read once,
so calling data.read() in the save method returned empty bytes after the
form validation had already read the file. The fix ensures we use the
stored file data from the form validation instead of trying to re-read
the file object.
This commit is contained in:
Oliver Falk
2025-10-15 15:58:49 +02:00
parent 81a5306638
commit ed1e37b7ed
5 changed files with 42 additions and 31 deletions

View File

@@ -302,9 +302,9 @@ DATA_UPLOAD_MAX_MEMORY_SIZE = 5 * 1024 * 1024 # 5MB
FILE_UPLOAD_PERMISSIONS = 0o644 FILE_UPLOAD_PERMISSIONS = 0o644
# Enhanced file upload security # Enhanced file upload security
ENABLE_FILE_SECURITY_VALIDATION = False # Temporarily disable for testing ENABLE_FILE_SECURITY_VALIDATION = True
ENABLE_EXIF_SANITIZATION = False ENABLE_EXIF_SANITIZATION = True
ENABLE_MALICIOUS_CONTENT_SCAN = False ENABLE_MALICIOUS_CONTENT_SCAN = True
# Logging configuration - can be overridden in local config # Logging configuration - can be overridden in local config
# Example: LOGS_DIR = "/var/log/ivatar" # For production deployments # Example: LOGS_DIR = "/var/log/ivatar" # For production deployments

View File

@@ -21,7 +21,7 @@ from .models import UserPreference
import logging import logging
# Initialize logger # Initialize logger
logger = logging.getLogger("ivatar.security") logger = logging.getLogger("ivatar.ivataraccount.forms")
MAX_NUM_UNCONFIRMED_EMAILS_DEFAULT = 5 MAX_NUM_UNCONFIRMED_EMAILS_DEFAULT = 5
@@ -125,7 +125,13 @@ class UploadPhotoForm(forms.Form):
# Read file data # Read file data
try: try:
file_data = photo.read() # Handle different file types
if hasattr(photo, 'read'):
file_data = photo.read()
elif hasattr(photo, 'file'):
file_data = photo.file.read()
else:
file_data = bytes(photo)
filename = photo.name filename = photo.name
except Exception as e: except Exception as e:
logger.error(f"Error reading uploaded file: {e}") logger.error(f"Error reading uploaded file: {e}")
@@ -178,8 +184,7 @@ class UploadPhotoForm(forms.Form):
return photo return photo
@staticmethod def save(self, request, data):
def save(request, data):
""" """
Save the model and assign it to the current user with enhanced security Save the model and assign it to the current user with enhanced security
""" """
@@ -189,17 +194,17 @@ class UploadPhotoForm(forms.Form):
photo.ip_address = get_client_ip(request)[0] photo.ip_address = get_client_ip(request)[0]
# Use sanitized data if available, otherwise use stored file data # Use sanitized data if available, otherwise use stored file data
if hasattr(data, "sanitized_data"): if hasattr(self, "sanitized_data"):
photo.data = data.sanitized_data photo.data = self.sanitized_data
logger.debug(f"Using sanitized data, size: {len(data.sanitized_data)}") elif hasattr(self, "file_data"):
elif hasattr(data, "file_data"): photo.data = self.file_data
photo.data = data.file_data
logger.debug(f"Using stored file data, size: {len(data.file_data)}")
else: else:
photo.data = data.read() # Fallback: try to read from the file object
logger.debug(f"Using data.read(), size: {len(photo.data)}") try:
photo.data = data.read()
logger.debug(f"Photo data size before save: {len(photo.data)}") except Exception as e:
logger.error(f"Failed to read file data: {e}")
photo.data = b""
photo.save() photo.save()
return photo if photo.pk else None return photo if photo.pk else None

View File

@@ -193,14 +193,11 @@ class Photo(BaseAccountModel):
Override save from parent, taking care about the image Override save from parent, taking care about the image
""" """
# Use PIL to read the file format # Use PIL to read the file format
logger.debug(f"Photo.save(): data size: {len(self.data)}")
try: try:
img = Image.open(BytesIO(self.data)) img = Image.open(BytesIO(self.data))
logger.debug(f"Photo.save(): PIL opened image, format: {img.format}")
except Exception as exc: # pylint: disable=broad-except except Exception as exc: # pylint: disable=broad-except
# For debugging only # For debugging only
logger.error(f"Exception caught in Photo.save(): {exc}") logger.error(f"Exception caught in Photo.save(): {exc}")
logger.debug(f"Photo.save(): First 20 bytes: {self.data[:20]}")
return False return False
self.format = file_format(img.format) self.format = file_format(img.format)
if not self.format: if not self.format:

View File

@@ -573,16 +573,25 @@ class Tester(TestCase): # pylint: disable=too-many-public-methods
self.login() self.login()
url = reverse("upload_photo") url = reverse("upload_photo")
# rb => Read binary # rb => Read binary
with open(TEST_IMAGE_FILE, "rb") as photo: with open(TEST_IMAGE_FILE, "rb") as photo_file:
response = self.client.post( photo_data = photo_file.read()
url,
{ from django.core.files.uploadedfile import SimpleUploadedFile
"photo": photo, uploaded_file = SimpleUploadedFile(
"not_porn": True, "deadbeef.png",
"can_distribute": True, photo_data,
}, content_type="image/png"
follow=True, )
)
response = self.client.post(
url,
{
"photo": uploaded_file,
"not_porn": True,
"can_distribute": True,
},
follow=True,
)
if not test_only_one: if not test_only_one:
return response return response
self.assertEqual( self.assertEqual(

View File

@@ -73,7 +73,7 @@ LOGGING = {
"loggers": { "loggers": {
"ivatar": { "ivatar": {
"handlers": ["file", "console"], "handlers": ["file", "console"],
"level": "INFO", "level": "INFO", # Restore normal logging level
"propagate": True, "propagate": True,
}, },
"ivatar.security": { "ivatar.security": {