fix(hosters): defensive null-payload guards in result parsers + 7 tests
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.
This commit is contained in:
parent
74d7f8ce5a
commit
0ba8bd3a2c
@ -160,7 +160,9 @@ function sleep(ms, signal) {
|
|||||||
// Doodstream: { result: [{ download_url, protected_embed, filecode, protected_dl }] }
|
// Doodstream: { result: [{ download_url, protected_embed, filecode, protected_dl }] }
|
||||||
function parseDoodstreamResult(payload) {
|
function parseDoodstreamResult(payload) {
|
||||||
let item = {};
|
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) {
|
if (Array.isArray(result) && result.length > 0) {
|
||||||
item = result[0];
|
item = result[0];
|
||||||
} else if (result && typeof result === 'object') {
|
} else if (result && typeof result === 'object') {
|
||||||
@ -195,18 +197,20 @@ function parseVoeResult(payload) {
|
|||||||
|
|
||||||
// Byse: { files: [{ filecode, filename, status }] }
|
// Byse: { files: [{ filecode, filename, status }] }
|
||||||
function parseByseResult(payload) {
|
function parseByseResult(payload) {
|
||||||
|
// Defensive: bypass-callers may pass null/non-object directly.
|
||||||
|
if (!payload || typeof payload !== 'object') payload = {};
|
||||||
let file_code = null;
|
let file_code = null;
|
||||||
let perFileError = null;
|
let perFileError = null;
|
||||||
|
|
||||||
// Primary: files array (per official Byse API docs)
|
// Primary: files array (per official Byse API docs)
|
||||||
if (Array.isArray(payload.files) && payload.files.length > 0) {
|
if (Array.isArray(payload.files) && payload.files.length > 0) {
|
||||||
const f = payload.files[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
|
// Byse returns HTTP 200 + msg=OK even when a specific file was rejected
|
||||||
// ("Not video file format", "Duplicate", "File too small", ...). When
|
// ("Not video file format", "Duplicate", "File too small", ...). When
|
||||||
// filecode is empty and status carries a non-OK message, that IS the
|
// filecode is empty and status carries a non-OK message, that IS the
|
||||||
// actual per-file error, not a server problem.
|
// 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();
|
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}` : ')'}`
|
`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) {
|
if (statusCode < 200 || statusCode >= 300) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
@ -540,6 +554,8 @@ module.exports = {
|
|||||||
HOSTER_CONFIGS,
|
HOSTER_CONFIGS,
|
||||||
__test: {
|
__test: {
|
||||||
extractUploadServerUrl,
|
extractUploadServerUrl,
|
||||||
parseVoeResult
|
parseVoeResult,
|
||||||
|
parseDoodstreamResult,
|
||||||
|
parseByseResult
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@ -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.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.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.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)
|
## Open items (priorisiert)
|
||||||
|
|
||||||
### Stabilität (neu aus deep-audit)
|
### 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.
|
- [ ] 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)
|
### Code-Qualität (deferred)
|
||||||
|
|||||||
@ -31,4 +31,65 @@ describe('hosters helpers', () => {
|
|||||||
|
|
||||||
assert.equal(url, 'https://delivery-hydra.voe-network.net/upload/01');
|
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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user