Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions baseapp/files/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ This adds the following fields to the File model:
- `uploaded_parts` - JSON tracking ETags
- `upload_expires_at` - Expiration timestamp

`file_size` is a `PositiveBigIntegerField`, so files over 2 GB fit. A project with its own concrete
`File` model needs `python manage.py makemigrations files` after upgrading to pick that up.

### 2. URLs Already Configured

The URLs are automatically included in your project via `testproject/urls.py`.
Expand Down
3 changes: 2 additions & 1 deletion baseapp/files/UPLOAD_FLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,10 @@ Without it uploads fail in the browser with a CORS error and nothing reaches the

### Common Errors
- **400 Bad Request**: Invalid parameters, part numbers, or file size
- **400 Bad Request** on complete: the assembled file's size differs from the declared `file_size` (e.g. a part carried extra bytes). The stored object is deleted and the upload is not completed.
- **403 Forbidden**: User doesn't own the file/upload
- **404 Not Found**: File or upload not found
- **413 Payload Too Large**: File exceeds `MAX_FILE_UPLOAD_SIZE`
- **400 Bad Request** `{"file_size": ["File is too large. The maximum size is 5.0 GB."]}`: File exceeds `MAX_FILE_UPLOAD_SIZE`. Clients can show this message as is.

### Upload Status
- `pending`: Upload initiated but not started
Expand Down
2 changes: 1 addition & 1 deletion baseapp/files/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ class AbstractFile(*file_inheritances, DocumentIdMixin, RelayModel, TimeStampedM
verbose_name=_("file name"),
help_text=_("Original name of the uploaded file"),
)
file_size = models.PositiveIntegerField(
file_size = models.PositiveBigIntegerField(
null=True,
verbose_name=_("file size"),
help_text=_("File size in bytes"),
Expand Down
16 changes: 16 additions & 0 deletions baseapp/files/rest_framework/uploads/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import swapper
from django.conf import settings
from django.template.defaultfilters import filesizeformat
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

Expand All @@ -19,6 +21,20 @@ class InitiateUploadSerializer(serializers.Serializer):
# Optional parent relationship via DocumentId public_id
parent_id = serializers.UUIDField(required=False, allow_null=True)

def validate_file_size(self, value: int) -> int:
"""Reject a declared size above MAX_FILE_UPLOAD_SIZE, naming the limit.

Validated here, not only in UploadService, so the client gets the reason
instead of the view's generic "Invalid upload parameters".
"""
max_file_size = getattr(settings, "MAX_FILE_UPLOAD_SIZE", 5 * 1024 * 1024 * 1024)
if value > max_file_size:
raise serializers.ValidationError(
_("File is too large. The maximum size is %(max)s.")
% {"max": filesizeformat(max_file_size)}
)
return value

def validate_parent_id(self, value):
"""Validate parent_id exists as a DocumentId public_id."""
if not value:
Expand Down
22 changes: 22 additions & 0 deletions baseapp/files/services/upload_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import swapper
from django.conf import settings
from django.core.files.storage import default_storage
from django.db import transaction
from django.utils import timezone

Expand Down Expand Up @@ -109,6 +110,7 @@ def complete_multipart_upload(self, file_id: int, parts: list):
# Complete with storage handler (network I/O — outside the transaction)
try:
file_path = self.handler.complete_upload(file_obj, file_obj.upload_id, parts)
self._verify_stored_size(file_obj, file_path)
except Exception:
File.objects.filter(id=file_id).update(upload_status=File.UploadStatus.FAILED)
raise
Expand Down Expand Up @@ -152,6 +154,26 @@ def abort_multipart_upload(self, file_id: int):
locked.save()
return locked

def _verify_stored_size(self, file_obj, file_path: str) -> None:
"""Reject an assembled file whose bytes differ from the declared size.

MAX_FILE_UPLOAD_SIZE is checked against the declared ``file_size`` at
initiation, but neither the local part endpoint nor a presigned S3 part URL
bounds how many bytes are actually sent. Both handlers store through
``default_storage`` (it is what selects the handler), so it can measure
either. An oversized object is deleted rather than kept.
"""
stored_size = default_storage.size(file_path)
if stored_size == file_obj.file_size:
return
try:
default_storage.delete(file_path)
except Exception:
logger.exception("Could not delete mismatched upload %s", file_path)
raise ValueError(
f"Stored size {stored_size} does not match declared size {file_obj.file_size}"
)

def _validate_file_params(self, file_size, num_parts, part_size):
"""Validate file upload parameters."""
max_file_size = getattr(settings, "MAX_FILE_UPLOAD_SIZE", 5 * 1024 * 1024 * 1024) # 5GB
Expand Down
30 changes: 30 additions & 0 deletions baseapp/files/tests/test_graphql.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import pytest
import swapper
from django.contrib.contenttypes.models import ContentType
from django.test import Client as DjangoClient

from baseapp_comments.tests.factories import CommentFactory
from baseapp_core.graphql.testing.fixtures import GraphQLClient
from baseapp_core.models import DocumentId

File = swapper.load_model("baseapp_files", "File")
Expand Down Expand Up @@ -795,3 +797,31 @@ def test_file_node_query_returns_file_for_owner(graphql_user_client, django_user
assert "errors" not in content
assert content["data"]["file"]["id"] == file.relay_id
assert content["data"]["file"]["fileName"] == "mine.txt"


FILE_SIZE_QUERY = """
query FileSize($id: ID!) {
node(id: $id) {
... on File {
fileSize
}
}
}
"""


def test_file_size_larger_than_32_bit_int(
graphql_user_client: GraphQLClient, django_user_client: DjangoClient
) -> None:
file_size = 3 * 1024 * 1024 * 1024
file = File.objects.create(
file_name="long-video.mp4",
file_size=file_size,
upload_status=File.UploadStatus.COMPLETED,
created_by=django_user_client.user,
)

content = graphql_user_client(FILE_SIZE_QUERY, variables={"id": file.relay_id}).json()

assert "errors" not in content
assert content["data"]["node"]["fileSize"] == file_size
30 changes: 26 additions & 4 deletions baseapp/files/tests/test_presigned_uploads.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

from datetime import timedelta
from typing import Any, Dict
from unittest.mock import patch
from urllib.parse import urlparse

import pytest
import swapper
from django.conf import settings
from django.test import override_settings
from django.utils import timezone
from freezegun import freeze_time
from rest_framework import status
Expand All @@ -29,8 +28,11 @@

@pytest.fixture
def temp_media_root(tmp_path):
"""Use a temporary directory for media root."""
with patch.object(settings, "MEDIA_ROOT", str(tmp_path)):
"""Use a temporary directory for media root.

override_settings (not patch.object) so default_storage drops its cached location.
"""
with override_settings(MEDIA_ROOT=str(tmp_path)):
yield tmp_path


Expand Down Expand Up @@ -99,6 +101,26 @@ def test_full_upload_flow(self, client, user_client, temp_media_root, initiated_
assert file_obj.upload_status == File.UploadStatus.COMPLETED
assert (temp_media_root / file_obj.file.name).read_bytes() == PART_DATA

def test_complete_rejects_more_bytes_than_declared(
self, client, user_client, temp_media_root, initiated_upload
) -> None:
"""Declaring a small size and PUTting more must not yield a COMPLETED, oversized file."""
path = presigned_path(initiated_upload["presigned_urls"][0])
etag = put_part(client, path, data=PART_DATA * 4).json()["etag"]

response = user_client.post(
f"/v1/files/uploads/{initiated_upload['id']}/complete",
{"parts": [{"part_number": 1, "etag": etag}]},
format="json",
)

assert response.status_code == status.HTTP_400_BAD_REQUEST
file_obj = File.get_by_public_id(initiated_upload["id"])
# ATOMIC_REQUESTS rolls back the FAILED flag with the 400; what matters is no COMPLETED file.
assert file_obj.upload_status != File.UploadStatus.COMPLETED
assert not file_obj.file
assert not list((temp_media_root / "files").rglob("*"))

def test_missing_token(self, client, initiated_upload):
"""A request without a token is rejected."""
path = presigned_path(initiated_upload["presigned_urls"][0]).split("?")[0]
Expand Down
73 changes: 73 additions & 0 deletions baseapp/files/tests/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import swapper
from django.contrib.auth import get_user_model
from django.contrib.contenttypes.models import ContentType
from django.test import override_settings
from django.utils import timezone
from rest_framework import status

Expand Down Expand Up @@ -210,6 +211,48 @@ def test_initiate_upload_file_size_mismatch(self, user_client, mock_s3_handler):
error_message = str(response_data)
assert "doesn't match" in error_message

@override_settings(MAX_FILE_UPLOAD_SIZE=10 * 1024 * 1024)
def test_initiate_upload_too_large_explains_the_limit(
self, user_client: Client, mock_s3_handler: MagicMock
) -> None:
response = user_client.post(
"/v1/files/uploads",
{
"file_name": "big.pdf",
"file_size": 15 * 1024 * 1024,
"file_content_type": "application/pdf",
"num_parts": 3,
"part_size": 5242880,
},
format="json",
)

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {
"file_size": ["File is too large. The maximum size is 10.0\xa0MB."]
}
mock_s3_handler.initiate_upload.assert_not_called()

def test_initiate_upload_larger_than_32_bit_size(
self, user_client: Client, mock_s3_handler: MagicMock
) -> None:
file_size = 3 * 1024 * 1024 * 1024
response = user_client.post(
"/v1/files/uploads",
{
"file_name": "long-video.mp4",
"file_size": file_size,
"file_content_type": "video/mp4",
"num_parts": 31,
"part_size": 100 * 1024 * 1024,
},
format="json",
)

assert response.status_code == status.HTTP_201_CREATED
file_obj = File.get_by_public_id(response.json()["id"])
assert file_obj.file_size == file_size

def test_initiate_upload_too_many_parts(self, user_client, mock_s3_handler):
"""Test validation for maximum parts (10,000)."""
response = user_client.post(
Expand Down Expand Up @@ -264,6 +307,36 @@ def pending_upload(self, user_client):
upload_expires_at=timezone.now() + timedelta(hours=24),
)

@pytest.fixture(autouse=True)
def stored_storage(self):
"""The handler is mocked, so nothing is stored; report the declared size by default."""
with patch("baseapp.files.services.upload_service.default_storage") as storage:
storage.size.return_value = 10485760
yield storage

def test_complete_upload_rejects_bytes_beyond_the_declared_size(
self,
user_client: Client,
pending_upload: "File",
mock_s3_handler: MagicMock,
stored_storage: MagicMock,
) -> None:
"""A client that declares a small size and sends more must not get a COMPLETED file."""
stored_storage.size.return_value = pending_upload.file_size + 1

response = user_client.post(
f"/v1/files/uploads/{pending_upload.public_id}/complete",
{"parts": [{"part_number": 1, "etag": "a"}, {"part_number": 2, "etag": "b"}]},
format="json",
)

assert response.status_code == status.HTTP_400_BAD_REQUEST
stored_storage.delete.assert_called_once_with("files/test-file.mp4")
pending_upload.refresh_from_db()
# ATOMIC_REQUESTS rolls back the FAILED flag with the 400; what matters is no COMPLETED file.
assert pending_upload.upload_status != File.UploadStatus.COMPLETED
assert not pending_upload.file

def test_complete_upload_success(self, user_client, pending_upload, mock_s3_handler):
"""Test successful upload completion."""
response = user_client.post(
Expand Down
20 changes: 20 additions & 0 deletions testproject/files/migrations/0004_file_size_big_integer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 5.2.17 on 2026-10-13 00:20

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("files", "0003_alter_file_options_alter_filetarget_options_and_more"),
]

operations = [
migrations.AlterField(
model_name="file",
name="file_size",
field=models.PositiveBigIntegerField(
help_text="File size in bytes", null=True, verbose_name="file size"
),
),
]
Loading