This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch SLING-13368 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
commit 2de3f95e0a2eaaf8c03a96279f3d8091ca3ad57e Author: Joerg Hoh <[email protected]> AuthorDate: Tue Sep 29 12:19:41 2026 +0200 SLING-13368 do not send a stacktrace if no ErrorHandler is registered --- .../sling/engine/impl/DefaultErrorHandler.java | 34 +++---- .../sling/engine/impl/DefaultErrorHandlerTest.java | 113 +++++++++++++++++++++ 2 files changed, 125 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java index 8320bf2..73788da 100644 --- a/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java +++ b/src/main/java/org/apache/sling/engine/impl/DefaultErrorHandler.java @@ -28,7 +28,6 @@ import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.SlingHttpServletResponse; import org.apache.sling.api.SlingJakartaHttpServletRequest; import org.apache.sling.api.SlingJakartaHttpServletResponse; -import org.apache.sling.api.request.RequestProgressTracker; import org.apache.sling.api.request.ResponseUtil; import org.apache.sling.api.servlets.ErrorHandler; import org.apache.sling.api.servlets.JakartaErrorHandler; @@ -171,6 +170,11 @@ public class DefaultErrorHandler implements JakartaErrorHandler { return; } + log.warn( + "handleError: No ErrorHandler service registered; every Sling instance should have one. " + + "Falling back to the minimal built-in error response for status {}", + status); + if (message == null) { message = "HTTP ERROR:" + String.valueOf(status); } else { @@ -185,8 +189,10 @@ public class DefaultErrorHandler implements JakartaErrorHandler { * <p> * This implementation resets the response before sending back a * standardized response which just conveys the status as 500/INTERNAL - * SERVER ERROR, the message from the throwable, the stacktrace, and server - * information. + * SERVER ERROR, the message from the throwable, and server information. + * The exception's stacktrace and the {@code RequestProgressTracker} dump + * are not sent to the client; they are only available in the server-side + * log (see the caller of this method). * <p> * This method logs error and does not write back and response data if the * response has already been committed. @@ -209,6 +215,9 @@ public class DefaultErrorHandler implements JakartaErrorHandler { return; } + log.warn("handleError: No ErrorHandler service registered; every Sling instance should have one. " + + "Falling back to the minimal built-in error response."); + sendError(status, throwable.getMessage(), throwable, request, response); } @@ -254,25 +263,6 @@ public class DefaultErrorHandler implements JakartaErrorHandler { } pw.println("</p>"); - if (throwable != null) { - final PrintWriter escapingWriter = new PrintWriter(ResponseUtil.getXmlEscapingWriter(pw)); - pw.println("<h3>Exception stacktrace:</h3>"); - pw.println("<pre>"); - pw.flush(); - throwable.printStackTrace(escapingWriter); - escapingWriter.flush(); - pw.println("</pre>"); - - final RequestProgressTracker tracker = - ((SlingJakartaHttpServletRequest) request).getRequestProgressTracker(); - pw.println("<h3>Request Progress:</h3>"); - pw.println("<pre>"); - pw.flush(); - tracker.dump(new PrintWriter(escapingWriter)); - escapingWriter.flush(); - pw.println("</pre>"); - } - pw.println("<hr /><address>"); pw.println(ResponseUtil.escapeXml(serverInfo)); pw.println("</address></body></html>"); diff --git a/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java b/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java new file mode 100644 index 0000000..a31979d --- /dev/null +++ b/src/test/java/org/apache/sling/engine/impl/DefaultErrorHandlerTest.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.engine.impl; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import org.apache.sling.api.SlingJakartaHttpServletRequest; +import org.apache.sling.api.SlingJakartaHttpServletResponse; +import org.apache.sling.api.request.RequestProgressTracker; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link DefaultErrorHandler}: with no {@code ErrorHandler}/ + * {@code JakartaErrorHandler} service bound, the built-in error response must + * not leak the exception's stacktrace or the {@link RequestProgressTracker} + * dump to the client. + */ +public class DefaultErrorHandlerTest { + + private DefaultErrorHandler handler; + private SlingJakartaHttpServletRequest request; + private SlingJakartaHttpServletResponse response; + private StringWriter responseBody; + private RequestProgressTracker tracker; + + @Before + public void setup() throws Exception { + handler = new DefaultErrorHandler(); + + tracker = mock(RequestProgressTracker.class); + + request = mock(SlingJakartaHttpServletRequest.class); + when(request.getRequestURI()).thenReturn("/content/test"); + when(request.getRequestProgressTracker()).thenReturn(tracker); + + response = mock(SlingJakartaHttpServletResponse.class); + responseBody = new StringWriter(); + when(response.getWriter()).thenReturn(new PrintWriter(responseBody)); + } + + @Test + public void testHandleThrowableWithoutDelegateDoesNotLeakStacktraceOrTracker() throws Exception { + final Exception cause = new IllegalStateException("some internal detail: /etc/secret-path"); + + handler.handleError(cause, request, response); + + verify(response).setStatus(500); + responseBody.flush(); + final String body = responseBody.toString(); + + // the exception's stacktrace must never be written to the response + assertFalse( + "response body must not contain a stacktrace frame", + body.contains("at org.apache.sling.engine.impl.DefaultErrorHandlerTest")); + assertFalse("response body must not mention the stacktrace section", body.contains("Exception stacktrace")); + + // the RequestProgressTracker must never be dumped into the response + assertFalse("response body must not contain the tracker dump section", body.contains("Request Progress")); + verify(tracker, never()).dump(org.mockito.ArgumentMatchers.any(PrintWriter.class)); + + // a minimal, generic error page is still rendered + assertTrue(body.contains("RequestURI=")); + } + + @Test + public void testHandleStatusWithoutDelegateStillRendersMessage() throws Exception { + handler.handleError(404, "not found", request, response); + + verify(response).setStatus(404); + responseBody.flush(); + assertTrue(responseBody.toString().contains("not found")); + } + + @Test + public void testHandleThrowableWithDelegateDoesNotUseFallback() throws Exception { + final org.apache.sling.api.servlets.JakartaErrorHandler delegate = + mock(org.apache.sling.api.servlets.JakartaErrorHandler.class); + handler.setDelegate(null, delegate); + + final Exception cause = new IllegalStateException("boom"); + handler.handleError(cause, request, response); + + verify(delegate).handleError(cause, request, response); + // the built-in fallback must not have written anything to the response + responseBody.flush(); + assertTrue(responseBody.toString().isEmpty()); + } +}
