diff --git a/CHANGELOG.md b/CHANGELOG.md index b24ac95ed..66b87d770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +- A Radio Browser mirror answering 204, 205 or 304, or a status outside + 200–599, no longer crashes the local dev/preview server. Null-body statuses + get an empty body; anything else that cannot become a response fails that + mirror so the next one is tried. + - Treat explicitly current, complete empty vessel coverage as a successful update, clearing obsolete contacts while retaining stale or incomplete snapshots. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 26d56e66b..9de0ebf31 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1135,9 +1135,12 @@ CCTV catalog acquisition, source normalization and frame/media delivery now live in separate modules. Each CCTV factory owns its catalog and health state; its `sourceRoot` option resolves relative source files against the application root. Radio Browser station normalization, restricted outbound transport and directory -caching are separate modules. Node-only package entries expose both provider -factories. Routes, payloads, fallback behavior and existing dev/preview hook -registration are preserved; browser rendering is unchanged. +caching are separate modules. The pinned transport gives a mirror's 204, 205 or +304 reply an empty body and fails any reply outside 200–599 without throwing +outside its promise, so an unusual mirror status cannot crash the server. +Node-only package entries expose both provider factories. Routes, payloads, +fallback behavior and existing dev/preview hook registration are preserved; +browser rendering is unchanged. ## Terrain, traffic, fire and bike-share provider modules diff --git a/server/providers/radio/transport.js b/server/providers/radio/transport.js index 7bbe33bcd..c9922bb2f 100644 --- a/server/providers/radio/transport.js +++ b/server/providers/radio/transport.js @@ -1,6 +1,9 @@ import https from 'node:https'; import { Readable } from 'node:stream'; import { isNonGlobalIpv4 } from './stations.js'; + +const NULL_BODY_STATUSES = new Set([204, 205, 304]); + export function radioMirrorOrigin(value) { const hostname = String(value ?? '') .toLowerCase() @@ -124,19 +127,33 @@ export function fetchPinnedRadioResponse(url, options, addresses) { }, }, (response) => { - const headers = new Headers(); - for (const [name, value] of Object.entries(response.headers)) { - if (Array.isArray(value)) - value.forEach((item) => headers.append(name, item)); - else if (value !== undefined) headers.set(name, String(value)); + // Nothing thrown here may escape: this callback runs outside the + // promise, so an exception would be uncaught and crash the server. + try { + const status = response.statusCode; + // A `Response` only represents statuses up to 599; Node never hands + // this callback a 1xx. + if (status > 599) + throw new Error( + `Radio Browser returned unsupported status ${status}`, + ); + const headers = new Headers(); + for (const [name, value] of Object.entries(response.headers)) { + if (Array.isArray(value)) + value.forEach((item) => headers.append(name, item)); + else if (value !== undefined) headers.set(name, String(value)); + } + const nullBody = NULL_BODY_STATUSES.has(status); + const upstream = new Response( + nullBody ? null : Readable.toWeb(response), + { status, statusText: response.statusMessage || '', headers }, + ); + if (nullBody) response.resume(); + resolve(upstream); + } catch (error) { + response.destroy(); + reject(error); } - resolve( - new Response(Readable.toWeb(response), { - status: response.statusCode || 500, - statusText: response.statusMessage || '', - headers, - }), - ); }, ); request.on('error', reject); diff --git a/src/data/radioProxy.test.mjs b/src/data/radioProxy.test.mjs index 2f090b5f9..60463205b 100644 --- a/src/data/radioProxy.test.mjs +++ b/src/data/radioProxy.test.mjs @@ -1,5 +1,9 @@ import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; +import https from 'node:https'; +import { Readable } from 'node:stream'; import test from 'node:test'; +import { fetchPinnedRadioResponse } from '../../server/providers/radio/transport.js'; import { createRadioProxyMiddleware, isPublicRadioAddress, @@ -469,3 +473,63 @@ test('proxy rejects any hostname resolution containing a forbidden address befor } assert.equal(fetchCount, 0); }); + +// Answer every pinned request with `status`, invoking the response callback +// asynchronously as `https.request` does, outside the caller's promise. +// The returned promise rejects on an uncaught exception, so a regression fails +// the race at once instead of leaving the fetch pending. +function mockMirrorStatus(t, status, statusMessage = '') { + let crash; + const crashed = new Promise((_resolve, reject) => (crash = reject)); + const onUncaught = (error) => + crash(new Error(`uncaught exception: ${error.message}`)); + process.on('uncaughtException', onUncaught); + t.after(() => process.off('uncaughtException', onUncaught)); + t.mock.method(https, 'request', (_url, _options, callback) => { + const request = new EventEmitter(); + request.end = () => + setImmediate(() => + callback( + Object.assign(Readable.from([]), { + statusCode: status, + statusMessage, + headers: { 'content-type': 'application/json' }, + }), + ), + ); + return request; + }); + return crashed; +} + +function fetchMirror() { + return fetchPinnedRadioResponse( + new URL('https://de1.api.radio-browser.info/json/servers'), + { headers: {} }, + [{ address: '93.184.216.34', family: 4 }], + ); +} + +for (const status of [204, 205, 304]) { + test(`a mirror answering null-body status ${status} resolves with a null body instead of crashing`, async (t) => { + const crashed = mockMirrorStatus(t, status); + const response = await Promise.race([fetchMirror(), crashed]); + assert.equal(response.status, status); + assert.equal(response.body, null); + }); +} + +for (const status of [600, 999]) { + test(`a mirror answering out-of-range status ${status} rejects instead of crashing`, async (t) => { + const crashed = mockMirrorStatus(t, status); + await assert.rejects( + Promise.race([fetchMirror(), crashed]), + new RegExp(`unsupported status ${status}`), + ); + }); +} + +test('a mirror answering an invalid reason phrase rejects instead of crashing', async (t) => { + const crashed = mockMirrorStatus(t, 200, 'OK\x7f'); + await assert.rejects(Promise.race([fetchMirror(), crashed]), TypeError); +});