Skip to content

Commit 818c9cb

Browse files
committed
Migrate node-fetch to built-in fetch (undici) (#5487)
* Migrate node-fetch to built-in fetch (undici), remove the node-fetch dep * Better changelog details, fix IPv6 test issues (CI) * Fix batch total race
1 parent fc7b23d commit 818c9cb

12 files changed

Lines changed: 274 additions & 185 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"apostrophe": minor
3+
---
4+
5+
The server-side HTTP client (`apos.http`) now uses Node's built-in `fetch` instead of `node-fetch`.
6+
7+
`node-fetch` is no longer maintained, and Node's built-in `fetch` is its standard, actively maintained successor, available in every Node.js version Apostrophe supports - so this is the right time to adopt it. We do not consider this a breaking change: common `apos.http.*` usage is unchanged, and we deliberately preserved compatibility where it mattered - `form-data` request bodies, cookie jars, the `timeout` option (now backed by an `AbortSignal`), and absolute redirect `Location` headers all behave as before.
8+
9+
Most code that calls `apos.http.get()`, `apos.http.post()`, etc. needs no changes. A few things to be aware of if you use advanced options or read raw responses:
10+
11+
- The `agent` option is no longer supported (the built-in `fetch` has no equivalent). Pass an undici `dispatcher` instead; `apos.http` throws if `agent` is given.
12+
- A `Host` request header can no longer be set (it is disallowed by the fetch standard and is silently ignored).
13+
- `originalResponse: true` now resolves with the built-in `fetch` `Response`. Its `body` is a web `ReadableStream` (use `require('node:stream').Readable.fromWeb()` to read it as a Node stream), and node-fetch-only helpers such as `.buffer()` are no longer available.
14+
- Requests that send a conditional header (`If-None-Match` / `If-Modified-Since`) now also send `Cache-Control: no-cache`, as required by the fetch standard. An endpoint that returns `304 Not Modified` based on those headers may return `200` to such a request.
15+
16+
New capabilities:
17+
18+
- The `timeout` option (in milliseconds) and the standard `signal` (`AbortSignal`) and undici `dispatcher` options are supported.
19+
- A request `body` may be a native `FormData`, in addition to a `form-data` package instance.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"oembetter": patch
3+
---
4+
5+
Replaced the `node-fetch` dependency with Node's built-in `fetch`. This is an internal change with no effect on the public API.

.changeset/yummy-buckets-fly.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"apostrophe": patch
3+
---
4+
5+
Batch jobs now reliably record their total item count, so completion notifications no longer occasionally report a null total.

packages/apostrophe/modules/@apostrophecms/http/index.js

Lines changed: 175 additions & 139 deletions
Large diffs are not rendered by default.

packages/apostrophe/modules/@apostrophecms/job/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ module.exports = {
110110
// sends a response with a jobId to the browser
111111
job = await self.start(options);
112112

113-
self.setTotal(job, ids.length);
113+
// Persist the total before work begins so the completed notification
114+
// and job document always report it, even if processing finishes fast.
115+
await self.setTotal(job, ids.length);
114116
// Runs after response is already sent
115117
run();
116118

packages/apostrophe/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
},
2424
"homepage": "https://github.com/apostrophecms/apostrophe/tree/main/packages/apostrophe",
2525
"engines": {
26-
"node": ">=16.0.0"
26+
"node": ">=22.0.0"
2727
},
2828
"keywords": [
2929
"apostrophe",
@@ -102,7 +102,6 @@
102102
"minimatch": "^3.1.4",
103103
"mkdirp": "^0.5.5",
104104
"multer": "^2.1.1",
105-
"node-fetch": "^2.6.1",
106105
"nodemailer": "^9.0.1",
107106
"nunjucks": "^3.2.1",
108107
"oembetter": "workspace:^",

packages/apostrophe/test-lib/test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const fs = require('fs-extra');
22
const path = require('path');
3+
const http = require('node:http');
34

45
const setupPackages = ({ folder = 'test' }) => {
56
const testNodeModules = path.join(__dirname, '../', folder, 'node_modules/');
@@ -57,5 +58,36 @@ const setupPackages = ({ folder = 'test' }) => {
5758
};
5859
setupPackages({ folder: 'test' });
5960

61+
// Performs a GET via the raw node:http client, sending `headers` to the server
62+
// verbatim. Use this in tests that must control headers the built-in fetch
63+
// (used by apos.http) would otherwise refuse or rewrite: e.g. a forbidden
64+
// `Host` header, or the `Cache-Control: no-cache` it adds to any request that
65+
// carries a conditional header (If-None-Match / If-Modified-Since). `url` may
66+
// be absolute or site-relative (resolved against `apos.http.getBase()`).
67+
// Resolves with a fullResponse-shaped { status, headers, body }.
68+
const rawGet = (apos, url, headers = {}) => {
69+
const target = url.startsWith('/') ? `${apos.http.getBase()}${url}` : url;
70+
return new Promise((resolve, reject) => {
71+
// Pass the URL string (rather than a split hostname/port) so an IPv6 base
72+
// such as `http://[::1]:3000` from getBase() is handled correctly; a
73+
// bracketed hostname passed on its own is treated as a name to DNS-resolve.
74+
const req = http.request(target, {
75+
method: 'GET',
76+
headers
77+
}, (res) => {
78+
const chunks = [];
79+
res.on('data', (chunk) => chunks.push(chunk));
80+
res.on('end', () => resolve({
81+
status: res.statusCode,
82+
headers: res.headers,
83+
body: Buffer.concat(chunks).toString()
84+
}));
85+
});
86+
req.on('error', reject);
87+
req.end();
88+
});
89+
};
90+
6091
module.exports = require('./util.js');
6192
module.exports.setupPackages = setupPackages;
93+
module.exports.rawGet = rawGet;

packages/apostrophe/test/files.js

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
const t = require('../test-lib/test.js');
22
const assert = require('assert/strict');
33
const fs = require('fs');
4+
// rawGet (raw node:http) lets this suite send a spoofed `Host` header, which
5+
// the built-in fetch used by apos.http would drop as a forbidden header.
6+
const { rawGet } = t;
47

58
describe('Files', function() {
69

@@ -142,17 +145,13 @@ describe('Files', function() {
142145
const attachment = apos.attachment.first(file);
143146
const url = apos.attachment.url(attachment);
144147
assert(url);
145-
// Send an attacker-controlled Host header (e.g. the cloud metadata
146-
// address from the advisory). The upstream fetch must be resolved
147-
// against the server-trusted baseUrl, not this header, so the
148-
// legitimate content is still served and the request is never
149-
// steered at the spoofed host.
150-
const response = await apos.http.get(url, {
151-
headers: {
152-
Host: '169.254.169.254'
153-
},
154-
fullResponse: true
155-
});
148+
// Spoof the Host header (the cloud-metadata address from the advisory)
149+
// over a raw request: apos.http uses the built-in fetch, which drops a
150+
// forbidden `Host` header and so cannot deliver the spoof. The server
151+
// must resolve the upstream fetch against its trusted baseUrl, not this
152+
// header, so the legitimate content is still served and the request is
153+
// never steered at the spoofed host.
154+
const response = await rawGet(apos, url, { Host: '169.254.169.254' });
156155
assert.strictEqual(response.status, 200);
157156
assert.strictEqual(response.body, attachment.data);
158157
} finally {

packages/apostrophe/test/pages.js

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
const t = require('../test-lib/test.js');
22
const assert = require('assert');
33
const _ = require('lodash');
4+
// The REST API etag tests below issue their conditional request via rawGet
5+
// (raw node:http): the built-in fetch used by apos.http adds Cache-Control:
6+
// no-cache to any request carrying a conditional header (Fetch standard), which
7+
// would suppress the asserted 304s. The page-serving etag tests stay on
8+
// apos.http (those routes set 304 explicitly).
9+
const { rawGet } = t;
410

511
describe('Pages', function() {
612
let apos;
@@ -1087,11 +1093,8 @@ describe('Pages', function() {
10871093
};
10881094

10891095
const response1 = await apos.http.get(`/api/v1/@apostrophecms/page/${homeId}`, { fullResponse: true });
1090-
const response2 = await apos.http.get(`/api/v1/@apostrophecms/page/${homeId}`, {
1091-
fullResponse: true,
1092-
headers: {
1093-
'if-none-match': response1.headers.etag
1094-
}
1096+
const response2 = await rawGet(apos, `/api/v1/@apostrophecms/page/${homeId}`, {
1097+
'if-none-match': response1.headers.etag
10951098
});
10961099

10971100
assert(response1.status === 200);
@@ -1125,11 +1128,8 @@ describe('Pages', function() {
11251128
// so requesting it again should not return a 304 status code
11261129
const pageUpdateResponse = await apos.doc.update(apos.task.getReq(), pageDoc);
11271130

1128-
const response2 = await apos.http.get(`/api/v1/@apostrophecms/page/${homeId}`, {
1129-
fullResponse: true,
1130-
headers: {
1131-
'if-none-match': response1.headers.etag
1132-
}
1131+
const response2 = await rawGet(apos, `/api/v1/@apostrophecms/page/${homeId}`, {
1132+
'if-none-match': response1.headers.etag
11331133
});
11341134

11351135
const eTag1Parts = response1.headers.etag.split(':');
@@ -1165,11 +1165,8 @@ describe('Pages', function() {
11651165
outOfDateETagParts[2] = Number(outOfDateETagParts[2]) -
11661166
(4444 + 1) * 1000; // 1s outdated
11671167

1168-
const response2 = await apos.http.get(`/api/v1/@apostrophecms/page/${homeId}`, {
1169-
fullResponse: true,
1170-
headers: {
1171-
'if-none-match': outOfDateETagParts.join(':')
1172-
}
1168+
const response2 = await rawGet(apos, `/api/v1/@apostrophecms/page/${homeId}`, {
1169+
'if-none-match': outOfDateETagParts.join(':')
11731170
});
11741171

11751172
const eTag1Parts = response1.headers.etag.split(':');

packages/apostrophe/test/pieces.js

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ const _ = require('lodash');
66
const FormData = require('form-data');
77
const t = require('../test-lib/test.js');
88

9+
// The etag tests below issue their conditional request via rawGet (raw
10+
// node:http): the built-in fetch used by apos.http adds Cache-Control: no-cache
11+
// to any request carrying a conditional header (Fetch standard), which would
12+
// suppress the asserted 304s.
13+
const { rawGet } = t;
14+
915
describe('Pieces', function() {
1016

1117
let apos;
@@ -1807,11 +1813,8 @@ describe('Pieces', function() {
18071813
};
18081814

18091815
const response1 = await apos.http.get('/api/v1/thing/testThing:en:published', { fullResponse: true });
1810-
const response2 = await apos.http.get('/api/v1/thing/testThing:en:published', {
1811-
fullResponse: true,
1812-
headers: {
1813-
'if-none-match': response1.headers.etag
1814-
}
1816+
const response2 = await rawGet(apos, '/api/v1/thing/testThing:en:published', {
1817+
'if-none-match': response1.headers.etag
18151818
});
18161819

18171820
assert(response1.status === 200);
@@ -1845,11 +1848,8 @@ describe('Pieces', function() {
18451848
// so requesting it again should not return a 304 status code
18461849
const pieceUpdateResponse = await apos.doc.update(apos.task.getReq(), pieceDoc);
18471850

1848-
const response2 = await apos.http.get('/api/v1/thing/testThing:en:published', {
1849-
fullResponse: true,
1850-
headers: {
1851-
'if-none-match': response1.headers.etag
1852-
}
1851+
const response2 = await rawGet(apos, '/api/v1/thing/testThing:en:published', {
1852+
'if-none-match': response1.headers.etag
18531853
});
18541854

18551855
const eTag1Parts = response1.headers.etag.split(':');
@@ -1885,11 +1885,8 @@ describe('Pieces', function() {
18851885
outOfDateETagParts[2] = Number(outOfDateETagParts[2]) -
18861886
(4444 + 1) * 1000; // 1s outdated
18871887

1888-
const response2 = await apos.http.get('/api/v1/thing/testThing:en:published', {
1889-
fullResponse: true,
1890-
headers: {
1891-
'if-none-match': outOfDateETagParts.join(':')
1892-
}
1888+
const response2 = await rawGet(apos, '/api/v1/thing/testThing:en:published', {
1889+
'if-none-match': outOfDateETagParts.join(':')
18931890
});
18941891

18951892
const eTag1Parts = response1.headers.etag.split(':');

0 commit comments

Comments
 (0)