From 88aa1d80085350bbb7b97370cfd1e8dd701f18f1 Mon Sep 17 00:00:00 2001 From: John Oliver <1615532+johnoliver@users.noreply.github.com> Date: Thu, 4 Jun 2026 12:39:33 +0000 Subject: [PATCH] Address code review feedback on pagination implementation - Tighten rel regex with word boundary to prevent false positives (e.g., rel="nextsomething" no longer matches). - Use parsed.origin comparison in validatePaginationUrl to correctly handle explicit default ports (e.g., :443 for HTTPS). - Fix pagination safeguard tests to use same-origin URLs so they actually exercise the 1000-page limit instead of being rejected by origin validation on the first request. - Add test for rel="nextsomething" not matching. - Add test for explicit default port acceptance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- __tests__/distributors/adopt-installer.test.ts | 3 ++- __tests__/distributors/semeru-installer.test.ts | 3 ++- __tests__/distributors/temurin-installer.test.ts | 3 ++- __tests__/util.test.ts | 13 +++++++++++++ src/util.ts | 8 ++++---- 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/__tests__/distributors/adopt-installer.test.ts b/__tests__/distributors/adopt-installer.test.ts index 5442d3f6..55649255 100644 --- a/__tests__/distributors/adopt-installer.test.ts +++ b/__tests__/distributors/adopt-installer.test.ts @@ -170,7 +170,8 @@ describe('getAvailableVersions', () => { }); it('stops pagination after 1000 pages as a safeguard', async () => { - const nextPageUrl = 'https://example.com/releases?page=2'; + const nextPageUrl = + 'https://api.adoptopenjdk.net/v3/assets/version/%5B1.0,100.0%5D?page=2&page_size=20'; spyHttpClient.mockReturnValue({ statusCode: 200, headers: {link: `<${nextPageUrl}>; rel="next"`}, diff --git a/__tests__/distributors/semeru-installer.test.ts b/__tests__/distributors/semeru-installer.test.ts index ffef9530..03b08b72 100644 --- a/__tests__/distributors/semeru-installer.test.ts +++ b/__tests__/distributors/semeru-installer.test.ts @@ -113,7 +113,8 @@ describe('getAvailableVersions', () => { }); it('stops pagination after 1000 pages as a safeguard', async () => { - const nextPageUrl = 'https://example.com/releases?page=2'; + const nextPageUrl = + 'https://api.adoptopenjdk.net/v3/assets/version/%5B1.0,100.0%5D?page=2&page_size=20'; spyHttpClient.mockReturnValue({ statusCode: 200, headers: {link: `<${nextPageUrl}>; rel="next"`}, diff --git a/__tests__/distributors/temurin-installer.test.ts b/__tests__/distributors/temurin-installer.test.ts index 49a52a7d..161a2d08 100644 --- a/__tests__/distributors/temurin-installer.test.ts +++ b/__tests__/distributors/temurin-installer.test.ts @@ -127,7 +127,8 @@ describe('getAvailableVersions', () => { }); it('stops pagination after 1000 pages as a safeguard', async () => { - const nextPageUrl = 'https://example.com/releases?page=2'; + const nextPageUrl = + 'https://api.adoptium.net/v3/assets/version/%5B1.0,100.0%5D?page=2&page_size=20'; spyHttpClient.mockReturnValue({ statusCode: 200, headers: {link: `<${nextPageUrl}>; rel="next"`}, diff --git a/__tests__/util.test.ts b/__tests__/util.test.ts index d3746cc7..2df3bb94 100644 --- a/__tests__/util.test.ts +++ b/__tests__/util.test.ts @@ -108,6 +108,10 @@ describe('getNextPageUrlFromLinkHeader', () => { 'https://api.adoptium.net/v3/versions?page=3' ], [{link: '; rel="last"'}, null], + [ + {link: '; rel="nextsomething"'}, + null + ], [undefined, null] ])('returns %s -> %s', (headers, expected) => { expect(getNextPageUrlFromLinkHeader(headers)).toBe(expected); @@ -147,6 +151,15 @@ describe('validatePaginationUrl', () => { validatePaginationUrl('not-a-url', 'https://api.adoptium.net') ).toBe(false); }); + + it('accepts URL with explicit default port', () => { + expect( + validatePaginationUrl( + 'https://api.adoptium.net:443/v3/assets?page=2', + 'https://api.adoptium.net' + ) + ).toBe(true); + }); }); describe('getVersionFromFileContent', () => { diff --git a/src/util.ts b/src/util.ts index 3b507214..5fe84c52 100644 --- a/src/util.ts +++ b/src/util.ts @@ -227,7 +227,9 @@ export function getNextPageUrlFromLinkHeader( if (!urlMatch) continue; const params = linkValue.slice(urlMatch[0].length); - if (/;\s*rel="?next"?/i.test(params)) { + // Use word boundary to match "next" as a standalone relation type + // RFC 8288 allows space-separated relation types like rel="next prev" + if (/;\s*rel="?[^"]*\bnext\b/i.test(params)) { return urlMatch[1]; } } @@ -242,9 +244,7 @@ export function validatePaginationUrl( try { const parsed = new URL(url); const allowed = new URL(allowedOrigin); - return ( - parsed.protocol === allowed.protocol && parsed.host === allowed.host - ); + return parsed.origin === allowed.origin; } catch { return false; }