From a8f4743142e8b6b37187c25a72c35ec048ae8c4f Mon Sep 17 00:00:00 2001 From: Gabriel Roldan Date: Tue, 22 Sep 2026 14:45:47 -0300 Subject: [PATCH] Stop REST errors answering 500 depending on the jar order in WEB-INF/lib Make RestExceptionHandler answer every RestException with the status of the exception regardless of the jar order. Otherwise GeoServer's RestControllerAdvice may take precedence and return 500 instead of 404/400. --- .../rest/controller/RestExceptionHandler.java | 10 +- .../controller/RestExceptionHandlerTest.java | 147 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 geowebcache/rest/src/test/java/org/geowebcache/rest/controller/RestExceptionHandlerTest.java diff --git a/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/RestExceptionHandler.java b/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/RestExceptionHandler.java index c5b52719ff..8dd25c26d0 100644 --- a/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/RestExceptionHandler.java +++ b/geowebcache/rest/src/main/java/org/geowebcache/rest/controller/RestExceptionHandler.java @@ -20,14 +20,22 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.geowebcache.rest.exception.RestException; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; import org.springframework.http.MediaType; import org.springframework.util.StreamUtils; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.context.request.WebRequest; -/** Common Rest Exception Handler for Spring MVC Controllers. */ +/** + * Common Rest Exception Handler for Spring MVC Controllers. + * + *

Ordered first so GeoServer's catch-all handler, {@code org.geoserver.rest.RestControllerAdvice}, which returns + * 500, cannot take over a {@link RestException} when Spring registers its jar before this one. + */ @ControllerAdvice +@Order(Ordered.HIGHEST_PRECEDENCE) public class RestExceptionHandler { private static final Logger LOGGER = Logger.getLogger(RestExceptionHandler.class.getName()); diff --git a/geowebcache/rest/src/test/java/org/geowebcache/rest/controller/RestExceptionHandlerTest.java b/geowebcache/rest/src/test/java/org/geowebcache/rest/controller/RestExceptionHandlerTest.java new file mode 100644 index 0000000000..27320b9109 --- /dev/null +++ b/geowebcache/rest/src/test/java/org/geowebcache/rest/controller/RestExceptionHandlerTest.java @@ -0,0 +1,147 @@ +/** + * This program is free software: you can redistribute it and/or modify it under the terms of the GNU Lesser General + * Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any + * later version. + * + *

This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied + * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + *

You should have received a copy of the GNU Lesser General Public License along with this program. If not, see + * . + */ +package org.geowebcache.rest.controller; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import jakarta.servlet.ServletException; +import org.geowebcache.rest.exception.RestException; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RunWith(Enclosed.class) +public class RestExceptionHandlerTest { + + @RestController + static class FailingController { + @GetMapping("/not-found") + public void notFound() { + throw new RestException("Unknown layer: missing", HttpStatus.NOT_FOUND); + } + + @GetMapping("/bad-request") + public void badRequest() { + throw new RestException("Layer name not provided", HttpStatus.BAD_REQUEST); + } + + @GetMapping("/server-error") + public void serverError() { + throw new RestException("Truncation failed", HttpStatus.INTERNAL_SERVER_ERROR); + } + + @GetMapping("/broken") + public void broken() { + throw new IllegalStateException("Broken layer"); + } + } + + /** Stands in for the catch-all advice of a host application such as GeoServer. */ + @ControllerAdvice + static class CatchAllAdvice { + @ExceptionHandler(Exception.class) + public ResponseEntity handleAnything(Exception e) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(e.getMessage()); + } + } + + static MockMvc mockMvc(Object... controllerAdvice) { + return MockMvcBuilders.standaloneSetup(new FailingController()) + .setControllerAdvice(controllerAdvice) + .build(); + } + + public static class AsOnlyAdvice { + + private MockMvc mockMvc; + + @Before + public void setUp() { + mockMvc = mockMvc(new RestExceptionHandler()); + } + + @Test + public void testNotFound() throws Exception { + mockMvc.perform(get("/not-found")) + .andExpect(status().isNotFound()) + .andExpect(content().contentType(MediaType.TEXT_PLAIN)) + .andExpect(content().string("Unknown layer: missing")); + } + + @Test + public void testBadRequest() throws Exception { + mockMvc.perform(get("/bad-request")) + .andExpect(status().isBadRequest()) + .andExpect(content().contentType(MediaType.TEXT_PLAIN)) + .andExpect(content().string("Layer name not provided")); + } + + @Test + public void testInternalServerError() throws Exception { + mockMvc.perform(get("/server-error")) + .andExpect(status().isInternalServerError()) + .andExpect(content().contentType(MediaType.TEXT_PLAIN)) + .andExpect(content().string("Truncation failed")); + } + + @Test + public void testOtherExceptionsNotHandled() { + ServletException unhandled = assertThrows(ServletException.class, () -> mockMvc.perform(get("/broken"))); + + assertTrue(unhandled.getCause() instanceof IllegalStateException); + } + } + + /** + * Spring loads advice beans of equal precedence in registration order, which is a race following on the order of + * the jars in {@code WEB-INF/lib}. A catch-all advice registered first must not take over a {@link RestException}, + * and must still get every other exception. + */ + public static class RegisteredAfterCatchAllAdvice { + + private MockMvc mockMvc; + + @Before + public void setUp() { + mockMvc = mockMvc(new CatchAllAdvice(), new RestExceptionHandler()); + } + + @Test + public void testRestExceptionStatus() throws Exception { + mockMvc.perform(get("/not-found")) + .andExpect(status().isNotFound()) + .andExpect(content().contentType(MediaType.TEXT_PLAIN)) + .andExpect(content().string("Unknown layer: missing")); + } + + @Test + public void testOtherExceptionsLeftToCatchAllAdvice() throws Exception { + mockMvc.perform(get("/broken")) + .andExpect(status().isInternalServerError()) + .andExpect(content().string("Broken layer")); + } + } +}