diff --git a/src/http/downloads.ts b/src/http/downloads.ts index a66d9fb..89208c4 100644 --- a/src/http/downloads.ts +++ b/src/http/downloads.ts @@ -50,9 +50,20 @@ export function createDownloadHandler(ctx: PecansHttpContext) { // platform autodetection from the user agent was removed in 2.0; // selecting a platform is the client's responsibility - const _platform = filename - ? filenameToPlatform(filename) - : getStringParam(req, "platform"); + let _platform: string | undefined; + if (filename) { + try { + _platform = filenameToPlatform(filename); + } catch (err) { + // a client-supplied filename that doesn't parse to a platform is + // a client error, not a server fault (it was a 500 before) + throw new BadRequestError( + err instanceof Error ? err.message : String(err), + ); + } + } else { + _platform = getStringParam(req, "platform"); + } if (!_platform) { throw new BadRequestError( "Platform is required. Specify a platform in the URL, e.g. /download/osx_64.", diff --git a/src/utils/platforms.ts b/src/utils/platforms.ts index 00d9795..82ccb03 100644 --- a/src/utils/platforms.ts +++ b/src/utils/platforms.ts @@ -213,5 +213,15 @@ export function filenameToPlatform(filename: string): Platform { const arch = filenameToArchitecture(name, os); parts.push(arch); const platformKey = parts.join("_").toUpperCase(); - return platforms[platformKey]; + const platform = platforms[platformKey]; + if (!platform) { + // discrete parts that don't combine into a modeled composite id, e.g. + // a "universal" token on a linux or non-msix windows asset; throwing + // (instead of returning undefined) keeps the drop visible - ingestion + // logs it rather than skipping the asset silently + throw new Error( + `Unrecognized platform combination (${parts.join("_")}) from filename (${filename})`, + ); + } + return platform; } diff --git a/test/integration/download.spec.ts b/test/integration/download.spec.ts index afaf9cd..df6a7eb 100644 --- a/test/integration/download.spec.ts +++ b/test/integration/download.spec.ts @@ -359,6 +359,13 @@ describe("/download/:tag/:filename", () => { "app-2.6.0-x64.dmg", ); }); + + it("400s on a filename that doesn't parse to a platform", async () => { + const { app } = configureTestAppWithReleases( + buildStableReleaseSet(OWNER, REPO), + ); + await supertest(app).get("/download/2.7.0/notes.txt").expect(400); + }); }); describe("/download/version/:tag/:platform?", () => { diff --git a/test/unit/platforms.spec.ts b/test/unit/platforms.spec.ts index 7a0747c..1f429d6 100644 --- a/test/unit/platforms.spec.ts +++ b/test/unit/platforms.spec.ts @@ -392,6 +392,18 @@ describe("Platforms", function () { } }); }); + + it("throws for discrete parts with no composite id", () => { + // a "universal" token on linux / non-msix windows assets has no + // modeled composite id; the lookup used to return undefined, which + // made ingestion skip the asset without logging anything + expect(() => filenameToPlatform("myapp-universal.tar.gz")).toThrow( + /Unrecognized platform combination \(linux_universal\)/, + ); + expect(() => filenameToPlatform("myapp-win64-universal.zip")).toThrow( + /Unrecognized platform combination \(windows_universal\)/, + ); + }); }); describe("resolveReleaseAssetForVersion", function () {