From 0ba8bd3a2ce113d2aa0cf50a9be6c4ae3d7bd94f Mon Sep 17 00:00:00 2001 From: Administrator Date: Tue, 28 Apr 2026 10:12:32 +0200 Subject: [PATCH] fix(hosters): defensive null-payload guards in result parsers + 7 tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a hoster server replies with a body that JSON-parses to a non-object (literal "null", a bare string, a number, a top-level array), uploadFile's downstream code crashed: payload.msg → TypeError on null payload.status → TypeError on null config.parseResult() → TypeError inside parseDoodstreamResult (payload.result) and parseByseResult (payload.files / payload.result) The user saw a confusing "Cannot read properties of null" instead of a useful "server returned no JSON object". Found by deep-audit pass. Fix in three places: 1. uploadFile (lib/hosters.js): after JSON.parse, normalise non-object payloads to {}. Subsequent `payload.X` accesses then return undefined and the existing fallback paths handle the empty case. 2. parseDoodstreamResult: defensive `payload && payload.result` so direct callers (tests, hypothetical future callers) get the same guarantee instead of relying on uploadFile to have normalised. 3. parseByseResult: same `payload || typeof payload !== 'object'` short-circuit at entry, plus null-checks on `f` (the first files entry) so a server returning [null] in files doesn't crash either. Tests: 7 new unit tests covering null/undefined/string/number/array payloads, malformed files entries, the fileRejected/accountError classification (regression-pinning the 3.1.4 phrasing tweaks), and the valid-filecode happy path. 126/126 green. --- lib/hosters.js | 24 ++++++++++++++--- tasks/todo.md | 2 +- tests/hosters.test.js | 61 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/lib/hosters.js b/lib/hosters.js index 7442a39..2b8fe7a 100644 --- a/lib/hosters.js +++ b/lib/hosters.js @@ -160,7 +160,9 @@ function sleep(ms, signal) { // Doodstream: { result: [{ download_url, protected_embed, filecode, protected_dl }] } function parseDoodstreamResult(payload) { let item = {}; - const result = payload.result; + // Defensive: also handle direct callers that bypass uploadFile's payload + // normalisation (e.g. unit tests, future callers). + const result = payload && payload.result; if (Array.isArray(result) && result.length > 0) { item = result[0]; } else if (result && typeof result === 'object') { @@ -195,18 +197,20 @@ function parseVoeResult(payload) { // Byse: { files: [{ filecode, filename, status }] } function parseByseResult(payload) { + // Defensive: bypass-callers may pass null/non-object directly. + if (!payload || typeof payload !== 'object') payload = {}; let file_code = null; let perFileError = null; // Primary: files array (per official Byse API docs) if (Array.isArray(payload.files) && payload.files.length > 0) { const f = payload.files[0]; - file_code = f.filecode || f.file_code || null; + file_code = f && (f.filecode || f.file_code) || null; // Byse returns HTTP 200 + msg=OK even when a specific file was rejected // ("Not video file format", "Duplicate", "File too small", ...). When // filecode is empty and status carries a non-OK message, that IS the // actual per-file error, not a server problem. - if (!file_code && f.status && !/^(ok|success|done)$/i.test(String(f.status))) { + if (!file_code && f && f.status && !/^(ok|success|done)$/i.test(String(f.status))) { perFileError = String(f.status).trim(); } } @@ -479,6 +483,16 @@ async function uploadFile(hosterName, filePath, apiKey, onProgress, signal, thro `Upload-Antwort von ${hosterName} war kein JSON (HTTP ${statusCode}${snippet ? `): ${snippet}` : ')'}` ); } + // Normalize valid-but-not-object JSON (JSON.parse('null') → null; + // JSON.parse('"foo"') → string; JSON.parse('[1]') → array). Without this + // the downstream `payload.msg` / `payload.status` / parseResult(payload) + // calls crash with a confusing TypeError instead of letting the existing + // fallback defaults kick in. Arrays from servers that return a top-level + // list (rare but seen in the wild) are kept addressable as `payload.X` + // → undefined, which the parsers already handle. + if (payload === null || typeof payload !== 'object') { + payload = {}; + } if (statusCode < 200 || statusCode >= 300) { throw new Error( @@ -540,6 +554,8 @@ module.exports = { HOSTER_CONFIGS, __test: { extractUploadServerUrl, - parseVoeResult + parseVoeResult, + parseDoodstreamResult, + parseByseResult } }; diff --git a/tasks/todo.md b/tasks/todo.md index b02af1a..94ea3ec 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -15,11 +15,11 @@ - ✅ 3.3.11 — Patch-Bumps `eslint 10.1→10.2`, `undici 7.24→7.25`, `ws 8.19→8.20` (semver-compatible) - ✅ 3.3.12 — Race condition fix: `uploadManager = null` in batch-done clobberte einen frisch gespawnten Manager wenn user mid-await neuen batch startete (deep-audit finding HIGH-1) - ✅ 3.3.13 — `save-global-settings-sync` reportet jetzt `returnValue=false` bei Fehlern + debugLog statt silent swallow; TOCTOU bei .bak-Refresh in beiden Pfaden (main.js + lib/config-store.js _atomicWrite) entkoppelt: bak-Read-Failure failt nicht mehr den ganzen Save (deep-audit findings HIGH-2 + MED-4) +- ✅ 3.3.14 — Parser-null-payload guard: `uploadFile` normalisiert payload zu `{}` falls `JSON.parse('null')` o.ä.; `parseDoodstreamResult` + `parseByseResult` haben defensive guards für direct callers + 7 neue Unit-Tests (null/non-object, malformed entries, fileRejected/accountError flips, valid filecode happy path) ## Open items (priorisiert) ### Stabilität (neu aus deep-audit) -- [ ] `parseByseResult` / `parseDoodstreamResult` crash on null payload (lib/hosters.js:202, 214, 163, 491) — `JSON.parse('null')` returns null, then `payload.files` throws TypeError. Guard `if (!payload || typeof payload !== 'object') throw...` after JSON.parse. - [ ] Cancellation latency in retry-loop's account-rotation block (lib/upload-manager.js:680-792) — re-check signal.aborted after each await. ### Code-Qualität (deferred) diff --git a/tests/hosters.test.js b/tests/hosters.test.js index 9a54b2f..9f41eef 100644 --- a/tests/hosters.test.js +++ b/tests/hosters.test.js @@ -31,4 +31,65 @@ describe('hosters helpers', () => { assert.equal(url, 'https://delivery-hydra.voe-network.net/upload/01'); }); + + it('parseDoodstreamResult tolerates null/non-object payload without throwing', () => { + // Direct callers may bypass uploadFile's normalisation. The parser must + // never throw on bad input — empty fields are the contract. + for (const bad of [null, undefined, 'string', 42, true]) { + const r = __test.parseDoodstreamResult(bad); + assert.equal(r.file_code, null); + assert.equal(r.download_url, null); + assert.equal(r.embed_url, null); + } + }); + + it('parseDoodstreamResult handles result-as-array and result-as-object', () => { + const arr = __test.parseDoodstreamResult({ result: [{ filecode: 'AB1', protected_dl: 'https://x/1', protected_embed: 'https://x/e/1' }] }); + assert.equal(arr.file_code, 'AB1'); + assert.equal(arr.download_url, 'https://x/1'); + assert.equal(arr.embed_url, 'https://x/e/1'); + + const obj = __test.parseDoodstreamResult({ result: { filecode: 'OBJ1', download_url: 'https://x/2' } }); + assert.equal(obj.file_code, 'OBJ1'); + assert.equal(obj.download_url, 'https://x/2'); + }); + + it('parseByseResult tolerates null/non-object payload without throwing', () => { + for (const bad of [null, undefined, 'string', 42, []]) { + const r = __test.parseByseResult(bad); + assert.equal(r.file_code, null); + assert.equal(r.download_url, null); + assert.equal(r.embed_url, null); + } + }); + + it('parseByseResult handles malformed files entries (null, missing fields)', () => { + // Files array with a null first element (server returned [null]) + const a = __test.parseByseResult({ files: [null] }); + assert.equal(a.file_code, null); + // Files array with object missing both filecode and status + const b = __test.parseByseResult({ files: [{}] }); + assert.equal(b.file_code, null); + }); + + it('parseByseResult throws fileRejected for non-OK status with empty filecode', () => { + assert.throws( + () => __test.parseByseResult({ files: [{ status: 'Not video file format' }] }), + (err) => err.fileRejected === true && /Not video file format/i.test(err.message) + ); + }); + + it('parseByseResult flips to accountError for storage-exhausted phrasing', () => { + assert.throws( + () => __test.parseByseResult({ files: [{ status: 'not enough disk space on your account' }] }), + (err) => err.accountError === true + ); + }); + + it('parseByseResult succeeds with valid filecode in files[0]', () => { + const r = __test.parseByseResult({ files: [{ filecode: 'GOOD123', status: 'OK' }] }); + assert.equal(r.file_code, 'GOOD123'); + assert.equal(r.download_url, 'https://byse.sx/d/GOOD123'); + assert.equal(r.embed_url, 'https://byse.sx/e/GOOD123'); + }); });