From fe8c6ed162613269bcc7fc6fa6ef27f2a29f8a0b Mon Sep 17 00:00:00 2001 From: Zakaria El Orche Date: Wed, 9 Sep 2026 12:20:33 +0000 Subject: [PATCH] fix(core): honour HttpException status in the default exception handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HttpException carries the status the caller meant, and its own javadoc says extensions map it to a structured response — but nothing did. Every one reached the catch-all and came back as 500, including the 400s that RequestHelper raises for a malformed query param and that flash-ext-jackson raises for an unparseable body. A handler doing the documented thing produced the wrong status. The default handler now renders HttpException at its own status, in both dev and prod modes, with the message JSON-escaped. Not pre-encoded like JSON_404 and JSON_500: the message is per-exception, and a path that already unwound a stack does not need the allocation shaved. Co-Authored-By: Claude Opus 5 --- .../relism/flash/routing/AbstractRouter.java | 40 +++++++++++++++++++ .../flash/routing/AbstractRouterTest.java | 23 +++++++++++ 2 files changed, 63 insertions(+) diff --git a/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java b/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java index 4473359..dc4e5ae 100644 --- a/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java +++ b/flash/src/main/java/dev/relism/flash/routing/AbstractRouter.java @@ -4,6 +4,7 @@ import dev.relism.flash.extension.FlashApp; import dev.relism.flash.models.*; import dev.relism.flash.routing.routers.fastpathrouter.FastPathRouterImpl; import dev.relism.flash.Flash; +import dev.relism.flash.exceptions.HttpException; import dev.relism.flash.http.ContentType; import dev.relism.flash.http.HttpMethod; import dev.relism.flash.template.ErrorPages; @@ -56,17 +57,56 @@ public abstract class AbstractRouter { protected ExceptionHandler exceptionHandler = Flash.DEV ? (ex, req, res) -> { + if (ex instanceof HttpException http) return renderHttpException(http, res); res.status(500); res.type(ContentType.TEXT_HTML); return ErrorPages.renderException(req, ex); } : (ex, req, res) -> { + if (ex instanceof HttpException http) return renderHttpException(http, res); log.error("Unhandled exception in {} {}", req.method(), req.path(), ex); res.status(500); res.type(ContentType.JSON); return JSON_500; }; + /** + * {@link HttpException} carries the status the caller meant; without this it reached the + * catch-all above and every one of them came back as 500 — including the 400s + * {@code RequestHelper} and {@code flash-ext-jackson} raise for malformed input. + * + *

Deliberately not pre-encoded like {@link #JSON_404}: the message is per-exception, and + * an error path that already unwound a stack does not need the allocation shaved. + */ + private static byte[] renderHttpException(HttpException failure, Response res) { + res.status(failure.status()); + res.type(ContentType.JSON); + String message = failure.getMessage(); + StringBuilder out = new StringBuilder(48 + (message == null ? 0 : message.length())); + out.append("{\"error\":\""); + escapeJson(message == null ? "" : message, out); + out.append("\",\"status\":").append(failure.status()).append('}'); + return out.toString().getBytes(StandardCharsets.UTF_8); + } + + /** Minimal RFC 8259 string escaping — enough for an exception message. */ + private static void escapeJson(String text, StringBuilder out) { + for (int i = 0; i < text.length(); i++) { + char c = text.charAt(i); + switch (c) { + case '"' -> out.append("\\\""); + case '\\' -> out.append("\\\\"); + case '\n' -> out.append("\\n"); + case '\r' -> out.append("\\r"); + case '\t' -> out.append("\\t"); + default -> { + if (c < 0x20) out.append(String.format("\\u%04x", (int) c)); + else out.append(c); + } + } + } + } + public SimpleHandler getNotFoundHandler() { return notFoundHandler; } public ExceptionHandler getExceptionHandler() { return exceptionHandler; } diff --git a/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java b/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java index ede5000..9871486 100644 --- a/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java +++ b/flash/src/test/java/dev/relism/flash/routing/AbstractRouterTest.java @@ -1,5 +1,7 @@ package dev.relism.flash.routing; +import dev.relism.flash.exceptions.HttpException; +import dev.relism.flash.http.ContentType; import dev.relism.flash.http.HttpMethod; import dev.relism.flash.models.Request; import dev.relism.flash.models.RequestHandler; @@ -7,6 +9,8 @@ import dev.relism.flash.models.Response; import dev.relism.flash.models.SimpleHandler; import org.junit.jupiter.api.Test; +import java.nio.charset.StandardCharsets; + import static org.junit.jupiter.api.Assertions.*; class AbstractRouterTest { @@ -77,4 +81,23 @@ class AbstractRouterTest { router.onException((ex, req, res) -> "Caught"); assertEquals("Caught", router.getExceptionHandler().handle(new RuntimeException(), null, null)); } + + + /** + * HttpException carries the status the caller meant. Before this was honoured every one of + * them came back as 500, including the 400s RequestHelper and flash-ext-jackson raise. + */ + @Test + void defaultExceptionHandlerHonoursHttpExceptionStatus() throws Exception { + DummyRouter router = new DummyRouter(); + Response res = new Response(200, ContentType.TEXT_PLAIN); + + Object body = router.getExceptionHandler() + .handle(HttpException.badRequest("bad \"input\""), null, res); + + assertEquals(400, res.getStatusCode()); + String rendered = new String((byte[]) body, StandardCharsets.UTF_8); + assertTrue(rendered.contains("\\\"input\\\""), "message must be JSON-escaped: " + rendered); + assertTrue(rendered.contains("\"status\":400"), rendered); + } }