Improve file upload validation (v6) (#3075)

vivid-planet · Jan 10, 2025 · ed126f8 · ed126f8
1 parent 7291335
commit ed126f8
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 22 deletions.
diff --git a/.changeset/hip-seahorses-wash.md b/.changeset/hip-seahorses-wash.md
@@ -0,0 +1,8 @@
+---
+"@comet/cms-admin": minor
+"@comet/cms-api": minor
+---
+
+Validate filename length for uploads to DAM or FileUploads
+
+The filename can't exceed 255 characters.
diff --git a/packages/admin/cms-admin/src/dam/DataGrid/fileUpload/fileUploadErrorMessages.tsx b/packages/admin/cms-admin/src/dam/DataGrid/fileUpload/fileUploadErrorMessages.tsx
@@ -85,3 +85,13 @@ export const SvgContainsJavaScriptError = (): React.ReactElement => (
         }}
     />
 );
+
+export const FilenameTooLongError = () => (
+    <FormattedMessage
+        id="comet.file.errors.filenameTooLong"
+        defaultMessage="<strong>Filename is too long.</strong> The filename can't exceed 255 characters."
+        values={{
+            strong: formatStrong,
+        }}
+    />
+);
diff --git a/packages/admin/cms-admin/src/dam/DataGrid/fileUpload/useDamFileUpload.tsx b/packages/admin/cms-admin/src/dam/DataGrid/fileUpload/useDamFileUpload.tsx
@@ -15,6 +15,7 @@ import { NewlyUploadedItem, useFileUploadContext } from "./FileUploadContext";
 import { FileUploadErrorDialog } from "./FileUploadErrorDialog";
 import {
     FileExtensionTypeMismatchError,
+    FilenameTooLongError,
     FileSizeError,
     MaxResolutionError,
     MissingFileExtensionError,
@@ -445,6 +446,8 @@ export const useDamFileUpload = (options: UploadDamFileOptions): FileUploadApi =
                             addValidationError(file, <MissingFileExtensionError />);
                         } else if (message.includes("File type and extension mismatch")) {
                             addValidationError(file, <FileExtensionTypeMismatchError extension={extension} mimetype={file.type} />);
+                        } else if (message.includes("Filename is too long")) {
+                            addValidationError(file, <FilenameTooLongError />);
                         } else {
                             addValidationError(file, <UnknownError />);
                         }

diff --git a/packages/api/cms-api/src/dam/files/dam-upload-file.interceptor.ts b/packages/api/cms-api/src/dam/files/dam-upload-file.interceptor.ts
@@ -35,20 +35,20 @@ export function DamUploadFileInterceptor(fieldName: string): Type<NestIntercepto
                     filename: function (req, file, cb) {
                         // otherwise special characters aren't decoded properly (https://github.com/expressjs/multer/issues/836#issuecomment-1264338996)
                         file.originalname = Buffer.from(file.originalname, "latin1").toString("utf8");
-                        cb(null, `${uuid()}-${file.originalname}`);
+                        cb(null, uuid());
                     },
                 }),
                 limits: {
                     fileSize: this.fileValidationService.config.maxFileSize * 1024 * 1024,
                 },
                 fileFilter: (req, file, cb) => {
-                    this.fileValidationService.validateFileMetadata(file).then((result) => {
-                        if (result === undefined) {
-                            return cb(null, true);
-                        } else {
-                            return cb(new CometValidationException(result), false);
-                        }
-                    });
+                    const errorMessage = this.fileValidationService.validateFileMetadata(file);
+
+                    if (errorMessage === undefined) {
+                        cb(null, true);
+                    } else {
+                        cb(new CometValidationException(errorMessage), false);
+                    }
                 },
             };
 

diff --git a/packages/api/cms-api/src/dam/files/file-validation.service.ts b/packages/api/cms-api/src/dam/files/file-validation.service.ts
@@ -7,7 +7,7 @@ export class FileValidationService {
     constructor(public config: { maxFileSize: number; acceptedMimeTypes: string[] }) {}
 
     async validateFile(file: FileUploadInput): Promise<undefined | string> {
-        let error = await this.validateFileMetadata(file);
+        let error = this.validateFileMetadata(file);
 
         if (error === undefined) {
             error = await this.validateFileContents(file);
@@ -16,10 +16,10 @@ export class FileValidationService {
         return error;
     }
 
-    async validateFileMetadata(file: FileUploadInput): Promise<undefined | string> {
-        //maximum file size
-        if (file.size > this.config.maxFileSize * 1024 * 1024) {
-            return "File is too large";
+    validateFileMetadata(file: Pick<FileUploadInput, "fieldname" | "originalname" | "encoding" | "mimetype">): undefined | string {
+        //maximum filename length
+        if (file.originalname.length > 255) {
+            return "Filename is too long";
         }
 
         //mime type in an accepted mime type
@@ -30,7 +30,7 @@ export class FileValidationService {
         //extension matched mime type
         const extension = file.originalname.split(".").pop()?.toLowerCase();
         if (extension === undefined) {
-            return `Invalid file name: Missing file extension`;
+            return "Invalid file name: Missing file extension";
         }
 
         const supportedExtensions = getValidExtensionsForMimetype(file.mimetype);

diff --git a/packages/api/cms-api/src/public-upload/public-upload-file.interceptor.ts b/packages/api/cms-api/src/public-upload/public-upload-file.interceptor.ts
@@ -35,20 +35,20 @@ export function PublicUploadFileInterceptor(fieldName: string): Type<NestInterce
                     filename: function (req, file, cb) {
                         // otherwise special characters aren't decoded properly (https://github.com/expressjs/multer/issues/836#issuecomment-1264338996)
                         file.originalname = Buffer.from(file.originalname, "latin1").toString("utf8");
-                        cb(null, `${uuid()}-${file.originalname}`);
+                        cb(null, uuid());
                     },
                 }),
                 limits: {
                     fileSize: fileValidationService.config.maxFileSize * 1024 * 1024,
                 },
                 fileFilter: (req, file, cb) => {
-                    this.fileValidationService.validateFileMetadata(file).then((result) => {
-                        if (result === undefined) {
-                            return cb(null, true);
-                        } else {
-                            return cb(new CometValidationException(result), false);
-                        }
-                    });
+                    const errorMessage = this.fileValidationService.validateFileMetadata(file);
+
+                    if (errorMessage === undefined) {
+                        cb(null, true);
+                    } else {
+                        cb(new CometValidationException(errorMessage), false);
+                    }
                 },
             };