@uppy/companion: only body parse when needed & increased body size for s3 (#4372)

* refine body parsers

to make it possible to adjust them individually
also improves security and speed
because we don't have to parse all body types (and all sizes) for all endpoints

* increase body size for s3 complete endpoint

fixes #1945

packages/@uppy/companion/src/companion.js

@@ -97,8 +97,11 @@ module.exports.app = (optionsArg = {}) => {
   app.use(cookieParser()) // server tokens are added to cookies
 
   app.use(interceptGrantErrorResponse)
+
   // override provider credentials at request time
-  app.use('/connect/:authProvider/:override?', getCredentialsOverrideMiddleware(providers, options))
+  // Making `POST` request to the `/connect/:provider/:override?` route requires a form body parser middleware:
+  // See https://github.com/simov/grant#dynamic-http
+  app.use('/connect/:authProvider/:override?', express.urlencoded({ extended: false }), getCredentialsOverrideMiddleware(providers, options))
   app.use(Grant(grantConfig))
 
   app.use((req, res, next) => {
@@ -116,19 +119,19 @@ module.exports.app = (optionsArg = {}) => {
   app.use('/s3', s3(options.s3))
   app.use('/url', url())
 
-  app.post('/:providerName/preauth', middlewares.hasSessionAndProvider, controllers.preauth)
+  app.post('/:providerName/preauth', express.json(), express.urlencoded({ extended: false }), middlewares.hasSessionAndProvider, middlewares.hasBody, controllers.preauth)
   app.get('/:providerName/connect', middlewares.hasSessionAndProvider, controllers.connect)
   app.get('/:providerName/redirect', middlewares.hasSessionAndProvider, controllers.redirect)
   app.get('/:providerName/callback', middlewares.hasSessionAndProvider, controllers.callback)
-  app.post('/:providerName/deauthorization/callback', middlewares.hasSessionAndProvider, controllers.deauthorizationCallback)
+  app.post('/:providerName/deauthorization/callback', express.json(), middlewares.hasSessionAndProvider, middlewares.hasBody, controllers.deauthorizationCallback)
   app.get('/:providerName/logout', middlewares.hasSessionAndProvider, middlewares.gentleVerifyToken, controllers.logout)
   app.get('/:providerName/send-token', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.sendToken)
   app.get('/:providerName/list/:id?', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.list)
-  app.post('/:providerName/get/:id', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.get)
+  app.post('/:providerName/get/:id', express.json(), middlewares.hasSessionAndProvider, middlewares.hasBody, middlewares.verifyToken, controllers.get)
   app.get('/:providerName/thumbnail/:id', middlewares.hasSessionAndProvider, middlewares.cookieAuthToken, middlewares.verifyToken, controllers.thumbnail)
   // @ts-ignore Type instantiation is excessively deep and possibly infinite.
   app.get('/search/:searchProviderName/list', middlewares.hasSearchQuery, middlewares.loadSearchProviderToken, controllers.list)
-  app.post('/search/:searchProviderName/get/:id', middlewares.loadSearchProviderToken, controllers.get)
+  app.post('/search/:searchProviderName/get/:id', express.json(), middlewares.loadSearchProviderToken, controllers.get)
 
   app.param('providerName', providerManager.getProviderMiddleware(providers, true))
   app.param('searchProviderName', providerManager.getProviderMiddleware(searchProviders))

packages/@uppy/companion/src/server/controllers/s3.js

@@ -1,4 +1,4 @@
-const router = require('express').Router
+const express = require('express')
 
 module.exports = function s3 (config) {
   if (typeof config.acl !== 'string' && config.acl != null) {
@@ -342,13 +342,14 @@ module.exports = function s3 (config) {
     })
   }
 
-  return router()
+  return express.Router()
     .get('/params', getUploadParameters)
-    .post('/multipart', createMultipartUpload)
+    .post('/multipart', express.json(), createMultipartUpload)
     .get('/multipart/:uploadId', getUploadedParts)
     .get('/multipart/:uploadId/batch', batchSignPartsUpload)
     .get('/multipart/:uploadId/:partNumber', signPartUpload)
-    .post('/multipart/:uploadId/complete', completeMultipartUpload)
+    // limit 1mb because maybe large upload with a lot of parts, see https://github.com/transloadit/uppy/issues/1945
+    .post('/multipart/:uploadId/complete', express.json({ limit: '1mb' }), completeMultipartUpload)
     .delete('/multipart/:uploadId', abortMultipartUpload)
 }
 

packages/@uppy/companion/src/server/controllers/url.js

@@ -1,4 +1,4 @@
-const router = require('express').Router
+const express = require('express')
 const validator = require('validator')
 
 const { startDownUpload } = require('../helpers/upload')
@@ -112,6 +112,6 @@ const get = async (req, res) => {
   startDownUpload({ req, res, getSize, download, onUnhandledError })
 }
 
-module.exports = () => router()
-  .post('/meta', meta)
-  .post('/get', get)
+module.exports = () => express.Router()
+  .post('/meta', express.json(), meta)
+  .post('/get', express.json(), get)

packages/@uppy/companion/src/server/middlewares.js

@@ -9,8 +9,8 @@ const getS3Client = require('./s3-client')
 const { getURLBuilder } = require('./helpers/utils')
 
 exports.hasSessionAndProvider = (req, res, next) => {
-  if (!req.session || !req.body) {
-    logger.debug('No session/body attached to req object. Exiting dispatcher.', null, req.id)
+  if (!req.session) {
+    logger.debug('No session attached to req object. Exiting dispatcher.', null, req.id)
     return res.sendStatus(400)
   }
 
@@ -22,6 +22,15 @@ exports.hasSessionAndProvider = (req, res, next) => {
   return next()
 }
 
+exports.hasBody = (req, res, next) => {
+  if (!req.body) {
+    logger.debug('No body attached to req object. Exiting dispatcher.', null, req.id)
+    return res.sendStatus(400)
+  }
+
+  return next()
+}
+
 exports.hasSearchQuery = (req, res, next) => {
   if (typeof req.query.q !== 'string') {
     logger.debug('search request has no search query', 'search.query.check', req.id)

packages/@uppy/companion/src/standalone/index.js

@@ -2,7 +2,6 @@ const express = require('express')
 const qs = require('node:querystring')
 const helmet = require('helmet')
 const morgan = require('morgan')
-const bodyParser = require('body-parser')
 const { URL } = require('node:url')
 const session = require('express-session')
 const addRequestId = require('express-request-id')()
@@ -96,9 +95,6 @@ module.exports = function server (inputCompanionOptions) {
     return undefined
   })
 
-  router.use(bodyParser.json())
-  router.use(bodyParser.urlencoded({ extended: false }))
-
   // Use helmet to secure Express headers
   router.use(helmet.frameguard())
   router.use(helmet.xssFilter())