From 1683c86ee5f9e8d9576d4b1228a2e46636f0f912 Mon Sep 17 00:00:00 2001 From: simojenki Date: Sat, 13 Mar 2021 12:22:37 +1100 Subject: [PATCH] Fix bug where navidrome doesnt always send range headers --- src/server.ts | 6 ++-- src/smapi.ts | 6 ++-- tests/navidrome.test.ts | 65 +++++++++++++++++++++++++++++++++++++++-- tests/server.test.ts | 53 +++++++++++++++++++++++++++++++++ tests/smapi.test.ts | 50 +++++++++++++++++-------------- 5 files changed, 151 insertions(+), 29 deletions(-) diff --git a/src/server.ts b/src/server.ts index 2cf9f52..28afab5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -141,9 +141,9 @@ function server( ) .then((stream) => { res.status(stream.status); - Object.entries(stream.headers).forEach(([header, value]) => - res.setHeader(header, value) - ); + Object.entries(stream.headers) + .filter(([_, v]) => v !== undefined) + .forEach(([header, value]) => res.setHeader(header, value)); res.send(stream.data); }); } diff --git a/src/smapi.ts b/src/smapi.ts index c6da341..97a6085 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -210,6 +210,8 @@ const genre = (genre: string) => ({ title: genre, }); +export const defaultAlbumArtURI = (webAddress: string, accessToken: string, album: AlbumSummary) => `${webAddress}/album/${album.id}/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessToken}` + const album = ( webAddress: string, accessToken: string, @@ -218,7 +220,7 @@ const album = ( itemType: "album", id: `album:${album.id}`, title: album.name, - albumArtURI: `${webAddress}/album/${album.id}/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessToken}`, + albumArtURI: defaultAlbumArtURI(webAddress, accessToken, album), }); export const track = (webAddress: string, accessToken: string, track: Track) => ({ @@ -232,7 +234,7 @@ export const track = (webAddress: string, accessToken: string, track: Track) => albumId: track.album.id, albumArtist: track.artist.name, albumArtistId: track.artist.id, - albumArtURI: `${webAddress}/album/${track.album.id}/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessToken}`, + albumArtURI: defaultAlbumArtURI(webAddress, accessToken, track.album), artist: track.artist.name, artistId: track.artist.id, duration: track.duration, diff --git a/tests/navidrome.test.ts b/tests/navidrome.test.ts index 46c87f4..5510810 100644 --- a/tests/navidrome.test.ts +++ b/tests/navidrome.test.ts @@ -815,6 +815,67 @@ describe("Navidrome", () => { describe("streaming a track", () => { const trackId = uuid(); + describe("when navidrome doesnt return a content-range, accept-ranges or content-length", () => { + it("should return undefined values", async () => { + const streamResponse = { + status: 200, + headers: { + "content-type": "audio/mpeg", + }, + data: Buffer.from("the track", "ascii"), + }; + + mockGET + .mockImplementationOnce(() => Promise.resolve(ok(PING_OK))) + .mockImplementationOnce(() => Promise.resolve(streamResponse)); + + const result = await navidrome + .generateToken({ username, password }) + .then((it) => it as AuthSuccess) + .then((it) => navidrome.login(it.authToken)) + .then((it) => it.stream({ trackId, range: undefined })); + + expect(result.headers).toEqual({ + "content-type": "audio/mpeg", + "content-length": undefined, + "content-range": undefined, + "accept-ranges": undefined, + }); + }); + }); + + describe("when navidrome returns a undefined for content-range, accept-ranges or content-length", () => { + it("should return undefined values", async () => { + const streamResponse = { + status: 200, + headers: { + "content-type": "audio/mpeg", + "content-length": undefined, + "content-range": undefined, + "accept-ranges": undefined, + }, + data: Buffer.from("the track", "ascii"), + }; + + mockGET + .mockImplementationOnce(() => Promise.resolve(ok(PING_OK))) + .mockImplementationOnce(() => Promise.resolve(streamResponse)); + + const result = await navidrome + .generateToken({ username, password }) + .then((it) => it as AuthSuccess) + .then((it) => navidrome.login(it.authToken)) + .then((it) => it.stream({ trackId, range: undefined })); + + expect(result.headers).toEqual({ + "content-type": "audio/mpeg", + "content-length": undefined, + "content-range": undefined, + "accept-ranges": undefined, + }); + }); + }); + describe("with no range specified", () => { describe("navidrome returns a 200", () => { it("should return the content", async () => { @@ -935,7 +996,7 @@ describe("Navidrome", () => { describe("fetching cover art", () => { describe("fetching album art", () => { - describe("when no size is specified", async () => { + describe("when no size is specified", () => { it("should fetch the image", async () => { const streamResponse = { status: 200, @@ -972,7 +1033,7 @@ describe("Navidrome", () => { }); }); - describe("when size is specified", async () => { + describe("when size is specified", () => { it("should fetch the image", async () => { const streamResponse = { status: 200, diff --git a/tests/server.test.ts b/tests/server.test.ts index 7496d84..9c77f5f 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -239,6 +239,59 @@ describe("server", () => { }); describe("when sonos does not ask for a range", () => { + describe("when the music service does not return a content-range, content-length or accept-ranges", () => { + it("should return a 200 with the data, without adding the undefined headers", async () => { + const stream = { + status: 200, + headers: { + "content-type": "audio/mp3", + }, + data: Buffer.from("some track", "ascii"), + }; + + musicService.login.mockResolvedValue(musicLibrary); + musicLibrary.stream.mockResolvedValue(stream); + + const res = await request(server) + .get(`/stream/track/${trackId}`) + .set(BONOB_ACCESS_TOKEN_HEADER, accessToken); + + expect(res.status).toEqual(stream.status); + expect(res.headers["content-type"]).toEqual("audio/mp3") + expect(res.headers["content-length"]).toEqual(`${stream.data.length}`) + expect(Object.keys(res.headers)).not.toContain("content-range") + expect(Object.keys(res.headers)).not.toContain("accept-ranges") + }); + }); + + describe("when the music service returns undefined values for content-range, content-length or accept-ranges", () => { + it("should return a 200 with the data, without adding the undefined headers", async () => { + const stream = { + status: 200, + headers: { + "content-type": "audio/mp3", + "content-length": undefined, + "accept-ranges": undefined, + "content-range": undefined, + }, + data: Buffer.from("some track", "ascii"), + }; + + musicService.login.mockResolvedValue(musicLibrary); + musicLibrary.stream.mockResolvedValue(stream); + + const res = await request(server) + .get(`/stream/track/${trackId}`) + .set(BONOB_ACCESS_TOKEN_HEADER, accessToken); + + expect(res.status).toEqual(stream.status); + expect(res.headers["content-type"]).toEqual("audio/mp3") + expect(res.headers["content-length"]).toEqual(`${stream.data.length}`) + expect(Object.keys(res.headers)).not.toContain("content-range") + expect(Object.keys(res.headers)).not.toContain("accept-ranges") + }); + }); + describe("when the music service returns a 200", () => { it("should return a 200 with the data", async () => { const stream = { diff --git a/tests/smapi.test.ts b/tests/smapi.test.ts index d5da915..62c72cb 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -18,6 +18,7 @@ import { PRESENTATION_MAP_ROUTE, SONOS_RECOMMENDED_IMAGE_SIZES, track, + defaultAlbumArtURI, } from "../src/smapi"; import { @@ -163,6 +164,17 @@ describe("track", () => { }); }); +describe("defaultAlbumArtURI", () => { + it("should create the correct URI", () => { + const webAddress = "http://localhost:1234"; + const accessToken = uuid(); + const album = anAlbum(); + + expect(defaultAlbumArtURI(webAddress, accessToken, album)).toEqual( + `${webAddress}/album/${album.id}/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessToken}` + ); + }); +}); class Base64AccessTokens implements AccessTokens { mint(authToken: string) { return Buffer.from(authToken).toString("base64"); @@ -427,6 +439,7 @@ describe("api", () => { const password = "validPassword"; let token: AuthSuccess; let ws: Client; + let accessToken: string; beforeEach(async () => { musicService.hasUser({ username, password }); @@ -434,6 +447,9 @@ describe("api", () => { username, password, })) as AuthSuccess; + + accessToken = accessTokens.mint(token.authToken); + ws = await createClientAsync(`${service.uri}?wsdl`, { endpoint: service.uri, httpClient: supersoap(server, rootUrl), @@ -545,11 +561,7 @@ describe("api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: `${rootUrl}/album/${ - it.id - }/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessTokens.mint( - token.authToken - )}`, + albumArtURI: defaultAlbumArtURI(rootUrl, accessToken, it), })), index: 0, total: artistWithManyAlbums.albums.length, @@ -574,11 +586,7 @@ describe("api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: `${rootUrl}/album/${ - it.id - }/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessTokens.mint( - token.authToken - )}`, + albumArtURI: defaultAlbumArtURI(rootUrl, accessToken, it), })), index: 2, total: artistWithManyAlbums.albums.length, @@ -683,11 +691,7 @@ describe("api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: `${rootUrl}/album/${ - it.id - }/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessTokens.mint( - token.authToken - )}`, + albumArtURI: defaultAlbumArtURI(rootUrl, accessToken, it), })), index: 0, total: 6, @@ -713,11 +717,7 @@ describe("api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: `${rootUrl}/album/${ - it.id - }/art/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${accessTokens.mint( - token.authToken - )}`, + albumArtURI: defaultAlbumArtURI(rootUrl, accessToken, it), })), index: 2, total: 6, @@ -779,7 +779,9 @@ describe("api", () => { }); expect(result[0]).toEqual( getMetadataResult2({ - mediaMetadata: [track3, track4].map(it => track(rootUrl, accessTokens.mint(token.authToken), it)), + mediaMetadata: [track3, track4].map((it) => + track(rootUrl, accessTokens.mint(token.authToken), it) + ), index: 2, total: 5, }) @@ -986,7 +988,11 @@ describe("api", () => { id: `track:${someTrack.id}`, }); expect(root[0]).toEqual({ - getMediaMetadataResult: track(rootUrl, accessTokens.mint(token.authToken), someTrack), + getMediaMetadataResult: track( + rootUrl, + accessTokens.mint(token.authToken), + someTrack + ), }); }); });