From efd6f07773876492189115c6e6aabe47cd4b8ddb Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Fri, 17 Apr 2026 23:58:20 -0400 Subject: [PATCH 1/8] Apply suggested fix to src/ThumbnailApi.ts from Copilot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/ThumbnailApi.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index 35c2404..bc5e8eb 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -147,9 +147,18 @@ export class ThumbnailApi { } expressResponse.status(status); - expressResponse.set( - this.responseHelper.getHeadersFromTarget(response.headers), - ); + try { + expressResponse.set( + this.responseHelper.getHeadersFromTarget(response.headers), + ); + } catch (err) { + this.releaseUpstreamBody(response); + const error = err instanceof Error + ? err + : new Error("Failed to set upstream headers."); + this.sendError(expressResponse, itemId, 502, error); + return; + } if (this.responseHelper.okBody(response.body)) { await this.responseHelper.pipe( response.body as ReadableStream, From 783dd25d8f658aa05587c151555f45d455f904a6 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Fri, 17 Apr 2026 23:58:20 -0400 Subject: [PATCH 2/8] Apply suggested fix to src/ThumbnailApi.ts from Copilot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/ThumbnailApi.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index bc5e8eb..aab9135 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -103,8 +103,6 @@ export class ThumbnailApi { return; } - expressResponse.status(status); - if (!this.responseHelper.okHeaders(remoteImageResponse.headers)) { const error = new Error(`Got bad headers from upstream.`); this.releaseUpstreamBody(remoteImageResponse); @@ -122,6 +120,8 @@ export class ThumbnailApi { return; } + expressResponse.status(status); + await this.responseHelper.pipe( remoteImageResponse.body as ReadableStream, expressResponse, From 44ab9dc3b941163fec47640244f785d0a095c770 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Fri, 17 Apr 2026 23:58:21 -0400 Subject: [PATCH 3/8] Apply suggested fix to src/ThumbnailApi.ts from Copilot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/ThumbnailApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index aab9135..77add76 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -79,7 +79,7 @@ export class ThumbnailApi { // for a short amount because it won't have been sized down expressResponse.set(this.responseHelper.getCacheHeaders(SHORT_CACHE_TIME)); - let remoteImageResponse: Response | undefined = undefined; + let remoteImageResponse: Response | undefined; try { remoteImageResponse = From 9ba5e36728b9af441b16b12237ff6ee3d8b1fe0f Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Fri, 17 Apr 2026 23:58:21 -0400 Subject: [PATCH 4/8] Apply suggested fix to src/ThumbnailApi.ts from Copilot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/ThumbnailApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index 77add76..0eef8cc 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -52,7 +52,7 @@ export class ThumbnailApi { itemId: string, expressResponse: express.Response, ): Promise { - let imageUrl = undefined; + let imageUrl: string | undefined; try { imageUrl = await this.dplaApi.getThumbnailUrl(itemId); From 124a977e4ffbd495eaf012c6047f96198e73e25b Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Fri, 17 Apr 2026 23:58:21 -0400 Subject: [PATCH 5/8] Apply suggested fix to src/ThumbnailApi.ts from Copilot Autofix Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- src/ThumbnailApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index 0eef8cc..cbc1f92 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -8,7 +8,7 @@ import { getLogger } from "./logger"; import { Logger } from "winston"; const LONG_CACHE_TIME: number = 60 * 60 * 24 * 30; // 30 days in seconds -const SHORT_CACHE_TIME = 60 * 60; // 1 hr in seconds +const SHORT_CACHE_TIME: number = 60 * 60; // 1 hr in seconds const PATH_PATTERN = /^\/thumb\/([a-f0-9]{32})$/; export class ThumbnailApi { From 4992827d690356a6a8957c0766b308de98d54902 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Sat, 18 Apr 2026 00:27:40 -0400 Subject: [PATCH 6/8] Guard expressResponse.set() in both proxy paths against resource leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If getHeadersFromTarget() or expressResponse.set() throws, the upstream body was left open (connection pool leak). Wrap the call in a try/catch in both proxyItemFromContributor and serveItemFromS3, releasing the body and sending a 502 on failure. Also add releaseUpstreamBody() before sendError() in the okBody-false branches of both methods — previously those paths skipped cleanup. Co-Authored-By: Claude Sonnet 4.6 --- src/ThumbnailApi.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index cbc1f92..44cf0ac 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -110,12 +110,21 @@ export class ThumbnailApi { return; } - expressResponse.set( - this.responseHelper.getHeadersFromTarget(remoteImageResponse.headers), - ); + try { + expressResponse.set( + this.responseHelper.getHeadersFromTarget(remoteImageResponse.headers), + ); + } catch (err) { + this.releaseUpstreamBody(remoteImageResponse); + const error = + err instanceof Error ? err : new Error("Failed to set upstream headers."); + this.sendError(expressResponse, itemId, 502, error); + return; + } if (!this.responseHelper.okBody(remoteImageResponse.body)) { const error = new Error("Received no body from upstream."); + this.releaseUpstreamBody(remoteImageResponse); this.sendError(expressResponse, itemId, 502, error); return; } @@ -166,6 +175,7 @@ export class ThumbnailApi { ); } else { const error = new Error("Upstream response had no body."); + this.releaseUpstreamBody(response); this.sendError(expressResponse, itemId, 502, error); } } From 8d3b828d24c3775533b89dc5cf2c9fc6ebe87299 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Sun, 19 Apr 2026 11:21:12 -0400 Subject: [PATCH 7/8] fix: guard sendError against headers-already-sent in both proxy methods Centralise the headersSent check inside sendError() rather than duplicating it at every call site. When expressResponse.set() throws because headers are already committed, calling sendError() previously threw ERR_HTTP_HEADERS_SENT; now sendError() logs and returns cleanly. Co-Authored-By: Claude Sonnet 4.6 --- src/ThumbnailApi.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index 44cf0ac..2c5248f 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -201,6 +201,10 @@ export class ThumbnailApi { code: number, error?: Error, ): void { + if (res.headersSent) { + this.logger.error("Unable to send %s for %s: headers already sent", code, itemId, error); + return; + } if (error) { this.logger.error("Sending %s for %s:", code, itemId, error); } From 9b3a4558968e67b48e1f447cefe5363c13bace45 Mon Sep 17 00:00:00 2001 From: Dominic Byrd-McDevitt Date: Sun, 19 Apr 2026 11:49:12 -0400 Subject: [PATCH 8/8] fix: move status() after try/catch in serveItemFromS3 for consistency Mirrors the ordering in proxyItemFromContributor, where status is set after headers are validated and set successfully. Co-Authored-By: Claude Sonnet 4.6 --- src/ThumbnailApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ThumbnailApi.ts b/src/ThumbnailApi.ts index 2c5248f..811488e 100644 --- a/src/ThumbnailApi.ts +++ b/src/ThumbnailApi.ts @@ -155,7 +155,6 @@ export class ThumbnailApi { return; } - expressResponse.status(status); try { expressResponse.set( this.responseHelper.getHeadersFromTarget(response.headers), @@ -168,6 +167,7 @@ export class ThumbnailApi { this.sendError(expressResponse, itemId, 502, error); return; } + expressResponse.status(status); if (this.responseHelper.okBody(response.body)) { await this.responseHelper.pipe( response.body as ReadableStream,