Issue #3806 async sendError (minimal fix) (#3875)

* Issue #3806 async sendError

This is a minimal fix for the async race of sendError using isHandled
at the same time as the normal dispatch is exiting.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #3806 async sendError

Fixed isStreaming method in minimal fix (showing the need for actual
test harnesses).

Signed-off-by: Greg Wilkins <gregw@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -203,6 +203,7 @@ public class Request implements HttpServletRequest
     private String _contentType;
     private String _characterEncoding;
     private ContextHandler.Context _context;
+    private ContextHandler.Context _errorContext;
     private CookieCutter _cookies;
     private DispatcherType _dispatcherType;
     private int _inputState = INPUT_NONE;
@@ -727,6 +728,14 @@ public class Request implements HttpServletRequest
         return _context;
     }
 
+    /**
+     * @return The current {@link Context context} used for this request, or <code>null</code> if {@link #setContext} has not yet been called.
+     */
+    public Context getErrorContext()
+    {
+        return _errorContext;
+    }
+
     /*
      * @see javax.servlet.http.HttpServletRequest#getContextPath()
      */
@@ -1917,7 +1926,13 @@ public class Request implements HttpServletRequest
     public void setContext(Context context)
     {
         _newContext = _context != context;
-        _context = context;
+        if (context == null)
+            _context = null;
+        else
+        {
+            _context = context;
+            _errorContext = context;
+        }
     }
 
     /**

jetty-server/src/main/java/org/eclipse/jetty/server/Response.java

@@ -440,7 +440,7 @@ public class Response implements HttpServletResponse
         if (code != SC_NO_CONTENT && code != SC_NOT_MODIFIED &&
             code != SC_PARTIAL_CONTENT && code >= SC_OK)
         {
-            ContextHandler.Context context = request.getContext();
+            ContextHandler.Context context = request.getErrorContext();
             ContextHandler contextHandler = context == null ? _channel.getState().getContextHandler() : context.getContextHandler();
             request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, code);
             request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message);
@@ -722,6 +722,11 @@ public class Response implements HttpServletResponse
         return _outputType == OutputType.WRITER;
     }
 
+    public boolean isStreaming()
+    {
+        return _outputType == OutputType.STREAM;
+    }
+
     @Override
     public PrintWriter getWriter() throws IOException
     {

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java

@@ -27,7 +27,6 @@ import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 import javax.servlet.RequestDispatcher;
-import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -91,10 +90,10 @@ public class ErrorHandler extends AbstractHandler
             if (errorPage != null)
             {
                 String oldErrorPage = (String)request.getAttribute(ERROR_PAGE);
-                ServletContext servletContext = request.getServletContext();
-                if (servletContext == null)
-                    servletContext = ContextHandler.getCurrentContext();
-                if (servletContext == null)
+                ContextHandler.Context context = baseRequest.getErrorContext();
+                if (context == null)
+                    context = ContextHandler.getCurrentContext();
+                if (context == null)
                 {
                     LOG.warn("No ServletContext for error page {}", errorPage);
                 }
@@ -106,7 +105,7 @@ public class ErrorHandler extends AbstractHandler
                 {
                     request.setAttribute(ERROR_PAGE, errorPage);
 
-                    Dispatcher dispatcher = (Dispatcher)servletContext.getRequestDispatcher(errorPage);
+                    Dispatcher dispatcher = (Dispatcher)context.getRequestDispatcher(errorPage);
                     try
                     {
                         if (LOG.isDebugEnabled())
@@ -168,11 +167,10 @@ public class ErrorHandler extends AbstractHandler
             for (String mimeType : acceptable)
             {
                 generateAcceptableResponse(baseRequest, request, response, code, message, mimeType);
-                if (baseRequest.isHandled())
+                if (response.isCommitted() || baseRequest.getResponse().isWriting() || baseRequest.getResponse().isStreaming())
                     break;
             }
         }
-        baseRequest.setHandled(true);
         baseRequest.getResponse().closeOutput();
     }
 

jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java

@@ -221,6 +221,20 @@ public class ErrorHandlerTest
         assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1"));
     }
 
+    @Test
+    public void testMoreSpecificAccept() throws Exception
+    {
+        String response = connector.getResponse(
+            "GET / HTTP/1.1\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html, some/other;specific=true\r\n" +
+                "\r\n");
+
+        assertThat(response, startsWith("HTTP/1.1 404 "));
+        assertThat(response, not(containsString("Content-Length: 0")));
+        assertThat(response, containsString("Content-Type: text/html;charset=iso-8859-1"));
+    }
+
     @Test
     public void test404HtmlAcceptAnyCharset() throws Exception
     {