From da5ba4eecdbfb4e49d054f13cebebbf6d1c18e0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 4 May 2026 14:27:58 +0200 Subject: [PATCH] Handle non-numeric Retry-After header values gracefully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate the parsed Retry-After value is a finite positive number before using it as a delay, falling back to exponential backoff otherwise. This prevents a near-zero delay tight retry loop when the header contains an HTTP-date or unexpected value. Signed-off-by: Fredrik Adelöw Co-authored-by: Cursor Signed-off-by: Fredrik Adelöw Co-authored-by: Cursor Signed-off-by: Fredrik Adelöw Co-authored-by: Cursor --- .../src/gitlab/GitLabIntegration.test.ts | 34 ++++++++++++++++ .../src/gitlab/GitLabIntegration.ts | 40 ++++++++++++++++--- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/integration/src/gitlab/GitLabIntegration.test.ts b/packages/integration/src/gitlab/GitLabIntegration.test.ts index dcd2965cb3..156ea0ef3b 100644 --- a/packages/integration/src/gitlab/GitLabIntegration.test.ts +++ b/packages/integration/src/gitlab/GitLabIntegration.test.ts @@ -19,6 +19,7 @@ import { rest } from 'msw'; import { ConfigReader } from '@backstage/config'; import { GitLabIntegration, + parseRetryAfterMs, replaceGitLabUrlType, sleep, } from './GitLabIntegration'; @@ -477,6 +478,39 @@ describe('sleep', () => { }); }); +describe('parseRetryAfterMs', () => { + it('parses delay-seconds', () => { + expect(parseRetryAfterMs('120', 5000)).toBe(120_000); + expect(parseRetryAfterMs('1', 5000)).toBe(1000); + }); + + it('parses an HTTP-date and computes a delta', () => { + const futureDate = new Date(Date.now() + 30_000).toUTCString(); + const result = parseRetryAfterMs(futureDate, 5000); + expect(result).toBeGreaterThan(29_000); + expect(result).toBeLessThanOrEqual(30_000); + }); + + it('returns 0 for an HTTP-date in the past', () => { + const pastDate = new Date(Date.now() - 10_000).toUTCString(); + expect(parseRetryAfterMs(pastDate, 5000)).toBe(0); + }); + + it('returns fallback for null or unparseable values', () => { + expect(parseRetryAfterMs(null, 5000)).toBe(5000); + expect(parseRetryAfterMs('', 5000)).toBe(5000); + expect(parseRetryAfterMs('not-a-date-or-number', 5000)).toBe(5000); + }); + + it('treats zero seconds as retry-immediately', () => { + expect(parseRetryAfterMs('0', 5000)).toBe(0); + }); + + it('returns fallback for negative seconds', () => { + expect(parseRetryAfterMs('-5', 5000)).toBe(5000); + }); +}); + describe('replaceGitLabUrlType', () => { it('should replace with expected type', () => { expect( diff --git a/packages/integration/src/gitlab/GitLabIntegration.ts b/packages/integration/src/gitlab/GitLabIntegration.ts index 4f6985e344..4e7f98c649 100644 --- a/packages/integration/src/gitlab/GitLabIntegration.ts +++ b/packages/integration/src/gitlab/GitLabIntegration.ts @@ -124,6 +124,7 @@ export class GitLabIntegration implements ScmIntegration { // No more attempts left — propagate the network error. if (attempt++ >= maxRetries) throw e; await sleep(backoffDelay(attempt), abortSignal); + if (abortSignal?.aborted) throw e; continue; } @@ -137,22 +138,51 @@ export class GitLabIntegration implements ScmIntegration { return response; } - // Determine delay from Retry-After header if present, otherwise exponential backoff - const retryAfter = response.headers.get('Retry-After'); - const delay = retryAfter - ? parseInt(retryAfter, 10) * 1000 - : backoffDelay(attempt); + // Retry-After is either delay-seconds or an HTTP-date (RFC 9110 §10.2.3). + const delay = parseRetryAfterMs( + response.headers.get('Retry-After'), + backoffDelay(attempt), + ); // Release the underlying connection so it can be reused, since we're // about to discard this response in favor of a retry. await response.body?.cancel().catch(() => {}); await sleep(delay, abortSignal); + if (abortSignal?.aborted) return response; } }; } } +/** @internal */ +export function parseRetryAfterMs( + headerValue: string | null, + fallbackMs: number, +): number { + if (!headerValue) { + return fallbackMs; + } + + // delay-seconds per RFC 9110 is 1*DIGIT + if (/^\d+$/.test(headerValue)) { + return Number(headerValue) * 1000; + } + + // HTTP-dates (IMF-fixdate) always contain a comma, e.g. + // "Sun, 06 Nov 1994 08:49:37 GMT" — use that as a prerequisite + // to avoid Date.parse interpreting random strings as dates. + if (headerValue.includes(',')) { + const dateMs = Date.parse(headerValue); + if (Number.isFinite(dateMs)) { + const deltaMs = dateMs - Date.now(); + return deltaMs > 0 ? deltaMs : 0; + } + } + + return fallbackMs; +} + /** @internal */ export async function sleep( durationMs: number,