Skip to content

Commit

Permalink
Add more robust input sanitization to the upload_url endpoint (#4931)
Browse files Browse the repository at this point in the history
* Fix upload_url endpoint

* fixed breaking python test cl

* Use DRF serializer with strict checksum & numeric duration

* reverted some changes
  • Loading branch information
GautamBytes authored Mar 4, 2025
1 parent 9dcd8b9 commit 657db92
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 25 deletions.
14 changes: 12 additions & 2 deletions contentcuration/contentcuration/tests/viewsets/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,10 @@ def setUp(self):

def test_required_keys(self):
del self.file["name"]

self.client.force_authenticate(user=self.user)
response = self.client.post(
reverse("file-upload-url"), self.file, format="json",
)

self.assertEqual(response.status_code, 400)

def test_duration_invalid(self):
Expand All @@ -379,7 +377,19 @@ def test_invalid_file_format_upload(self):
response = self.client.post(
reverse("file-upload-url"), file, format="json",
)
self.assertEqual(response.status_code, 400)

def test_invalid_preset_upload(self):
self.client.force_authenticate(user=self.user)
file = {
"size": 1000,
"checksum": uuid.uuid4().hex,
"name": "le_studio",
"file_format": file_formats.MP3,
"preset": "invalid_preset", # Deliberately invalid
"duration": 10.123
}
response = self.client.post(reverse("file-upload-url"), file, format="json")
self.assertEqual(response.status_code, 400)

def test_insufficient_storage(self):
Expand Down
71 changes: 48 additions & 23 deletions contentcuration/contentcuration/viewsets/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from django.core.exceptions import PermissionDenied
from django.http import HttpResponseBadRequest
from le_utils.constants import file_formats
from le_utils.constants import format_presets
from rest_framework import serializers
from rest_framework.decorators import action
from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response
Expand All @@ -30,6 +32,41 @@
from contentcuration.viewsets.sync.utils import generate_update_event


class StrictFloatField(serializers.FloatField):
def to_internal_value(self, data):
# If data is a string, reject it even if it represents a number.
if isinstance(data, str):
raise serializers.ValidationError("A valid number is required.")
return super().to_internal_value(data)


# New Serializer for validating upload_url inputs
class FileUploadURLSerializer(serializers.Serializer):
"""
Serializer to validate inputs for the upload_url endpoint.
Required:
- size: a float value
- checksum: a 32-digit hex string
- name: a string (note: mapped from request.data['name'])
- file_format: a valid file format choice from file_formats.choices
- preset: a valid preset choice from format_presets.choices
Optional:
- duration: a number that will be floored to an integer and must be > 0
"""
size = serializers.FloatField(required=True)
checksum = serializers.RegexField(regex=r'^[0-9a-f]{32}$', required=True)
name = serializers.CharField(required=True)
file_format = serializers.ChoiceField(choices=file_formats.choices, required=True)
preset = serializers.ChoiceField(choices=format_presets.choices, required=True)
duration = StrictFloatField(required=False)

def validate_duration(self, value):
floored = math.floor(value)
if floored <= 0:
raise serializers.ValidationError("File duration is equal to or less than 0")
return floored


class FileFilter(RequiredFilterSet):
id__in = UUIDInFilter(field_name="id")
contentnode__in = UUIDInFilter(field_name="contentnode")
Expand Down Expand Up @@ -162,26 +199,17 @@ def delete_from_changes(self, changes):

@action(detail=False, methods=["post"])
def upload_url(self, request):
try:
size = request.data["size"]
checksum = request.data["checksum"]
filename = request.data["name"]
file_format = request.data["file_format"]
preset = request.data["preset"]
except KeyError:
return HttpResponseBadRequest(
reason="Must specify: size, checksum, name, file_format, and preset"
)

duration = request.data.get("duration")
if duration is not None:
if not isinstance(duration, (int, float)):
return HttpResponseBadRequest(reason="File duration must be a number")
duration = math.floor(duration)
if duration <= 0:
return HttpResponseBadRequest(
reason="File duration is equal to or less than 0"
)
# Validate input using the new serializer
serializer = FileUploadURLSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

validated_data = serializer.validated_data
size = validated_data["size"]
checksum = validated_data["checksum"]
filename = validated_data["name"]
file_format = validated_data["file_format"]
preset = validated_data["preset"]
duration = validated_data.get("duration")

try:
request.user.check_space(float(size), checksum)
Expand All @@ -203,9 +231,6 @@ def upload_url(self, request):
filepath, checksum_base64, 600, content_length=size
)

if file_format not in dict(file_formats.choices):
return HttpResponseBadRequest("Invalid file_format!")

file = File(
file_size=size,
checksum=checksum,
Expand Down

0 comments on commit 657db92

Please sign in to comment.