From f32b2c6eb701228516eac35d7ff18a7f70dc48f2 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:34:43 +0000 Subject: [PATCH] fix: abort ZIP responses when the streamer fails --- src/server/routes/repository-public.ts | 6 ++- test/zip-proxy-errors.test.js | 72 ++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 test/zip-proxy-errors.test.js diff --git a/src/server/routes/repository-public.ts b/src/server/routes/repository-public.ts index 092b3ea..f5a0071 100644 --- a/src/server/routes/repository-public.ts +++ b/src/server/routes/repository-public.ts @@ -112,7 +112,7 @@ router.get( upstreamStatus, upstreamBody: upstreamBody?.slice(0, 500), url: config.STREAMER_ENTRYPOINT - ? join(config.STREAMER_ENTRYPOINT, "api/zip") + ? join(config.STREAMER_ENTRYPOINT, "api/download") : undefined, err: serializeError(err), }); @@ -124,6 +124,10 @@ router.get( }), res ); + // Once ZIP bytes have been sent, handleError cannot send JSON. + // Terminate the truncated response so the client reports failure + // instead of waiting forever for the ZIP's central directory. + if (res.headersSent && !res.writableEnded) res.destroy(); }); reqStream.pipe(res); res.on("close", () => { diff --git a/test/zip-proxy-errors.test.js b/test/zip-proxy-errors.test.js new file mode 100644 index 0000000..5d97327 --- /dev/null +++ b/test/zip-proxy-errors.test.js @@ -0,0 +1,72 @@ +const { expect } = require("chai"); +const express = require("express"); +const http = require("node:http"); +const { PassThrough } = require("node:stream"); +const got = require("got"); +require("ts-node/register/transpile-only"); +const config = require("../src/config").default; +const utils = require("../src/server/routes/route-utils"); +const router = require("../src/server/routes/repository-public").default; + +describe("ZIP proxy failures", function () { + let restore, upstream, server; + beforeEach(async function () { + const previous = { download: config.ENABLE_DOWNLOAD, endpoint: config.STREAMER_ENTRYPOINT, + getRepo: utils.getRepo, getUser: utils.getUser, stream: got.stream }; + restore = () => { + config.ENABLE_DOWNLOAD = previous.download; + config.STREAMER_ENTRYPOINT = previous.endpoint; + utils.getRepo = previous.getRepo; utils.getUser = previous.getUser; got.stream = previous.stream; + }; + config.ENABLE_DOWNLOAD = true; + config.STREAMER_ENTRYPOINT = "http://streamer.test/"; + utils.getUser = async () => { throw new Error("anonymous"); }; + utils.getRepo = async () => ({ repoId: "test", options: {}, + model: { source: { repositoryName: "owner/repo", commit: "abc" } }, + countView: async () => {}, getToken: async () => "", + generateAnonymizeTransformer: () => ({ opt: {} }), + }); + upstream = new PassThrough(); + got.stream = () => upstream; + const app = express(); app.use(router); + server = await new Promise(resolve => { + const listening = app.listen(0, "127.0.0.1", () => resolve(listening)); + }); + }); + afterEach(async function () { + upstream.destroy(); server.closeAllConnections(); + await new Promise(resolve => server.close(resolve)); restore(); + }); + function request(onResponse) { + return http.get(`http://127.0.0.1:${server.address().port}/test/zip`, onResponse); + } + it("aborts a partial ZIP when the upstream fails after headers", async function () { + const result = new Promise((resolve, reject) => { + const client = request(response => { + expect(response.statusCode).to.equal(200); + response.once("data", () => upstream.destroy(new Error("upstream disconnected"))); + response.on("aborted", resolve); + response.on("end", () => reject(new Error("truncated ZIP reported as complete"))); + response.on("error", () => {}); + }); + client.on("error", reject); + }); + upstream.write(Buffer.from("PK partial archive")); + await result; + }); + it("keeps JSON errors before any ZIP bytes are sent", async function () { + const result = new Promise((resolve, reject) => { + request(response => { + let body = ""; response.on("data", chunk => { body += chunk; }); + response.on("end", () => resolve({ status: response.statusCode, body: JSON.parse(body) })); + }).on("error", reject); + }); + // Fail only after the route has attached its upstream listener. + upstream.once("newListener", event => { + if (event === "error") globalThis.queueMicrotask(() => upstream.destroy(Object.assign(new Error("unavailable"), { + response: { statusCode: 503, body: '{"error":"zip_not_available"}' }, + }))); + }); + expect(await result).to.deep.equal({ status: 502, body: { error: "zip_not_available" } }); + }); +});