Browse Source

@uppy/companion: invert some internal boolean options (#5198)

blockLocalUrls
see https://github.com/transloadit/uppy/pull/4554/files#r1268677162
Mikael Finstad 11 months ago
parent
commit
1b1e5c7809

+ 7 - 0
docs/guides/migration-guides.md

@@ -25,6 +25,13 @@ These cover all the major Uppy versions and how to migrate to them.
 - `access-control-allow-headers` is no longer included in
   `Access-Control-Expose-Headers`, and `uppy-versions` is no longer an allowed
   header. We are not aware of any issues this might cause.
+- Internal refactoring (probably won’t affect you)
+  - `getProtectedGot` parameter `blockLocalIPs` changed to `allowLocalIPs`
+    (inverted boolean).
+  - `getURLMeta` 2nd (boolean) argument inverted.
+  - `getProtectedHttpAgent` parameter `blockLocalIPs` changed to `allowLocalIPs`
+    (inverted boolean).
+  - `downloadURL` 2nd (boolean) argument inverted.
 
 ### `@uppy/companion-client`
 

+ 6 - 8
packages/@uppy/companion/src/server/controllers/url.js

@@ -17,15 +17,13 @@ const logger = require('../logger')
  * to the callback chunk by chunk.
  *
  * @param {string} url
- * @param {boolean} blockLocalIPs
+ * @param {boolean} allowLocalIPs
  * @param {string} traceId
  * @returns {Promise}
  */
-const downloadURL = async (url, blockLocalIPs, traceId) => {
-  // TODO in next major, rename all blockLocalIPs to allowLocalUrls and invert the bool, to make it consistent
-  // see discussion https://github.com/transloadit/uppy/pull/4554/files#r1268677162
+const downloadURL = async (url, allowLocalIPs, traceId) => {
   try {
-    const protectedGot = await getProtectedGot({ blockLocalIPs })
+    const protectedGot = await getProtectedGot({ allowLocalIPs })
     const stream = protectedGot.stream.get(url, { responseType: 'json' })
     await prepareStream(stream)
     return stream
@@ -50,7 +48,7 @@ const meta = async (req, res) => {
       return res.status(400).json({ error: 'Invalid request body' })
     }
 
-    const urlMeta = await getURLMeta(req.body.url, !allowLocalUrls)
+    const urlMeta = await getURLMeta(req.body.url, allowLocalUrls)
     return res.json(urlMeta)
   } catch (err) {
     logger.error(err, 'controller.url.meta.error', req.id)
@@ -75,12 +73,12 @@ const get = async (req, res) => {
   }
 
   async function getSize () {
-    const { size } = await getURLMeta(req.body.url, !allowLocalUrls)
+    const { size } = await getURLMeta(req.body.url, allowLocalUrls)
     return size
   }
 
   async function download () {
-    return downloadURL(req.body.url, !allowLocalUrls, req.id)
+    return downloadURL(req.body.url, allowLocalUrls, req.id)
   }
 
   try {

+ 9 - 9
packages/@uppy/companion/src/server/helpers/request.js

@@ -46,7 +46,7 @@ module.exports.validateURL = validateURL
 /**
  * Returns http Agent that will prevent requests to private IPs (to prevent SSRF)
  */
-const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => {
+const getProtectedHttpAgent = ({ protocol, allowLocalIPs }) => {
   function dnsLookup (hostname, options, callback) {
     dns.lookup(hostname, options, (err, addresses, maybeFamily) => {
       if (err) {
@@ -58,7 +58,7 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => {
       // because dns.lookup seems to be called with option `all: true`, if we are on an ipv6 system,
       // `addresses` could contain a list of ipv4 addresses as well as ipv6 mapped addresses (rfc6052) which we cannot allow
       // however we should still allow any valid ipv4 addresses, so we filter out the invalid addresses
-      const validAddresses = !blockLocalIPs ? toValidate : toValidate.filter(({ address }) => !isDisallowedIP(address))
+      const validAddresses = allowLocalIPs ? toValidate : toValidate.filter(({ address }) => !isDisallowedIP(address))
 
       // and check if there's anything left after we filtered:
       if (validAddresses.length === 0) {
@@ -73,7 +73,7 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => {
 
   return class HttpAgent extends (protocol.startsWith('https') ? https : http).Agent {
     createConnection (options, callback) {
-      if (ipaddr.isValid(options.host) && blockLocalIPs && isDisallowedIP(options.host)) {
+      if (ipaddr.isValid(options.host) && !allowLocalIPs && isDisallowedIP(options.host)) {
         callback(new Error(FORBIDDEN_IP_ADDRESS))
         return undefined
       }
@@ -85,9 +85,9 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => {
 
 module.exports.getProtectedHttpAgent = getProtectedHttpAgent
 
-async function getProtectedGot ({ blockLocalIPs }) {
-  const HttpAgent = getProtectedHttpAgent({ protocol: 'http', blockLocalIPs })
-  const HttpsAgent = getProtectedHttpAgent({ protocol: 'https', blockLocalIPs })
+async function getProtectedGot ({ allowLocalIPs }) {
+  const HttpAgent = getProtectedHttpAgent({ protocol: 'http', allowLocalIPs })
+  const HttpsAgent = getProtectedHttpAgent({ protocol: 'https', allowLocalIPs })
   const httpAgent = new HttpAgent()
   const httpsAgent = new HttpsAgent()
 
@@ -102,12 +102,12 @@ module.exports.getProtectedGot = getProtectedGot
  * Gets the size and content type of a url's content
  *
  * @param {string} url
- * @param {boolean} blockLocalIPs
+ * @param {boolean} allowLocalIPs
  * @returns {Promise<{name: string, type: string, size: number}>}
  */
-exports.getURLMeta = async (url, blockLocalIPs = false) => {
+exports.getURLMeta = async (url, allowLocalIPs = false) => {
   async function requestWithMethod (method) {
-    const protectedGot = await getProtectedGot({ blockLocalIPs })
+    const protectedGot = await getProtectedGot({ allowLocalIPs })
     const stream = protectedGot.stream(url, { method, throwHttpErrors: false })
 
     return new Promise((resolve, reject) => (

+ 1 - 1
packages/@uppy/companion/src/server/provider/facebook/index.js

@@ -69,7 +69,7 @@ class Facebook extends Provider {
   async size ({ id, token }) {
     return this.#withErrorHandling('provider.facebook.size.error', async () => {
       const url = await getMediaUrl({ token, id })
-      const { size } = await getURLMeta(url, true)
+      const { size } = await getURLMeta(url)
       return size
     })
   }

+ 1 - 1
packages/@uppy/companion/src/server/provider/instagram/graph/index.js

@@ -70,7 +70,7 @@ class Instagram extends Provider {
   async size ({ id, token }) {
     return this.#withErrorHandling('provider.instagram.size.error', async () => {
       const url = await getMediaUrl({ token, id })
-      const { size } = await getURLMeta(url, true)
+      const { size } = await getURLMeta(url)
       return size
     })
   }

+ 1 - 1
packages/@uppy/companion/src/server/provider/unsplash/index.js

@@ -58,7 +58,7 @@ class Unsplash extends Provider {
   async size ({ id, token }) {
     return this.#withErrorHandling('provider.unsplash.size.error', async () => {
       const { links: { download: url } } = await getPhotoMeta(await getClient({ token }), id)
-      const { size } = await getURLMeta(url, true)
+      const { size } = await getURLMeta(url)
       return size
     })
   }

+ 6 - 6
packages/@uppy/companion/test/__tests__/http-agent.js

@@ -11,24 +11,24 @@ describe('test protected request Agent', () => {
   test('allows URLs without IP addresses', async () => {
     nock('https://transloadit.com').get('/').reply(200)
     const url = 'https://transloadit.com'
-    return (await getProtectedGot({ blockLocalIPs: true })).get(url)
+    return (await getProtectedGot({ allowLocalIPs: false })).get(url)
   })
 
   test('blocks url that resolves to forbidden IP', async () => {
     const url = 'https://localhost'
-    const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url))
+    const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url))
     await expect(promise).rejects.toThrow(/^Forbidden resolved IP address/)
   })
 
   test('blocks private http IP address', async () => {
     const url = 'http://172.20.10.4:8090'
-    const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url))
+    const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url))
     await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS))
   })
 
   test('blocks private https IP address', async () => {
     const url = 'https://172.20.10.4:8090'
-    const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url))
+    const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url))
     await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS))
   })
 
@@ -57,12 +57,12 @@ describe('test protected request Agent', () => {
 
     for (const ip of ipv4s) {
       const url = `http://${ip}:8090`
-      const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url))
+      const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url))
       await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS))
     }
     for (const ip of ipv6s) {
       const url = `http://[${ip}]:8090`
-      const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url))
+      const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url))
       await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS))
     }
   })