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>
This commit is contained in:
John Oliver 2026-06-04 12:39:33 +00:00
parent 284c50b083
commit 88aa1d8008
5 changed files with 23 additions and 7 deletions

View File

@ -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"`},

View File

@ -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"`},

View File

@ -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"`},

View File

@ -108,6 +108,10 @@ describe('getNextPageUrlFromLinkHeader', () => {
'https://api.adoptium.net/v3/versions?page=3'
],
[{link: '<https://example.com/last?page=5>; rel="last"'}, null],
[
{link: '<https://example.com/page?p=2>; 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', () => {

View File

@ -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;
}