Return 400 status codes when query parameters are bad (#678)

* add utility to validate year param, localize error message

* return 400 on invalid document query params

* return 400 on invalid committee year

* return 400 on requirements and governing

* Refactor functions

* use new functions where applicable

* throw if semester is faulty

* add pageSize helper

* use upper bound for page

* fix off by one errors

* use 1-indexed page on expenses

* remove transaction

src/lib/events/getEvents.ts

@@ -1,6 +1,5 @@
 import { BASIC_EVENT_FILTER } from "$lib/events/events";
 import type { Prisma, PrismaClient } from "@prisma/client";
-import * as m from "$paraglide/messages";
 
 type EventFilters = {
   tags?: string[];
@@ -111,19 +110,19 @@ export const getAllEvents = async (
       },
     ],
   };
-  const [events, count] = await prisma.$transaction(async (tx) => {
-    const events = tx.event.findMany({
+  // Don't run as transaction, a little read only data race is fine
+  const [events, count] = await Promise.all([
+    prisma.event.findMany({
       where,
       orderBy: {
         startDatetime: filters.pastEvents ? "desc" : "asc",
       },
-      skip: pageNumber * pageSize,
+      skip: Math.max(pageNumber - 1, 0) * pageSize,
       take: pageSize,
       include,
-    });
-    const count = tx.event.count({ where });
-    return [await events, await count];
-  });
+    }),
+    prisma.event.count({ where }),
+  ]);
   return [events, Math.ceil(count / pageSize)];
 };
 
@@ -140,11 +139,3 @@ export const getEvent = async (prisma: PrismaClient, slug: string) => {
 export type EventWithIncludes = NonNullable<
   Awaited<ReturnType<typeof getEvent>>
 >;
-
-export const getAndValidatePage = (url: URL) => {
-  const page = url.searchParams.get("page");
-  if (page && Number.isNaN(Number.parseInt(page))) {
-    throw new Error(m.events_errors_invalidPage());
-  }
-  return page ? Math.max(Number.parseInt(page) - 1, 0) : undefined;
-};

src/lib/news/getArticles.ts

@@ -106,19 +106,19 @@ export const getAllArticles = async (
         }
       : {}),
   };
-  const [articles, count] = await prisma.$transaction(async (tx) => {
-    const articles = tx.article.findMany({
+  // Don't run as transaction, a little read only data race is fine
+  const [articles, count] = await Promise.all([
+    prisma.article.findMany({
       where,
       orderBy: {
         publishedAt: "desc",
       },
-      skip: pageNumber * pageSize,
+      skip: Math.max(pageNumber - 1, 0) * pageSize,
       take: pageSize,
       include,
-    });
-    const count = tx.article.count({ where });
-    return await Promise.all([articles, count]);
-  });
+    }),
+    prisma.article.count({ where }),
+  ]);
   return [articles, Math.ceil(count / pageSize)];
 };
 

src/lib/utils/semesters.test.ts

@@ -1,18 +1,16 @@
 import {
   toString,
-  parseSemester,
   semesterRange,
   startDate,
   endDate,
+  parseSemesterFromString,
 } from "./semesters";
 
 import { describe, expect, it } from "vitest";
 
 describe("semester", () => {
   it("to string", () => expect(toString(4048)).toBe("VT 2024"));
 
-  it("parse semester", () => expect(parseSemester("VT 2024")).toBe(4048));
-
   it("range", () =>
     expect(semesterRange(4048, 4050)).toEqual([4048, 4049, 4050]));
 
@@ -28,3 +26,40 @@ describe("semester", () => {
   it("end date fall", () =>
     expect(endDate(4049)).toEqual(new Date(2025, 0, 1)));
 });
+
+describe("parseSemester", () => {
+  it("parses VT 2024", () =>
+    expect(parseSemesterFromString("VT 2024", () => new Error())).toBe(4048));
+  it("parses HT 2024", () =>
+    expect(parseSemesterFromString("HT 2024", () => new Error())).toBe(4049));
+  it("parses VT 2025", () =>
+    expect(parseSemesterFromString("VT 2025", () => new Error())).toBe(4050));
+  it("parses HT 2025", () =>
+    expect(parseSemesterFromString("HT 2025", () => new Error())).toBe(4051));
+  it("parses VT 2026", () =>
+    expect(parseSemesterFromString("VT 2026", () => new Error())).toBe(4052));
+  it("parses HT 2026", () =>
+    expect(parseSemesterFromString("HT 2026", () => new Error())).toBe(4053));
+
+  it("throws on invalid semester", () => {
+    expect(() =>
+      parseSemesterFromString("VT 202", () => new Error()),
+    ).toThrow();
+    expect(() =>
+      parseSemesterFromString("VT 2024a", () => new Error()),
+    ).toThrow();
+    expect(() =>
+      parseSemesterFromString("VT 2024 ", () => new Error()),
+    ).toThrow();
+    expect(() =>
+      parseSemesterFromString("XT 2024", () => new Error()),
+    ).toThrow();
+    expect(() =>
+      parseSemesterFromString("VT 2024 2024", () => new Error()),
+    ).toThrow();
+    expect(() => parseSemesterFromString("VT24", () => new Error())).toThrow();
+    expect(() => parseSemesterFromString("VT", () => new Error())).toThrow();
+    expect(() => parseSemesterFromString("2024", () => new Error())).toThrow();
+    expect(() => parseSemesterFromString("", () => new Error())).toThrow();
+  });
+});

src/lib/utils/semesters.ts

@@ -43,6 +43,31 @@ export const coveredSemesters = (
 export const toString = (semester: Semester): string =>
   semesterTerm(semester) + " " + semesterYear(semester);
 
-export const parseSemester = (string: string): Semester =>
-  (string.slice(0, 2) === "HT" ? 1 : 0) +
-  (parseInt(string.slice(3, 7)) ?? 2024) * 2;
+/**
+ * Parse a semester from a string.
+ * @param string The string to parse
+ * @param errorFunction The function to call if the string is invalid
+ * @returns The parsed semester
+ * @throws If the string is invalid
+ */
+export const parseSemesterFromString = (
+  string: string,
+  errorFunction: (() => Error) | (() => never),
+): Semester => {
+  const match = string.match(/^(VT|HT) (\d{4})$/);
+  if (match === null) {
+    throw errorFunction();
+  }
+  const [, term, year] = match;
+  if (term !== "VT" && term !== "HT") {
+    throw errorFunction();
+  }
+  if (year === undefined) {
+    throw errorFunction();
+  }
+  const yearInt = parseInt(year);
+  if (isNaN(yearInt)) {
+    throw errorFunction();
+  }
+  return semesterFromYearAndTerm(yearInt, term);
+};

src/lib/utils/url.server.ts

@@ -0,0 +1,109 @@
+import { error } from "@sveltejs/kit";
+import * as m from "$paraglide/messages";
+import {
+  dateToSemester,
+  parseSemesterFromString,
+  type Semester,
+} from "$lib/utils/semesters";
+
+interface Options {
+  fallbackValue: number;
+  lowerBound: number;
+  upperBound: number;
+  errorMessage: string;
+}
+
+/**
+ * Get `year` from the URL search params or throw a Svelte error if `year` is invalid.
+ * If `year` is not set, the default is the current year.
+ * @param url The URL object
+ * @param options The options for the function. Default is `{ fallbackValue: new Date().getFullYear(), lowerBound: 1982, upperBound: new Date().getFullYear(), errorMessage: m.error_invalid_year() }`
+ * @returns `year`, or the {@link fallbackValue} if `year` is not set
+ * @throws Svelte error if `year` is invalid
+ */
+export const getYearOrThrowSvelteError = (
+  url: URL,
+  options?: Partial<Options>,
+) => {
+  return getIntegerParamOrThrowSvelteError(url, "year", {
+    fallbackValue: options?.fallbackValue ?? new Date().getFullYear(),
+    lowerBound: options?.lowerBound ?? 1982,
+    upperBound: options?.upperBound ?? new Date().getFullYear(),
+    errorMessage: options?.errorMessage ?? m.error_invalid_year(),
+  });
+};
+
+/**
+ * Get `page` from the URL search params or throw a Svelte error if `page` is invalid.
+ * If `page` is not set, the default is 1.
+ * @param url The URL object
+ * @param options The options for the function. Default is `{ fallbackValue: 1, lowerBound: 1, upperBound: Number.MAX_SAFE_INTEGER, errorMessage: m.error_invalid_page() }`
+ * @returns `page`, or the {@link fallbackValue} if `page` is not set
+ * @throws Svelte error if `page` is invalid
+ */
+export const getPageOrThrowSvelteError = (
+  url: URL,
+  options?: Partial<Options>,
+) => {
+  return getIntegerParamOrThrowSvelteError(url, "page", {
+    fallbackValue: options?.fallbackValue ?? 1,
+    lowerBound: options?.lowerBound ?? 1,
+    upperBound: options?.upperBound ?? Number.MAX_SAFE_INTEGER,
+    errorMessage: options?.errorMessage ?? m.error_invalid_page(),
+  });
+};
+
+/**
+ * Get `pageSize` from the URL search params or throw a Svelte error if `pageSize` is invalid
+ * If `pageSize` is not set, the default is 10.
+ * @param url The URL object
+ * @param options The options for the function. Default is `{ fallbackValue: 10, lowerBound: 1, upperBound: 100, errorMessage: m.error_invalid_page_size() }`
+ * @returns `pageSize`, or the {@link fallbackValue} if `pageSize` is not set
+ * @throws Svelte error if `pageSize` is invalid
+ */
+export const getPageSizeOrThrowSvelteError = (
+  url: URL,
+  options?: Partial<Options>,
+) => {
+  return getIntegerParamOrThrowSvelteError(url, "pageSize", {
+    fallbackValue: options?.fallbackValue ?? 10,
+    lowerBound: options?.lowerBound ?? 1,
+    upperBound: options?.upperBound ?? 100,
+    errorMessage: options?.errorMessage ?? m.error_invalid_page_size(),
+  });
+};
+
+/**
+ * Get an integer parameter from the URL object or throw a Svelte error if the parameter is invalid.
+ * If the parameter is not set, the default is the fallback value.
+ * @param url The URL object
+ * @param param The parameter to get
+ * @param options The options for the function.
+ * @returns The parameter, or the {@link fallbackValue} if the parameter is not set
+ * @throws Svelte error if the parameter is invalid
+ */
+export const getIntegerParamOrThrowSvelteError = (
+  url: URL,
+  param: string,
+  options: Options,
+): number => {
+  const value = parseInt(
+    url.searchParams.get(param) || options.fallbackValue.toString(),
+  );
+  if (isNaN(value)) throw error(400, options.errorMessage);
+  if (value < options.lowerBound || value > options.upperBound)
+    throw error(400, options.errorMessage);
+  return value;
+};
+
+export const getSemesterOrThrowSvelteError = (
+  url: URL,
+  fallbackValue = dateToSemester(new Date()),
+): Semester => {
+  const semester = url.searchParams.get("semester");
+  if (semester === null) return fallbackValue;
+  const parsed = parseSemesterFromString(semester, () =>
+    error(400, m.error_invalid_semester()),
+  );
+  return parsed;
+};

src/routes/(app)/api/members/phadders/+server.ts

@@ -2,20 +2,16 @@ import { error } from "@sveltejs/kit";
 import type { RequestHandler } from "./$types";
 import { searchForMembers } from "../membersSearch";
 import { phadderMandateFilter } from "$lib/nollning/groups/types";
+import { getYearOrThrowSvelteError } from "$lib/utils/url.server";
 
 // Like member search but filters on members who were phadders during the given year
 export const GET: RequestHandler = async ({ locals, url }) => {
   const { prisma } = locals;
   const search = url.searchParams.get("search")?.toLowerCase();
-  const year = Number.parseInt(
-    url.searchParams.get("year") ?? new Date().getFullYear().toString(),
-  );
+  const year = getYearOrThrowSvelteError(url);
   if (search == undefined || search.length === 0) {
     throw error(400, "you need to provide a search value");
   }
-  if (Number.isNaN(year)) {
-    throw error(400, "invalid year");
-  }
 
   return new Response(
     JSON.stringify(

src/routes/(app)/committees/committee.server.ts

@@ -6,24 +6,25 @@ import { zod } from "sveltekit-superforms/adapters";
 import { message, superValidate, withFiles } from "sveltekit-superforms/server";
 import { updateSchema } from "./types";
 import { updateMarkdown } from "$lib/news/markdown/mutations.server";
+import { getYearOrThrowSvelteError } from "$lib/utils/url.server";
 
-export const getYear = (url: URL) => {
-  const yearQuery = url.searchParams.get("year");
-  const parsedYear = parseInt(yearQuery ?? "");
-  const year = isNaN(parsedYear) ? new Date().getFullYear() : parsedYear;
-  return year;
-};
 /**
+ * Load all data that every committee load function needs
+ * @param prisma The Prisma client
  * @param shortName The committee's short name
- * @param year The year to load the committee for, defaults to current year
+ * @param url The URL object
  * @returns All data that the every committee load function needs
  */
 export const committeeLoad = async (
   prisma: PrismaClient,
   shortName: string,
   url: URL,
 ) => {
-  const year = getYear(url);
+  const currentYear = new Date().getFullYear();
+  // Allow to see committees from 1982 to the NEXT year
+  const year = getYearOrThrowSvelteError(url, {
+    upperBound: currentYear + 1,
+  });
 
   const firstDayOfYear = new Date(`${year}-01-01`);
   const lastDayOfYear = new Date(`${year}-12-31`);

src/routes/(app)/committees/nollu/+page.server.ts

@@ -1,9 +1,14 @@
 import type { PageServerLoad } from "./$types";
-import { committeeActions, committeeLoad, getYear } from "../committee.server";
+import { committeeActions, committeeLoad } from "../committee.server";
+import { getYearOrThrowSvelteError } from "$lib/utils/url.server";
 
 export const load: PageServerLoad = async ({ locals, url }) => {
   const { prisma } = locals;
-  const year = getYear(url);
+  const currentYear = new Date().getFullYear();
+  // Allow to see committees from 1982 to the NEXT year
+  const year = getYearOrThrowSvelteError(url, {
+    upperBound: currentYear + 1,
+  });
   const phadderGroups = prisma.phadderGroup.findMany({
     where: {
       year,