@uppy/companion: Use filename from content-disposition instead of relying on url, with fallback (#4489)

* Use filename from content-disposition instead of relying on url, with fallback

* Update packages/@uppy/companion/src/server/helpers/request.js

* Update packages/@uppy/companion/src/server/helpers/request.js

* Add random string to file name coming from basename in case ?param is a differentiator

* Add e2e test for Companion /url/meta file name + mock server with Content-Disposition

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* Update e2e/cypress/integration/dashboard-xhr.spec.ts

Co-authored-by: Mikael Finstad <finstaden@gmail.com>

* Update e2e/mock-server.mjs

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* Update package.json

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* Update e2e/mock-server.mjs

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* Update e2e/cypress/integration/dashboard-xhr.spec.ts

Co-authored-by: Mikael Finstad <finstaden@gmail.com>

* Add comment about why randomUUID is added to file name

* yarn

* res vs response

---------

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
Co-authored-by: Mikael Finstad <finstaden@gmail.com>

e2e/cypress.config.mjs

@@ -1,5 +1,6 @@
 import { defineConfig } from 'cypress'
 import installLogsPrinter from 'cypress-terminal-report/src/installLogsPrinter.js'
+import startMockServer from './mock-server.mjs'
 
 export default defineConfig({
   defaultCommandTimeout: 16000,
@@ -11,6 +12,8 @@ export default defineConfig({
     setupNodeEvents (on) {
       // implement node event listeners here
       installLogsPrinter(on)
+
+      startMockServer('localhost', 4678)
     },
   },
 })

e2e/cypress/integration/dashboard-xhr.spec.ts

@@ -11,6 +11,43 @@ describe('Dashboard with XHR', () => {
     runRemoteUrlImageUploadTest()
   })
 
+  it('should return correct file name with URL plugin from remote image with Content-Disposition', () => {
+    const fileName = `DALL·E IMG_9078 - 学中文 🤑`
+    cy.get('[data-cy="Url"]').click()
+    cy.get('.uppy-Url-input').type('http://localhost:4678/file-with-content-disposition')
+    cy.get('.uppy-Url-importButton').click()
+    cy.wait('@url').then(() => {
+      cy.get('.uppy-Dashboard-Item-name').should('contain', fileName)
+      cy.get('.uppy-Dashboard-Item-status').should('contain', '84 KB')
+    })
+  })
+
+  it('should return correct file name with URL plugin from remote image without Content-Disposition', () => {
+    cy.get('[data-cy="Url"]').click()
+    cy.get('.uppy-Url-input').type('http://localhost:4678/file-no-headers')
+    cy.get('.uppy-Url-importButton').click()
+    cy.wait('@url').then(() => {
+      cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-no')
+      cy.get('.uppy-Dashboard-Item-status').should('contain', '0')
+    })
+  })
+
+  it('should return correct file name even when Companion doesnt supply it', () => {
+    cy.intercept('POST', 'http://localhost:3020/url/meta', {
+      statusCode: 200,
+      headers: {},
+      body: JSON.stringify({ size: 123, type: 'image/jpeg' }),
+    }).as('url')
+
+    cy.get('[data-cy="Url"]').click()
+    cy.get('.uppy-Url-input').type('http://localhost:4678/file-with-content-disposition')
+    cy.get('.uppy-Url-importButton').click()
+    cy.wait('@url').then(() => {
+      cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-with')
+      cy.get('.uppy-Dashboard-Item-status').should('contain', '123 B')
+    })
+  })
+
   it('should upload remote image with Unsplash plugin', () => {
     runRemoteUnsplashUploadTest()
   })

e2e/mock-server.mjs

@@ -0,0 +1,30 @@
+import http from 'node:http'
+
+const requestListener = (req, res) => {
+  const endpoint = req.url
+
+  switch (endpoint) {
+    case '/file-with-content-disposition': {
+      const fileName = `DALL·E IMG_9078 - 学中文 🤑`
+      res.setHeader('Content-Disposition', `attachment; filename="ASCII-name.zip"; filename*=UTF-8''${encodeURIComponent(fileName)}`)
+      res.setHeader('Content-Type', 'image/jpeg')
+      res.setHeader('Content-Length', '86500')
+      break
+    }
+    case '/file-no-headers':
+      break
+    default:
+      res.writeHead(404).end('Unhandled request')
+  }
+
+  res.end()
+}
+
+export default function startMockServer (host, port) {
+  const server = http.createServer(requestListener)
+  server.listen(port, host, () => {
+    console.log(`Server is running on http://${host}:${port}`)
+  })
+}
+
+// startMockServer('localhost', 4678)

packages/@uppy/companion/package.json

@@ -34,6 +34,7 @@
     "chalk": "4.1.2",
     "common-tags": "1.8.2",
     "connect-redis": "6.1.3",
+    "content-disposition": "^0.5.4",
     "cookie-parser": "1.4.6",
     "cors": "^2.8.5",
     "escape-goat": "3.0.0",

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

@@ -5,6 +5,9 @@ const { URL } = require('node:url')
 const dns = require('node:dns')
 const ipaddr = require('ipaddr.js')
 const got = require('got').default
+const path = require('node:path')
+const { randomUUID } = require('node:crypto')
+const contentDisposition = require('content-disposition')
 
 const logger = require('../logger')
 
@@ -119,7 +122,7 @@ module.exports.getProtectedGot = getProtectedGot
  *
  * @param {string} url
  * @param {boolean} blockLocalIPs
- * @returns {Promise<{type: string, size: number}>}
+ * @returns {Promise<{name: string, type: string, size: number}>}
  */
 exports.getURLMeta = async (url, blockLocalIPs = false) => {
   async function requestWithMethod (method) {
@@ -131,11 +134,18 @@ exports.getURLMeta = async (url, blockLocalIPs = false) => {
         .on('response', (response) => {
           // Can be undefined for unknown length URLs, e.g. transfer-encoding: chunked
           const contentLength = parseInt(response.headers['content-length'], 10)
+          // If Content-Disposition with file name is missing, fallback to the URL path for the name,
+          // but if multiple files are served via query params like foo.com?file=file-1, foo.com?file=file-2,
+          // we add random string to avoid duplicate files
+          const filename = response.headers['content-disposition']
+            ? contentDisposition.parse(response.headers['content-disposition']).parameters.filename
+            : `${path.basename(response.request.requestUrl)}-${randomUUID()}`
 
           // No need to get the rest of the response, as we only want header (not really relevant for HEAD, but why not)
           stream.destroy()
 
           resolve({
+            name: filename,
             type: response.headers['content-type'],
             size: Number.isNaN(contentLength) ? null : contentLength,
             statusCode: response.statusCode,
@@ -167,6 +177,6 @@ exports.getURLMeta = async (url, blockLocalIPs = false) => {
     throw new Error(`URL server responded with status: ${urlMeta.statusCode}`)
   }
 
-  const { size, type } = urlMeta
-  return { size, type }
+  const { name, size, type } = urlMeta
+  return { name, size, type }
 }

packages/@uppy/url/src/Url.jsx

@@ -116,7 +116,7 @@ export default class Url extends UIPlugin {
       const tagFile = {
         meta: optionalMeta,
         source: this.id,
-        name: getFileNameFromUrl(url),
+        name: meta.name || getFileNameFromUrl(url),
         type: meta.type,
         data: {
           size: meta.size,

yarn.lock

@@ -9315,6 +9315,7 @@ __metadata:
     chalk: 4.1.2
     common-tags: 1.8.2
     connect-redis: 6.1.3
+    content-disposition: ^0.5.4
     cookie-parser: 1.4.6
     cors: ^2.8.5
     escape-goat: 3.0.0
@@ -13762,7 +13763,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"content-disposition@npm:0.5.4":
+"content-disposition@npm:0.5.4, content-disposition@npm:^0.5.4":
   version: 0.5.4
   resolution: "content-disposition@npm:0.5.4"
   dependencies: