From 22c0218f18dc7f4d116a8d8194472a6eb3795127 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sun, 26 Jun 2022 21:55:09 -0700 Subject: [PATCH] do not use a default upload protocol (#3834) --- packages/@uppy/companion/src/server/Uploader.js | 8 +++----- packages/@uppy/companion/test/__tests__/companion.js | 2 +- packages/@uppy/companion/test/__tests__/uploader.js | 7 +++++++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index d1099eb41c..77274c6eb9 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -93,9 +93,8 @@ function validateOptions (options) { } // validate protocol - // @todo this validation should not be conditional once the protocol field is mandatory - if (options.protocol && !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) { - throw new ValidationError('unsupported protocol specified') + if (options.protocol == null || !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) { + throw new ValidationError('please specify a valid protocol') } // s3 uploads don't require upload destination @@ -208,8 +207,7 @@ class Uploader { } async _uploadByProtocol () { - // @todo a default protocol should not be set. We should ensure that the user specifies their protocol. - const protocol = this.options.protocol || PROTOCOLS.multipart + const { protocol } = this.options switch (protocol) { case PROTOCOLS.multipart: diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js index c71c624d37..fe6d2f7ee6 100644 --- a/packages/@uppy/companion/test/__tests__/companion.js +++ b/packages/@uppy/companion/test/__tests__/companion.js @@ -36,7 +36,7 @@ describe('validate upload data', () => { protocol: 'tusInvalid', }) .expect(400) - .then((res) => expect(res.body.message).toBe('unsupported protocol specified')) + .then((res) => expect(res.body.message).toBe('please specify a valid protocol')) }) test('invalid upload fieldname gets rejected', () => { diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index fcb3640aff..577895ab83 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -20,10 +20,13 @@ process.env.COMPANION_DATADIR = './test/output' process.env.COMPANION_DOMAIN = 'localhost:3020' const { companionOptions } = standalone() +const protocol = 'tus' + describe('uploader with tus protocol', () => { test('uploader respects uploadUrls', async () => { const opts = { endpoint: 'http://localhost/files', + protocol, companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] }, } @@ -33,6 +36,7 @@ describe('uploader with tus protocol', () => { test('uploader respects uploadUrls, valid', async () => { const opts = { endpoint: 'http://url.myendpoint.com/files', + protocol, companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] }, } @@ -43,6 +47,7 @@ describe('uploader with tus protocol', () => { test('uploader respects uploadUrls, localhost', async () => { const opts = { endpoint: 'http://localhost:1337/', + protocol, companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/localhost:1337\//] }, } @@ -226,6 +231,7 @@ describe('uploader with tus protocol', () => { const opts = { companionOptions, endpoint: 'http://localhost', + protocol, } // eslint-disable-next-line no-new @@ -247,6 +253,7 @@ describe('uploader with tus protocol', () => { test('uploader respects maxFileSize correctly', async () => { const opts = { endpoint: 'http://url.myendpoint.com/files', + protocol, companionOptions: { ...companionOptions, maxFileSize: 100 }, size: 99, }