fix(rest-api): unwrap WebApplicationException from JasperException in rules include endpoint

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

@@ -121,19 +121,22 @@ private String getJspResponse ( HttpServletRequest request, HttpServletResponse
 		} catch (WebApplicationException e) {
 			throw e;
 		} catch (Exception e) {
-			Logger.debug(this.getClass(), "unable to parse: " + path);
-			Logger.error( this.getClass(), e.toString(), e );
-			StringWriter sw = new StringWriter();
-			sw.append("<div style='padding:30px;'>");
-			sw.append("unable to parse: <a href='" + path + "' target='debug'>" + path + "</a>");
-			sw.append("<hr>");
-			sw.append("<pre style='width:90%;overflow:hidden;white-space:pre-wrap'>");
-			sw.append(e.toString());
-
-			sw.append("</pre>");
-			sw.append("</div>");
-			return sw.toString();
-
+			// Servlet containers (Jasper) wrap any Throwable raised inside a JSP in a
+			// ServletException/JasperException, so a WebApplicationException thrown by
+			// the JSP arrives here as a *cause* — walk the chain and re-throw it so
+			// JAX-RS maps the proper HTTP status (e.g. 400/404) instead of returning
+			// a 200 with debug HTML.
+			for (Throwable t = e.getCause(); t != null; t = t.getCause()) {
+				if (t instanceof WebApplicationException) {
+					throw (WebApplicationException) t;
+				}
+			}
+			// For any other failure, log the cause so admins/devs can investigate
+			// and return a plain 500 — never leak internal exception details in
+			// the response body, and never return HTTP 200 for a failed render.
+			Logger.error(this.getClass(), "Failed to render JSP: " + path, e);
+			throw new WebApplicationException(
+					Response.status(Response.Status.INTERNAL_SERVER_ERROR).build());
 		}
 
 	}

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

@@ -1,9 +1,12 @@
 <%@ page import="com.dotmarketing.util.Config" %>
 <%@page import="com.dotmarketing.business.APILocator"%>
+<%@page import="com.dotmarketing.business.PermissionAPI"%>
 <%@page import="com.dotcms.repackage.org.apache.struts.Globals"%>
 <%@ page import="com.dotmarketing.util.PortletURLUtil" %>
 <%@page import="com.dotmarketing.portlets.contentlet.model.Contentlet"%>
 <%@page import="com.dotmarketing.util.UtilMethods"%>
+<%@page import="com.dotmarketing.util.UUIDUtil"%>
+<%@page import="com.dotmarketing.util.Logger"%>
 <%@page import="com.liferay.util.Xss"%>
 <%@page import="javax.ws.rs.WebApplicationException"%>
 <%@page import="javax.ws.rs.core.Response"%>
@@ -12,17 +15,28 @@
 <%
     final String id = request.getParameter("id");
     if (!UtilMethods.isSet(id)) {
+        Logger.debug(this, "rules/include called with missing or empty 'id' parameter");
         throw new WebApplicationException(
             Response.status(Response.Status.BAD_REQUEST)
                 .entity("Missing or empty required parameter: id")
                 .build()
         );
     }
 
+    if (!UUIDUtil.isUUID(id)) {
+        Logger.debug(this, "rules/include called with invalid id format");
+        throw new WebApplicationException(
+            Response.status(Response.Status.BAD_REQUEST)
+                .entity("Invalid id format: " + Xss.encodeForHTML(id))
+                .build()
+        );
+    }
+
     Contentlet contentlet;
     try {
         contentlet = APILocator.getContentletAPI().findContentletByIdentifierAnyLanguage(id);
     } catch (final Exception e) {
+        Logger.debug(this, "rules/include - findContentletByIdentifierAnyLanguage failed for id=" + id, e);
         throw new WebApplicationException(
             Response.status(Response.Status.BAD_REQUEST)
                 .entity("Invalid id parameter: " + Xss.encodeForHTML(id))
@@ -31,12 +45,23 @@
     }
 
     if (contentlet == null || !UtilMethods.isSet(contentlet.getIdentifier())) {
+        Logger.debug(this, "rules/include - no contentlet found for id=" + id);
         throw new WebApplicationException(
             Response.status(Response.Status.NOT_FOUND)
                 .entity("No content found for id: " + Xss.encodeForHTML(id))
                 .build()
         );
     }
+
+    if (user == null || !APILocator.getPermissionAPI().doesUserHavePermission(
+            contentlet, PermissionAPI.PERMISSION_READ, user, false)) {
+        Logger.debug(this, "rules/include - user lacks READ on id=" + id);
+        throw new WebApplicationException(
+            Response.status(Response.Status.FORBIDDEN)
+                .entity("Access denied")
+                .build()
+        );
+    }
 %>
 
 

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

@@ -1,6 +1,7 @@
 package com.dotcms.rest;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
@@ -14,6 +15,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.WebApplicationException;
@@ -107,32 +109,97 @@ public void getJspResponse_propagates404WhenJspThrowsWebApplicationException()
     }
 
     // -------------------------------------------------------------------------
-    // Ordinary exceptions → error HTML (existing behaviour preserved)
+    // ServletException-wrapped WebApplicationException (real Jasper behaviour)
     // -------------------------------------------------------------------------
 
     @Test
-    public void getJspResponse_returnsErrorHtmlForGenericException() throws Exception {
-        doThrow(new IOException("disk full")).when(mockDispatcher).include(any(), any());
+    public void getJspResponse_unwrapsWebApplicationExceptionFromServletException()
+            throws Exception {
+        final WebApplicationException root = new WebApplicationException(
+                Response.status(Response.Status.NOT_FOUND).entity("not found").build());
+        // Tomcat/Jasper wraps any throwable raised inside a JSP in a ServletException
+        // (specifically JasperException) before it reaches RequestDispatcher.include().
+        final ServletException wrapped =
+                new ServletException("jsp failed", root);
+        doThrow(wrapped).when(mockDispatcher).include(any(), any());
+
+        try {
+            getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
+            fail("Expected wrapped WebApplicationException to be unwrapped and re-thrown");
+        } catch (final InvocationTargetException e) {
+            assertTrue("Cause must be WebApplicationException",
+                    e.getCause() instanceof WebApplicationException);
+            assertEquals("HTTP status must be 404",
+                    Response.Status.NOT_FOUND.getStatusCode(),
+                    ((WebApplicationException) e.getCause()).getResponse().getStatus());
+        }
+    }
 
-        final Object result = getJspResponse.invoke(
-                portlet, mockRequest, mockResponse, "rules", "include");
+    @Test
+    public void getJspResponse_unwrapsWebApplicationExceptionFromNestedCauseChain()
+            throws Exception {
+        final WebApplicationException root = new WebApplicationException(
+                Response.status(Response.Status.BAD_REQUEST).entity("bad id").build());
+        // Real Jasper wraps the JSP throwable; wrap again to simulate any extra layer
+        // (e.g. Jersey or filter-level wrappers) so the unwrap walks the full chain.
+        final ServletException middle =
+                new ServletException("jsp failed", root);
+        final RuntimeException outer = new RuntimeException("outer", middle);
+        doThrow(outer).when(mockDispatcher).include(any(), any());
 
-        assertTrue("Result must be a String", result instanceof String);
-        final String html = (String) result;
-        assertTrue("Error HTML must mention the JSP path", html.contains("rules"));
-        assertTrue("Error HTML must include the exception message", html.contains("disk full"));
+        try {
+            getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
+            fail("Expected nested WebApplicationException to be unwrapped and re-thrown");
+        } catch (final InvocationTargetException e) {
+            assertTrue("Cause must be WebApplicationException",
+                    e.getCause() instanceof WebApplicationException);
+            assertEquals("HTTP status must be 400",
+                    Response.Status.BAD_REQUEST.getStatusCode(),
+                    ((WebApplicationException) e.getCause()).getResponse().getStatus());
+        }
     }
 
+    // -------------------------------------------------------------------------
+    // Non-WebApplicationException failures → WebApplicationException(500)
+    // (no debug HTML, no HTTP 200, no internal details leaked in the response)
+    // -------------------------------------------------------------------------
+
     @Test
-    public void getJspResponse_returnsErrorHtmlForRuntimeException() throws Exception {
+    public void getJspResponse_throws500ForGenericException() throws Exception {
+        doThrow(new IOException("disk full")).when(mockDispatcher).include(any(), any());
+
+        try {
+            getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
+            fail("Expected WebApplicationException(500) for non-WAE failures");
+        } catch (final InvocationTargetException e) {
+            assertTrue("Cause must be WebApplicationException",
+                    e.getCause() instanceof WebApplicationException);
+            final WebApplicationException wae = (WebApplicationException) e.getCause();
+            assertEquals("HTTP status must be 500",
+                    Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
+                    wae.getResponse().getStatus());
+            assertFalse("Response body must not leak the internal exception message",
+                    wae.getResponse().hasEntity());
+        }
+    }
+
+    @Test
+    public void getJspResponse_throws500ForRuntimeException() throws Exception {
         doThrow(new IllegalArgumentException("bad uuid"))
                 .when(mockDispatcher).include(any(), any());
 
-        final Object result = getJspResponse.invoke(
-                portlet, mockRequest, mockResponse, "rules", "include");
-
-        assertTrue("Result must be a String", result instanceof String);
-        assertTrue("Error HTML must include the exception message",
-                ((String) result).contains("bad uuid"));
+        try {
+            getJspResponse.invoke(portlet, mockRequest, mockResponse, "rules", "include");
+            fail("Expected WebApplicationException(500) for non-WAE failures");
+        } catch (final InvocationTargetException e) {
+            assertTrue("Cause must be WebApplicationException",
+                    e.getCause() instanceof WebApplicationException);
+            final WebApplicationException wae = (WebApplicationException) e.getCause();
+            assertEquals("HTTP status must be 500",
+                    Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
+                    wae.getResponse().getStatus());
+            assertFalse("Response body must not leak the internal exception message",
+                    wae.getResponse().hasEntity());
+        }
     }
 }