Cleanup req files on bad request

9 files changed, 50 insertions, 22 deletions

diff --git a/server/helpers/core-utils.ts b/server/helpers/core-utils.ts
index 2951aef1e..884206aad 100644
--- a/server/helpers/core-utils.ts
+++ b/server/helpers/core-utils.ts
@@ -58,7 +58,7 @@ function escapeHTML (stringParam) {
     '<': '&lt;',
     '>': '&gt;',
     '"': '&quot;',
-    "'": '&#39;',
+    '\'': '&#39;',
     '/': '&#x2F;',
     '`': '&#x60;',
     '=': '&#x3D;'
diff --git a/server/helpers/custom-validators/video-captions.ts b/server/helpers/custom-validators/video-captions.ts
index 6a9c6d75c..6b1729f36 100644
--- a/server/helpers/custom-validators/video-captions.ts
+++ b/server/helpers/custom-validators/video-captions.ts
@@ -1,4 +1,4 @@
-import { CONSTRAINTS_FIELDS, VIDEO_CAPTIONS_MIMETYPE_EXT, VIDEO_LANGUAGES, VIDEO_MIMETYPE_EXT } from '../../initializers'
+import { CONSTRAINTS_FIELDS, VIDEO_CAPTIONS_MIMETYPE_EXT, VIDEO_LANGUAGES } from '../../initializers'
 import { exists, isFileValid } from './misc'
 import { Response } from 'express'
 import { VideoModel } from '../../models/video/video'
diff --git a/server/helpers/utils.ts b/server/helpers/utils.ts
index cfb427570..7abcec5d7 100644
--- a/server/helpers/utils.ts
+++ b/server/helpers/utils.ts
@@ -6,11 +6,35 @@ import { CONFIG } from '../initializers'
 import { UserModel } from '../models/account/user'
 import { ActorModel } from '../models/activitypub/actor'
 import { ApplicationModel } from '../models/application/application'
-import { pseudoRandomBytesPromise } from './core-utils'
+import { pseudoRandomBytesPromise, unlinkPromise } from './core-utils'
 import { logger } from './logger'
+import { isArray } from './custom-validators/misc'
 
 const isCidr = require('is-cidr')
 
+function cleanUpReqFiles (req: { files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[] }) {
+  const files = req.files
+
+  if (!files) return
+
+  if (isArray(files)) {
+    (files as Express.Multer.File[]).forEach(f => deleteFileAsync(f.path))
+    return
+  }
+
+  for (const key of Object.keys(files)) {
+    const file = files[key]
+
+    if (isArray(file)) file.forEach(f => deleteFileAsync(f.path))
+    else deleteFileAsync(file.path)
+  }
+}
+
+function deleteFileAsync (path: string) {
+  unlinkPromise(path)
+    .catch(err => logger.error('Cannot delete the file %s asynchronously.', path, { err }))
+}
+
 async function generateRandomString (size: number) {
   const raw = await pseudoRandomBytesPromise(size)
 
@@ -162,6 +186,8 @@ type SortType = { sortModel: any, sortValue: string }
 // ---------------------------------------------------------------------------
 
 export {
+  cleanUpReqFiles,
+  deleteFileAsync,
   generateRandomString,
   getFormattedObjects,
   isSignupAllowed,
diff --git a/server/middlewares/validators/avatar.ts b/server/middlewares/validators/avatar.ts
index f346ea92f..5860735c6 100644
--- a/server/middlewares/validators/avatar.ts
+++ b/server/middlewares/validators/avatar.ts
@@ -4,6 +4,7 @@ import { isAvatarFile } from '../../helpers/custom-validators/users'
 import { areValidationErrors } from './utils'
 import { CONSTRAINTS_FIELDS } from '../../initializers'
 import { logger } from '../../helpers/logger'
+import { cleanUpReqFiles } from '../../helpers/utils'
 
 const updateAvatarValidator = [
   body('avatarfile').custom((value, { req }) => isAvatarFile(req.files)).withMessage(
@@ -14,7 +15,7 @@ const updateAvatarValidator = [
   (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking updateAvatarValidator parameters', { files: req.files })
 
-    if (areValidationErrors(req, res)) return
+    if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
 
     return next()
   }
diff --git a/server/middlewares/validators/video-captions.ts b/server/middlewares/validators/video-captions.ts
index b6d92d380..18d537bc4 100644
--- a/server/middlewares/validators/video-captions.ts
+++ b/server/middlewares/validators/video-captions.ts
@@ -7,6 +7,7 @@ import { CONSTRAINTS_FIELDS } from '../../initializers'
 import { UserRight } from '../../../shared'
 import { logger } from '../../helpers/logger'
 import { isVideoCaptionExist, isVideoCaptionFile, isVideoCaptionLanguageValid } from '../../helpers/custom-validators/video-captions'
+import { cleanUpReqFiles } from '../../helpers/utils'
 
 const addVideoCaptionValidator = [
   param('videoId').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid video id'),
@@ -20,12 +21,12 @@ const addVideoCaptionValidator = [
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking addVideoCaption parameters', { parameters: req.body })
 
-    if (areValidationErrors(req, res)) return
-    if (!await isVideoExist(req.params.videoId, res)) return
+    if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
+    if (!await isVideoExist(req.params.videoId, res)) return cleanUpReqFiles(req)
 
     // Check if the user who did the request is able to update the video
     const user = res.locals.oauth.token.User
-    if (!checkUserCanManageVideo(user, res.locals.video, UserRight.UPDATE_ANY_VIDEO, res)) return
+    if (!checkUserCanManageVideo(user, res.locals.video, UserRight.UPDATE_ANY_VIDEO, res)) return cleanUpReqFiles(req)
 
     return next()
   }
diff --git a/server/middlewares/validators/video-channels.ts b/server/middlewares/validators/video-channels.ts
index 7f65f7290..143ce9582 100644
--- a/server/middlewares/validators/video-channels.ts
+++ b/server/middlewares/validators/video-channels.ts
@@ -1,7 +1,7 @@
 import * as express from 'express'
 import { body, param } from 'express-validator/check'
 import { UserRight } from '../../../shared'
-import { isAccountIdExist, isAccountNameWithHostExist } from '../../helpers/custom-validators/accounts'
+import { isAccountNameWithHostExist } from '../../helpers/custom-validators/accounts'
 import { isIdOrUUIDValid } from '../../helpers/custom-validators/misc'
 import {
   isVideoChannelDescriptionValid,
@@ -13,8 +13,6 @@ import { logger } from '../../helpers/logger'
 import { UserModel } from '../../models/account/user'
 import { VideoChannelModel } from '../../models/video/video-channel'
 import { areValidationErrors } from './utils'
-import { isAvatarFile } from '../../helpers/custom-validators/users'
-import { CONSTRAINTS_FIELDS } from '../../initializers'
 
 const listVideoAccountChannelsValidator = [
   param('accountName').exists().withMessage('Should have a valid account name'),
diff --git a/server/middlewares/validators/videos.ts b/server/middlewares/validators/videos.ts
index d9af2aa0a..9357c1e39 100644
--- a/server/middlewares/validators/videos.ts
+++ b/server/middlewares/validators/videos.ts
@@ -35,6 +35,7 @@ import { CONSTRAINTS_FIELDS } from '../../initializers'
 import { VideoShareModel } from '../../models/video/video-share'
 import { authenticate } from '../oauth'
 import { areValidationErrors } from './utils'
+import { cleanUpReqFiles } from '../../helpers/utils'
 
 const videosAddValidator = getCommonVideoAttributes().concat([
   body('videofile')
@@ -50,13 +51,13 @@ const videosAddValidator = getCommonVideoAttributes().concat([
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking videosAdd parameters', { parameters: req.body, files: req.files })
 
-    if (areValidationErrors(req, res)) return
-    if (areErrorsInScheduleUpdate(req, res)) return
+    if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
+    if (areErrorsInScheduleUpdate(req, res)) return cleanUpReqFiles(req)
 
     const videoFile: Express.Multer.File = req.files['videofile'][0]
     const user = res.locals.oauth.token.User
 
-    if (!await isVideoChannelOfAccountExist(req.body.channelId, user, res)) return
+    if (!await isVideoChannelOfAccountExist(req.body.channelId, user, res)) return cleanUpReqFiles(req)
 
     const isAble = await user.isAbleToUploadVideo(videoFile)
     if (isAble === false) {
@@ -64,7 +65,7 @@ const videosAddValidator = getCommonVideoAttributes().concat([
          .json({ error: 'The user video quota is exceeded with this video.' })
          .end()
 
-      return
+      return cleanUpReqFiles(req)
     }
 
     let duration: number
@@ -77,7 +78,7 @@ const videosAddValidator = getCommonVideoAttributes().concat([
          .json({ error: 'Invalid input file.' })
          .end()
 
-      return
+      return cleanUpReqFiles(req)
     }
 
     videoFile['duration'] = duration
@@ -99,23 +100,24 @@ const videosUpdateValidator = getCommonVideoAttributes().concat([
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
     logger.debug('Checking videosUpdate parameters', { parameters: req.body })
 
-    if (areValidationErrors(req, res)) return
-    if (areErrorsInScheduleUpdate(req, res)) return
-    if (!await isVideoExist(req.params.id, res)) return
+    if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
+    if (areErrorsInScheduleUpdate(req, res)) return cleanUpReqFiles(req)
+    if (!await isVideoExist(req.params.id, res)) return cleanUpReqFiles(req)
 
     const video = res.locals.video
 
     // Check if the user who did the request is able to update the video
     const user = res.locals.oauth.token.User
-    if (!checkUserCanManageVideo(user, res.locals.video, UserRight.UPDATE_ANY_VIDEO, res)) return
+    if (!checkUserCanManageVideo(user, res.locals.video, UserRight.UPDATE_ANY_VIDEO, res)) return cleanUpReqFiles(req)
 
     if (video.privacy !== VideoPrivacy.PRIVATE && req.body.privacy === VideoPrivacy.PRIVATE) {
+      cleanUpReqFiles(req)
       return res.status(409)
         .json({ error: 'Cannot set "private" a video that was not private.' })
         .end()
     }
 
-    if (req.body.channelId && !await isVideoChannelOfAccountExist(req.body.channelId, user, res)) return
+    if (req.body.channelId && !await isVideoChannelOfAccountExist(req.body.channelId, user, res)) return cleanUpReqFiles(req)
 
     return next()
   }
diff --git a/server/tests/api/server/follows.ts b/server/tests/api/server/follows.ts
index a19b47509..25c87b4dc 100644
--- a/server/tests/api/server/follows.ts
+++ b/server/tests/api/server/follows.ts
@@ -178,7 +178,7 @@ describe('Test follows', function () {
   })
 
   it('Should upload a video on server 2 and 3 and propagate only the video of server 2', async function () {
-    this.timeout(10000)
+    this.timeout(35000)
 
     await uploadVideo(servers[1].url, servers[1].accessToken, { name: 'server2' })
     await uploadVideo(servers[2].url, servers[2].accessToken, { name: 'server3' })
diff --git a/server/tests/api/server/handle-down.ts b/server/tests/api/server/handle-down.ts
index fb9722630..18a0d9ce3 100644
--- a/server/tests/api/server/handle-down.ts
+++ b/server/tests/api/server/handle-down.ts
@@ -176,7 +176,7 @@ describe('Test handle downs', function () {
   })
 
   it('Should re-follow server 1', async function () {
-    this.timeout(15000)
+    this.timeout(35000)
 
     await reRunServer(servers[1])
     await reRunServer(servers[2])