Remove request and form-data from companion

[mifi]

form-data

As discovered in #3477, https://github.com/form-data/form-data can cause strange bugs and crashes. Although that pr #3478 works around two of these bugs by sanitizing all metadata to strings, form-data seems unmaintained with many open issues/PRs and we should really rewrite our code to use something else.

Although we don't depend on it directly, request does. We use request with form-data in Uploader for multipart XHR uploads.

request

request is deprecated and receives no more security updates or bugfixes. It does not support promises and will probably never support modern features like http2 and other neat stuff. We only use request a few places so it should be fairly straightforward to replace, although:

• we use the streaming features so it needs to be replaced carefully:
  

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

  Line 67 in 7e9f34e

  resp.pause()

• We must also handle with access control in redirects (URL validation) getRedirectEvaluator https://github.com/transloadit/uppy/blob/main/packages/%40uppy/companion/src/server/helpers/request.js
• We must handle DNS lookup IP validation (custom HttpAgent and HttpsAgent https://github.com/transloadit/uppy/blob/main/packages/%40uppy/companion/src/server/helpers/request.js

Alternatives to replace request with:

undici

advantages:

• Node.js has started vendoring it in and plans on exposing its implementation of fetch and FormData as globals in Node.js 18
  drawbacks:
• lower level API, doesn't support progress: Is there any list of 'high-level abstraction' libraries using undici? nodejs/undici#1195

got

advantages:

• popular and actively maintained
• Has a modern, maintained, spec-compliant FormData implementation
  • uses https://github.com/octet-stream/form-data, see Add support for FormData request body sindresorhus/got#1835
  • also supports https://github.com/jimmywarting/FormData
• feature rich, easy to use and provides almost everything you'd need out of the box, which node-fetch does not (retry, exponential back-off, progress, formdata) https://github.com/sindresorhus/got#comparison
• has a node-fetch API wrapper
• node-only api so no browser support awkwardness

drawbacks:

• as of v12, ESM only so either need to rewrite companion to ESM, or use dynamic await import.

node-fetch

advantages:

• popular and actively maintained
• similar to browser api
• maybe same API as node.js upcoming built in fetch api (or maybe not?)

drawbacks:

• depends on the old form-data
• maybe fetch is a bit awkward in node because it was originally designed for browsers?
• not so many features built in, like progress and http2

axios

advantages:

• popular and actively maintained

drawbacks:

• a bit awkward in node because it was originally made for browser. some things are quirky for this reason

purest

Purest is a module that provides configurable REST API for different providers with overrides for unknown providers. It is tightly integrated into companion providers.

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

Line 24 in 7e9f34e

purestConfig.zoom = {

We can either upgrade or replace purest with one of the above alternatives (replacing yields 1 request library instead of 2, so lower size).

Upgrading purest

purest seems to have been rewritten completely recently and removed the use of request in v4.0.0, so we could look into upgrading to that

There seems to be no change log for v4, just Complete rewrite of the module, so we may have to look into the code itself to see what breaking changes we need to consider.

It now depends on these modules for http request and multipart support, which seem to be not so popular:

• https://www.npmjs.com/package/request-multipart
• https://www.npmjs.com/package/request-compose

[aduh95]

I think we should go with undici: Node.js has started vendoring it in and plans on exposing its implementation of fetch and FormData as globals in Node.js 18.

[mifi]

Ooo nice. Never heard of it but it sounds promising. I'll add it.

[Murderlon]

I have a feeling that got has more things baked-in that we would need to make around undici ourselves, but I haven't looked close enough at it how we use requests in companion to know how much self implementation is required.

[mifi]

sindresorhus, creator of got, has voiced out that undici might not be so stable yet: sindresorhus/got#2075 (comment) although for us it might not be a big problem because we would not be using the full surface of their api.

Another option might be to use the fetch api because it seems that undici includes a fetch layer, which one would think is more stable because fetch is quite well established. Or if undici's fetch API is not stable/complete yet maybe we could start out with node-fetch and then later just switch it out

[Murderlon]

I think the best thing to do is make a list of how we use request in Companion, and see how that list does in https://github.com/sindresorhus/got#comparison. If we need some custom or advanced things, I would prefer to go with got.

[mifi]

Features that we need:

• possibility to intercept request after headers are received, and make decisions based on headers
  • possibility to either abort the now unfinished request, or
  • pause the stream, so that once the stream is resumed, the first byte of the body will be the first byte read from the stream
• handle redirects
  • allow custom logic and rejecting requests based on redirected URL (getRedirectEvaluator)
• support FormData with Streams (also unknown length streams) (Uploader.js)
• timeouts
• support custom HttpAgent / HttpsAgent to be able to reject connections to certain IP addresses (after DNS lookup has happened)

[mifi]

I'm not 100% sure but it seems like undici doesn't support custom redirects or hooks nodejs/undici#491

Only got, ky and axios support hooks like this, according to the got table.

• I'm guessing fetch will never support custom redirect hooks because redirects are AFAIK not exposed to the browser JS code due to security issues.
• I don't think we want to use axios in node.js because it's old and was made for the browser
• ky is also made primarily for the browser

so I guess we're going with got then, unless I'm missing something.

[aduh95]

Another option might be to use the fetch api

Note that Undici's fetch is only available for Node.js 16.8+, and we still want to support v14.20+.