Forward-compatible on Spring 6.2 (Spring 7 readiness PR-F). */ @@ -98,33 +105,73 @@ protected HandlerMethod getHandlerInternal(HttpServletRequest request) throws Ex // 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. + // back toward the original path. When both a trailing slash and an extension were present, + // try keeping the extension first (slash-only strip) before the raw original URI. // // Paths without a registered extension: match as-is, then fall back to trailing-slash strip. HttpServletRequest normalized = normalize(request); - if (normalized != request) { - clearAndReparsePathCaches(normalized); + if (normalized == request) { + return super.getHandlerInternal(request); + } + + HttpMediaTypeNotAcceptableException notAcceptable = null; + + clearAndReparsePathCaches(normalized); + try { + HandlerMethod stripped = super.getHandlerInternal(normalized); + if (stripped != null) { + return stripped; + } + } catch (HttpMediaTypeNotAcceptableException ex) { + // 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. + notAcceptable = ex; + } + + // /api/analytics.xlsx/ fully strips to /api/analytics (JSON produces → 406). Retry with only + // the trailing slash removed so the literal /api/analytics.xlsx mapping can still win. + HttpServletRequest slashOnly = trailingSlashOnly(request); + if (slashOnly != null) { + clearAndReparsePathCaches(slashOnly); try { - HandlerMethod stripped = super.getHandlerInternal(normalized); - if (stripped != null) { - return stripped; + HandlerMethod literalWithExtension = super.getHandlerInternal(slashOnly); + if (literalWithExtension != null) { + return literalWithExtension; } - } 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; + } catch (HttpMediaTypeNotAcceptableException ex) { + if (notAcceptable == null) { + notAcceptable = ex; } - throw notAcceptable; } - // No stripped handler - try the original path (literal extension mappings). - clearAndReparsePathCaches(request); } - return super.getHandlerInternal(request); + clearAndReparsePathCaches(request); + HandlerMethod original = super.getHandlerInternal(request); + if (original != null) { + return original; + } + if (notAcceptable != null) { + throw notAcceptable; + } + return null; + } + + /** + * When the original URI ends with a registered media-type extension and a trailing + * slash, returns a request with only the trailing slash removed (extension kept). Otherwise + * {@code null}. + */ + private HttpServletRequest trailingSlashOnly(HttpServletRequest request) { + String uri = request.getRequestURI(); + if (uri == null || uri.length() <= 1 || uri.charAt(uri.length() - 1) != PATH_SEPARATOR) { + return null; + } + String withoutSlash = uri.substring(0, uri.length() - 1); + if (getRegisteredExtension(withoutSlash) == null) { + return null; + } + return new PathNormalizingRequestWrapper(request, null, true); } private static void clearAndReparsePathCaches(HttpServletRequest 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 93beeb613882..8a9430dd082e 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 @@ -99,6 +99,24 @@ void setUp() throws Exception { new AnalyticsStyleController(), queryXml); + Method queryXlsx = AnalyticsStyleController.class.getMethod("queryXlsx", String.class); + mapping.registerMapping( + mapping.getMappingForMethod(queryXlsx, AnalyticsStyleController.class), + new AnalyticsStyleController(), + queryXlsx); + + Method aggregateJson = AnalyticsStyleController.class.getMethod("aggregateJson"); + mapping.registerMapping( + mapping.getMappingForMethod(aggregateJson, AnalyticsStyleController.class), + new AnalyticsStyleController(), + aggregateJson); + + Method aggregateXlsx = AnalyticsStyleController.class.getMethod("aggregateXlsx"); + mapping.registerMapping( + mapping.getMappingForMethod(aggregateXlsx, AnalyticsStyleController.class), + new AnalyticsStyleController(), + aggregateXlsx); + Method onlyJson = AnalyticsStyleController.class.getMethod("onlyJson", String.class); mapping.registerMapping( mapping.getMappingForMethod(onlyJson, AnalyticsStyleController.class), @@ -137,6 +155,43 @@ void literalDownloadMappingWinsWhenStrippedProducesIsIncompatible() throws Excep SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE)); } + /** + * Follow-up to #24479: RestAssured-style {@code basePath("analytics.xlsx") + get("")} produces + * {@code /api/analytics.xlsx/}. Fully stripping both slash and extension hits the JSON produces + * handler (406); the literal download map must still win via a slash-only retry. + */ + @Test + void literalDownloadMappingWinsWithTrailingSlashAfterExtension() 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 aggregateLiteralXlsxDownloadWinsWithTrailingSlash() throws Exception { + MockHttpServletRequest request = request("/api/analytics.xlsx/"); + HandlerMethod handler = mapping.getHandlerInternal(request); + assertNotNull(handler); + assertEquals("aggregateXlsx", handler.getMethod().getName()); + assertEquals( + MediaType.parseMediaType("application/vnd.ms-excel"), + request.getAttribute( + SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE)); + } + + @Test + void rethrowsNotAcceptableWhenTrailingSlashAndNoLiteralFallback() { + MockHttpServletRequest request = request("/api/analytics/only-json/nEenWmSyUEp.xml/"); + assertThrows( + HttpMediaTypeNotAcceptableException.class, () -> mapping.getHandlerInternal(request)); + } + @Test void rethrowsNotAcceptableWhenNoLiteralFallbackExists() { MockHttpServletRequest request = request("/api/analytics/only-json/nEenWmSyUEp.xml"); @@ -261,6 +316,23 @@ public String queryXml(String trackedEntityType) { return "xml"; } + @GetMapping(value = "/api/analytics/trackedEntities/query/{trackedEntityType}.xlsx") + public String queryXlsx(String trackedEntityType) { + return "xlsx"; + } + + @GetMapping( + value = "/api/analytics", + produces = {APPLICATION_JSON_VALUE, "application/javascript"}) + public String aggregateJson() { + return "aggregate-json"; + } + + @GetMapping(value = "/api/analytics.xlsx") + public String aggregateXlsx() { + return "aggregate-xlsx"; + } + @GetMapping( value = "/api/analytics/only-json/{id}", produces = {APPLICATION_JSON_VALUE})