Companion 2.0 (#2273)

* companion: check if body is present in error response

fixes #2254

* companion: deprecate (and remove) req.uppy

* companion: remove duplex stream code

* companion: drop node 6 url parser

* companion: send back a consistent error response with the URL controller

* companion: allow videos from instagram graph API (the bug has been fixed)

* build: drop node 6 tests from travis

* build: run companion test for node.js 10

.travis.yml

@@ -77,13 +77,13 @@ jobs:
         script: bin/travis-deploy-companion
         on:
           branch: master
-    - name: 'Run Companion tests (Node.js 6)'
+    - name: 'Run Companion tests (Node.js 10)'
       node_js: 12
       services:
         - docker
       script:
-        - nvm install 6.0.0
-        - nvm use 6.0.0
+        - nvm install 10.0.0
+        - nvm use 10.0.0
         - npm run test:companion
     # Build the website when things are merged to master
     # https://docs.travis-ci.com/user/deployment/#Conditional-Releases-with-on

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

@@ -271,8 +271,6 @@ const getOptionsMiddleware = (options) => {
     }
 
     logger.info(`uppy client version ${req.companion.clientVersion}`, 'companion.client.version')
-    // @todo remove req.uppy in next major release
-    req.uppy = req.companion
     next()
   }
 

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

@@ -56,12 +56,6 @@ class Uploader {
     this.uploadFileName = this.options.metadata.name || path.basename(this.path)
     this.streamsEnded = false
     this.uploadStopped = false
-    this.duplexStream = null
-    // @TODO disabling parallel uploads and downloads for now
-    // if (this.options.protocol === PROTOCOLS.tus) {
-    //   this.duplexStream = new stream.PassThrough()
-    //     .on('error', (err) => logger.error(`${this.shortToken} ${err}`, 'uploader.duplex.error'))
-    // }
     this.writeStream = fs.createWriteStream(this.path, { mode: 0o666 }) // no executable files
       .on('error', (err) => logger.error(`${err}`, 'uploader.write.error', this.shortToken))
     /** @type {number} */
@@ -302,31 +296,8 @@ class Uploader {
     })
   }
 
-  /**
-   * @param {Buffer | Buffer[]} chunk
-   * @param {function} cb
-   */
-  writeToStreams (chunk, cb) {
-    const done = []
-    const doneLength = this.duplexStream ? 2 : 1
-    const onDone = () => {
-      done.push(true)
-      if (done.length >= doneLength) {
-        cb()
-      }
-    }
-
-    this.writeStream.write(chunk, onDone)
-    if (this.duplexStream) {
-      this.duplexStream.write(chunk, onDone)
-    }
-  }
-
   endStreams () {
     this.writeStream.end()
-    if (this.duplexStream) {
-      this.duplexStream.end()
-    }
   }
 
   getResponse () {

packages/@uppy/companion/src/server/controllers/oauth-redirect.js

@@ -1,5 +1,5 @@
 const qs = require('querystring')
-const parseUrl = require('url').parse // eslint-disable-line node/no-deprecated-api
+const { URL } = require('url')
 const { hasMatch } = require('../helpers/utils')
 const oAuthState = require('../helpers/oauth-state')
 
@@ -15,7 +15,7 @@ module.exports = function oauthRedirect (req, res) {
     return res.status(400).send('Cannot find state in session')
   }
   const handler = oAuthState.getFromState(state, 'companionInstance', req.companion.options.secret)
-  const handlerHostName = parseUrl(handler).host
+  const handlerHostName = (new URL(handler)).host
 
   if (hasMatch(handlerHostName, req.companion.options.server.validHosts)) {
     const providerName = req.companion.provider.authProvider

packages/@uppy/companion/src/server/controllers/send-token.js

@@ -3,7 +3,7 @@
  * sends auth token to uppy client
  */
 const tokenService = require('../helpers/jwt')
-const parseUrl = require('url').parse // eslint-disable-line node/no-deprecated-api
+const { URL } = require('url')
 const { hasMatch, sanitizeHtml } = require('../helpers/utils')
 const oAuthState = require('../helpers/oauth-state')
 const versionCmp = require('../helpers/version')
@@ -32,7 +32,7 @@ module.exports = function sendToken (req, res, next) {
     )
     const allowedClients = req.companion.options.clients
     // if no preset clients then allow any client
-    if (!allowedClients || hasMatch(origin, allowedClients) || hasMatch(parseUrl(origin).host, allowedClients)) {
+    if (!allowedClients || hasMatch(origin, allowedClients) || hasMatch((new URL(origin)).host, allowedClients)) {
       const allowsStringMessage = versionCmp.gte(clientVersion, '1.0.2')
       return res.send(allowsStringMessage ? htmlContent(uppyAuthToken, origin) : oldHtmlContent(uppyAuthToken, origin))
     }

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

@@ -1,5 +1,6 @@
 const router = require('express').Router
 const request = require('request')
+const { URL } = require('url')
 const Uploader = require('../Uploader')
 const validator = require('validator')
 const utils = require('../helpers/utils')
@@ -30,7 +31,7 @@ const meta = (req, res) => {
     .then((meta) => res.json(meta))
     .catch((err) => {
       logger.error(err, 'controller.url.meta.error', req.id)
-      return res.status(500).json({ error: err })
+      return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
     })
 }
 
@@ -71,8 +72,7 @@ const get = (req, res) => {
       res.status(response.status).json(response.body)
     }).catch((err) => {
       logger.error(err, 'controller.url.get.error', req.id)
-      // @todo this should send back error (not err)
-      res.json({ err })
+      return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
     })
 }
 
@@ -114,7 +114,7 @@ const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => {
     uri: url,
     method: 'GET',
     followAllRedirects: true,
-    agentClass: getProtectedHttpAgent(utils.parseURL(url).protocol, blockLocalIPs)
+    agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs)
   }
 
   request(opts)

packages/@uppy/companion/src/server/helpers/utils.js

@@ -1,5 +1,5 @@
 const request = require('request')
-const urlParser = require('url')
+const { URL } = require('url')
 const crypto = require('crypto')
 const { getProtectedHttpAgent } = require('./request')
 
@@ -43,17 +43,6 @@ exports.sanitizeHtml = (text) => {
   return text ? text.replace(/<\/?[^>]+(>|$)/g, '') : text
 }
 
-/**
- * Node 6(and beyond) compatible url parser
- * @todo drop the use of url.parse when support for node 6 is dropped
- *
- * @param {string} url URL to be parsed
- */
-exports.parseURL = (url) => {
-  // eslint-disable-next-line
-  return urlParser.URL ? new urlParser.URL(url) : urlParser.parse(url)
-}
-
 /**
  * Gets the size and content type of a url's content
  *
@@ -67,7 +56,7 @@ exports.getURLMeta = (url, blockLocalIPs = false) => {
       uri: url,
       method: 'HEAD',
       followAllRedirects: true,
-      agentClass: getProtectedHttpAgent(exports.parseURL(url).protocol, blockLocalIPs)
+      agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs)
     }
 
     request(opts, (err, response, body) => {

packages/@uppy/companion/src/server/provider/index.js

@@ -46,7 +46,7 @@ module.exports.getProviderMiddleware = (providers) => {
  */
 module.exports.getDefaultProviders = (companionOptions) => {
   const { providerOptions } = companionOptions || { providerOptions: null }
-  // @todo: 2.0 we should rename drive to googledrive or google-drive or google
+  // @todo: we should rename drive to googledrive or google-drive or google
   const providers = { dropbox, drive, facebook, onedrive }
   // Instagram's Graph API key is just numbers, while the old API key is hex
   const usesGraphAPI = () => /^\d+$/.test(providerOptions.instagram.key)

packages/@uppy/companion/src/server/provider/instagram/graph/adapter.js

@@ -19,22 +19,8 @@ exports.getItemIcon = (item) => {
 exports.getItemSubList = (item) => {
   const newItems = []
   item.data.forEach((subItem) => {
-    // exclude videos because of bug https://developers.facebook.com/support/bugs/801145630390846/
-    // @todo remove this clause when bug is fixed
-    if (isVideo(subItem)) {
-      return
-    }
-
     if (subItem.media_type === MEDIA_TYPES.carousel) {
-      subItem.children.data.forEach((i) => {
-        // exclude videos because of bug https://developers.facebook.com/support/bugs/801145630390846/
-        // @todo remove this clause when bug is fixed
-        if (isVideo(i)) {
-          return
-        }
-
-        newItems.push(i)
-      })
+      subItem.children.data.forEach((i) => newItems.push(i))
     } else {
       newItems.push(subItem)
     }

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

@@ -6,7 +6,7 @@ const morgan = require('morgan')
 const bodyParser = require('body-parser')
 const redis = require('../server/redis')
 const logger = require('../server/logger')
-const { parseURL } = require('../server/helpers/utils')
+const { URL } = require('url')
 const merge = require('lodash.merge')
 // @ts-ignore
 const promBundle = require('express-prom-bundle')
@@ -80,7 +80,7 @@ morgan.token('url', (req, res) => {
 morgan.token('referrer', (req, res) => {
   const ref = req.headers.referer || req.headers.referrer
   if (typeof ref === 'string') {
-    const parsed = parseURL(ref)
+    const parsed = new URL(ref)
     const rawQuery = qs.parse(parsed.search.replace('?', ''))
     const { query, censored } = censorQuery(rawQuery)
     return censored ? `${parsed.href.split('?')[0]}?${qs.stringify(query)}` : parsed.href