From 8dd4d3b434523416dfa71bc59d359521844283d6 Mon Sep 17 00:00:00 2001
From: Thomas Dax <thomas.dax@vivid-planet.com>
Date: Thu, 19 Dec 2024 16:47:05 +0100
Subject: [PATCH] Improve file upload validation (#2952)

---
 .../src/dam/files/dam-upload-file.interceptor.ts | 16 ++++++++--------
 .../src/dam/files/file-validation.service.ts     | 11 +++--------
 .../file-uploads-file.interceptor.ts             | 16 ++++++++--------
 3 files changed, 19 insertions(+), 24 deletions(-)

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
index 7648eee149..ce81e881b4 100644
--- 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
index 70e7ce18b7..505dbca0c3 100644
--- 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,12 +16,7 @@ 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 {
         //mime type in an accepted mime type
         if (!this.config.acceptedMimeTypes.includes(file.mimetype)) {
             return "Unsupported mime type";
@@ -30,7 +25,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/file-uploads/file-uploads-file.interceptor.ts b/packages/api/cms-api/src/file-uploads/file-uploads-file.interceptor.ts
index 909b4bd831..9f258edd5e 100644
--- a/packages/api/cms-api/src/file-uploads/file-uploads-file.interceptor.ts
+++ b/packages/api/cms-api/src/file-uploads/file-uploads-file.interceptor.ts
@@ -35,20 +35,20 @@ export function FileUploadsFileInterceptor(fieldName: string): Type<NestIntercep
                     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);
+                    }
                 },
             };