Merge pull request #16728 from overleaf/bg-clsi-timeout-fetchutils-connect-timeout

add connect timeout to fetch-utils using custom agents

GitOrigin-RevId: 877dbc1311197461cf1e9bfee74196bd5d3e42dc

libraries/fetch-utils/index.js

@@ -2,6 +2,8 @@ const _ = require('lodash')
 const { Readable } = require('stream')
 const OError = require('@overleaf/o-error')
 const fetch = require('node-fetch')
+const http = require('http')
+const https = require('https')
 
 /**
  * Make a request and return the parsed JSON response.
@@ -245,6 +247,42 @@ async function maybeGetResponseBody(response) {
   }
 }
 
+// Define custom http and https agents with support for connect timeouts
+
+class ConnectTimeoutError extends OError {
+  constructor(options) {
+    super('connect timeout', options)
+  }
+}
+
+function withTimeout(createConnection, options, callback) {
+  if (options.connectTimeout) {
+    // Wrap createConnection in a timeout
+    const timer = setTimeout(() => {
+      socket.destroy(new ConnectTimeoutError(options))
+    }, options.connectTimeout)
+    const socket = createConnection(options, (err, stream) => {
+      clearTimeout(timer)
+      callback(err, stream)
+    })
+    return socket
+  } else {
+    // Fallback to default createConnection
+    return createConnection(options, callback)
+  }
+}
+
+class CustomHttpAgent extends http.Agent {
+  createConnection(options, callback) {
+    return withTimeout(super.createConnection.bind(this), options, callback)
+  }
+}
+class CustomHttpsAgent extends https.Agent {
+  createConnection(options, callback) {
+    return withTimeout(super.createConnection.bind(this), options, callback)
+  }
+}
+
 module.exports = {
   fetchJson,
   fetchJsonWithResponse,
@@ -255,4 +293,7 @@ module.exports = {
   fetchString,
   fetchStringWithResponse,
   RequestFailedError,
+  ConnectTimeoutError,
+  CustomHttpAgent,
+  CustomHttpsAgent,
 }

libraries/fetch-utils/package.json

@@ -26,6 +26,7 @@
   "dependencies": {
     "@overleaf/o-error": "*",
     "lodash": "^4.17.21",
-    "node-fetch": "^2.6.11"
+    "node-fetch": "^2.6.11",
+    "selfsigned": "^2.4.1"
   }
 }

libraries/fetch-utils/test/unit/FetchUtilsTests.js

@@ -3,6 +3,7 @@ const { FetchError, AbortError } = require('node-fetch')
 const { Readable } = require('stream')
 const { once } = require('events')
 const { TestServer } = require('./helpers/TestServer')
+const selfsigned = require('selfsigned')
 const {
   fetchJson,
   fetchStream,
@@ -10,15 +11,41 @@ const {
   fetchRedirect,
   fetchString,
   RequestFailedError,
+  CustomHttpAgent,
+  CustomHttpsAgent,
 } = require('../..')
 
-const PORT = 30001
+const HTTP_PORT = 30001
+const HTTPS_PORT = 30002
+
+const attrs = [{ name: 'commonName', value: 'example.com' }]
+const pems = selfsigned.generate(attrs, { days: 365 })
+
+const PRIVATE_KEY = pems.private
+const PUBLIC_CERT = pems.cert
+
+const dns = require('dns')
+const _originalLookup = dns.lookup
+// Custom DNS resolver function
+dns.lookup = (hostname, options, callback) => {
+  if (hostname === 'example.com') {
+    // If the hostname is our test case, return the ip address for the test server
+    callback(null, '127.0.0.1', 4)
+  } else {
+    // Otherwise, use the default lookup
+    _originalLookup(hostname, options, callback)
+  }
+}
 
 describe('fetch-utils', function () {
   before(async function () {
     this.server = new TestServer()
-    await this.server.start(PORT)
-    this.url = path => `http://127.0.0.1:${PORT}${path}`
+    await this.server.start(HTTP_PORT, HTTPS_PORT, {
+      key: PRIVATE_KEY,
+      cert: PUBLIC_CERT,
+    })
+    this.url = path => `http://example.com:${HTTP_PORT}${path}`
+    this.httpsUrl = path => `https://example.com:${HTTPS_PORT}${path}`
   })
 
   after(async function () {
@@ -236,6 +263,55 @@ describe('fetch-utils', function () {
       await expectRequestAborted(this.server.lastReq)
     })
   })
+
+  describe('CustomHttpAgent', function () {
+    it('makes an http request successfully', async function () {
+      const agent = new CustomHttpAgent({ connectTimeout: 100 })
+      const body = await fetchString(this.url('/hello'), { agent })
+      expect(body).to.equal('hello')
+    })
+
+    it('times out when accessing a non-routable address', async function () {
+      const agent = new CustomHttpAgent({ connectTimeout: 10 })
+      await expect(fetchString('http://10.255.255.255/', { agent }))
+        .to.be.rejectedWith(FetchError)
+        .and.eventually.have.property('message')
+        .and.to.equal(
+          'request to http://10.255.255.255/ failed, reason: connect timeout'
+        )
+    })
+  })
+
+  describe('CustomHttpsAgent', function () {
+    it('makes an https request successfully', async function () {
+      const agent = new CustomHttpsAgent({
+        connectTimeout: 100,
+        ca: PUBLIC_CERT,
+      })
+      const body = await fetchString(this.httpsUrl('/hello'), { agent })
+      expect(body).to.equal('hello')
+    })
+
+    it('rejects an untrusted server', async function () {
+      const agent = new CustomHttpsAgent({
+        connectTimeout: 100,
+      })
+      await expect(fetchString(this.httpsUrl('/hello'), { agent }))
+        .to.be.rejectedWith(FetchError)
+        .and.eventually.have.property('code')
+        .and.to.equal('DEPTH_ZERO_SELF_SIGNED_CERT')
+    })
+
+    it('times out when accessing a non-routable address', async function () {
+      const agent = new CustomHttpsAgent({ connectTimeout: 10 })
+      await expect(fetchString('https://10.255.255.255/', { agent }))
+        .to.be.rejectedWith(FetchError)
+        .and.eventually.have.property('message')
+        .and.to.equal(
+          'request to https://10.255.255.255/ failed, reason: connect timeout'
+        )
+    })
+  })
 })
 
 async function streamToString(stream) {

libraries/fetch-utils/test/unit/helpers/TestServer.js

@@ -1,8 +1,11 @@
 const express = require('express')
 const bodyParser = require('body-parser')
+const http = require('http')
+const https = require('https')
+const { promisify } = require('util')
 
 class TestServer {
-  constructor(port) {
+  constructor() {
     this.app = express()
 
     this.app.use(bodyParser.json())
@@ -87,9 +90,9 @@ class TestServer {
     })
   }
 
-  start(port) {
-    return new Promise((resolve, reject) => {
-      this.server = this.app.listen(port, err => {
+  start(port, httpsPort, httpsOptions) {
+    const startHttp = new Promise((resolve, reject) => {
+      this.server = http.createServer(this.app).listen(port, err => {
         if (err) {
           reject(err)
         } else {
@@ -97,18 +100,24 @@ class TestServer {
         }
       })
     })
+    const startHttps = new Promise((resolve, reject) => {
+      this.https_server = https
+        .createServer(httpsOptions, this.app)
+        .listen(httpsPort, err => {
+          if (err) {
+            reject(err)
+          } else {
+            resolve()
+          }
+        })
+    })
+    return Promise.all([startHttp, startHttps])
   }
 
   stop() {
-    return new Promise((resolve, reject) => {
-      this.server.close(err => {
-        if (err) {
-          reject(err)
-        } else {
-          resolve()
-        }
-      })
-    })
+    const stopHttp = promisify(this.server.close).bind(this.server)
+    const stopHttps = promisify(this.https_server.close).bind(this.https_server)
+    return Promise.all([stopHttp(), stopHttps()])
   }
 }