Move filename validation out of the Router and into the FilesAdaptor (p…

…arse-community#6157)

* Move filename validation out of the Router and into the FilesAdaptor

* Address PR comments

* Update unittests to handle FilesAdapter interface change

* Make validateFilename optional

spec/AdaptableController.spec.js

@@ -64,6 +64,7 @@ describe('AdaptableController', () => {
  deleteFile: function() {},
  getFileData: function() {},
  getFileLocation: function() {},
+ validateFilename: function() {},
  };
  expect(() => {
  new FilesController(adapter);
@@ -77,6 +78,7 @@ describe('AdaptableController', () => {
  AGoodAdapter.prototype.deleteFile = function() {};
  AGoodAdapter.prototype.getFileData = function() {};
  AGoodAdapter.prototype.getFileLocation = function() {};
+ AGoodAdapter.prototype.validateFilename = function() {};
 
  const adapter = new AGoodAdapter();
  expect(() => {

spec/FilesController.spec.js

@@ -4,6 +4,8 @@ const WinstonLoggerAdapter = require('../lib/Adapters/Logger/WinstonLoggerAdapte
  .WinstonLoggerAdapter;
 const GridFSBucketAdapter = require('../lib/Adapters/Files/GridFSBucketAdapter')
  .GridFSBucketAdapter;
+const GridStoreAdapter = require('../lib/Adapters/Files/GridStoreAdapter')
+ .GridStoreAdapter;
 const Config = require('../lib/Config');
 const FilesController = require('../lib/Controllers/FilesController').default;
 
@@ -14,6 +16,7 @@ const mockAdapter = {
  deleteFile: () => {},
  getFileData: () => {},
  getFileLocation: () => 'xyz',
+ validateFilename: () => {},
 };
 
 // Small additional tests to improve overall coverage
@@ -118,4 +121,22 @@ 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();
+ });
+
+ it('should also reject slashes in file names', done => {
+ const gridStoreAdapter = new GridStoreAdapter(
+ 'mongodb://localhost:27017/parse'
+ );
+ const fileName = 'foo/randomFileName.pdf';
+ expect(gridStoreAdapter.validateFilename(fileName)).not.toBe(null);
+ done();
+ });
 });

src/Adapters/Files/FilesAdapter.js

@@ -8,12 +8,16 @@
 // * deleteFile(filename)
 // * getFileData(filename)
 // * getFileLocation(config, filename)
+// Adapter classes should implement the following functions:
+// * validateFilename(filename)
+// * handleFileStream(filename, req, res, contentType)
 //
 // 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/node';
 /**
  * @module Adapters
  */
@@ -56,6 +60,46 @@ export class FilesAdapter {
  * @return {string} Absolute URL
  */
  getFileLocation(config: Config, filename: string): string {}
+
+ /** Validate a filename for this adapter type
+ *
+ * @param {string} filename
+ *
+ * @returns {null|Parse.Error} null if there are no errors
+ */
+ // validateFilename(filename: string): ?Parse.Error {}
+
+ /** Handles Byte-Range Requests for Streaming
+ *
+ * @param {string} filename
+ * @param {object} req
+ * @param {object} res
+ * @param {string} contentType
+ *
+ * @returns {Promise} Data for byte range
+ */
+ // handleFileStream(filename: string, res: any, req: any, contentType: string): Promise
+}
+
+/**
+ * Simple filename validation
+ *
+ * @param filename
+ * @returns {null|Parse.Error}
+ */
+export function validateFilename(filename): ?Parse.Error {
+ if (filename.length > 128) {
+ return new Parse.Error(Parse.Error.INVALID_FILE_NAME, 'Filename too long.');
+ }
+
+ 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;
 }
 
 export default FilesAdapter;

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;

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/).

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;

src/Routers/FilesRouter.js

@@ -69,35 +69,24 @@ 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.')
  );
  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 => {