Skip to content

Commit fc846bf

Browse files
netromsnetroms
authored
fix: match literal path-extension downloads with trailing slash (#24537)
Follow-up to #24479. Paths like /api/analytics.xlsx/ fully strip to /api/analytics, hit the JSON produces handler (406), then failed the literal fallback because the original URI still had the trailing slash and Spring 7 does not trailing-slash-match. Retry with only the trailing slash removed before the raw original path so literal download mappings still win. Covers the RestAssured basePath pattern used by AnalyticsAggregateDownloadTest.
1 parent cb2977c commit fc846bf

2 files changed

Lines changed: 141 additions & 22 deletions

File tree

‎dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMapping.java‎

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,17 @@
5858
* registered media-type extension (or trailing slash), the suffix-stripped path is matched first so
5959
* content negotiation works and generic {@code /{property}} handlers do not swallow names like
6060
* {@code metadata.json}. If the stripped lookup finds no handler, or finds one whose {@code
61-
* produces} cannot satisfy the extension-derived media type ({@code 406}), we fall back to the
62-
* original path so controllers that map an extension literally (analytics {@code .xml}/{@code .csv}
63-
* downloads, OpenAPI's {@code /openapi/openapi.json}) keep working. The resolved media type is
64-
* recorded for {@link SuffixMediaTypeContentNegotiationStrategy}.
61+
* produces} cannot satisfy the extension-derived media type ({@code 406}), we fall back in order:
62+
*
63+
* <ol>
64+
* <li>slash-only strip when both a trailing slash and a registered extension were present (so
65+
* literal maps like {@code /api/analytics.xlsx} still match {@code /api/analytics.xlsx/})
66+
* <li>the original path (literal-extension controllers such as OpenAPI's {@code
67+
* /openapi/openapi.json})
68+
* </ol>
69+
*
70+
* Rethrow the 406 only when no fallback mapping exists. The resolved media type is recorded for
71+
* {@link SuffixMediaTypeContentNegotiationStrategy}.
6572
*
6673
* <p>Forward-compatible on Spring 6.2 (Spring 7 readiness PR-F).
6774
*/
@@ -98,33 +105,73 @@ protected HandlerMethod getHandlerInternal(HttpServletRequest request) throws Ex
98105
// controller mappings such as /openapi/openapi.json), or the stripped path matches a handler
99106
// whose produces cannot satisfy the extension-derived media type (406 - common for analytics
100107
// download endpoints that map .xml/.csv/... literally next to a JSON produces handler), fall
101-
// back to the original path.
108+
// back toward the original path. When both a trailing slash and an extension were present,
109+
// try keeping the extension first (slash-only strip) before the raw original URI.
102110
//
103111
// Paths without a registered extension: match as-is, then fall back to trailing-slash strip.
104112
HttpServletRequest normalized = normalize(request);
105-
if (normalized != request) {
106-
clearAndReparsePathCaches(normalized);
113+
if (normalized == request) {
114+
return super.getHandlerInternal(request);
115+
}
116+
117+
HttpMediaTypeNotAcceptableException notAcceptable = null;
118+
119+
clearAndReparsePathCaches(normalized);
120+
try {
121+
HandlerMethod stripped = super.getHandlerInternal(normalized);
122+
if (stripped != null) {
123+
return stripped;
124+
}
125+
} catch (HttpMediaTypeNotAcceptableException ex) {
126+
// Stripped path matched a handler, but its produces does not include the media type forced
127+
// by the path extension. Prefer a literal-suffix mapping when one exists; otherwise rethrow
128+
// so the client still gets 406 rather than a misleading 404.
129+
notAcceptable = ex;
130+
}
131+
132+
// /api/analytics.xlsx/ fully strips to /api/analytics (JSON produces → 406). Retry with only
133+
// the trailing slash removed so the literal /api/analytics.xlsx mapping can still win.
134+
HttpServletRequest slashOnly = trailingSlashOnly(request);
135+
if (slashOnly != null) {
136+
clearAndReparsePathCaches(slashOnly);
107137
try {
108-
HandlerMethod stripped = super.getHandlerInternal(normalized);
109-
if (stripped != null) {
110-
return stripped;
138+
HandlerMethod literalWithExtension = super.getHandlerInternal(slashOnly);
139+
if (literalWithExtension != null) {
140+
return literalWithExtension;
111141
}
112-
} catch (HttpMediaTypeNotAcceptableException notAcceptable) {
113-
// Stripped path matched a handler, but its produces does not include the media type forced
114-
// by the path extension. Prefer a literal-suffix mapping when one exists; otherwise rethrow
115-
// so the client still gets 406 rather than a misleading 404.
116-
clearAndReparsePathCaches(request);
117-
HandlerMethod literal = super.getHandlerInternal(request);
118-
if (literal != null) {
119-
return literal;
142+
} catch (HttpMediaTypeNotAcceptableException ex) {
143+
if (notAcceptable == null) {
144+
notAcceptable = ex;
120145
}
121-
throw notAcceptable;
122146
}
123-
// No stripped handler - try the original path (literal extension mappings).
124-
clearAndReparsePathCaches(request);
125147
}
126148

127-
return super.getHandlerInternal(request);
149+
clearAndReparsePathCaches(request);
150+
HandlerMethod original = super.getHandlerInternal(request);
151+
if (original != null) {
152+
return original;
153+
}
154+
if (notAcceptable != null) {
155+
throw notAcceptable;
156+
}
157+
return null;
158+
}
159+
160+
/**
161+
* When the original URI ends with a registered media-type extension <em>and</em> a trailing
162+
* slash, returns a request with only the trailing slash removed (extension kept). Otherwise
163+
* {@code null}.
164+
*/
165+
private HttpServletRequest trailingSlashOnly(HttpServletRequest request) {
166+
String uri = request.getRequestURI();
167+
if (uri == null || uri.length() <= 1 || uri.charAt(uri.length() - 1) != PATH_SEPARATOR) {
168+
return null;
169+
}
170+
String withoutSlash = uri.substring(0, uri.length() - 1);
171+
if (getRegisteredExtension(withoutSlash) == null) {
172+
return null;
173+
}
174+
return new PathNormalizingRequestWrapper(request, null, true);
128175
}
129176

130177
private static void clearAndReparsePathCaches(HttpServletRequest request) {

‎dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/mvc/CustomRequestMappingHandlerMappingNormalizeTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,24 @@ void setUp() throws Exception {
9999
new AnalyticsStyleController(),
100100
queryXml);
101101

102+
Method queryXlsx = AnalyticsStyleController.class.getMethod("queryXlsx", String.class);
103+
mapping.registerMapping(
104+
mapping.getMappingForMethod(queryXlsx, AnalyticsStyleController.class),
105+
new AnalyticsStyleController(),
106+
queryXlsx);
107+
108+
Method aggregateJson = AnalyticsStyleController.class.getMethod("aggregateJson");
109+
mapping.registerMapping(
110+
mapping.getMappingForMethod(aggregateJson, AnalyticsStyleController.class),
111+
new AnalyticsStyleController(),
112+
aggregateJson);
113+
114+
Method aggregateXlsx = AnalyticsStyleController.class.getMethod("aggregateXlsx");
115+
mapping.registerMapping(
116+
mapping.getMappingForMethod(aggregateXlsx, AnalyticsStyleController.class),
117+
new AnalyticsStyleController(),
118+
aggregateXlsx);
119+
102120
Method onlyJson = AnalyticsStyleController.class.getMethod("onlyJson", String.class);
103121
mapping.registerMapping(
104122
mapping.getMappingForMethod(onlyJson, AnalyticsStyleController.class),
@@ -137,6 +155,43 @@ void literalDownloadMappingWinsWhenStrippedProducesIsIncompatible() throws Excep
137155
SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE));
138156
}
139157

158+
/**
159+
* Follow-up to #24479: RestAssured-style {@code basePath("analytics.xlsx") + get("")} produces
160+
* {@code /api/analytics.xlsx/}. Fully stripping both slash and extension hits the JSON produces
161+
* handler (406); the literal download map must still win via a slash-only retry.
162+
*/
163+
@Test
164+
void literalDownloadMappingWinsWithTrailingSlashAfterExtension() throws Exception {
165+
MockHttpServletRequest request =
166+
request("/api/analytics/trackedEntities/query/nEenWmSyUEp.xml/");
167+
HandlerMethod handler = mapping.getHandlerInternal(request);
168+
assertNotNull(handler);
169+
assertEquals("queryXml", handler.getMethod().getName());
170+
assertEquals(
171+
MediaType.APPLICATION_XML,
172+
request.getAttribute(
173+
SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE));
174+
}
175+
176+
@Test
177+
void aggregateLiteralXlsxDownloadWinsWithTrailingSlash() throws Exception {
178+
MockHttpServletRequest request = request("/api/analytics.xlsx/");
179+
HandlerMethod handler = mapping.getHandlerInternal(request);
180+
assertNotNull(handler);
181+
assertEquals("aggregateXlsx", handler.getMethod().getName());
182+
assertEquals(
183+
MediaType.parseMediaType("application/vnd.ms-excel"),
184+
request.getAttribute(
185+
SuffixMediaTypeContentNegotiationStrategy.SUFFIX_MEDIA_TYPE_ATTRIBUTE));
186+
}
187+
188+
@Test
189+
void rethrowsNotAcceptableWhenTrailingSlashAndNoLiteralFallback() {
190+
MockHttpServletRequest request = request("/api/analytics/only-json/nEenWmSyUEp.xml/");
191+
assertThrows(
192+
HttpMediaTypeNotAcceptableException.class, () -> mapping.getHandlerInternal(request));
193+
}
194+
140195
@Test
141196
void rethrowsNotAcceptableWhenNoLiteralFallbackExists() {
142197
MockHttpServletRequest request = request("/api/analytics/only-json/nEenWmSyUEp.xml");
@@ -261,6 +316,23 @@ public String queryXml(String trackedEntityType) {
261316
return "xml";
262317
}
263318

319+
@GetMapping(value = "/api/analytics/trackedEntities/query/{trackedEntityType}.xlsx")
320+
public String queryXlsx(String trackedEntityType) {
321+
return "xlsx";
322+
}
323+
324+
@GetMapping(
325+
value = "/api/analytics",
326+
produces = {APPLICATION_JSON_VALUE, "application/javascript"})
327+
public String aggregateJson() {
328+
return "aggregate-json";
329+
}
330+
331+
@GetMapping(value = "/api/analytics.xlsx")
332+
public String aggregateXlsx() {
333+
return "aggregate-xlsx";
334+
}
335+
264336
@GetMapping(
265337
value = "/api/analytics/only-json/{id}",
266338
produces = {APPLICATION_JSON_VALUE})

0 commit comments

Comments
 (0)