1 files changed, 47 insertions, 38 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java
index 511451b3577..7f3af1258b6 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java
@@ -6,10 +6,13 @@ import com.yahoo.jdisc.handler.BindingNotFoundException;
 import com.yahoo.jdisc.handler.CompletionHandler;
 import com.yahoo.jdisc.handler.ContentChannel;
 import com.yahoo.jdisc.handler.ResponseHandler;
+import com.yahoo.jdisc.http.HttpHeaders;
 import com.yahoo.jdisc.http.HttpResponse;
 import com.yahoo.jdisc.service.BindingSetNotFoundException;
+import org.eclipse.jetty.http.MimeTypes;
 
 import javax.annotation.concurrent.GuardedBy;
+import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.io.PrintWriter;
@@ -24,8 +27,10 @@ import java.util.logging.Logger;
 
 /**
  * @author tonytv
+ * @author bjorncs
  */
 public class ServletResponseController {
+
     private static Logger log = Logger.getLogger(ServletResponseController.class.getName());
 
     /**
@@ -37,9 +42,10 @@ public class ServletResponseController {
     private final Object monitor = new Object();
 
     //servletResponse must not be modified after the response has been committed.
+    private final HttpServletRequest servletRequest;
     private final HttpServletResponse servletResponse;
     private final boolean developerMode;
-    private final Executor executor;
+    private final ErrorResponseContentCreator errorResponseContentCreator = new ErrorResponseContentCreator();
 
     //all calls to the servletOutputStreamWriter must hold the monitor first to ensure visibility of servletResponse changes.
     private final ServletOutputStreamWriter servletOutputStreamWriter;
@@ -47,17 +53,16 @@ public class ServletResponseController {
     @GuardedBy("monitor")
     private boolean responseCommitted = false;
 
-
-
     public ServletResponseController(
+            HttpServletRequest servletRequest,
             HttpServletResponse servletResponse,
             Executor executor,
             MetricReporter metricReporter,
             boolean developerMode) throws IOException {
 
+        this.servletRequest = servletRequest;
         this.servletResponse = servletResponse;
         this.developerMode = developerMode;
-        this.executor = executor;
         this.servletOutputStreamWriter =
                 new ServletOutputStreamWriter(servletResponse.getOutputStream(), executor, metricReporter);
     }
@@ -89,50 +94,60 @@ public class ServletResponseController {
 
 
     public void trySendError(Throwable t) {
-        final boolean responseWasCommitted;
-
-        synchronized (monitor) {
-            responseWasCommitted = responseCommitted;
+        String reasonPhrase = getReasonPhrase(t, developerMode);
+        int statusCode = getStatusCode(t);
 
-            if (!responseCommitted) {
-                responseCommitted = true;
-                servletOutputStreamWriter.setSendingError();
+        final boolean responseWasCommitted;
+        try {
+            synchronized (monitor) {
+                responseWasCommitted = responseCommitted;
+                if (!responseCommitted) {
+                    responseCommitted = true;
+                    sendErrorAsync(statusCode, reasonPhrase);
+                }
             }
+        } catch (Throwable e) {
+            servletOutputStreamWriter.fail(t);
+            return;
         }
 
         //Must be evaluated after state transition for test purposes(See ConformanceTestException)
         //Done outside the monitor since it causes a callback in tests.
-        String reasonPhrase = getReasonPhrase(t, developerMode);
-        int statusCode = getStatusCode(t);
-
         if (responseWasCommitted) {
-
             RuntimeException exceptionWithStackTrace = new RuntimeException(t);
             log.log(Level.FINE, "Response already committed, can't change response code", exceptionWithStackTrace);
             // TODO: should always have failed here, but that breaks test assumptions. Doing soft close instead.
             //assert !Thread.holdsLock(monitor);
             //servletOutputStreamWriter.fail(t);
             servletOutputStreamWriter.close(null);
-            return;
         }
 
-        try {
-
-            // HttpServletResponse.sendError() is blocking and must not be executed in Jetty/RequestHandler thread.
-            executor.execute(() -> {
-                try {
-                    // TODO We should control the response content this method generates
-                    // a response body based on Jetty's own response templates ("Powered by Jetty").
-                    servletResponse.sendError(statusCode, reasonPhrase);
-                    finishedFuture().complete(null);
-                } catch (IOException e) {
-                    log.severe("Failed to send error response: " + e.getMessage());
-                    throw new RuntimeException(e);
-                }
-            });
+    }
 
-        } catch (Throwable e) {
-            servletOutputStreamWriter.fail(t);
+    /**
+     * Async version of {@link org.eclipse.jetty.server.Response#sendError(int, String)}.
+     */
+    private void sendErrorAsync(int statusCode, String reasonPhrase) {
+        servletResponse.setHeader(HttpHeaders.Names.EXPIRES, null);
+        servletResponse.setHeader(HttpHeaders.Names.LAST_MODIFIED, null);
+        servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, null);
+        servletResponse.setHeader(HttpHeaders.Names.CONTENT_TYPE, null);
+        servletResponse.setHeader(HttpHeaders.Names.CONTENT_LENGTH, null);
+        setStatus(servletResponse, statusCode, Optional.of(reasonPhrase));
+
+        // If we are allowed to have a body
+        if (statusCode != HttpServletResponse.SC_NO_CONTENT &&
+                statusCode != HttpServletResponse.SC_NOT_MODIFIED &&
+                statusCode != HttpServletResponse.SC_PARTIAL_CONTENT &&
+                statusCode >= HttpServletResponse.SC_OK) {
+            servletResponse.setHeader(HttpHeaders.Names.CACHE_CONTROL, "must-revalidate,no-cache,no-store");
+            servletResponse.setContentType(MimeTypes.Type.TEXT_HTML_8859_1.toString());
+            byte[] errorContent = errorResponseContentCreator
+                    .createErrorContent(servletRequest.getRequestURI(), statusCode, Optional.ofNullable(reasonPhrase));
+            servletResponse.setContentLength(errorContent.length);
+            servletOutputStreamWriter.sendErrorContentAndCloseAsync(ByteBuffer.wrap(errorContent));
+        } else {
+            servletOutputStreamWriter.close(null);
         }
     }
 
@@ -207,12 +222,6 @@ public class ServletResponseController {
         }
     }
 
-    public boolean isResponseCommitted() {
-        synchronized (monitor) {
-            return responseCommitted;
-        }
-    }
-
     public final ResponseHandler responseHandler = new ResponseHandler() {
         @Override
         public ContentChannel handleResponse(Response response) {