Skip to content

Commit fb2fee4

Browse files
dsolistorresclaude
andcommitted
fix(rest-api): unwrap WebApplicationException from JasperException in rules include endpoint (#34888)
Tomcat/Jasper wraps any Throwable raised inside a JSP in a ServletException before it leaves RequestDispatcher.include(), so the dedicated catch(WebApplicationException) added in #35337 never matched at runtime and /api/portlet/rules/include kept returning HTTP 200 with debug HTML instead of the expected 400/404. The generic catch in BaseRestPortlet.getJspResponse() now walks the cause chain and re-throws the WebApplicationException so JAX-RS can map the proper status. Added unit tests that simulate the real Jasper wrapping (the existing test mocked the dispatcher to throw the exception directly, bypassing the wrapper). Also in include.jsp: - Validate the id parameter against UUIDUtil.isUUID before hitting the API, so an invalid format returns 400 instead of 404. - Add DEBUG logging on every rejection branch; only log the id value once it has passed UUID validation, to avoid CWE-117 log injection from the unvalidated query parameter. Refs: #34888 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 855c7d5 commit fb2fee4

3 files changed

Lines changed: 75 additions & 0 deletions

File tree

dotCMS/src/main/java/com/dotcms/rest/BaseRestPortlet.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ private String getJspResponse ( HttpServletRequest request, HttpServletResponse
121121
} catch (WebApplicationException e) {
122122
throw e;
123123
} catch (Exception e) {
124+
// Servlet containers (Jasper) wrap any Throwable raised inside a JSP in a
125+
// ServletException/JasperException, so a WebApplicationException thrown by
126+
// the JSP arrives here as a *cause* — walk the chain and re-throw it so
127+
// JAX-RS maps the proper HTTP status (e.g. 400/404) instead of returning
128+
// a 200 with debug HTML.
129+
for (Throwable t = e.getCause(); t != null; t = t.getCause()) {
130+
if (t instanceof WebApplicationException) {
131+
throw (WebApplicationException) t;
132+
}
133+
}
124134
Logger.debug(this.getClass(), "unable to parse: " + path);
125135
Logger.error( this.getClass(), e.toString(), e );
126136
StringWriter sw = new StringWriter();

dotCMS/src/main/webapp/WEB-INF/jsp/rules/include.jsp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
<%@ page import="com.dotmarketing.util.PortletURLUtil" %>
55
<%@page import="com.dotmarketing.portlets.contentlet.model.Contentlet"%>
66
<%@page import="com.dotmarketing.util.UtilMethods"%>
7+
<%@page import="com.dotmarketing.util.UUIDUtil"%>
8+
<%@page import="com.dotmarketing.util.Logger"%>
79
<%@page import="com.liferay.util.Xss"%>
810
<%@page import="javax.ws.rs.WebApplicationException"%>
911
<%@page import="javax.ws.rs.core.Response"%>
@@ -12,17 +14,28 @@
1214
<%
1315
final String id = request.getParameter("id");
1416
if (!UtilMethods.isSet(id)) {
17+
Logger.debug(this, "rules/include called with missing or empty 'id' parameter");
1518
throw new WebApplicationException(
1619
Response.status(Response.Status.BAD_REQUEST)
1720
.entity("Missing or empty required parameter: id")
1821
.build()
1922
);
2023
}
2124
25+
if (!UUIDUtil.isUUID(id)) {
26+
Logger.debug(this, "rules/include called with invalid id format");
27+
throw new WebApplicationException(
28+
Response.status(Response.Status.BAD_REQUEST)
29+
.entity("Invalid id format: " + Xss.encodeForHTML(id))
30+
.build()
31+
);
32+
}
33+
2234
Contentlet contentlet;
2335
try {
2436
contentlet = APILocator.getContentletAPI().findContentletByIdentifierAnyLanguage(id);
2537
} catch (final Exception e) {
38+
Logger.debug(this, "rules/include - findContentletByIdentifierAnyLanguage failed for id=" + id, e);
2639
throw new WebApplicationException(
2740
Response.status(Response.Status.BAD_REQUEST)
2841
.entity("Invalid id parameter: " + Xss.encodeForHTML(id))
@@ -31,6 +44,7 @@
3144
}
3245
3346
if (contentlet == null || !UtilMethods.isSet(contentlet.getIdentifier())) {
47+
Logger.debug(this, "rules/include - no contentlet found for id=" + id);
3448
throw new WebApplicationException(
3549
Response.status(Response.Status.NOT_FOUND)
3650
.entity("No content found for id: " + Xss.encodeForHTML(id))

dotCMS/src/test/java/com/dotcms/rest/BaseRestPortletTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,57 @@ public void getJspResponse_propagates404WhenJspThrowsWebApplicationException()
106106
}
107107
}
108108

109+
// -------------------------------------------------------------------------
110+
// ServletException-wrapped WebApplicationException (real Jasper behaviour)
111+
// -------------------------------------------------------------------------
112+
113+
@Test
114+
public void getJspResponse_unwrapsWebApplicationExceptionFromServletException()
115+
throws Exception {
116+
final WebApplicationException root = new WebApplicationException(
117+
Response.status(Response.Status.NOT_FOUND).entity("not found").build());
118+
// Tomcat/Jasper wraps any throwable raised inside a JSP in a ServletException
119+
// (specifically JasperException) before it reaches RequestDispatcher.include().
120+
final javax.servlet.ServletException wrapped =
121+
new javax.servlet.ServletException("jsp failed", root);
122+
doThrow(wrapped).when(mockDispatcher).include(any(), any());
123+
124+
try {
125+
getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
126+
fail("Expected wrapped WebApplicationException to be unwrapped and re-thrown");
127+
} catch (final InvocationTargetException e) {
128+
assertTrue("Cause must be WebApplicationException",
129+
e.getCause() instanceof WebApplicationException);
130+
assertEquals("HTTP status must be 404",
131+
Response.Status.NOT_FOUND.getStatusCode(),
132+
((WebApplicationException) e.getCause()).getResponse().getStatus());
133+
}
134+
}
135+
136+
@Test
137+
public void getJspResponse_unwrapsWebApplicationExceptionFromNestedCauseChain()
138+
throws Exception {
139+
final WebApplicationException root = new WebApplicationException(
140+
Response.status(Response.Status.BAD_REQUEST).entity("bad id").build());
141+
// Real Jasper wraps the JSP throwable; wrap again to simulate any extra layer
142+
// (e.g. Jersey or filter-level wrappers) so the unwrap walks the full chain.
143+
final javax.servlet.ServletException middle =
144+
new javax.servlet.ServletException("jsp failed", root);
145+
final RuntimeException outer = new RuntimeException("outer", middle);
146+
doThrow(outer).when(mockDispatcher).include(any(), any());
147+
148+
try {
149+
getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
150+
fail("Expected nested WebApplicationException to be unwrapped and re-thrown");
151+
} catch (final InvocationTargetException e) {
152+
assertTrue("Cause must be WebApplicationException",
153+
e.getCause() instanceof WebApplicationException);
154+
assertEquals("HTTP status must be 400",
155+
Response.Status.BAD_REQUEST.getStatusCode(),
156+
((WebApplicationException) e.getCause()).getResponse().getStatus());
157+
}
158+
}
159+
109160
// -------------------------------------------------------------------------
110161
// Ordinary exceptions → error HTML (existing behaviour preserved)
111162
// -------------------------------------------------------------------------

0 commit comments

Comments
 (0)