From 72f3fc8436a0191a65159d1f75cc26dbd31db443 Mon Sep 17 00:00:00 2001 From: Samu Date: Sat, 29 Aug 2026 16:24:40 +0200 Subject: [PATCH] fix: harden discogs mappers against malformed payloads, name error classes, assert auth header --- server/src/discogs.ts | 67 +++++++++++++++++++++---------------- server/test/discogs.test.ts | 20 ++++++++--- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/server/src/discogs.ts b/server/src/discogs.ts index de845c9..16722dc 100644 --- a/server/src/discogs.ts +++ b/server/src/discogs.ts @@ -25,6 +25,7 @@ export class DiscogsError extends Error { message: string ) { super(message) + this.name = new.target.name } } export class DiscogsAuthError extends DiscogsError { @@ -52,41 +53,43 @@ export function toYear(year: unknown): number | null { } export function mapSearchResult(r: any): DiscogsReleaseSummary { - const { artist, title } = splitTitle(String(r.title ?? '')) + const src = r ?? {} + const { artist, title } = splitTitle(String(src.title ?? '')) return { - id: Number(r.id), + id: Number(src.id), artist, title, - year: toYear(r.year), - formats: Array.isArray(r.format) ? r.format.map(String) : [], - labels: Array.isArray(r.label) ? r.label.map(String) : [], - country: r.country ?? null, - catno: r.catno ?? null, - thumbUrl: r.thumb ?? r.cover_image ?? null, + year: toYear(src.year), + formats: Array.isArray(src.format) ? src.format.filter((f: any) => f != null).map(String) : [], + labels: Array.isArray(src.label) ? src.label.filter((l: any) => l != null).map(String) : [], + country: src.country ?? null, + catno: src.catno ?? null, + thumbUrl: src.thumb ?? src.cover_image ?? null, } } export function mapRelease(r: any): DiscogsReleaseFull { + const src = r ?? {} return { - id: Number(r.id), - artist: Array.isArray(r.artists) && r.artists[0] ? String(r.artists[0].name) : '', - title: String(r.title ?? ''), - year: toYear(r.year), - formats: Array.isArray(r.formats) ? r.formats.map((f: any) => String(f.name)) : [], - labels: Array.isArray(r.labels) ? r.labels.map((l: any) => String(l.name)) : [], - country: r.country ?? null, - catno: Array.isArray(r.labels) && r.labels[0] ? (r.labels[0].catno ?? null) : null, - thumbUrl: r.thumb ?? (r.images?.[0]?.uri ?? null), - genres: Array.isArray(r.genres) ? r.genres.map(String) : [], - tracklist: Array.isArray(r.tracklist) - ? r.tracklist - .filter((t: any) => t.title) - .map((t: any) => ({ position: String(t.position ?? ''), title: String(t.title) })) + id: Number(src.id), + artist: Array.isArray(src.artists) && src.artists[0] ? String(src.artists[0]?.name ?? '') : '', + title: String(src.title ?? ''), + year: toYear(src.year), + formats: Array.isArray(src.formats) ? src.formats.map((f: any) => String(f?.name ?? '')) : [], + labels: Array.isArray(src.labels) ? src.labels.map((l: any) => String(l?.name ?? '')) : [], + country: src.country ?? null, + catno: Array.isArray(src.labels) && src.labels[0] ? (src.labels[0]?.catno ?? null) : null, + thumbUrl: src.thumb ?? (src.images?.[0]?.uri ?? null), + genres: Array.isArray(src.genres) ? src.genres.filter((g: any) => g != null).map(String) : [], + tracklist: Array.isArray(src.tracklist) + ? src.tracklist + .filter((t: any) => t?.title) + .map((t: any) => ({ position: String(t?.position ?? ''), title: String(t?.title ?? '') })) : [], - coverUrl: r.images?.[0]?.uri ?? null, - barcodes: Array.isArray(r.identifiers) - ? r.identifiers - .filter((i: any) => i.type === 'Barcode' && i.value) + coverUrl: src.images?.[0]?.uri ?? null, + barcodes: Array.isArray(src.identifiers) + ? src.identifiers + .filter((i: any) => i?.type === 'Barcode' && i?.value) .map((i: any) => String(i.value)) : [], } @@ -125,21 +128,27 @@ export class DiscogsClient { if (res.status === 401) throw new DiscogsAuthError() if (res.status === 429) throw new DiscogsRateLimitError() if (!res.ok) throw new DiscogsError(res.status, `discogs HTTP ${res.status}`) - return res.json() + try { + return await res.json() + } catch { + throw new DiscogsError(res.status, 'discogs returned a non-JSON response') + } } return this.queue ? this.queue.run(doFetch) : doFetch() } async searchByBarcode(barcode: string): Promise { const body = await this.get('/database/search', { barcode, type: 'release', per_page: '20' }) - return (body.results ?? []).map(mapSearchResult) + const results: any[] = Array.isArray(body?.results) ? body.results : [] + return results.map(mapSearchResult) } async searchByText(query: string, format?: string): Promise { const params: Record = { q: query, type: 'release', per_page: '20' } if (format) params.format = format const body = await this.get('/database/search', params) - return (body.results ?? []).map(mapSearchResult) + const results: any[] = Array.isArray(body?.results) ? body.results : [] + return results.map(mapSearchResult) } async getRelease(id: number): Promise { diff --git a/server/test/discogs.test.ts b/server/test/discogs.test.ts index 90f64cf..d31bbd2 100644 --- a/server/test/discogs.test.ts +++ b/server/test/discogs.test.ts @@ -15,17 +15,19 @@ function jsonResponse(body: unknown, status = 200): Response { }) } -function stubFetch(routes: (url: string) => Response): typeof fetch { - return (async (input: any) => routes(String(input))) as typeof fetch +function stubFetch(routes: (url: string, init?: any) => Response): typeof fetch { + return (async (input: any, init?: any) => routes(String(input), init)) as typeof fetch } describe('DiscogsClient', () => { it('searches by barcode with token and maps results', async () => { const seen: string[] = [] + const seenAuth: string[] = [] const c = new DiscogsClient( 'testtoken', - stubFetch((url) => { - seen.push(url) + stubFetch((_url, init) => { + seen.push(_url) + seenAuth.push(String(new Headers(init?.headers).get('Authorization'))) return jsonResponse(discogsSearchFixture) }) ) @@ -34,6 +36,7 @@ describe('DiscogsClient', () => { expect(seen[0]).toContain('barcode=5021592210629') expect(seen[0]).toContain('type=release') expect(seen[0]).toContain('token=testtoken') + expect(seenAuth[0]).toBe('Discogs token=testtoken') expect(results).toHaveLength(2) expect(results[0]).toEqual({ id: 1001, @@ -105,4 +108,13 @@ describe('mappers', () => { { position: '2', title: 'Theme de Yoyo' }, ]) }) + + it('does not throw on malformed payloads', () => { + expect(() => mapSearchResult(null)).not.toThrow() + expect(() => mapRelease(undefined)).not.toThrow() + expect(mapRelease({ tracklist: [null, { title: 'X' }] }).tracklist).toEqual([{ position: '', title: 'X' }]) + expect(mapSearchResult({ format: [null, 'CD'] }).formats).toEqual(['CD']) + expect(new DiscogsAuthError().name).toBe('DiscogsAuthError') + expect(new DiscogsRateLimitError().name).toBe('DiscogsRateLimitError') + }) })