fix: fall back to literal path on path-extension 406 (#24479)

Author: netroms

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 @@
  *
  * <p>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}.
  *
  * <p>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);

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";
+    }
+  }
 }