From cf7a61b5a2b68fd966c4a355e37e84b048ed296b Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 31 Jul 2018 15:09:34 +0200 Subject: [PATCH] Cleanup req files on bad request --- server/helpers/core-utils.ts | 2 +- .../custom-validators/video-captions.ts | 2 +- server/helpers/utils.ts | 28 ++++++++++++++++++- server/middlewares/validators/avatar.ts | 3 +- .../middlewares/validators/video-captions.ts | 7 +++-- .../middlewares/validators/video-channels.ts | 4 +-- server/middlewares/validators/videos.ts | 22 ++++++++------- server/tests/api/server/follows.ts | 2 +- server/tests/api/server/handle-down.ts | 2 +- 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) { '<': '<', '>': '>', '"': '"', - "'": ''', + '\'': ''', '/': '/', '`': '`', '=': '=' 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]) -- 2.41.0