From 82597080bed91cc5475546ad47fe490f1d8e3656 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 14 Dec 2017 21:25:51 +0100 Subject: [PATCH] Burn dirty require of api utils refs #9178 - `checkFileExists` and `checkFileIsValid` where dirty required from web/middleware - these two functions are only used in the target middleware - let's move them --- core/server/api/utils.js | 13 ------- .../web/middleware/validation/upload.js | 12 +++--- core/server/web/utils.js | 19 +++++++++- core/test/unit/api/utils_spec.js | 37 +------------------ core/test/unit/web/utils_spec.js | 37 +++++++++++++++++++ 5 files changed, 63 insertions(+), 55 deletions(-) create mode 100644 core/test/unit/web/utils_spec.js diff --git a/core/server/api/utils.js b/core/server/api/utils.js index 48a4f636a1..57acaa940c 100644 --- a/core/server/api/utils.js +++ b/core/server/api/utils.js @@ -2,7 +2,6 @@ // Shared helpers for working with the API var Promise = require('bluebird'), _ = require('lodash'), - path = require('path'), permissions = require('../services/permissions'), validation = require('../data/validation'), common = require('../lib/common'), @@ -322,18 +321,6 @@ utils = { } return Promise.resolve(object); - }, - checkFileExists: function checkFileExists(fileData) { - return !!(fileData.mimetype && fileData.path); - }, - checkFileIsValid: function checkFileIsValid(fileData, types, extensions) { - var type = fileData.mimetype, - ext = path.extname(fileData.name).toLowerCase(); - - if (_.includes(types, type) && _.includes(extensions, ext)) { - return true; - } - return false; } }; diff --git a/core/server/web/middleware/validation/upload.js b/core/server/web/middleware/validation/upload.js index 4223c4cacb..9962a8155c 100644 --- a/core/server/web/middleware/validation/upload.js +++ b/core/server/web/middleware/validation/upload.js @@ -1,6 +1,8 @@ -var apiUtils = require('../../../api/utils'), - common = require('../../../lib/common'), - config = require('../../../config'); +'use strict'; + +const common = require('../../../lib/common'), + config = require('../../../config'), + localUtils = require('../../utils'); module.exports = function upload(options) { var type = options.type; @@ -15,14 +17,14 @@ module.exports = function upload(options) { req.file.type = req.file.mimetype; // Check if a file was provided - if (!apiUtils.checkFileExists(req.file)) { + if (!localUtils.checkFileExists(req.file)) { return next(new common.errors.NoPermissionError({ message: common.i18n.t('errors.api.' + type + '.missingFile') })); } // Check if the file is valid - if (!apiUtils.checkFileIsValid(req.file, contentTypes, extensions)) { + if (!localUtils.checkFileIsValid(req.file, contentTypes, extensions)) { return next(new common.errors.UnsupportedMediaTypeError({ message: common.i18n.t('errors.api.' + type + '.invalidFile', {extensions: extensions}) })); diff --git a/core/server/web/utils.js b/core/server/web/utils.js index bab99e259f..10e2eee3f3 100644 --- a/core/server/web/utils.js +++ b/core/server/web/utils.js @@ -1,6 +1,9 @@ 'use strict'; -const url = require('url'); +const url = require('url'), + path = require('path'), + _ = require('lodash'); + let _private = {}; _private.removeDoubleCharacters = function removeDoubleCharacters(character, string) { @@ -30,3 +33,17 @@ module.exports.removeOpenRedirectFromUrl = function removeOpenRedirectFromUrl(ur (parsedUrl.hash || '') ); }; + +module.exports.checkFileExists = function checkFileExists(fileData) { + return !!(fileData.mimetype && fileData.path); +}; + +module.exports.checkFileIsValid = function checkFileIsValid(fileData, types, extensions) { + var type = fileData.mimetype, + ext = path.extname(fileData.name).toLowerCase(); + + if (_.includes(types, type) && _.includes(extensions, ext)) { + return true; + } + return false; +}; diff --git a/core/test/unit/api/utils_spec.js b/core/test/unit/api/utils_spec.js index db20a8469d..ea24489ddc 100644 --- a/core/test/unit/api/utils_spec.js +++ b/core/test/unit/api/utils_spec.js @@ -32,9 +32,7 @@ describe('API Utils', function () { 'prepareFields', 'prepareFormats', 'convertOptions', - 'checkObject', - 'checkFileExists', - 'checkFileIsValid' + 'checkObject' ]); }); @@ -427,39 +425,6 @@ describe('API Utils', function () { }); }); - describe('checkFileExists', function () { - it('should return true if file exists in input', function () { - apiUtils.checkFileExists({mimetype: 'file', path: 'path'}).should.be.true(); - }); - - it('should return false if file does not exist in input', function () { - apiUtils.checkFileExists({}).should.be.false(); - }); - - it('should return false if file is incorrectly structured', function () { - apiUtils.checkFileExists({type: 'file'}).should.be.false(); - }); - }); - - describe('checkFileIsValid', function () { - it('returns true if file has valid extension and type', function () { - apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.txt']).should.be.true(); - apiUtils.checkFileIsValid({ - name: 'test.jpg', - mimetype: 'jpeg' - }, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true(); - }); - - it('returns false if file has invalid extension', function () { - apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).should.be.false(); - apiUtils.checkFileIsValid({name: 'test', mimetype: 'text'}, ['text'], ['.txt']).should.be.false(); - }); - - it('returns false if file has invalid type', function () { - apiUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['archive'], ['.txt']).should.be.false(); - }); - }); - describe('isPublicContext', function () { it('should call out to permissions', function () { var permsStub = sandbox.stub(permissions, 'parseContext').returns({public: true}); diff --git a/core/test/unit/web/utils_spec.js b/core/test/unit/web/utils_spec.js new file mode 100644 index 0000000000..ebb60ee7b6 --- /dev/null +++ b/core/test/unit/web/utils_spec.js @@ -0,0 +1,37 @@ +var should = require('should'), + webUtils = require('../../../server/web/utils'); + +describe('web utils', function () { + describe('checkFileExists', function () { + it('should return true if file exists in input', function () { + webUtils.checkFileExists({mimetype: 'file', path: 'path'}).should.be.true(); + }); + + it('should return false if file does not exist in input', function () { + webUtils.checkFileExists({}).should.be.false(); + }); + + it('should return false if file is incorrectly structured', function () { + webUtils.checkFileExists({type: 'file'}).should.be.false(); + }); + }); + + describe('checkFileIsValid', function () { + it('returns true if file has valid extension and type', function () { + webUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.txt']).should.be.true(); + webUtils.checkFileIsValid({ + name: 'test.jpg', + mimetype: 'jpeg' + }, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true(); + }); + + it('returns false if file has invalid extension', function () { + webUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).should.be.false(); + webUtils.checkFileIsValid({name: 'test', mimetype: 'text'}, ['text'], ['.txt']).should.be.false(); + }); + + it('returns false if file has invalid type', function () { + webUtils.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['archive'], ['.txt']).should.be.false(); + }); + }); +});