Skip to content

Commit 648ea51

Browse files
committed
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: 100) 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)
1 parent 8bdc9fe commit 648ea51

3 files changed

Lines changed: 65 additions & 15 deletions

File tree

ghost/core/core/server/lib/image/image-size.js

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ class ImageSize {
7373
// use probe-image-size to download enough of an image to get it's dimensions
7474
// returns promise which resolves dimensions
7575
_probeImageSizeFromUrl(imageUrl) {
76-
// probe-image-size uses `request` npm module which doesn't have our `got`
77-
// override with custom URL validation so it needs duplicating here
76+
// Fast-fail invalid URLs; the underlying got instance also enforces
77+
// URL validation + SSRF protection in its beforeRequest hooks.
7878
if (_.isEmpty(imageUrl) || !this.validator.isURL(imageUrl)) {
7979
return Promise.reject(new errors.InternalServerError({
8080
message: 'URL empty or invalid.',
@@ -83,18 +83,25 @@ class ImageSize {
8383
}));
8484
}
8585

86-
// wrap probe-image-size in a promise in case it is unresponsive/the timeout itself doesn't work
87-
return (Promise.race([
88-
this.probe(imageUrl, this.NEEDLE_OPTIONS),
89-
new Promise((res, rej) => {
90-
setTimeout(() => {
91-
rej(new errors.InternalServerError({
92-
message: 'Probe unresponsive.',
93-
code: 'IMAGE_SIZE_URL'
94-
}));
95-
}, this.NEEDLE_OPTIONS.response_timeout);
96-
})
97-
]));
86+
// wrap probe-image-size in a promise in case it is unresponsive/the timeout itself doesn't work.
87+
// If the outer timeout wins, destroy the underlying stream (when available) so the pooled
88+
// keep-alive socket isn't leaked.
89+
const probeResult = this.probe(imageUrl, this.NEEDLE_OPTIONS);
90+
let timeoutHandle;
91+
const timeoutPromise = new Promise((res, rej) => {
92+
timeoutHandle = setTimeout(() => {
93+
if (probeResult && probeResult.stream && typeof probeResult.stream.destroy === 'function') {
94+
probeResult.stream.destroy();
95+
}
96+
rej(new errors.InternalServerError({
97+
message: 'Probe unresponsive.',
98+
code: 'IMAGE_SIZE_URL'
99+
}));
100+
}, this.NEEDLE_OPTIONS.response_timeout);
101+
});
102+
return Promise.race([probeResult, timeoutPromise]).finally(() => {
103+
clearTimeout(timeoutHandle);
104+
});
98105
}
99106

100107
// download full image then use image-size to get it's dimensions

ghost/core/core/server/lib/image/image-utils.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,27 @@ const BlogIcon = require('./blog-icon');
22
const CachedImageSizeFromUrl = require('./cached-image-size-from-url');
33
const Gravatar = require('./gravatar');
44
const ImageSize = require('./image-size');
5-
const probe = require('probe-image-size');
5+
const probeImageSize = require('probe-image-size');
6+
const externalRequest = require('../request-external');
7+
8+
// Probe image dimensions over the shared keep-alive `got` instance instead of
9+
// probe-image-size's built-in needle client. This reuses sockets across the
10+
// parallel cover/author/og/twitter/logo fetches in meta/image-dimensions.js
11+
// and routes them through the same SSRF protections as other external requests.
12+
// The returned promise exposes the underlying stream via `.stream` so callers
13+
// can destroy it on early abort — otherwise a pooled keep-alive socket leaks.
14+
function probe(url, options = {}) {
15+
const stream = externalRequest.stream(url, {
16+
headers: options.headers,
17+
timeout: {
18+
request: options.response_timeout || 10000
19+
},
20+
retry: {limit: 0}
21+
});
22+
const promise = probeImageSize(stream);
23+
promise.stream = stream;
24+
return promise;
25+
}
626

727
class ImageUtils {
828
constructor({config, urlUtils, settingsCache, storageUtils, storage, validator, request, cacheStore}) {

ghost/core/core/server/lib/request-external.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,30 @@
66
const got = /** @type {Got} */ (/** @type {unknown} */ (require('got').default));
77
const dns = require('dns');
88
const net = require('net');
9+
const http = require('http');
10+
const https = require('https');
911
const dnsPromises = require('dns').promises;
1012
const errors = require('@tryghost/errors');
1113
const config = require('../../shared/config');
1214
const validator = require('@tryghost/validator');
1315

16+
// Shared keep-alive agents so outbound HTTPS connections are pooled and reused
17+
// across page renders / oEmbed / webmention / recommendations / image probes.
18+
// Without this, each request opens a fresh socket which on a NAT-gatewayed VPC
19+
// holds the gateway at its connection-rate ceiling and causes port-collision drops.
20+
const httpAgent = new http.Agent({
21+
keepAlive: true,
22+
keepAliveMsecs: 60000,
23+
maxSockets: 256,
24+
maxFreeSockets: 256
25+
});
26+
const httpsAgent = new https.Agent({
27+
keepAlive: true,
28+
keepAliveMsecs: 60000,
29+
maxSockets: 256,
30+
maxFreeSockets: 256
31+
});
32+
1433
/**
1534
* Normalize an IPv4 address from any format (decimal, octal, hex, integer)
1635
* to standard dotted-decimal notation using the WHATWG URL parser.
@@ -280,6 +299,10 @@ const gotOpts = {
280299
timeout: {
281300
request: 10000
282301
}, // default is no timeout
302+
agent: {
303+
http: httpAgent,
304+
https: httpsAgent
305+
},
283306
hooks: {
284307
init: process.env.NODE_ENV?.startsWith('test') ? [disableRetries] : [],
285308
beforeRequest: [errorIfInvalidUrl, errorIfHostnameResolvesToPrivateIp, installSafeDnsLookup],

0 commit comments

Comments
 (0)