Issue #5605 unconsumed input on sendError

Add Connection:close if content can't be consumed during a sendError. Processed after the request has returned to the container. Signed-off-by: Greg Wilkins <[email protected]>
jetty · Nov 10, 2020 · 0cdc78e · 0cdc78e
1 parent 30a9161
commit 0cdc78e
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 1 deletion.
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -31,14 +31,18 @@
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.servlet.DispatcherType;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletException;
 
 import org.eclipse.jetty.http.BadMessageException;
+import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpGenerator;
 import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpVersion;
@@ -406,7 +410,33 @@ public boolean 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)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
-                            _response.setStatus(code != null ? code : HttpStatus.INTERNAL_SERVER_ERROR_500);
+                            if (code == null)
+                                code = HttpStatus.INTERNAL_SERVER_ERROR_500;
+                            _response.setStatus(code);
+
+                            // Close the connection if we can't consume the input
+                            if (!_request.getHttpInput().consumeAll())
+                            {
+                                _response.getHttpFields().computeField(HttpHeader.CONNECTION, (h, fields) ->
+                                {
+                                    if (fields == null || fields.isEmpty())
+                                        return HttpConnection.CONNECTION_CLOSE;
+
+                                    if (fields.stream().anyMatch(f -> f.contains(HttpHeaderValue.CLOSE.asString())))
+                                    {
+                                        if (fields.size() == 1)
+                                            return fields.get(0);
+
+                                        return new HttpField(HttpHeader.CONNECTION, fields.stream()
+                                            .flatMap(field -> Stream.of(field.getValues()))
+                                            .collect(Collectors.joining(", ")));
+                                    }
+
+                                    return new HttpField(HttpHeader.CONNECTION, fields.stream()
+                                        .flatMap(field -> Stream.of(field.getValues()))
+                                        .collect(Collectors.joining(", ")) + ",close");
+                                });
+                            }
 
                             ContextHandler.Context context = (ContextHandler.Context)_request.getAttribute(ErrorHandler.ERROR_CONTEXT);
                             ErrorHandler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler());

diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java
@@ -233,6 +233,46 @@ public void test404HtmlAccept() throws Exception
         assertContent(response);
     }
 
+    @Test
+    public void test404Post() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.1\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 10\r\n" +
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat("Response status code", response.getStatus(), is(404));
+        assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.getField(HttpHeader.CONNECTION), nullValue());
+        assertContent(response);
+    }
+
+    @Test
+    public void test404PostCantConsume() throws Exception
+    {
+        String rawResponse = connector.getResponse(
+            "POST / HTTP/1.1\r\n" +
+                "Host: Localhost\r\n" +
+                "Accept: text/html\r\n" +
+                "Content-Length: 100\r\n" +
+                "\r\n" +
+                "0123456789");
+
+        HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+
+        assertThat("Response status code", response.getStatus(), is(404));
+        assertThat("Response Content-Length", response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
+        assertThat("Response Content-Type", response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
+        assertThat(response.getField(HttpHeader.CONNECTION).getValue(), is("close"));
+        assertContent(response);
+    }
+
     @Test
     public void testMoreSpecificAccept() throws Exception
     {