From d2727f9fba53cba21cfb6fdc75b61a1b12c3301b Mon Sep 17 00:00:00 2001 From: Paul Herron Date: Thu, 4 Jun 2026 14:32:19 +0100 Subject: [PATCH 1/2] Improve validation for entityIssueDetails We are receiving URLs with alphanumeric reference numbers instead of page numbers. We do not know where these originate from at this stage but they are currently generating 500 errors, which is: - not a good experience for users - flooding Sentry with unhandled 500 errors This change updates Valibot validation to assert that a number is passed. When a number is not passed, then we return a 400 Bad Request error, which is more appropriate and provides better feedback to both us for debugging, and to the user. --- .../entityIssueDetails.middleware.js | 2 +- .../entityIssueDetails.middleware.test.js | 29 ++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/middleware/entityIssueDetails.middleware.js b/src/middleware/entityIssueDetails.middleware.js index 866431cd4..6f8aca6f3 100644 --- a/src/middleware/entityIssueDetails.middleware.js +++ b/src/middleware/entityIssueDetails.middleware.js @@ -26,7 +26,7 @@ export const IssueDetailsQueryParams = v.object({ dataset: v.string(), issue_type: v.string(), issue_field: v.string(), - pageNumber: v.optional(v.pipe(v.string(), v.transform(s => parseInt(s, 10)), v.minValue(1)), '1'), + pageNumber: v.optional(v.pipe(v.string(), v.transform(s => parseInt(s, 10)), v.number(), v.integer(), v.minValue(1)), '1'), resourceId: v.optional(v.string()) }) diff --git a/test/unit/middleware/entityIssueDetails.middleware.test.js b/test/unit/middleware/entityIssueDetails.middleware.test.js index 7e4704ca0..61dea1d9f 100644 --- a/test/unit/middleware/entityIssueDetails.middleware.test.js +++ b/test/unit/middleware/entityIssueDetails.middleware.test.js @@ -1,5 +1,7 @@ import { describe, it, vi, expect, beforeEach } from 'vitest' -import { getIssueDetails, getIssueField, prepareEntity, setRecordCount, show404ifNoIssues } from '../../../src/middleware/entityIssueDetails.middleware.js' +import * as v from 'valibot' +import { isValiError } from 'valibot' +import { IssueDetailsQueryParams, getIssueDetails, getIssueField, prepareEntity, setRecordCount, show404ifNoIssues } from '../../../src/middleware/entityIssueDetails.middleware.js' import { MiddlewareError } from '../../../src/utils/errors.js' vi.mock('../../../src/services/performanceDbApi.js') @@ -255,4 +257,29 @@ describe('issueDetails.middleware.js', () => { expect(next).toHaveBeenCalledWith() }) }) + + describe('IssueDetailsQueryParams', () => { + const validBaseParams = { + lpa: 'local-authority:LIV', + dataset: 'tree-preservation-zone', + issue_type: 'missing associated entity', + issue_field: 'tree-preservation-order' + } + + it('transforms a valid numeric pageNumber string to a number', () => { + const result = v.parse(IssueDetailsQueryParams, { ...validBaseParams, pageNumber: '2' }) + expect(result.pageNumber).toBe(2) + }) + + it('throws a ValiError when pageNumber is not numeric', () => { + let thrown + try { + v.parse(IssueDetailsQueryParams, { ...validBaseParams, pageNumber: 'TPO335.G19' }) + } catch (error) { + thrown = error + } + expect(thrown).toBeDefined() + expect(isValiError(thrown)).toBe(true) + }) + }) }) From a48902c163cbb568064f2be919388dc25552873d Mon Sep 17 00:00:00 2001 From: jpherr <57010814+jpherr@users.noreply.github.com> Date: Mon, 8 Jun 2026 15:13:27 +0100 Subject: [PATCH 2/2] Update src/middleware/entityIssueDetails.middleware.js Optimisation from code rabbit, where it identified cases where decimals would be accepted and routed to integers. e.g. 2.2 >> 2 Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/middleware/entityIssueDetails.middleware.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/middleware/entityIssueDetails.middleware.js b/src/middleware/entityIssueDetails.middleware.js index 6f8aca6f3..19ce19707 100644 --- a/src/middleware/entityIssueDetails.middleware.js +++ b/src/middleware/entityIssueDetails.middleware.js @@ -26,7 +26,17 @@ export const IssueDetailsQueryParams = v.object({ dataset: v.string(), issue_type: v.string(), issue_field: v.string(), - pageNumber: v.optional(v.pipe(v.string(), v.transform(s => parseInt(s, 10)), v.number(), v.integer(), v.minValue(1)), '1'), + pageNumber: v.optional( + v.pipe( + v.string(), + v.regex(/^\d+$/), + v.transform(s => Number(s)), + v.number(), + v.integer(), + v.minValue(1) + ), + '1' + ), resourceId: v.optional(v.string()) })