diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 3d479670005f..869803064b93 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -1384,8 +1384,18 @@ public void demand() @Override public void prepareResponse(HttpFields.Mutable headers) { - if (_connectionKeepAlive && _version == HttpVersion.HTTP_1_0 && !headers.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) - headers.add(HttpFields.CONNECTION_KEEPALIVE); + // If the HTTP/1 generator is set to no longer be persistent then the + // logic for handling HTTP/1.0 shouldn't occur. This typically happens + // in the case of errors with the request or response. +// if (_version == HttpVersion.HTTP_1_0 && _generator.isPersistent()) + { + // attempt to cover HTTP/1.0 case where the Connection response header + // doesn't have a close value (set by the application) and it needs to + // be represented as a `Connection: keep-alive` response instead due + // to the existence of the `Connection: keep-alive` request header + if (_connectionKeepAlive && !headers.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) + headers.add(HttpFields.CONNECTION_KEEPALIVE); + } } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ErrorHandlerTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ErrorHandlerTest.java index f9ec70681b74..d3398df95480 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ErrorHandlerTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ErrorHandlerTest.java @@ -31,6 +31,7 @@ import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.io.Content; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; @@ -38,7 +39,6 @@ import org.eclipse.jetty.util.ajax.JSON; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -55,7 +55,6 @@ import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertTrue; -@Disabled // TODO public class ErrorHandlerTest { StacklessLogging stacklessLogging; @@ -281,17 +280,21 @@ public void test404PostHttp11() throws Exception @Test public void test404PostCantConsumeHttp10() throws Exception { - String rawResponse = connector.getResponse( - "POST / HTTP/1.0\r\n" + - "Host: Localhost\r\n" + - "Accept: text/html\r\n" + - "Content-Length: 100\r\n" + - "Connection: keep-alive\r\n" + - "\r\n" + - "0123456789"); - + String rawRequest = """ + POST / HTTP/1.0\r + Host: Localhost\r + Accept: text/html\r + Content-Length: 100\r + Connection: keep-alive\r + \r + 0123456789 + """; + + String rawResponse = connector.getResponse(rawRequest); HttpTester.Response response = HttpTester.parseResponse(rawResponse); + dump(response); + assertThat(response.getStatus(), is(404)); assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0)); assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1")); @@ -308,7 +311,6 @@ public void test404PostCantConsumeHttp11() throws Exception "Host: Localhost\r\n" + "Accept: text/html\r\n" + "Content-Length: 100\r\n" + - "Connection: keep-alive\r\n" + // This is not need by HTTP/1.1 but sometimes sent anyway "\r\n" + "0123456789"); @@ -703,7 +705,7 @@ public void testErrorContextRecycle() throws Exception context.setErrorHandler(new ErrorHandler() { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException { baseRequest.setHandled(true); response.getOutputStream().println("Context Error"); @@ -717,35 +719,32 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques response.sendError(444); } }); - - context.setErrorHandler(new ErrorHandler() + server.setErrorHandler((request, response, callback) -> { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - baseRequest.setHandled(true); - response.getOutputStream().println("Server Error"); - } + Content.Sink.write(response, true, "Server Error", callback); + return true; }); server.start(); - LocalConnector.LocalEndPoint connection = connector.connect(); - connection.addInputAndExecute(BufferUtil.toBuffer( - "GET /foo/test HTTP/1.1\r\n" + - "Host: Localhost\r\n" + - "\r\n")); - String response = connection.getResponse(); - - assertThat(response, containsString("HTTP/1.1 444 444")); - assertThat(response, containsString("Context Error")); - - connection.addInputAndExecute(BufferUtil.toBuffer( - "GET /test HTTP/1.1\r\n" + - "Host: Localhost\r\n" + - "\r\n")); - response = connection.getResponse(); - assertThat(response, containsString("HTTP/1.1 404 Not Found")); - assertThat(response, containsString("Server Error")); + try (LocalConnector.LocalEndPoint connection = connector.connect()) + { + connection.addInputAndExecute(BufferUtil.toBuffer( + "GET /foo/test HTTP/1.1\r\n" + + "Host: Localhost\r\n" + + "\r\n")); + String response = connection.getResponse(); + + assertThat(response, containsString("HTTP/1.1 444 444")); + assertThat(response, containsString("Context Error")); + + connection.addInputAndExecute(BufferUtil.toBuffer( + "GET /test HTTP/1.1\r\n" + + "Host: Localhost\r\n" + + "\r\n")); + response = connection.getResponse(); + assertThat(response, containsString("HTTP/1.1 404 Not Found")); + assertThat(response, containsString("Server Error")); + } } }