From a8f41ae99416eb3bd210d5cbfd48b1c7c92ee1fd Mon Sep 17 00:00:00 2001 From: Martin Alexander Kaminski Date: Mon, 9 Nov 2026 23:34:51 +0100 Subject: [PATCH 1/2] fix: check for negative limit, offset and page search param --- .../src/schema/books/resolvers/Query/searchBooks.ts | 10 ++++++++++ .../favorites/resolvers/Query/favoriteBooks.ts | 9 +++++++++ .../BookPagination/BookPagination.test.tsx | 12 ++++++++++++ .../app/components/BookPagination/BookPagination.tsx | 3 ++- frontend/app/hooks/useBookSearchParams.ts | 9 +++++++-- frontend/app/routes/profile.tsx | 3 ++- 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/backend/src/schema/books/resolvers/Query/searchBooks.ts b/backend/src/schema/books/resolvers/Query/searchBooks.ts index f45396c6..5f2a11e7 100644 --- a/backend/src/schema/books/resolvers/Query/searchBooks.ts +++ b/backend/src/schema/books/resolvers/Query/searchBooks.ts @@ -11,6 +11,7 @@ import { sql, type SQL, } from "drizzle-orm"; +import { GraphQLError } from "graphql/error"; import { db } from "~/db"; import { bookGenres, books, bookToGenres } from "~/db/schema"; import type { QueryResolvers } from "~/schema/types.generated"; @@ -20,6 +21,15 @@ export const searchBooks: NonNullable = async ( args, _ctx, ) => { + if ( + (args.limit != null && args.limit < 0) || + (args.offset != null && args.offset < 0) + ) { + throw new GraphQLError("Limit and offset cannot be negative.", { + extensions: { code: "BAD_USER_INPUT" }, + }); + } + const conditions: SQL[] = []; if (args.title && args.fuzzy) { diff --git a/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts b/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts index af54cc9d..54ef5aa1 100644 --- a/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts +++ b/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts @@ -16,6 +16,15 @@ export const favoriteBooks: NonNullable< ); } + if ( + (arg.limit != null && arg.limit < 0) || + (arg.offset != null && arg.offset < 0) + ) { + throw new GraphQLError("Limit and offset cannot be negative.", { + extensions: { code: "BAD_USER_INPUT" }, + }); + } + const [foundBooks, totalCount] = await Promise.all([ db.query.favoriteBooks.findMany({ where: { diff --git a/frontend/app/components/BookPagination/BookPagination.test.tsx b/frontend/app/components/BookPagination/BookPagination.test.tsx index 8aabf9fa..975e8c58 100644 --- a/frontend/app/components/BookPagination/BookPagination.test.tsx +++ b/frontend/app/components/BookPagination/BookPagination.test.tsx @@ -56,4 +56,16 @@ describe("BookPagination", () => { screen.getByRole("link", { name: "Go to next page" }), ).toHaveAttribute("href", "/?q=blade&genres=Fantasy&page=3"); }); + + it.each(["abc", "0", "-2", "1.5"])( + "treats page=%s as the first page", + (page) => { + render(); + + expect(currentPage()).toBe("1"); + expect( + screen.getByRole("link", { name: "Go to previous page" }), + ).toHaveAttribute("aria-disabled", "true"); + }, + ); }); diff --git a/frontend/app/components/BookPagination/BookPagination.tsx b/frontend/app/components/BookPagination/BookPagination.tsx index 569195aa..aa96f57b 100644 --- a/frontend/app/components/BookPagination/BookPagination.tsx +++ b/frontend/app/components/BookPagination/BookPagination.tsx @@ -8,12 +8,13 @@ import { PaginationNext, PaginationPrevious, } from "~/components/ui/Pagination"; +import { parsePageNum } from "~/hooks/useBookSearchParams"; const DISABLED_STYLE = "pointer-events-none opacity-50"; function BookPagination({ totalPages }: { totalPages: number }) { const [searchParams] = useSearchParams(); - const currentPage = parseInt(searchParams.get("page") ?? "1"); + const currentPage = parsePageNum(searchParams.get("page")); const isFirstPage = currentPage === 1; const isLastPage = currentPage === totalPages; diff --git a/frontend/app/hooks/useBookSearchParams.ts b/frontend/app/hooks/useBookSearchParams.ts index 41c610ba..a8f04d35 100644 --- a/frontend/app/hooks/useBookSearchParams.ts +++ b/frontend/app/hooks/useBookSearchParams.ts @@ -8,6 +8,11 @@ function parseCount(value: string | null) { return value && Number.isInteger(count) && count >= 0 ? count : undefined; } +function parsePageNum(value: string | null) { + const page = Number(value); + return Number.isInteger(page) && page >= 1 ? page : 1; +} + const FILTER_KEYS = [ "genres", "language", @@ -21,7 +26,7 @@ function useBookSearchParams() { const [searchParams] = useSearchParams(); const query = searchParams.get("q") ?? ""; - const page = parseInt(searchParams.get("page") ?? "1", 10); + const page = parsePageNum(searchParams.get("page")); const sort = parseBookSort(searchParams.get("sort")); // Only update filters when searchParams change to avoid unnecessary re-renders @@ -44,4 +49,4 @@ function useBookSearchParams() { return { query, page, filters, sort }; } -export { FILTER_KEYS, useBookSearchParams }; +export { FILTER_KEYS, parsePageNum, useBookSearchParams }; diff --git a/frontend/app/routes/profile.tsx b/frontend/app/routes/profile.tsx index a0962ce9..98c97f98 100644 --- a/frontend/app/routes/profile.tsx +++ b/frontend/app/routes/profile.tsx @@ -1,6 +1,7 @@ import { Link, Navigate, useSearchParams } from "react-router"; import { authClient } from "~/api/auth"; import { useFavoriteBooks } from "~/hooks/useFavoriteBooks"; +import { parsePageNum } from "~/hooks/useBookSearchParams"; import { BookCard } from "~/components/BookCard/BookCard"; import { BookPagination } from "~/components/BookPagination/BookPagination"; import { PageBreadcrumb } from "~/components/PageBreadcrumb/PageBreadcrumb"; @@ -25,7 +26,7 @@ export function meta() { export default function ProfilePage() { const { data: session, isPending } = authClient.useSession(); const [searchParams] = useSearchParams(); - const page = parseInt(searchParams.get("page") ?? "1", 10); + const page = parsePageNum(searchParams.get("page")); const { books, totalCount, topGenres, loading, error } = useFavoriteBooks({ enabled: !!session, From dedf71514d03ea09a391ef6631359262ec69a0fc Mon Sep 17 00:00:00 2001 From: Martin Alexander Kaminski Date: Wed, 11 Nov 2026 15:04:45 +0100 Subject: [PATCH 2/2] refactor: do not check for null when not needed --- backend/src/schema/books/resolvers/Query/searchBooks.ts | 5 +---- .../src/schema/favorites/resolvers/Query/favoriteBooks.ts | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/backend/src/schema/books/resolvers/Query/searchBooks.ts b/backend/src/schema/books/resolvers/Query/searchBooks.ts index bd398143..288be837 100644 --- a/backend/src/schema/books/resolvers/Query/searchBooks.ts +++ b/backend/src/schema/books/resolvers/Query/searchBooks.ts @@ -20,10 +20,7 @@ export const searchBooks: NonNullable = async ( _parent, args, ) => { - if ( - (args.limit != null && args.limit < 0) || - (args.offset != null && args.offset < 0) - ) { + if (args.limit < 0 || args.offset < 0) { throw new GraphQLError("Limit and offset cannot be negative.", { extensions: { code: "BAD_USER_INPUT" }, }); diff --git a/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts b/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts index 54ef5aa1..041355ed 100644 --- a/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts +++ b/backend/src/schema/favorites/resolvers/Query/favoriteBooks.ts @@ -16,10 +16,7 @@ export const favoriteBooks: NonNullable< ); } - if ( - (arg.limit != null && arg.limit < 0) || - (arg.offset != null && arg.offset < 0) - ) { + if (arg.limit < 0 || arg.offset < 0) { throw new GraphQLError("Limit and offset cannot be negative.", { extensions: { code: "BAD_USER_INPUT" }, });