diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMapping.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMapping.java index ea61d6ac5d6d..5f4db5890ed0 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMapping.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMapping.java @@ -36,6 +36,7 @@ import org.hisp.dhis.webapi.security.config.WebMvcConfig; import org.hisp.dhis.webapi.view.SuffixMediaTypeContentNegotiationStrategy; import org.springframework.http.MediaType; +import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerMapping; @@ -53,12 +54,14 @@ * *

It also reinstates path-extension and trailing-slash matching without relying on Spring's * removed/deprecated handler-mapping flags ({@code setUseSuffixPatternMatch} / {@code - * setUseTrailingSlashMatch} / {@code favorPathExtension}). For each request the literal path is - * matched first so controllers that map an extension literally (e.g. OpenAPI's {@code - * /openapi/openapi.json}) keep working. Only if that has no handler do we fall back to matching the - * path with a trailing slash and/or a registered media-type extension removed (e.g. {@code - * /api/dataElements.json} to {@code /api/dataElements}), recording the resolved media type for - * {@link SuffixMediaTypeContentNegotiationStrategy}. + * setUseTrailingSlashMatch} / {@code favorPathExtension}). For each request that ends with a + * registered media-type extension (or trailing slash), the suffix-stripped path is matched first so + * content negotiation works and generic {@code /{property}} handlers do not swallow names like + * {@code metadata.json}. If the stripped lookup finds no handler, or finds one whose {@code + * produces} cannot satisfy the extension-derived media type ({@code 406}), we fall back to the + * original path so controllers that map an extension literally (analytics {@code .xml}/{@code .csv} + * downloads, OpenAPI's {@code /openapi/openapi.json}) keep working. The resolved media type is + * recorded for {@link SuffixMediaTypeContentNegotiationStrategy}. * *

Forward-compatible on Spring 6.2 (Spring 7 readiness PR-F). */ @@ -92,15 +95,30 @@ protected HandlerMethod getHandlerInternal(HttpServletRequest request) throws Ex // When the path ends with a registered media-type extension (e.g. .json / .json.zip), prefer // matching the suffix-stripped path so content negotiation works and generic /{property} // handlers do not swallow "metadata.json" as a property name. If that fails (literal-extension - // controller mappings such as /openapi/openapi.json), fall back to the original path. + // controller mappings such as /openapi/openapi.json), or the stripped path matches a handler + // whose produces cannot satisfy the extension-derived media type (406 - common for analytics + // download endpoints that map .xml/.csv/... literally next to a JSON produces handler), fall + // back to the original path. // // Paths without a registered extension: match as-is, then fall back to trailing-slash strip. HttpServletRequest normalized = normalize(request); if (normalized != request) { clearAndReparsePathCaches(normalized); - HandlerMethod stripped = super.getHandlerInternal(normalized); - if (stripped != null) { - return stripped; + try { + HandlerMethod stripped = super.getHandlerInternal(normalized); + if (stripped != null) { + return stripped; + } + } catch (HttpMediaTypeNotAcceptableException notAcceptable) { + // Stripped path matched a handler, but its produces does not include the media type forced + // by the path extension. Prefer a literal-suffix mapping when one exists; otherwise rethrow + // so the client still gets 406 rather than a misleading 404. + clearAndReparsePathCaches(request); + HandlerMethod literal = super.getHandlerInternal(request); + if (literal != null) { + return literal; + } + throw notAcceptable; } // No stripped handler - try the original path (literal extension mappings). clearAndReparsePathCaches(request); diff --git a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMappingNormalizeTest.java b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMappingNormalizeTest.java index 7f5b3ebda5e1..93beeb613882 100644 --- a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMappingNormalizeTest.java +++ b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMappingNormalizeTest.java @@ -33,6 +33,8 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import jakarta.servlet.http.HttpServletRequest; import java.lang.reflect.Method; @@ -45,6 +47,8 @@ import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.stereotype.Controller; +import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.accept.ContentNegotiationManager; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.method.HandlerMethod; @@ -65,6 +69,10 @@ class CustomRequestMappingHandlerMappingNormalizeTest { void setUp() throws Exception { mapping = new CustomRequestMappingHandlerMapping(); mapping.setApplicationContext(new StaticApplicationContext()); + // Needed so produces conditions evaluate the suffix-forced media type the same way production + // does via SuffixMediaTypeContentNegotiationStrategy (PR-F / #24463). + mapping.setContentNegotiationManager( + new ContentNegotiationManager(new SuffixMediaTypeContentNegotiationStrategy())); mapping.afterPropertiesSet(); Method method = SampleController.class.getMethod("list"); @@ -78,6 +86,24 @@ void setUp() throws Exception { mapping.getMappingForMethod(openapi, SampleController.class), new SampleController(), openapi); + + Method queryJson = AnalyticsStyleController.class.getMethod("queryJson", String.class); + mapping.registerMapping( + mapping.getMappingForMethod(queryJson, AnalyticsStyleController.class), + new AnalyticsStyleController(), + queryJson); + + Method queryXml = AnalyticsStyleController.class.getMethod("queryXml", String.class); + mapping.registerMapping( + mapping.getMappingForMethod(queryXml, AnalyticsStyleController.class), + new AnalyticsStyleController(), + queryXml); + + Method onlyJson = AnalyticsStyleController.class.getMethod("onlyJson", String.class); + mapping.registerMapping( + mapping.getMappingForMethod(onlyJson, AnalyticsStyleController.class), + new AnalyticsStyleController(), + onlyJson); } @Test @@ -93,6 +119,31 @@ void literalExtensionMappingWinsWhenStrippedPathHasNoHandler() throws Exception SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE)); } + /** + * Regression for analytics-style dual mappings: JSON produces on {@code /query/{id}} plus a + * literal {@code /query/{id}.xml} download handler. Stripping the suffix must not surface the + * JSON handler's 406 and block the literal download mapping (CI failure after #24463). + */ + @Test + void literalDownloadMappingWinsWhenStrippedProducesIsIncompatible() throws Exception { + MockHttpServletRequest request = + request("/api/analytics/trackedEntities/query/nEenWmSyUEp.xml"); + HandlerMethod handler = mapping.getHandlerInternal(request); + assertNotNull(handler); + assertEquals("queryXml", handler.getMethod().getName()); + assertEquals( + MediaType.APPLICATION_XML, + request.getAttribute( + SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE)); + } + + @Test + void rethrowsNotAcceptableWhenNoLiteralFallbackExists() { + MockHttpServletRequest request = request("/api/analytics/only-json/nEenWmSyUEp.xml"); + assertThrows( + HttpMediaTypeNotAcceptableException.class, () -> mapping.getHandlerInternal(request)); + } + @ParameterizedTest @CsvSource({ "/api/dataElements.json, application/json", @@ -190,4 +241,31 @@ public String openapi() { return "openapi"; } } + + /** + * Mirrors the analytics download pattern: a generic path with restricted {@code produces} next to + * a literal-suffix download mapping with no produces restriction. + */ + @Controller + @RequestMapping + static class AnalyticsStyleController { + @GetMapping( + value = "/api/analytics/trackedEntities/query/{trackedEntityType}", + produces = {APPLICATION_JSON_VALUE, "application/javascript"}) + public String queryJson(String trackedEntityType) { + return "json"; + } + + @GetMapping(value = "/api/analytics/trackedEntities/query/{trackedEntityType}.xml") + public String queryXml(String trackedEntityType) { + return "xml"; + } + + @GetMapping( + value = "/api/analytics/only-json/{id}", + produces = {APPLICATION_JSON_VALUE}) + public String onlyJson(String id) { + return "json-only"; + } + } }