From b0930b8da257842564f07acdefbfcbe9cd8bd363 Mon Sep 17 00:00:00 2001 From: Djalim Simaila Date: Sun, 26 Apr 2026 19:01:53 +0200 Subject: [PATCH] fix: correct handler bugs exposed by test suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ajustements [numEtud]/[idUE]: fix .where() missing and() — PUT/DELETE were applying only numEtud condition, modifying all rows for a student - modules/users/enseignements POST: add try/catch, return 500 on invalid JSON - modules/[idModule] PUT: add try/catch + type check on nom (string required) - modules POST: add .trim() check to reject whitespace-only id/nom - users POST: add .trim() check to reject whitespace-only id/nom/prenom - ues POST: add .trim() check to reject whitespace-only nom - notes POST: add type check (typeof number) and bounds check (0 ≤ note ≤ 20) - ue-modules POST: add coeff >= 0 validation Update robustness tests to reflect fixed behavior (remove [BUG] labels, replace assertRejects with status code assertions). --- routes/(apps)/admin/api/enseignements.ts | 11 +- routes/(apps)/admin/api/modules.ts | 9 +- routes/(apps)/admin/api/modules/[idModule].ts | 11 +- routes/(apps)/admin/api/users.ts | 13 +- .../notes/api/ajustements/[numEtud]/[idUE].ts | 8 +- routes/(apps)/notes/api/notes.ts | 6 + routes/(apps)/notes/api/ue-modules.ts | 6 + routes/(apps)/notes/api/ues.ts | 2 +- tests/e2e/robustness_test.ts | 199 ++++++++++-------- 9 files changed, 166 insertions(+), 99 deletions(-) diff --git a/routes/(apps)/admin/api/enseignements.ts b/routes/(apps)/admin/api/enseignements.ts index 06408bc..cb2ab47 100644 --- a/routes/(apps)/admin/api/enseignements.ts +++ b/routes/(apps)/admin/api/enseignements.ts @@ -26,11 +26,12 @@ export const handler: Handlers = { return FORBIDDEN; } - const body: { - idProf: string; - idModule: string; - idPromo: string; - } = await request.json(); + let body: { idProf: string; idModule: string; idPromo: string }; + try { + body = await request.json(); + } catch { + return new Response(null, { status: 500 }); + } if (!body.idProf || !body.idModule || !body.idPromo) { return new Response(null, { status: 400 }); diff --git a/routes/(apps)/admin/api/modules.ts b/routes/(apps)/admin/api/modules.ts index 2cb2fe7..bdb37b9 100644 --- a/routes/(apps)/admin/api/modules.ts +++ b/routes/(apps)/admin/api/modules.ts @@ -31,9 +31,14 @@ export const handler: Handlers = { return new Response(null, { status: 403 }); } - const body: { id: string; nom: string } = await request.json(); + let body: { id: string; nom: string }; + try { + body = await request.json(); + } catch { + return new Response(null, { status: 500 }); + } - if (!body.id || !body.nom) { + if (!body.id || !body.id.trim() || !body.nom || !body.nom.trim()) { return new Response(null, { status: 400 }); } diff --git a/routes/(apps)/admin/api/modules/[idModule].ts b/routes/(apps)/admin/api/modules/[idModule].ts index 6f17dfe..d3d9467 100644 --- a/routes/(apps)/admin/api/modules/[idModule].ts +++ b/routes/(apps)/admin/api/modules/[idModule].ts @@ -33,7 +33,16 @@ export const handler: Handlers = { request: Request, context: FreshContext, ): Promise { - const body: { nom: string } = await request.json(); + let body: { nom: string }; + try { + body = await request.json(); + } catch { + return new Response(null, { status: 500 }); + } + + if (typeof body.nom !== "string") { + return new Response(null, { status: 400 }); + } const [updated] = await db .update(modules) diff --git a/routes/(apps)/admin/api/users.ts b/routes/(apps)/admin/api/users.ts index d2fbd56..61317d7 100644 --- a/routes/(apps)/admin/api/users.ts +++ b/routes/(apps)/admin/api/users.ts @@ -27,10 +27,17 @@ export const handler: Handlers = { request: Request, _context: FreshContext, ): Promise { - const body: { id: string; nom: string; prenom: string; idRole: number } = - await request.json(); + let body: { id: string; nom: string; prenom: string; idRole: number }; + try { + body = await request.json(); + } catch { + return new Response(null, { status: 500 }); + } - if (!body.id || !body.nom || !body.prenom) { + if ( + !body.id || !body.id.trim() || !body.nom || !body.nom.trim() || + !body.prenom || !body.prenom.trim() + ) { return new Response(null, { status: 400 }); } diff --git a/routes/(apps)/notes/api/ajustements/[numEtud]/[idUE].ts b/routes/(apps)/notes/api/ajustements/[numEtud]/[idUE].ts index c9b3ab0..a165f44 100644 --- a/routes/(apps)/notes/api/ajustements/[numEtud]/[idUE].ts +++ b/routes/(apps)/notes/api/ajustements/[numEtud]/[idUE].ts @@ -2,7 +2,7 @@ import { FreshContext, Handlers } from "$fresh/server.ts"; import { db } from "$root/databases/db.ts"; import { ajustements } from "$root/databases/schema.ts"; import { AuthenticatedState } from "$root/defaults/interfaces.ts"; -import { eq } from "npm:drizzle-orm@0.45.2"; +import { and, eq } from "npm:drizzle-orm@0.45.2"; const NOT_FOUND = new Response( JSON.stringify({ error: "Ajustement introuvable" }), @@ -31,7 +31,7 @@ export const handler: Handlers = { const ajustement = await db .select() .from(ajustements) - .where(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE)) + .where(and(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE))) .then((rows) => rows[0] ?? null); if (!ajustement) return NOT_FOUND; @@ -69,7 +69,7 @@ export const handler: Handlers = { const [updated] = await db .update(ajustements) .set({ valeur: body.valeur }) - .where(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE)) + .where(and(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE))) .returning(); if (!updated) return NOT_FOUND; @@ -97,7 +97,7 @@ export const handler: Handlers = { const [deleted] = await db .delete(ajustements) - .where(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE)) + .where(and(eq(ajustements.numEtud, numEtud), eq(ajustements.idUE, idUE))) .returning(); if (!deleted) return NOT_FOUND; diff --git a/routes/(apps)/notes/api/notes.ts b/routes/(apps)/notes/api/notes.ts index 22d387e..b7fd580 100644 --- a/routes/(apps)/notes/api/notes.ts +++ b/routes/(apps)/notes/api/notes.ts @@ -49,6 +49,12 @@ export const handler: Handlers = { }); } + if (typeof note !== "number" || note < 0 || note > 20) { + return new Response("Champ 'note' doit être un nombre entre 0 et 20", { + status: 400, + }); + } + const result = await db.insert(notes).values({ note, numEtud, idModule }) .returning(); diff --git a/routes/(apps)/notes/api/ue-modules.ts b/routes/(apps)/notes/api/ue-modules.ts index 8cd48bc..1a825a6 100644 --- a/routes/(apps)/notes/api/ue-modules.ts +++ b/routes/(apps)/notes/api/ue-modules.ts @@ -47,6 +47,12 @@ export const handler: Handlers = { ); } + if (typeof coeff !== "number" || coeff < 0) { + return new Response("Champ 'coeff' doit être un nombre >= 0", { + status: 400, + }); + } + const result = await db.insert(ueModules).values({ idModule, idUE, diff --git a/routes/(apps)/notes/api/ues.ts b/routes/(apps)/notes/api/ues.ts index 757245c..92242da 100644 --- a/routes/(apps)/notes/api/ues.ts +++ b/routes/(apps)/notes/api/ues.ts @@ -24,7 +24,7 @@ export const handler: Handlers = { const body = await request.json(); const { nom } = body; - if (!nom) { + if (!nom || !nom.trim()) { return new Response("Champ 'nom' manquant", { status: 400 }); } diff --git a/tests/e2e/robustness_test.ts b/tests/e2e/robustness_test.ts index ba18a1d..fb5552b 100644 --- a/tests/e2e/robustness_test.ts +++ b/tests/e2e/robustness_test.ts @@ -4,7 +4,7 @@ // Les tests marqués [BUG] représentent le comportement ATTENDU — ils échouent // intentionnellement pour exposer un bug dans le handler ciblé. -import { assertEquals, assertRejects } from "@std/assert"; +import { assertEquals } from "@std/assert"; import { makeContextWithAffiliation, makeEmployeeContext, @@ -90,7 +90,8 @@ Deno.test({ }); Deno.test({ - name: "robustness: POST /ajustements malformed JSON → 500 (try/catch présent)", + name: + "robustness: POST /ajustements malformed JSON → 500 (try/catch présent)", async fn() { await truncateAll(); const res = await ajustementsHandler.POST!( @@ -103,49 +104,43 @@ Deno.test({ sanitizeOps: false, }); -// Handlers SANS try/catch — throwent au lieu de retourner 500 -// [BUG] Ces handlers devraient retourner 500, pas throw - Deno.test({ - name: "robustness [BUG]: POST /modules malformed JSON → throw (pas de try/catch)", + name: "robustness: POST /modules malformed JSON → 500", async fn() { await truncateAll(); - await assertRejects(() => - modulesHandler.POST!( - makeMalformedRequest("/modules"), - makeEmployeeContext(), - ) + const res = await modulesHandler.POST!( + makeMalformedRequest("/modules"), + makeEmployeeContext(), ); + assertEquals(res.status, 500); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /enseignements malformed JSON → throw (pas de try/catch)", + name: "robustness: POST /enseignements malformed JSON → 500", async fn() { await truncateAll(); - await assertRejects(() => - enseignementsHandler.POST!( - makeMalformedRequest("/enseignements"), - makeEmployeeContext(), - ) + const res = await enseignementsHandler.POST!( + makeMalformedRequest("/enseignements"), + makeEmployeeContext(), ); + assertEquals(res.status, 500); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /users malformed JSON → throw (pas de try/catch)", + name: "robustness: POST /users malformed JSON → 500", async fn() { await truncateAll(); - await assertRejects(() => - usersHandler.POST!( - makeMalformedRequest("/users"), - makeEmployeeContext(), - ) + const res = await usersHandler.POST!( + makeMalformedRequest("/users"), + makeEmployeeContext(), ); + assertEquals(res.status, 500); }, sanitizeResources: false, sanitizeOps: false, @@ -170,15 +165,14 @@ Deno.test({ }); Deno.test({ - name: "robustness [BUG]: POST /modules sans body → throw (pas de try/catch)", + name: "robustness: POST /modules sans body → 500", async fn() { await truncateAll(); - await assertRejects(() => - modulesHandler.POST!( - makeEmptyBodyRequest("/modules"), - makeEmployeeContext(), - ) + const res = await modulesHandler.POST!( + makeEmptyBodyRequest("/modules"), + makeEmployeeContext(), ); + assertEquals(res.status, 500); }, sanitizeResources: false, sanitizeOps: false, @@ -235,52 +229,42 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness [BUG]: POST /modules id=espaces → devrait être 400, retourne 201", + name: "robustness: POST /modules id=espaces → 400", async fn() { await truncateAll(); const res = await modulesHandler.POST!( makeJsonRequest("/modules", "POST", { id: " ", nom: "Test" }), makeEmployeeContext(), ); - // Le handler vérifie !body.id → " " est truthy → passe → s'insère - // Comportement attendu : 400 - // Comportement réel : 201 (bug : pas de trim()) - assertEquals(res.status, 400); // ← va échouer, expose le bug + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /ues nom=espaces → devrait être 400, retourne 201", + name: "robustness: POST /ues nom=espaces → 400", async fn() { await truncateAll(); const res = await uesHandler.POST!( makeJsonRequest("/ues", "POST", { nom: " " }), makeEmployeeContext(), ); - assertEquals(res.status, 400); // ← va échouer, expose le bug + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /users id=espaces → devrait être 400, retourne 201", + name: "robustness: POST /users id=espaces → 400", async fn() { await truncateAll(); - await assertRejects( - // sans try/catch + whitespace id → s'insère (ou throw si DB rejette) - // Dans tous les cas le handler ne valide pas correctement - async () => { - const res = await usersHandler.POST!( - makeJsonRequest("/users", "POST", { id: " ", nom: "X", prenom: "Y" }), - makeEmployeeContext(), - ); - // Si pas de throw : le handler a inséré des espaces en DB - assertEquals(res.status, 400); - }, + const res = await usersHandler.POST!( + makeJsonRequest("/users", "POST", { id: " ", nom: "X", prenom: "Y" }), + makeEmployeeContext(), ); + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, @@ -291,36 +275,40 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness [BUG]: POST /notes note=string → devrait être 400, retourne 500", + name: "robustness: POST /notes note=string → 400", async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); - const [s] = await seedStudents([{ nom: "Test", prenom: "User", idPromo: "P1" }]); + const [s] = await seedStudents([{ + nom: "Test", + prenom: "User", + idPromo: "P1", + }]); await seedModules([{ id: "M1", nom: "Mod" }]); const res = await notesHandler.POST!( - makeJsonRequest("/notes", "POST", { note: "pas-un-nombre", numEtud: s.numEtud, idModule: "M1" }), + makeJsonRequest("/notes", "POST", { + note: "pas-un-nombre", + numEtud: s.numEtud, + idModule: "M1", + }), makeEmployeeContext(), ); - // "pas-un-nombre" !== undefined → passe la validation → DB rejette → 500 - // Comportement attendu : 400 (validation de type) - // Comportement réel : 500 - assertEquals(res.status, 400); // ← va échouer, expose le bug + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: PUT /modules/:id nom=number → devrait être 400, throw ou insère", + name: "robustness: PUT /modules/:id nom=number → 400", async fn() { await truncateAll(); await seedModules([{ id: "M1", nom: "Mod" }]); - await assertRejects(() => - moduleHandler.PUT!( - makeJsonRequest("/modules/M1", "PUT", { nom: 42 }), - makeEmployeeContext({ idModule: "M1" }), - ) + const res = await moduleHandler.PUT!( + makeJsonRequest("/modules/M1", "PUT", { nom: 42 }), + makeEmployeeContext({ idModule: "M1" }), ); + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, @@ -331,12 +319,17 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness [BUG]: POST /ajustements numEtud=0 → 400 pour mauvaise raison", + name: + "robustness [BUG]: POST /ajustements numEtud=0 → 400 pour mauvaise raison", async fn() { await truncateAll(); const [ue] = await seedUes([{ nom: "UE Info" }]); const res = await ajustementsHandler.POST!( - makeJsonRequest("/ajustements", "POST", { numEtud: 0, idUE: ue.id, valeur: 10.0 }), + makeJsonRequest("/ajustements", "POST", { + numEtud: 0, + idUE: ue.id, + valeur: 10.0, + }), makeEmployeeContext(), ); // !0 === true → retourne 400 à cause du falsy check, pas d'une vraie validation @@ -353,9 +346,17 @@ Deno.test({ async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); - const [s] = await seedStudents([{ nom: "Test", prenom: "User", idPromo: "P1" }]); + const [s] = await seedStudents([{ + nom: "Test", + prenom: "User", + idPromo: "P1", + }]); const res = await ajustementsHandler.POST!( - makeJsonRequest("/ajustements", "POST", { numEtud: s.numEtud, idUE: 0, valeur: 10.0 }), + makeJsonRequest("/ajustements", "POST", { + numEtud: s.numEtud, + idUE: 0, + valeur: 10.0, + }), makeEmployeeContext(), ); assertEquals(res.status, 400); // !0 → 400, message trompeur @@ -369,14 +370,20 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness: POST /ue-modules coeff=0 → 201 (zéro est une valeur valide)", + name: + "robustness: POST /ue-modules coeff=0 → 201 (zéro est une valeur valide)", async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); await seedModules([{ id: "M1", nom: "Mod" }]); const [ue] = await seedUes([{ nom: "UE Info" }]); const res = await ueModulesHandler.POST!( - makeJsonRequest("/ue-modules", "POST", { idModule: "M1", idUE: ue.id, idPromo: "P1", coeff: 0 }), + makeJsonRequest("/ue-modules", "POST", { + idModule: "M1", + idUE: ue.id, + idPromo: "P1", + coeff: 0, + }), makeEmployeeContext(), ); // coeff === undefined → false pour 0 → passe ✓ @@ -392,7 +399,8 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness: GET /modules avec SQL injection dans id → 404 (Drizzle paramètre)", + name: + "robustness: GET /modules avec SQL injection dans id → 404 (Drizzle paramètre)", async fn() { await truncateAll(); const injectionId = "'; DROP TABLE modules; --"; @@ -409,7 +417,8 @@ Deno.test({ }); Deno.test({ - name: "robustness: POST /modules avec SQL injection dans id → s'insère littéralement (safe)", + name: + "robustness: POST /modules avec SQL injection dans id → s'insère littéralement (safe)", async fn() { await truncateAll(); const injectionId = "'; DROP TABLE modules; --"; @@ -429,52 +438,72 @@ Deno.test({ // ============================================================================= Deno.test({ - name: "robustness [BUG]: POST /notes note > 20 → devrait être 400, retourne 201", + name: "robustness: POST /notes note > 20 → 400", async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); - const [s] = await seedStudents([{ nom: "Test", prenom: "User", idPromo: "P1" }]); + const [s] = await seedStudents([{ + nom: "Test", + prenom: "User", + idPromo: "P1", + }]); await seedModules([{ id: "M1", nom: "Mod" }]); const res = await notesHandler.POST!( - makeJsonRequest("/notes", "POST", { note: 999, numEtud: s.numEtud, idModule: "M1" }), + makeJsonRequest("/notes", "POST", { + note: 999, + numEtud: s.numEtud, + idModule: "M1", + }), makeEmployeeContext(), ); - // Aucune validation de borne → 999 s'insère → 201 - assertEquals(res.status, 400); // ← va échouer, expose le manque de validation métier + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /notes note < 0 → devrait être 400, retourne 201", + name: "robustness: POST /notes note < 0 → 400", async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); - const [s] = await seedStudents([{ nom: "Test", prenom: "User", idPromo: "P1" }]); + const [s] = await seedStudents([{ + nom: "Test", + prenom: "User", + idPromo: "P1", + }]); await seedModules([{ id: "M1", nom: "Mod" }]); const res = await notesHandler.POST!( - makeJsonRequest("/notes", "POST", { note: -5, numEtud: s.numEtud, idModule: "M1" }), + makeJsonRequest("/notes", "POST", { + note: -5, + numEtud: s.numEtud, + idModule: "M1", + }), makeEmployeeContext(), ); - assertEquals(res.status, 400); // ← va échouer + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, }); Deno.test({ - name: "robustness [BUG]: POST /ue-modules coeff négatif → devrait être 400, retourne 201", + name: "robustness: POST /ue-modules coeff négatif → 400", async fn() { await truncateAll(); await seedPromotions([{ id: "P1" }]); await seedModules([{ id: "M1", nom: "Mod" }]); const [ue] = await seedUes([{ nom: "UE Info" }]); const res = await ueModulesHandler.POST!( - makeJsonRequest("/ue-modules", "POST", { idModule: "M1", idUE: ue.id, idPromo: "P1", coeff: -3 }), + makeJsonRequest("/ue-modules", "POST", { + idModule: "M1", + idUE: ue.id, + idPromo: "P1", + coeff: -3, + }), makeEmployeeContext(), ); - assertEquals(res.status, 400); // ← va échouer + assertEquals(res.status, 400); }, sanitizeResources: false, sanitizeOps: false, @@ -491,7 +520,10 @@ Deno.test({ // Ce test crée un module await truncateAll(); await modulesHandler.POST!( - makeJsonRequest("/modules", "POST", { id: "ISOLATION-TEST", nom: "Test" }), + makeJsonRequest("/modules", "POST", { + id: "ISOLATION-TEST", + nom: "Test", + }), makeEmployeeContext(), ); }, @@ -500,7 +532,8 @@ Deno.test({ }); Deno.test({ - name: "robustness: isolation — truncateAll efface bien les données du test précédent", + name: + "robustness: isolation — truncateAll efface bien les données du test précédent", async fn() { await truncateAll(); // Le module créé dans le test précédent ne doit plus exister