diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index ad3a40e7557..3dc11a79f5a 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -14,6 +14,7 @@ const mockAdapter = { deleteFile: () => {}, getFileData: () => {}, getFileLocation: () => 'xyz', + validateFilename: () => {}, }; // Small additional tests to improve overall coverage @@ -118,4 +119,13 @@ describe('FilesController', () => { done(); }); + + it('should reject slashes in file names', done => { + const gridStoreAdapter = new GridFSBucketAdapter( + 'mongodb://localhost:27017/parse' + ); + const fileName = 'foo/randomFileName.pdf'; + expect(gridStoreAdapter.validateFilename(fileName)).not.toBe(null); + done(); + }); }); diff --git a/src/Adapters/Files/FilesAdapter.js b/src/Adapters/Files/FilesAdapter.js index db02174ca6e..1bf3d287152 100644 --- a/src/Adapters/Files/FilesAdapter.js +++ b/src/Adapters/Files/FilesAdapter.js @@ -8,12 +8,15 @@ // * deleteFile(filename) // * getFileData(filename) // * getFileLocation(config, filename) +// Adapter classes should implement the following functions: +// * validateFilename(filename) // // Default is GridFSBucketAdapter, which requires mongo // and for the API server to be using the DatabaseController with Mongo // database adapter. import type { Config } from '../../Config'; +import Parse from 'parse/lib/node/Parse'; /** * @module Adapters */ @@ -56,6 +59,37 @@ export class FilesAdapter { * @return {string} Absolute URL */ getFileLocation(config: Config, filename: string): string {} + + /** Validate a filename for this adaptor type (optional)1G + * + * @param {string} filename + * + * @returns {null|*|Parse.Error} null if there are no errors + */ + // TODO: Make this required once enough people have updated their adaptors + // validateFilename(filename): ?Parse.Error {} +} + +/** + * Default filename validate pulled out of FilesRouter. Mostly used for Mongo storage + * + * @param filename + * @returns {null|*|Parse.Error|Parse.ParseError|ParseError|ParseError|Parse.ParseError} + */ +export function validateFilename(filename): ?Parse.Error { + if (filename.length > 128) { + return new Parse.Error(Parse.Error.INVALID_FILE_NAME, 'Filename too long.'); + } + + // US/ASCII centric default + const regx = /^[_a-zA-Z0-9][a-zA-Z0-9@. ~_-]*$/; + if (!filename.match(regx)) { + return new Parse.Error( + Parse.Error.INVALID_FILE_NAME, + 'Filename contains invalid characters.' + ); + } + return null; // No errors } export default FilesAdapter; diff --git a/src/Adapters/Files/GridFSBucketAdapter.js b/src/Adapters/Files/GridFSBucketAdapter.js index bc1b4ae920c..28b238efe38 100644 --- a/src/Adapters/Files/GridFSBucketAdapter.js +++ b/src/Adapters/Files/GridFSBucketAdapter.js @@ -8,7 +8,7 @@ // @flow-disable-next import { MongoClient, GridFSBucket, Db } from 'mongodb'; -import { FilesAdapter } from './FilesAdapter'; +import { FilesAdapter, validateFilename } from './FilesAdapter'; import defaults from '../../defaults'; export class GridFSBucketAdapter extends FilesAdapter { @@ -139,6 +139,10 @@ export class GridFSBucketAdapter extends FilesAdapter { } return this._client.close(false); } + + validateFilename(filename) { + return validateFilename(filename); + } } export default GridFSBucketAdapter; diff --git a/src/Adapters/Files/GridStoreAdapter.js b/src/Adapters/Files/GridStoreAdapter.js index 3eb50ee5614..3d6bba10724 100644 --- a/src/Adapters/Files/GridStoreAdapter.js +++ b/src/Adapters/Files/GridStoreAdapter.js @@ -9,7 +9,7 @@ // @flow-disable-next import { MongoClient, GridStore, Db } from 'mongodb'; -import { FilesAdapter } from './FilesAdapter'; +import { FilesAdapter, validateFilename } from './FilesAdapter'; import defaults from '../../defaults'; export class GridStoreAdapter extends FilesAdapter { @@ -110,6 +110,10 @@ export class GridStoreAdapter extends FilesAdapter { } return this._client.close(false); } + + validateFilename(filename) { + return validateFilename(filename); + } } // handleRangeRequest is licensed under Creative Commons Attribution 4.0 International License (https://creativecommons.org/licenses/by/4.0/). diff --git a/src/Controllers/FilesController.js b/src/Controllers/FilesController.js index 579e3e43d6e..63c259931bc 100644 --- a/src/Controllers/FilesController.js +++ b/src/Controllers/FilesController.js @@ -1,7 +1,7 @@ // FilesController.js import { randomHexString } from '../cryptoUtils'; import AdaptableController from './AdaptableController'; -import { FilesAdapter } from '../Adapters/Files/FilesAdapter'; +import validateFilename, { FilesAdapter } from '../Adapters/Files/FilesAdapter'; import path from 'path'; import mime from 'mime'; @@ -95,6 +95,13 @@ export class FilesController extends AdaptableController { handleFileStream(config, filename, req, res, contentType) { return this.adapter.handleFileStream(filename, req, res, contentType); } + + validateFilename(filename) { + if (typeof this.adapter.validateFilename === 'function') { + return this.adapter.validateFilename(filename); + } + return validateFilename(filename); + } } export default FilesController; diff --git a/src/Routers/FilesRouter.js b/src/Routers/FilesRouter.js index 72d032afa72..56fb5a3cc8b 100644 --- a/src/Routers/FilesRouter.js +++ b/src/Routers/FilesRouter.js @@ -69,6 +69,11 @@ export class FilesRouter { } createHandler(req, res, next) { + const config = req.config; + const filesController = config.filesController; + const filename = req.params.filename; + const contentType = req.get('Content-type'); + if (!req.body || !req.body.length) { next( new Parse.Error(Parse.Error.FILE_SAVE_ERROR, 'Invalid file upload.') @@ -76,28 +81,12 @@ export class FilesRouter { return; } - if (req.params.filename.length > 128) { - next( - new Parse.Error(Parse.Error.INVALID_FILE_NAME, 'Filename too long.') - ); + const error = filesController.validateFilename(filename); + if (error) { + next(error); return; } - if (!req.params.filename.match(/^[_a-zA-Z0-9][a-zA-Z0-9@\.\ ~_-]*$/)) { - next( - new Parse.Error( - Parse.Error.INVALID_FILE_NAME, - 'Filename contains invalid characters.' - ) - ); - return; - } - - const filename = req.params.filename; - const contentType = req.get('Content-type'); - const config = req.config; - const filesController = config.filesController; - filesController .createFile(config, filename, req.body, contentType) .then(result => {