Merge pull request #2160 from transloadit/validate-upload-data

companion: validate all upload data provided by Uppy Client

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 }
   }

packages/@uppy/companion/test/__tests__/companion.js

@@ -66,6 +66,97 @@ 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)