Issue #12505 - Server to Servlet Error Handling (#12586)

* Issue #12505 server to servlet api error handling

---------

Co-authored-by: gregw <[email protected]>

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java

@@ -42,6 +42,7 @@
 import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.pathmap.MatchedResource;
 import org.eclipse.jetty.io.WriterOutputStream;
+import org.eclipse.jetty.server.handler.ErrorHandler;
 import org.eclipse.jetty.util.Fields;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.StringUtil;
@@ -58,10 +59,10 @@ public class Dispatcher implements RequestDispatcher
      * Dispatch include attribute names
      */
     public static final String __FORWARD_PREFIX = "jakarta.servlet.forward.";
-    
+
     /**
      * Name of original request attribute
-     */ 
+     */
     public static final String __ORIGINAL_REQUEST = "org.eclipse.jetty.originalRequest";
 
     public static final String JETTY_INCLUDE_HEADER_PREFIX = "org.eclipse.jetty.server.include.";
@@ -316,15 +317,15 @@ public String getRequestURI()
         @Override
         public StringBuffer getRequestURL()
         {
-            return _uri == null ? super.getRequestURL() :  new StringBuffer(HttpURI.build(_uri).query(null).scheme(super.getScheme()).host(super.getServerName()).port(super.getServerPort()).asString());
+            return _uri == null ? super.getRequestURL() : new StringBuffer(HttpURI.build(_uri).query(null).scheme(super.getScheme()).host(super.getServerName()).port(super.getServerPort()).asString());
         }
 
         @Override
         public Object getAttribute(String name)
         {
             if (name == null)
                 return null;
-            
+
             //Servlet Spec 9.4.2 no forward attributes if a named dispatcher
             if (_named != null && name.startsWith(__FORWARD_PREFIX))
                 return null;
@@ -356,7 +357,9 @@ public Object getAttribute(String name)
                     return originalRequest == null ? _httpServletRequest : originalRequest;
                 }
                 // Forward should hide include.
-                case RequestDispatcher.INCLUDE_MAPPING, RequestDispatcher.INCLUDE_SERVLET_PATH, RequestDispatcher.INCLUDE_PATH_INFO, RequestDispatcher.INCLUDE_REQUEST_URI, RequestDispatcher.INCLUDE_CONTEXT_PATH, RequestDispatcher.INCLUDE_QUERY_STRING ->
+                case RequestDispatcher.INCLUDE_MAPPING, RequestDispatcher.INCLUDE_SERVLET_PATH,
+                     RequestDispatcher.INCLUDE_PATH_INFO, RequestDispatcher.INCLUDE_REQUEST_URI,
+                     RequestDispatcher.INCLUDE_CONTEXT_PATH, RequestDispatcher.INCLUDE_QUERY_STRING ->
                 {
                     return null;
                 }
@@ -380,11 +383,11 @@ public Object getAttribute(String name)
         public Enumeration<String> getAttributeNames()
         {
             ArrayList<String> names = new ArrayList<>(Collections.list(super.getAttributeNames()));
-            
+
             //Servlet Spec 9.4.2 no forward attributes if a named dispatcher
             if (_named != null)
                 return Collections.enumeration(names);
-            
+
             names.add(RequestDispatcher.FORWARD_REQUEST_URI);
             names.add(RequestDispatcher.FORWARD_SERVLET_PATH);
             names.add(RequestDispatcher.FORWARD_PATH_INFO);
@@ -416,7 +419,7 @@ public Object getAttribute(String name)
         {
             if (name == null)
                 return null;
-            
+
             //Servlet Spec 9.3.1 no include attributes if a named dispatcher
             if (_named != null && name.startsWith(__INCLUDE_PREFIX))
                 return null;
@@ -440,7 +443,7 @@ public Enumeration<String> getAttributeNames()
             ArrayList<String> names = new ArrayList<>(Collections.list(super.getAttributeNames()));
             if (_named != null)
                 return Collections.enumeration(names);
-            
+
             names.add(RequestDispatcher.INCLUDE_MAPPING);
             names.add(RequestDispatcher.INCLUDE_SERVLET_PATH);
             names.add(RequestDispatcher.INCLUDE_PATH_INFO);
@@ -462,7 +465,7 @@ private static class IncludeResponse extends HttpServletResponseWrapper
         ServletOutputStream _servletOutputStream;
         PrintWriter _printWriter;
         PrintWriter _mustFlush;
-        
+
         public IncludeResponse(HttpServletResponse response)
         {
             super(response);
@@ -753,9 +756,12 @@ public Enumeration<String> getAttributeNames()
 
     private class ErrorRequest extends ParameterRequestWrapper
     {
+        private final HttpServletRequest _httpServletRequest;
+
         public ErrorRequest(HttpServletRequest httpRequest)
         {
             super(httpRequest);
+            _httpServletRequest = httpRequest;
         }
 
         @Override
@@ -797,6 +803,34 @@ public StringBuffer getRequestURL()
                 .port(getServerPort())
                 .asString());
         }
+
+        @Override
+        public Object getAttribute(String name)
+        {
+            return switch (name)
+            {
+                case ERROR_REQUEST_URI -> _httpServletRequest.getRequestURI();
+                case ERROR_STATUS_CODE -> super.getAttribute(ErrorHandler.ERROR_STATUS);
+                case ERROR_MESSAGE -> super.getAttribute(ErrorHandler.ERROR_MESSAGE);
+                case ERROR_SERVLET_NAME -> super.getAttribute(ErrorHandler.ERROR_ORIGIN);
+                case ERROR_EXCEPTION -> super.getAttribute(ErrorHandler.ERROR_EXCEPTION);
+                case ERROR_EXCEPTION_TYPE ->
+                {
+                    Object err = super.getAttribute(ErrorHandler.ERROR_EXCEPTION);
+                    yield err == null ? null : err.getClass();
+                }
+                default -> super.getAttribute(name);
+            };
+        }
+
+        @Override
+        public Enumeration<String> getAttributeNames()
+        {
+            // TODO add all names?
+            List<String> names = new ArrayList<>(List.of(ERROR_REQUEST_URI, ERROR_STATUS_CODE, ERROR_MESSAGE, ERROR_SERVLET_NAME, ERROR_EXCEPTION, ERROR_EXCEPTION_TYPE));
+            names.addAll(Collections.list(super.getAttributeNames()));
+            return Collections.enumeration(names);
+        }
     }
 
     @Override

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorHandler.java

@@ -27,7 +27,6 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
@@ -128,7 +127,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
             }
         }
 
-        String message = (String)request.getAttribute(Dispatcher.ERROR_MESSAGE);
+        String message = (String)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_MESSAGE);
         if (message == null)
             message = HttpStatus.getMessage(response.getStatus());
         generateAcceptableResponse(servletContextRequest, httpServletRequest, httpServletResponse, response.getStatus(), message);
@@ -416,9 +415,9 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer,
         htmlRow(writer, "MESSAGE", message);
         if (isShowServlet())
         {
-            htmlRow(writer, "SERVLET", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
+            htmlRow(writer, "SERVLET", request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN));
         }
-        Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+        Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
         while (cause != null)
         {
             htmlRow(writer, "CAUSED BY", cause);
@@ -451,9 +450,9 @@ protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, i
         writer.printf("MESSAGE: %s%n", message);
         if (isShowServlet())
         {
-            writer.printf("SERVLET: %s%n", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
+            writer.printf("SERVLET: %s%n", request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN));
         }
-        Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+        Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
         while (cause != null)
         {
             writer.printf("CAUSED BY %s%n", cause);
@@ -467,8 +466,8 @@ protected void writeErrorPlain(HttpServletRequest request, PrintWriter writer, i
 
     protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, int code, String message)
     {
-        Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
-        Object servlet = request.getAttribute(Dispatcher.ERROR_SERVLET_NAME);
+        Throwable cause = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
+        Object servlet = request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_ORIGIN);
         Map<String, String> json = new HashMap<>();
 
         json.put("url", request.getRequestURI());
@@ -492,7 +491,7 @@ protected void writeErrorJson(HttpServletRequest request, PrintWriter writer, in
 
     protected void writeErrorPageStacks(HttpServletRequest request, Writer writer) throws IOException
     {
-        Throwable th = (Throwable)request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
+        Throwable th = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
         if (th != null)
         {
             writer.write("<h3>Caused by:</h3><pre>");

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ErrorPageErrorHandler.java

@@ -65,7 +65,7 @@ public String getErrorPage(HttpServletRequest request)
         PageLookupTechnique pageSource = null;
 
         Class<?> matchedThrowable = null;
-        Throwable error = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+        Throwable error = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
         Throwable cause = error;
 
         // Walk the cause hierarchy
@@ -98,6 +98,7 @@ public String getErrorPage(HttpServletRequest request)
             {
                 request.setAttribute(Dispatcher.ERROR_EXCEPTION, unwrapped);
                 request.setAttribute(Dispatcher.ERROR_EXCEPTION_TYPE, unwrapped.getClass());
+                request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION, unwrapped);
             }
         }
 
@@ -108,7 +109,7 @@ public String getErrorPage(HttpServletRequest request)
             pageSource = PageLookupTechnique.STATUS_CODE;
 
             // look for an exact code match
-            errorStatusCode = (Integer)request.getAttribute(Dispatcher.ERROR_STATUS_CODE);
+            errorStatusCode = (Integer)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS);
             if (errorStatusCode != null)
             {
                 errorPage = _errorPages.get(Integer.toString(errorStatusCode));
@@ -149,7 +150,7 @@ public String getErrorPage(HttpServletRequest request)
                     dbg.append(" (using matched Throwable ");
                     dbg.append(matchedThrowable.getName());
                     dbg.append(" / actually thrown as ");
-                    Throwable originalThrowable = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+                    Throwable originalThrowable = (Throwable)request.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION);
                     dbg.append(originalThrowable.getClass().getName());
                     dbg.append(')');
                     LOG.debug(dbg.toString(), cause);

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResourceServlet.java

@@ -780,7 +780,7 @@ protected void writeHttpError(Request coreRequest, Response coreResponse, Callba
                 if (isIncluded(request))
                     return;
                 if (cause != null)
-                    request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, cause);
+                    request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_EXCEPTION, cause);
                 response.sendError(statusCode, reason);
             }
             catch (IOException e)

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java

@@ -461,7 +461,7 @@ public void handle()
 
                             // the following is needed as you cannot trust the response code and reason
                             // as those could have been modified after calling sendError
-                            Integer code = (Integer)_servletContextRequest.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
+                            Integer code = (Integer)_servletContextRequest.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS);
                             if (code == null)
                                 code = HttpStatus.INTERNAL_SERVER_ERROR_500;
                             getServletContextResponse().setStatus(code);
@@ -474,7 +474,7 @@ public void handle()
                             if (!_httpInput.consumeAvailable())
                                 ResponseUtils.ensureNotPersistent(_servletContextRequest, _servletContextRequest.getServletContextResponse());
 
-                            ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(ErrorHandler.ERROR_CONTEXT);
+                            ContextHandler.ScopedContext context = (ContextHandler.ScopedContext)_servletContextRequest.getAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_CONTEXT);
                             Request.Handler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler());
 
                             // If we can't have a body or have no ErrorHandler, then create a minimal error response.

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java

@@ -40,6 +40,7 @@
 import static jakarta.servlet.RequestDispatcher.ERROR_REQUEST_URI;
 import static jakarta.servlet.RequestDispatcher.ERROR_SERVLET_NAME;
 import static jakarta.servlet.RequestDispatcher.ERROR_STATUS_CODE;
+import static org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS;
 
 /**
  * holder of the state of request-response cycle.
@@ -423,6 +424,16 @@ public Action handling()
                         throw new IllegalStateException(getStatusStringLocked());
                     _initial = true;
                     _state = State.HANDLING;
+                    if (_servletChannel.getResponse().getStatus() != 0)
+                    {
+                        if (_servletChannel.getRequest().getAttribute(ERROR_STATUS) instanceof Integer errorCode)
+                        {
+                            _servletChannel.getServletRequestState().sendError(errorCode, null);
+                            _requestState = RequestState.BLOCKING;
+                            _sendError = false;
+                            return Action.SEND_ERROR;
+                        }
+                    }
                     return Action.DISPATCH;
 
                 case WOKEN:
@@ -1022,7 +1033,6 @@ else if (cause instanceof UnavailableException)
 
     public void sendError(int code, String message)
     {
-        // This method is called by Response.sendError to organise for an error page to be generated when it is possible:
         //  + The response is reset and temporarily closed.
         //  + The details of the error are saved as request attributes
         //  + The _sendError boolean is set to true so that an ERROR_DISPATCH action will be generated:
@@ -1058,21 +1068,18 @@ public void sendError(int code, String message)
             response.setStatus(code);
             servletContextRequest.errorClose();
 
-            request.setAttribute(org.eclipse.jetty.ee10.servlet.ErrorHandler.ERROR_CONTEXT, servletContextRequest.getErrorContext());
-            request.setAttribute(ERROR_REQUEST_URI, httpServletRequest.getRequestURI());
-            request.setAttribute(ERROR_SERVLET_NAME, servletContextRequest.getServletName());
-            request.setAttribute(ERROR_STATUS_CODE, code);
-            request.setAttribute(ERROR_MESSAGE, message);
-
             // Set Jetty Specific Attributes.
             request.setAttribute(ErrorHandler.ERROR_CONTEXT, servletContextRequest.getServletContext());
             request.setAttribute(ErrorHandler.ERROR_MESSAGE, message);
             request.setAttribute(ErrorHandler.ERROR_STATUS, code);
+            request.setAttribute(ErrorHandler.ERROR_ORIGIN, servletContextRequest.getServletName());
 
             _sendError = true;
             if (_event != null)
             {
                 Throwable cause = (Throwable)request.getAttribute(ERROR_EXCEPTION);
+                if (cause == null)
+                    cause = (Throwable)request.getAttribute(ErrorHandler.ERROR_EXCEPTION);
                 if (cause != null)
                     _event.addThrowable(cause);
             }

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java

@@ -73,6 +73,7 @@
 import org.eclipse.jetty.ee10.servlet.security.ConstraintAware;
 import org.eclipse.jetty.ee10.servlet.security.ConstraintMapping;
 import org.eclipse.jetty.ee10.servlet.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpURI;
 import org.eclipse.jetty.http.pathmap.MatchedResource;
 import org.eclipse.jetty.io.IOResources;
@@ -1147,7 +1148,6 @@ protected ContextRequest wrapRequest(Request request, Response response)
         decodedPathInContext = URIUtil.decodePath(getContext().getPathInContext(request.getHttpURI().getCanonicalPath()));
         matchedResource = _servletHandler.getMatchedServlet(decodedPathInContext);
 
-
         if (matchedResource == null)
             return wrapNoServlet(request, response);
         ServletHandler.MappedServlet mappedServlet = matchedResource.getResource();
@@ -1196,7 +1196,17 @@ protected boolean handleByContextHandler(String pathInContext, ContextRequest re
         boolean initialDispatch = request instanceof ServletContextRequest;
         if (!initialDispatch)
             return false;
-        return super.handleByContextHandler(pathInContext, request, response, callback);
+
+        if (isProtectedTarget(pathInContext))
+        {
+            // At this point we have not entered the state machine of the ServletChannelState, so we do nothing here
+            // other than to set the error status attribute (and also status). When execution proceeds normally into the
+            // state machine the request will be treated as an error. Note that we set both the error status and request
+            // status because the request status is cheaper to check than the error status attribute.
+            request.setAttribute(org.eclipse.jetty.server.handler.ErrorHandler.ERROR_STATUS, 404);
+            response.setStatus(HttpStatus.NOT_FOUND_404);
+        }
+        return false;
     }
 
     @Override