From 6e39b59dd506ed149f973c89a876becfb59d371a Mon Sep 17 00:00:00 2001 From: Sli Date: Wed, 9 Apr 2025 22:15:12 +0200 Subject: [PATCH] Use UploadedImage to check image correctness and better error responses --- core/api.py | 31 +++++++------------ .../bundled/core/components/easymde-index.ts | 20 ++++++++++-- core/tests/test_files.py | 6 ++-- locale/fr/LC_MESSAGES/djangojs.po | 14 ++++++--- 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/core/api.py b/core/api.py index e038e0f3..fb8ec29d 100644 --- a/core/api.py +++ b/core/api.py @@ -1,15 +1,14 @@ -from typing import Annotated +from typing import Annotated, Any, Literal import annotated_types from django.conf import settings from django.db.models import F from django.http import HttpResponse -from ninja import Query, UploadedFile +from ninja import File, Query from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.exceptions import PermissionDenied from ninja_extra.pagination import PageNumberPaginationExtra from ninja_extra.schemas import PaginatedResponseSchema -from PIL import UnidentifiedImageError from club.models import Mailing from core.auth.api_permissions import CanAccessLookup, CanView, IsOldSubscriber @@ -20,6 +19,7 @@ from core.schemas import ( MarkdownSchema, SithFileSchema, UploadedFileSchema, + UploadedImage, UserFamilySchema, UserFilterSchema, UserProfileSchema, @@ -39,25 +39,18 @@ class MarkdownController(ControllerBase): class UploadController(ControllerBase): @route.post( "/image", - response=UploadedFileSchema, + response={ + 200: UploadedFileSchema, + 422: dict[Literal["detail"], list[dict[str, Any]]], + 403: dict[Literal["detail"], str], + }, permissions=[IsOldSubscriber], url_name="quick_upload_image", ) - def upload_image(self, file: UploadedFile): - if file.content_type.split("/")[0] != "image": - return self.create_response( - message=f"{file.name} isn't a file image", status_code=415 - ) - - try: - image = QuickUploadImage.create_from_uploaded( - file, uploader=self.context.request.user - ) - except UnidentifiedImageError: - return self.create_response( - message=f"{file.name} can't be processed", status_code=415 - ) - + def upload_image(self, file: File[UploadedImage]): + image = QuickUploadImage.create_from_uploaded( + file, uploader=self.context.request.user + ) return image diff --git a/core/static/bundled/core/components/easymde-index.ts b/core/static/bundled/core/components/easymde-index.ts index 39539409..af531236 100644 --- a/core/static/bundled/core/components/easymde-index.ts +++ b/core/static/bundled/core/components/easymde-index.ts @@ -6,7 +6,11 @@ import { inheritHtmlElement, registerComponent } from "#core:utils/web-component import type CodeMirror from "codemirror"; // biome-ignore lint/style/useNamingConvention: This is how they called their namespace import EasyMDE from "easymde"; -import { markdownRenderMarkdown, uploadUploadImage } from "#openapi"; +import { + type UploadUploadImageErrors, + markdownRenderMarkdown, + uploadUploadImage, +} from "#openapi"; const loadEasyMde = (textarea: HTMLTextAreaElement) => { const easymde = new EasyMDE({ @@ -21,8 +25,18 @@ const loadEasyMde = (textarea: HTMLTextAreaElement) => { file: file, }, }); - if (response.response.status !== 200) { - onError(gettext("Invalid file")); + if (!response.response.ok) { + if (response.response.status === 422) { + onError( + (response.error as UploadUploadImageErrors[422]).detail + .map((err: Record<"ctx", Record<"error", string>>) => err.ctx.error) + .join(" ; "), + ); + } else if (response.response.status === 403) { + onError(gettext("Not authorized, you need to have subscribed at least once")); + } else { + onError(gettext("Could not upload image")); + } return; } onSuccess(response.data.href); diff --git a/core/tests/test_files.py b/core/tests/test_files.py index 6c4c30e8..4afc4583 100644 --- a/core/tests/test_files.py +++ b/core/tests/test_files.py @@ -306,19 +306,19 @@ def test_apply_rights_recursively(): SimpleUploadedFile( "test.jpg", content=b"invalid", content_type="image/jpg" ), - 415, + 422, ), ( lambda: old_subscriber_user.make(), SimpleUploadedFile( "test.jpg", content=RED_PIXEL_PNG, content_type="invalid" ), - 415, + 200, # PIL can guess ), ( lambda: old_subscriber_user.make(), SimpleUploadedFile("test.jpg", content=b"invalid", content_type="invalid"), - 415, + 422, ), ], ) diff --git a/locale/fr/LC_MESSAGES/djangojs.po b/locale/fr/LC_MESSAGES/djangojs.po index 70c4bd4b..9bb0f79b 100644 --- a/locale/fr/LC_MESSAGES/djangojs.po +++ b/locale/fr/LC_MESSAGES/djangojs.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2025-04-08 11:42+0200\n" +"POT-Creation-Date: 2025-04-09 22:12+0200\n" "PO-Revision-Date: 2024-09-17 11:54+0200\n" "Last-Translator: Sli \n" "Language-Team: AE info \n" @@ -64,12 +64,18 @@ msgid "No results found" msgstr "Aucun résultat trouvé" #: core/static/bundled/core/components/easymde-index.ts -msgid "Invalid file" -msgstr "Fichier invalide" +msgid "Not authorized, you need to have subscribed at least once" +msgstr "" + +#: core/static/bundled/core/components/easymde-index.ts +msgid "Could not upload image" +msgstr "L'image n'a pas pu être téléversée" #: core/static/bundled/core/components/easymde-index.ts msgid "Attach files by drag and dropping or pasting from clipboard." -msgstr "Ajoutez des fichiez en glissant déposant ou collant depuis votre presse papier." +msgstr "" +"Ajoutez des fichiez en glissant déposant ou collant depuis votre presse " +"papier." #: core/static/bundled/core/components/easymde-index.ts msgid "Drop image to upload it."