Cleanup req files on bad request

diff --git a/server/helpers/core-utils.ts b/server/helpers/core-utils.ts
index 2951aef1eebc86abc90e032d63c3f63f6d51501b..884206aad0c4043830bf4e168346519e9bb97994 100644 (file)
--- 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 6a9c6d75ce5492dc84926c31a3c80a1c8f668edd..6b1729f36935c4afb845d415ef7607d8b6d66af0 100644 (file)
--- 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 cfb4275703280bd87e8a968b19b7f49a4ca46b46..7abcec5d727fe3f08ea7607d755cfe92a01fcc66 100644 (file)
--- 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 f346ea92f0377383d3612e8de32742ee966dc566..5860735c6355c7b34ab3026b4b6588376c3d833d 100644 (file)
--- 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 b6d92d3809be86d5aa6aaed70209ddc070a0d13f..18d537bc473f35148d3e19fa7af2dc88ddca198c 100644 (file)
--- 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 7f65f729085e521232d93ddec50267bc37e421a1..143ce95829ac6157b5413279ffb93f11033c3c5e 100644 (file)
--- 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 d9af2aa0ab9a99bfb5e6f7d25ba4c76d64a42571..9357c1e39709ea3c1e5869c73181fd8bffad804c 100644 (file)
--- 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 a19b47509792d6fb359c0f3be3b6b816e28d44e2..25c87b4dca380a4614c5f5518a4ffc46c2da1e1c 100644 (file)
--- 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 fb9722630d752fa9ee85ff03c33854f9c6b9fc17..18a0d9ce3187d47d8bce71725adb221762de6841 100644 (file)
--- 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])