Merge pull request #1102 from Rohilalala/fix/radio-mirror-null-body

fix(radio): reject null-body and out-of-range mirror statuses instead of crashing

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.
+
 - Snow flakes sway within a fixed range instead of drifting sideways faster the
   longer the app runs; the Wind setting still scales the sway.
 

docs/CURRENT-STATE.md

@@ -1220,9 +1220,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
+caching are separate modules. The pinned transport gives a mirror's 204, 205 or
-factories. Routes, payloads, fallback behavior and existing dev/preview hook
+304 reply an empty body and fails any reply outside 200–599 without throwing
-registration are preserved; browser rendering is unchanged.
+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
 

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();
+        // Nothing thrown here may escape: this callback runs outside the
-        for (const [name, value] of Object.entries(response.headers)) {
+        // promise, so an exception would be uncaught and crash the server.
-          if (Array.isArray(value))
+        try {
-            value.forEach((item) => headers.append(name, item));
+          const status = response.statusCode || 500;
-          else if (value !== undefined) headers.set(name, String(value));
+          // 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);

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);
+});