From 6680bedf6c2ae664e17b9038a8a1e2ed40a2b28e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 15 May 2024 10:17:40 +0100 Subject: [PATCH] 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 | 41 ++++++++++ libraries/fetch-utils/package.json | 3 +- .../fetch-utils/test/unit/FetchUtilsTests.js | 82 ++++++++++++++++++- .../test/unit/helpers/TestServer.js | 35 +++++--- package-lock.json | 36 ++++++-- 5 files changed, 171 insertions(+), 26 deletions(-) diff --git a/libraries/fetch-utils/index.js b/libraries/fetch-utils/index.js index f164dfaa9e..ba1c9663ae 100644 --- a/libraries/fetch-utils/index.js +++ b/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, } diff --git a/libraries/fetch-utils/package.json b/libraries/fetch-utils/package.json index 59e6328647..5120380118 100644 --- a/libraries/fetch-utils/package.json +++ b/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" } } diff --git a/libraries/fetch-utils/test/unit/FetchUtilsTests.js b/libraries/fetch-utils/test/unit/FetchUtilsTests.js index 280d7055b2..0c04afed19 100644 --- a/libraries/fetch-utils/test/unit/FetchUtilsTests.js +++ b/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) { diff --git a/libraries/fetch-utils/test/unit/helpers/TestServer.js b/libraries/fetch-utils/test/unit/helpers/TestServer.js index 0fb69d5454..57888ea297 100644 --- a/libraries/fetch-utils/test/unit/helpers/TestServer.js +++ b/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()]) } } diff --git a/package-lock.json b/package-lock.json index 3ff920d673..e4e7e6f8df 100644 --- a/package-lock.json +++ b/package-lock.json @@ -113,7 +113,8 @@ "dependencies": { "@overleaf/o-error": "*", "lodash": "^4.17.21", - "node-fetch": "^2.6.11" + "node-fetch": "^2.6.11", + "selfsigned": "^2.4.1" }, "devDependencies": { "body-parser": "^1.20.2", @@ -12784,6 +12785,14 @@ "form-data": "^3.0.0" } }, + "node_modules/@types/node-forge": { + "version": "1.3.11", + "resolved": "https://registry.npmjs.org/@types/node-forge/-/node-forge-1.3.11.tgz", + "integrity": "sha512-FQx220y22OKNTqaByeBGqHWYz4cl94tpcxeFdvBo3wjG6XPBuZ0BNgNZRV5J5TFmmcsJ4IzsLkmGRiQbnYsBEQ==", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/normalize-package-data": { "version": "2.4.4", "resolved": "https://registry.npmjs.org/@types/normalize-package-data/-/normalize-package-data-2.4.4.tgz", @@ -35549,11 +35558,11 @@ "dev": true }, "node_modules/selfsigned": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.1.1.tgz", - "integrity": "sha512-GSL3aowiF7wa/WtSFwnUrludWFoNhftq8bUkH9pkzjpN2XSPOAYEgg6e0sS9s0rZwgJzJiQRPU18A6clnoW5wQ==", - "dev": true, + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.4.1.tgz", + "integrity": "sha512-th5B4L2U+eGLq1TVh7zNRGBapioSORUeymIydxgFpwww9d2qyKvtuPU2jJuHvYAwwqi2Y596QBL3eEqcPEYL8Q==", "dependencies": { + "@types/node-forge": "^1.3.0", "node-forge": "^1" }, "engines": { @@ -51514,6 +51523,7 @@ "lodash": "^4.17.21", "mocha": "^10.2.0", "node-fetch": "^2.6.11", + "selfsigned": "^2.4.1", "typescript": "^5.0.4" }, "dependencies": { @@ -57088,6 +57098,14 @@ "form-data": "^3.0.0" } }, + "@types/node-forge": { + "version": "1.3.11", + "resolved": "https://registry.npmjs.org/@types/node-forge/-/node-forge-1.3.11.tgz", + "integrity": "sha512-FQx220y22OKNTqaByeBGqHWYz4cl94tpcxeFdvBo3wjG6XPBuZ0BNgNZRV5J5TFmmcsJ4IzsLkmGRiQbnYsBEQ==", + "requires": { + "@types/node": "*" + } + }, "@types/normalize-package-data": { "version": "2.4.4", "resolved": "https://registry.npmjs.org/@types/normalize-package-data/-/normalize-package-data-2.4.4.tgz", @@ -75324,11 +75342,11 @@ "dev": true }, "selfsigned": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.1.1.tgz", - "integrity": "sha512-GSL3aowiF7wa/WtSFwnUrludWFoNhftq8bUkH9pkzjpN2XSPOAYEgg6e0sS9s0rZwgJzJiQRPU18A6clnoW5wQ==", - "dev": true, + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.4.1.tgz", + "integrity": "sha512-th5B4L2U+eGLq1TVh7zNRGBapioSORUeymIydxgFpwww9d2qyKvtuPU2jJuHvYAwwqi2Y596QBL3eEqcPEYL8Q==", "requires": { + "@types/node-forge": "^1.3.0", "node-forge": "^1" } },