From 2b6ec9237299f4edea369587a6ec9205790e6ba7 Mon Sep 17 00:00:00 2001 From: Ricardo Pinto Date: Thu, 28 May 2026 11:00:14 +0100 Subject: [PATCH] Added shared keep-alive agent to external HTTP requests ref https://linear.app/ghost/issue/BER-3690 - got.extend in request-external.js had no agent configured, so every oEmbed / webmention / recommendation / image probe opened a fresh socket - on a NAT-gatewayed VPC this holds the gateway at its connection-rate ceiling and causes port-collision drops that gateway sizing won't fix - adds a shared http/https Agent (keepAlive: true, keepAliveMsecs: 60s, maxSockets: 256) wired into the central got instance - routes probe-image-size through externalRequest.stream so the parallel cover/author/og/twitter/logo fetches from meta/image-dimensions.js reuse the same pool (and pick up the existing SSRF protections) --- .../core/core/server/lib/image/image-size.js | 35 ++++---- .../core/core/server/lib/image/image-utils.js | 22 ++++- .../core/core/server/lib/request-external.js | 23 ++++++ .../unit/server/lib/image/image-utils.test.js | 81 +++++++++++++++++++ 4 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 ghost/core/test/unit/server/lib/image/image-utils.test.js diff --git a/ghost/core/core/server/lib/image/image-size.js b/ghost/core/core/server/lib/image/image-size.js index fecd52668ce..153982b4396 100644 --- a/ghost/core/core/server/lib/image/image-size.js +++ b/ghost/core/core/server/lib/image/image-size.js @@ -73,8 +73,8 @@ class ImageSize { // use probe-image-size to download enough of an image to get it's dimensions // returns promise which resolves dimensions _probeImageSizeFromUrl(imageUrl) { - // probe-image-size uses `request` npm module which doesn't have our `got` - // override with custom URL validation so it needs duplicating here + // Fast-fail invalid URLs; the underlying got instance also enforces + // URL validation + SSRF protection in its beforeRequest hooks. if (_.isEmpty(imageUrl) || !this.validator.isURL(imageUrl)) { return Promise.reject(new errors.InternalServerError({ message: 'URL empty or invalid.', @@ -83,18 +83,25 @@ class ImageSize { })); } - // wrap probe-image-size in a promise in case it is unresponsive/the timeout itself doesn't work - return (Promise.race([ - this.probe(imageUrl, this.NEEDLE_OPTIONS), - new Promise((res, rej) => { - setTimeout(() => { - rej(new errors.InternalServerError({ - message: 'Probe unresponsive.', - code: 'IMAGE_SIZE_URL' - })); - }, this.NEEDLE_OPTIONS.response_timeout); - }) - ])); + // wrap probe-image-size in a promise in case it is unresponsive/the timeout itself doesn't work. + // If the outer timeout wins, destroy the underlying stream (when available) so the pooled + // keep-alive socket isn't leaked. + const probeResult = this.probe(imageUrl, this.NEEDLE_OPTIONS); + let timeoutHandle; + const timeoutPromise = new Promise((res, rej) => { + timeoutHandle = setTimeout(() => { + if (probeResult && probeResult.stream && typeof probeResult.stream.destroy === 'function') { + probeResult.stream.destroy(); + } + rej(new errors.InternalServerError({ + message: 'Probe unresponsive.', + code: 'IMAGE_SIZE_URL' + })); + }, this.NEEDLE_OPTIONS.response_timeout); + }); + return Promise.race([probeResult, timeoutPromise]).finally(() => { + clearTimeout(timeoutHandle); + }); } // download full image then use image-size to get it's dimensions diff --git a/ghost/core/core/server/lib/image/image-utils.js b/ghost/core/core/server/lib/image/image-utils.js index 0cc1c6fc5ee..35b3f0fa71f 100644 --- a/ghost/core/core/server/lib/image/image-utils.js +++ b/ghost/core/core/server/lib/image/image-utils.js @@ -2,7 +2,27 @@ const BlogIcon = require('./blog-icon'); const CachedImageSizeFromUrl = require('./cached-image-size-from-url'); const Gravatar = require('./gravatar'); const ImageSize = require('./image-size'); -const probe = require('probe-image-size'); +const probeImageSize = require('probe-image-size'); +const externalRequest = require('../request-external'); + +// Probe image dimensions over the shared keep-alive `got` instance instead of +// probe-image-size's built-in needle client. This reuses sockets across the +// parallel cover/author/og/twitter/logo fetches in meta/image-dimensions.js +// and routes them through the same SSRF protections as other external requests. +// The returned promise exposes the underlying stream via `.stream` so callers +// can destroy it on early abort — otherwise a pooled keep-alive socket leaks. +function probe(url, options = {}) { + const stream = externalRequest.stream(url, { + headers: options.headers, + timeout: { + request: options.response_timeout || 10000 + }, + retry: {limit: 0} + }); + const promise = probeImageSize(stream); + promise.stream = stream; + return promise; +} class ImageUtils { constructor({config, urlUtils, settingsCache, storageUtils, storage, validator, request, cacheStore}) { diff --git a/ghost/core/core/server/lib/request-external.js b/ghost/core/core/server/lib/request-external.js index f959cd9c05a..82a5acd41a8 100644 --- a/ghost/core/core/server/lib/request-external.js +++ b/ghost/core/core/server/lib/request-external.js @@ -6,11 +6,30 @@ const got = /** @type {Got} */ (/** @type {unknown} */ (require('got').default)); const dns = require('dns'); const net = require('net'); +const http = require('http'); +const https = require('https'); const dnsPromises = require('dns').promises; const errors = require('@tryghost/errors'); const config = require('../../shared/config'); const validator = require('@tryghost/validator'); +// Shared keep-alive agents so outbound HTTPS connections are pooled and reused +// across page renders / oEmbed / webmention / recommendations / image probes. +// Without this, each request opens a fresh socket which on a NAT-gatewayed VPC +// holds the gateway at its connection-rate ceiling and causes port-collision drops. +const httpAgent = new http.Agent({ + keepAlive: true, + keepAliveMsecs: 60000, + maxSockets: 256, + maxFreeSockets: 256 +}); +const httpsAgent = new https.Agent({ + keepAlive: true, + keepAliveMsecs: 60000, + maxSockets: 256, + maxFreeSockets: 256 +}); + /** * Normalize an IPv4 address from any format (decimal, octal, hex, integer) * to standard dotted-decimal notation using the WHATWG URL parser. @@ -280,6 +299,10 @@ const gotOpts = { timeout: { request: 10000 }, // default is no timeout + agent: { + http: httpAgent, + https: httpsAgent + }, hooks: { init: process.env.NODE_ENV?.startsWith('test') ? [disableRetries] : [], beforeRequest: [errorIfInvalidUrl, errorIfHostnameResolvesToPrivateIp, installSafeDnsLookup], diff --git a/ghost/core/test/unit/server/lib/image/image-utils.test.js b/ghost/core/test/unit/server/lib/image/image-utils.test.js new file mode 100644 index 00000000000..a179a5255ea --- /dev/null +++ b/ghost/core/test/unit/server/lib/image/image-utils.test.js @@ -0,0 +1,81 @@ +const assert = require('node:assert/strict'); +const {PassThrough} = require('node:stream'); +const sinon = require('sinon'); +const rewire = require('rewire'); + +const MODULE_PATH = '../../../../../core/server/lib/image/image-utils'; + +describe('image-utils probe wrapper', function () { + let stream; + let externalRequestStub; + let probeImageSizeStub; + let probe; + let imageUtilsModule; + let revert; + + beforeEach(function () { + stream = new PassThrough(); + sinon.spy(stream, 'destroy'); + externalRequestStub = { + stream: sinon.stub().returns(stream) + }; + probeImageSizeStub = sinon.stub().resolves({width: 10, height: 20}); + + imageUtilsModule = rewire(MODULE_PATH); + revert = imageUtilsModule.__set__({ + externalRequest: externalRequestStub, + probeImageSize: probeImageSizeStub + }); + probe = imageUtilsModule.__get__('probe'); + }); + + afterEach(function () { + revert(); + sinon.restore(); + }); + + it('routes the request through externalRequest.stream (SSRF-protected got instance)', async function () { + await probe('https://example.com/cat.jpg', {}); + sinon.assert.calledOnce(externalRequestStub.stream); + const [calledUrl] = externalRequestStub.stream.firstCall.args; + assert.equal(calledUrl, 'https://example.com/cat.jpg'); + }); + + it('forwards headers and maps response_timeout → timeout.request', async function () { + await probe('https://example.com/cat.jpg', { + headers: {'User-Agent': 'Mozilla/5.0 Safari/537.36'}, + response_timeout: 1234 + }); + const [, opts] = externalRequestStub.stream.firstCall.args; + assert.deepEqual(opts.headers, {'User-Agent': 'Mozilla/5.0 Safari/537.36'}); + assert.equal(opts.timeout.request, 1234); + assert.equal(opts.retry.limit, 0); + }); + + it('defaults timeout.request to 10000ms when response_timeout is omitted', async function () { + await probe('https://example.com/cat.jpg', {}); + const [, opts] = externalRequestStub.stream.firstCall.args; + assert.equal(opts.timeout.request, 10000); + }); + + it('passes the got stream to probe-image-size', async function () { + await probe('https://example.com/cat.jpg', {}); + sinon.assert.calledOnceWithExactly(probeImageSizeStub, stream); + }); + + it('exposes the underlying stream on the returned promise so callers can destroy it on abort', async function () { + const result = probe('https://example.com/cat.jpg', {}); + assert.equal(result.stream, stream); + assert.equal(typeof result.stream.destroy, 'function'); + + result.stream.destroy(); + sinon.assert.calledOnce(stream.destroy); + + await result; + }); + + it('resolves with the dimensions returned by probe-image-size', async function () { + const result = await probe('https://example.com/cat.jpg', {}); + assert.deepEqual(result, {width: 10, height: 20}); + }); +});