diff --git a/README.md b/README.md index 2597c07..6d41658 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,10 @@ To support larger datasets, searching, filtering, and pagination are handled by The city dataset provides a larger set of records for evaluating database queries and scalability. +For both `countries` and `cities`, the backend caps GraphQL `limit` at 100 results per request (default 20) to prevent oversized queries. Search input escapes `%`, `_`, and backslashes before using `ILIKE`, so those characters are matched literally rather than acting as SQL wildcards. Queries still use parameterized values. + +Negative `offset` values are treated as `0` to prevent database errors. Input validation is covered by unit tests for pagination limits, search escaping, and UUID validation. GraphQL API tests also verify that negative offsets are handled correctly and that percent signs, underscores, and backslashes are treated as literal search characters. + For deployment details, see [Deployment on the NTNU VM](docs/backendDeployment.md). ## Pagination and Caching @@ -212,11 +216,14 @@ All AI-generated suggestions have been reviewed and evaluated before being appli ```text backend/ ├── src/ -│ ├── db.ts # PostgreSQL connection -│ ├── schema.graphql # GraphQL schema -│ ├── server.ts # Apollo Server and resolvers -│ ├── seed.ts # Database seeding -│ └── checkDb.ts # Database connection check +│ ├── app.ts # GraphQL resolvers used by API tests +│ ├── checkDb.ts # Database connection check +│ ├── db.ts # PostgreSQL connection +│ ├── queryInput.ts # Pagination and search input validation +│ ├── schema.graphql # GraphQL schema +│ ├── seed.ts # Database seeding +│ ├── server.ts # Apollo Server and production resolvers +│ └── userId.ts # UUID validation data/ ├── cities.json diff --git a/backend/src/app.ts b/backend/src/app.ts index 1864908..4909f59 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -1,5 +1,6 @@ import { readFile } from 'node:fs/promises'; import { ApolloServer } from '@apollo/server'; +import { escapeLikeSearch, normalizeLimit, normalizeOffset } from './queryInput.ts'; export interface Db { query: (sql: string, params?: unknown[]) => Promise<{ rows: Record[] }>; @@ -27,15 +28,16 @@ export function createResolvers(pool: Db) { }, countries: async (_parent, args) => { + // E'\\\\' makes backslash the ILIKE escape character. const filters = ` - WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%') + WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%' ESCAPE E'\\\\') AND ($2::text IS NULL OR continent = $2) `; - const filterValues = [args.search ?? null, args.continent ?? null]; + const filterValues = [escapeLikeSearch(args.search), args.continent ?? null]; const itemsResult = await pool.query( `SELECT ${COUNTRY_COLUMNS} FROM country ${filters} ORDER BY name LIMIT $3 OFFSET $4`, - [...filterValues, args.limit, args.offset] + [...filterValues, normalizeLimit(args.limit), normalizeOffset(args.offset)] ); const countResult = await pool.query( `SELECT count(*) FROM country ${filters}`, @@ -57,14 +59,14 @@ export function createResolvers(pool: Db) { cities: async (_parent, args) => { const filters = ` - WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%') + WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%' ESCAPE E'\\\\') AND ($2::text IS NULL OR country_code = $2) `; - const filterValues = [args.search ?? null, args.countryCode ?? null]; + const filterValues = [escapeLikeSearch(args.search), args.countryCode ?? null]; const itemsResult = await pool.query( `SELECT ${CITY_COLUMNS} FROM city ${filters} ORDER BY population DESC LIMIT $3 OFFSET $4`, - [...filterValues, args.limit, args.offset] + [...filterValues, normalizeLimit(args.limit), normalizeOffset(args.offset)] ); const countResult = await pool.query(`SELECT count(*) FROM city ${filters}`, filterValues); diff --git a/backend/src/queryInput.ts b/backend/src/queryInput.ts new file mode 100644 index 0000000..da4b9a1 --- /dev/null +++ b/backend/src/queryInput.ts @@ -0,0 +1,18 @@ +/** Maximum page size to prevent a single request from fetching an unbounded result set. */ +export const MAX_PAGE_LIMIT = 100; +const DEFAULT_PAGE_LIMIT = 20; + +/** Keep page sizes between 1 and MAX_PAGE_LIMIT (GraphQL defaults to 20). */ +export function normalizeLimit(limit: number | null | undefined): number { + return Math.min(Math.max(limit ?? DEFAULT_PAGE_LIMIT, 1), MAX_PAGE_LIMIT); +} + +/** Treat a negative offset as the first page. */ +export function normalizeOffset(offset: number | null | undefined): number { + return Math.max(offset ?? 0, 0); +} + +/** Treat %, _ and backslash as literal characters inside a PostgreSQL ILIKE pattern. */ +export function escapeLikeSearch(search: string | null | undefined): string | null { + return search == null ? null : search.replace(/[\\%_]/g, '\\$&'); +} diff --git a/backend/src/server.ts b/backend/src/server.ts index e93bdef..14de9ee 100644 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -4,6 +4,8 @@ import { ApolloServer } from '@apollo/server'; import { startStandaloneServer } from '@apollo/server/standalone'; import { GraphQLError } from 'graphql'; import { pool } from './db.ts'; +import { escapeLikeSearch, normalizeLimit, normalizeOffset } from './queryInput.ts'; +import { parseUserId } from './userId.ts'; const PORT = Number(process.env.PORT ?? 3001); const HOST = process.env.HOST ?? '0.0.0.0'; @@ -34,8 +36,6 @@ const SORT_ORDERS = new Map([ ]); const MAX_RANDOM_COUNTRIES = 50; -const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - const typeDefs = await readFile(new URL('./schema.graphql', import.meta.url), 'utf-8'); const resolvers = { @@ -59,9 +59,9 @@ const resolvers = { const sortExpression = args.sortBy === 'NAME' ? 'name COLLATE "C"' : sortColumn; const sortOrder = SORT_ORDERS.get(args.sortOrder) ?? 'ASC'; - // Bind search text as a value; ILIKE provides case-insensitive substring matching. + // E'\\\\' selects backslash as the ILIKE escape character; % and _ are escaped in input. const filters = ` - WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%') + WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%' ESCAPE E'\\\\') AND ($2::text IS NULL OR continent = $2) AND ($3::text IS NULL OR $3 = ANY(languages)) AND ( @@ -95,7 +95,7 @@ const resolvers = { `; const filterValues = [ - args.search ?? null, + escapeLikeSearch(args.search), args.continent ?? null, args.language ?? null, args.status ?? null, @@ -109,7 +109,7 @@ const resolvers = { ${filters} ORDER BY ${sortExpression} ${sortOrder} NULLS LAST, code ASC LIMIT $7 OFFSET $8`, - [...filterValues, args.limit ?? 20, args.offset ?? 0] + [...filterValues, normalizeLimit(args.limit), normalizeOffset(args.offset)] ); // Count all filtered matches separately so totalCount does not shrink with each page. @@ -144,9 +144,9 @@ const resolvers = { const sortColumn = CITY_SORT_COLUMNS.get(args.sortBy) ?? 'population'; const sortOrder = SORT_ORDERS.get(args.sortOrder) ?? 'DESC'; - // ILIKE makes city-name matching case-insensitive; values remain bound parameters. + // E'\\\\' selects backslash as the ILIKE escape character; % and _ are escaped in input. const filters = ` - WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%') + WHERE ($1::text IS NULL OR name ILIKE '%' || $1 || '%' ESCAPE E'\\\\') AND ($2::text IS NULL OR country_code = $2) AND ( $3::text IS NULL @@ -156,7 +156,11 @@ const resolvers = { ) `; - const filterValues = [args.search ?? null, args.countryCode ?? null, args.continent ?? null]; + const filterValues = [ + escapeLikeSearch(args.search), + args.countryCode ?? null, + args.continent ?? null, + ]; // Apply pagination after filtering/sorting, and keep the total independent of the page. const itemsResult = await pool.query( @@ -165,7 +169,7 @@ const resolvers = { ${filters} ORDER BY ${sortColumn} ${sortOrder} NULLS LAST, id ASC LIMIT $4 OFFSET $5`, - [...filterValues, args.limit ?? 20, args.offset ?? 0] + [...filterValues, normalizeLimit(args.limit), normalizeOffset(args.offset)] ); // Return the filtered total, not just the current LIMIT/OFFSET page. @@ -249,8 +253,7 @@ const server = new ApolloServer({ typeDefs, resolvers }); const { url } = await startStandaloneServer(server, { listen: { port: PORT, host: HOST }, context: async ({ req }) => { - const header = req.headers['x-user-id']; - const userId = typeof header === 'string' && UUID_PATTERN.test(header) ? header : null; + const userId = parseUserId(req.headers['x-user-id']); return { userId }; }, diff --git a/backend/src/userId.ts b/backend/src/userId.ts new file mode 100644 index 0000000..7ccf7cc --- /dev/null +++ b/backend/src/userId.ts @@ -0,0 +1,6 @@ +const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** Validate the anonymous browser ID before using it in SQL queries. */ +export function parseUserId(header: string | string[] | undefined): string | null { + return typeof header === 'string' && UUID_PATTERN.test(header) ? header : null; +} diff --git a/backend/test/graphql.test.ts b/backend/test/graphql.test.ts index 086b656..62b8e90 100644 --- a/backend/test/graphql.test.ts +++ b/backend/test/graphql.test.ts @@ -1,9 +1,11 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { createServer } from '../src/app.ts'; -import { countryFixtures, createTestDb } from './testDb.ts'; +import { cityFixtures, countryFixtures, createTestDb } from './testDb.ts'; +import { MAX_PAGE_LIMIT } from '../src/queryInput.ts'; let db: Awaited>; let server: ReturnType; +const observedPageLimits: number[] = []; // eslint-disable-next-line @typescript-eslint/no-explicit-any const data = (result: { data?: unknown }) => result.data as any; @@ -32,7 +34,14 @@ async function countries(variables: Record = {}) { beforeAll(async () => { db = await createTestDb(); - server = createServer(db); + server = createServer({ + query: async (sql, params) => { + if (sql.includes(' LIMIT ')) { + observedPageLimits.push(Number(params?.at(-2))); + } + return db.query(sql, params); + }, + }); await server.start(); }); @@ -102,6 +111,21 @@ describe('countries query', () => { expect(result).toEqual({ items: [], totalCount: 0 }); }); + it('caps excessively large country page requests on the server', async () => { + await countries({ limit: 100000 }); + expect(observedPageLimits.at(-1)).toBe(MAX_PAGE_LIMIT); + }); + + it('searches for literal percent and underscore characters in countries', async () => { + const percent = await countries({ search: '%', limit: 20 }); + expect(percent.items.map((c) => c.name)).toEqual(['Rate 100%']); + expect(percent.totalCount).toBe(1); + + const underscore = await countries({ search: '_', limit: 20 }); + expect(underscore.items.map((c) => c.name)).toEqual(['Code_A']); + expect(underscore.totalCount).toBe(1); + }); + it('returns nothing for a search without matches', async () => { expect(await countries({ search: 'zzzz', limit: 20, offset: 0 })).toEqual({ items: [], @@ -151,8 +175,97 @@ describe('cities query', () => { expect(page.items).toEqual([{ id: '2', name: 'Bergen', country: { code: 'XA' } }]); }); + it('caps excessively large city page requests on the server', async () => { + const result = await run(CITIES, { limit: 100000 }); + expect(result.errors).toBeUndefined(); + expect(observedPageLimits.at(-1)).toBe(MAX_PAGE_LIMIT); + }); + + it('treats percent and underscore as literal characters in city searches', async () => { + // Add both literal matches and lookalikes that must not be included. + const names = ['100% City', '100X City', 'A_City', 'AXCity']; + for (const [index, name] of names.entries()) { + await db.query( + `INSERT INTO city (id, name, latitude, longitude, country_code, population) + VALUES ($1, $2, 0, 0, 'XA', 1)`, + [index + 4, name] + ); + } + + try { + for (const [search, expectedName] of [ + ['%', '100% City'], + ['_', 'A_City'], + ]) { + const result = await run(CITIES, { search }); + expect(result.errors).toBeUndefined(); + expect(data(result).cities.items.map((city: { name: string }) => city.name)).toEqual([ + expectedName, + ]); + expect(data(result).cities.totalCount).toBe(1); + } + } finally { + await db.query('DELETE FROM city WHERE id >= $1', [4]); + } + }); + it('searches case-insensitively', async () => { const result = await run(CITIES, { search: 'OSL', limit: 10, offset: 0 }); expect(data(result).cities.items.map((c: { name: string }) => c.name)).toEqual(['Oslo']); }); }); +describe('GraphQL input validation', () => { + it('treats negative offsets as zero', async () => { + expect(await countries({ limit: 3, offset: -1 })).toEqual( + await countries({ limit: 3, offset: 0 }) + ); + + const query = `query($offset: Int) { + cities(limit: 2, offset: $offset) { + items { id } + totalCount + } + }`; + + const negative = await run(query, { offset: -1 }); + const zero = await run(query, { offset: 0 }); + + expect(negative.errors).toBeUndefined(); + expect(data(negative)).toEqual(data(zero)); + }); + + it('treats backslashes as literal search characters', async () => { + const originalCountry = countryFixtures.find((country) => country.code === 'XA'); + const originalCity = cityFixtures.find((city) => city.id === 1); + + if (!originalCountry || !originalCity) { + throw new Error('Missing test fixtures'); + } + await db.query('UPDATE country SET name = $1 WHERE code = $2', [String.raw`Back\Path`, 'XA']); + + await db.query('UPDATE city SET name = $1 WHERE id = $2', [String.raw`Back\Town`, 1]); + + try { + const countryResult = await countries({ search: '\\' }); + + expect(countryResult.items.map((country) => country.name)).toEqual([String.raw`Back\Path`]); + + const cityResult = await run( + `query($search: String) { + cities(search: $search) { + items { name } + totalCount + } + }`, + { search: '\\' } + ); + + expect(cityResult.errors).toBeUndefined(); + expect(data(cityResult).cities.items).toEqual([{ name: String.raw`Back\Town` }]); + } finally { + await db.query('UPDATE country SET name = $1 WHERE code = $2', [originalCountry.name, 'XA']); + + await db.query('UPDATE city SET name = $1 WHERE id = $2', [originalCity.name, 1]); + } + }); +}); diff --git a/backend/test/queryInput.test.ts b/backend/test/queryInput.test.ts new file mode 100644 index 0000000..7b2df36 --- /dev/null +++ b/backend/test/queryInput.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { + escapeLikeSearch, + normalizeLimit, + normalizeOffset, + MAX_PAGE_LIMIT, +} from '../src/queryInput.ts'; +import { parseUserId } from '../src/userId.ts'; + +describe('Query input and UUID helper functions', () => { + it('limits page sizes', () => { + expect(normalizeLimit(undefined)).toBe(20); + expect(normalizeLimit(0)).toBe(1); + expect(normalizeLimit(-10)).toBe(1); + expect(normalizeLimit(25)).toBe(25); + expect(normalizeLimit(100000)).toBe(MAX_PAGE_LIMIT); + }); + + it('prevents negative offsets', () => { + expect(normalizeOffset(undefined)).toBe(0); + expect(normalizeOffset(-1)).toBe(0); + expect(normalizeOffset(10)).toBe(10); + }); + + it('escapes SQL LIKE special characters', () => { + expect(escapeLikeSearch('%')).toBe('\\%'); + expect(escapeLikeSearch('_')).toBe('\\_'); + expect(escapeLikeSearch(String.raw`a\b`)).toBe(String.raw`a\\b`); + expect(escapeLikeSearch(null)).toBeNull(); + }); + + it('accepts valid UUIDs and rejects malformed ones', () => { + const valid = '11111111-2222-4333-8444-555555555555'; + + expect(parseUserId(valid)).toBe(valid); + expect(parseUserId('11111111-2222-3333-4444-5555-666666666666')).toBeNull(); + expect(parseUserId(undefined)).toBeNull(); + expect(parseUserId([valid])).toBeNull(); + }); +}); diff --git a/backend/test/testDb.ts b/backend/test/testDb.ts index 3f3ab44..3615a1e 100644 --- a/backend/test/testDb.ts +++ b/backend/test/testDb.ts @@ -18,10 +18,14 @@ export const countryFixtures = Array.from({ length: 25 }, (_, i) => { }; }); -// Names that exercise case-insensitive search. +// Names that exercise case-insensitive and literal wildcard searches. countryFixtures[0].name = 'Norway'; countryFixtures[1].name = 'Northland'; countryFixtures[2].name = 'Aland'; +countryFixtures[3].name = 'Rate 100%'; +countryFixtures[4].name = 'Rate 100X'; +countryFixtures[5].name = 'Code_A'; +countryFixtures[6].name = 'CodeXA'; export const cityFixtures = [ { id: 1, name: 'Oslo', latitude: 59.9, longitude: 10.7, country_code: 'XA', population: 700000 }, @@ -67,6 +71,7 @@ export async function createTestDb(): Promise Promise [c.code, c.name, c.capital, c.continent, c.languages, c.population, c.area, c.flagUrl] ); } + for (const c of cityFixtures) { await db.query( `INSERT INTO city (id, name, latitude, longitude, country_code, population) diff --git a/src/App.test.tsx b/src/App.test.tsx index 365964a..456d0c5 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -172,7 +172,13 @@ describe('App', () => { await waitFor(() => expect(searchResult).toHaveBeenCalledOnce(), { timeout: 2000 }); await waitFor(() => expect(screen.getAllByRole('listitem')).toHaveLength(1)); - expect(screen.getByRole('status')).toHaveTextContent('Showing 1 of 1'); + await waitFor(() => { + expect( + screen + .getAllByRole('status') + .some((element) => /Showing\s+1\s+of\s+1/.test(element.textContent ?? '')) + ).toBe(true); + }); }); it('trims the search text and treats an empty search as no filter', async () => {