From d272ff210e7ff09fd8d3802c3b707d7800e15b8a Mon Sep 17 00:00:00 2001 From: ifedapoolarewaju Date: Mon, 30 Mar 2020 14:00:14 +0100 Subject: [PATCH 1/2] companion: validate all client provided upload data --- .../@uppy/companion/src/server/Uploader.js | 57 ++++++++++-- .../companion/test/__tests__/companion.js | 92 +++++++++++++++++++ 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index a84265da2..319a49bc3 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -2,15 +2,17 @@ const fs = require('fs') const path = require('path') const tus = require('tus-js-client') const uuid = require('uuid') -const emitter = require('./emitter') +const isObject = require('isobject') +const validator = require('validator') const request = require('request') +const emitter = require('./emitter') const serializeError = require('serialize-error') const { jsonStringify, hasMatch } = require('./helpers/utils') const logger = require('./logger') -const validator = require('validator') const headerSanitize = require('./header-blacklist') const redis = require('./redis') +const DEFAULT_FIELD_NAME = 'files[]' const PROTOCOLS = Object.freeze({ multipart: 'multipart', s3Multipart: 's3-multipart', @@ -49,6 +51,7 @@ class Uploader { this.token = uuid.v4() this.path = `${this.options.pathPrefix}/${Uploader.FILE_NAME_PREFIX}-${this.token}` this.options.metadata = this.options.metadata || {} + this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME this.uploadFileName = this.options.metadata.name || path.basename(this.path) this.streamsEnded = false this.uploadStopped = false @@ -84,9 +87,6 @@ class Uploader { this._paused = true if (this.tus) { const shouldTerminate = !!this.tus.url - // @todo remove the ts-ignore when the tus-js-client type definitions - // have been updated with this change https://github.com/DefinitelyTyped/DefinitelyTyped/pull/40629 - // @ts-ignore this.tus.abort(shouldTerminate) } this.cleanUp() @@ -139,6 +139,45 @@ class Uploader { * @returns {boolean} */ validateOptions (options) { + // validate HTTP Method + if (options.httpMethod) { + if (typeof options.httpMethod !== 'string') { + this._errRespMessage = 'unsupported HTTP METHOD specified' + return false + } + + const method = options.httpMethod.toLowerCase() + if (method !== 'put' && method !== 'post') { + this._errRespMessage = 'unsupported HTTP METHOD specified' + return false + } + } + + // validate fieldname + if (options.fieldname && typeof options.fieldname !== 'string') { + this._errRespMessage = 'fieldname must be a string' + return false + } + + // validate metadata + if (options.metadata && !isObject(options.metadata)) { + this._errRespMessage = 'metadata must be an object' + return false + } + + // validate headers + if (options.headers && !isObject(options.headers)) { + this._errRespMessage = 'headers must be an object' + return false + } + + // validate protocol + // @todo this validation should not be conditional once the protocol field is mandatory + if (options.protocol && !Object.prototype.hasOwnProperty.call(PROTOCOLS, options.protocol)) { + this._errRespMessage = 'unsupported protocol specified' + return false + } + // s3 uploads don't require upload destination // validation, because the destination is determined // by the server's s3 config @@ -147,14 +186,14 @@ class Uploader { } if (!options.endpoint && !options.uploadUrl) { - this._errRespMessage = 'No destination specified' + this._errRespMessage = 'no destination specified' return false } const validatorOpts = { require_protocol: true, require_tld: !options.companionOptions.debug } return [options.endpoint, options.uploadUrl].every((url) => { if (url && !validator.isURL(url, validatorOpts)) { - this._errRespMessage = 'Invalid destination url' + this._errRespMessage = 'invalid destination url' return false } @@ -219,7 +258,7 @@ class Uploader { return } - // @todo a default protocol should not be set. We should ensure that the user specifies her protocol. + // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. const protocol = this.options.protocol || PROTOCOLS.multipart // The download has completed; close the file and start an upload if necessary. @@ -287,7 +326,7 @@ class Uploader { getResponse () { if (this._errRespMessage) { - return { body: this._errRespMessage, status: 400 } + return { body: { error: this._errRespMessage }, status: 400 } } return { body: { token: this.token }, status: 200 } } diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js index e03bf6fa7..0a3bf556f 100644 --- a/packages/@uppy/companion/test/__tests__/companion.js +++ b/packages/@uppy/companion/test/__tests__/companion.js @@ -66,6 +66,98 @@ describe('list provider files', () => { }) }) +describe('validate upload data', () => { + test('invalid upload protocol gets rejected', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tusInvalid' + }) + .expect(400) + .then((res) => expect(res.body.error).toBe('unsupported protocol specified')) + }) + + test('invalid upload fieldname gets rejected', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tus', + fieldname: 390 + }) + .expect(400) + .then((res) => expect(res.body.error).toBe('fieldname must be a string')) + }) + + test('invalid upload metadata gets rejected', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tus', + metadata: 'I am a string instead of object' + }) + .expect(400) + .then((res) => expect(res.body.error).toBe('metadata must be an object')) + }) + + test('invalid upload headers get rejected', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tus', + headers: 'I am a string instead of object' + }) + .expect(400) + .then((res) => expect(res.body.error).toBe('headers must be an object')) + }) + + test('invalid upload HTTP Method gets rejected', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tus', + httpMethod: 'DELETE' + }) + .expect(400) + .then((res) => expect(res.body.error).toBe('unsupported HTTP METHOD specified')) + }) + + test('valid upload data is allowed', () => { + return request(authServer) + .post('/drive/get/README.md') + .set('uppy-auth-token', token) + .set('Content-Type', 'application/json') + .send({ + endpoint: 'http://master.tus.com/files', + protocol: 'tus', + httpMethod: 'PUT', + headers: { + customheader: 'header value' + }, + metadata: { + mymetadata: 'matadata value' + }, + fieldname: 'uploadField' + + }) + .expect(200) + }) +}) + describe('download provdier file', () => { test('specified file gets downloaded from provider', () => { return request(authServer) From 8d5890ca3f99800466367d7b8d5cf435669088f1 Mon Sep 17 00:00:00 2001 From: ifedapoolarewaju Date: Mon, 30 Mar 2020 14:43:32 +0100 Subject: [PATCH 2/2] chores: remove blank line --- packages/@uppy/companion/test/__tests__/companion.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js index 0a3bf556f..0115d5ec5 100644 --- a/packages/@uppy/companion/test/__tests__/companion.js +++ b/packages/@uppy/companion/test/__tests__/companion.js @@ -152,7 +152,6 @@ describe('validate upload data', () => { mymetadata: 'matadata value' }, fieldname: 'uploadField' - }) .expect(200) })