From 385d943b0c29445da704763da7d107325b3d99d2 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 2 Aug 2024 14:26:42 +1000 Subject: [PATCH] Fix #12104 Empty Connection Header Fix #12104 by better handling of `close` and `keep-alive` values in the HttpGenerator, so that we never add an empty field. --- .../org/eclipse/jetty/http/HttpField.java | 35 +++ .../org/eclipse/jetty/http/HttpGenerator.java | 56 ++++- .../org/eclipse/jetty/http/QuotedCSV.java | 12 ++ .../org/eclipse/jetty/http/HttpFieldTest.java | 18 ++ .../jetty/server/HttpConnectionTest.java | 202 +++++++++++++++++- 5 files changed, 306 insertions(+), 17 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java index 88c812ca38cb..2598a5e364d2 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java @@ -550,6 +550,41 @@ public boolean is(String name) return _name.equalsIgnoreCase(name); } + /** + * Return a {@link HttpField} without a given value (case-insensitive) + * @param value The value to remove + * @return A new {@link HttpField} if the value was removed, but others remain; this {@link HttpField} if it + * did not contain the value; or {@code null} if the value was the only value. + */ + public HttpField withoutValue(String value) + { + if (_value.length() < value.length()) + return this; + + if (_value.equalsIgnoreCase(value)) + return null; + + if (_value.length() == value.length()) + return this; + + QuotedCSV csv = new QuotedCSV(false, _value); + boolean removed = false; + for (Iterator i = csv.iterator(); i.hasNext();) + { + String element = i.next(); + if (element.equalsIgnoreCase(value)) + { + removed = true; + i.remove(); + } + } + + if (!removed) + return this; + + return new HttpField(_header, _name, csv.asString()); + } + private int nameHashCode() { int h = this._hash; diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index b1282d67bd49..595fb5f5df5e 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -18,8 +18,6 @@ import java.nio.ByteBuffer; import java.util.Arrays; import java.util.function.Supplier; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.eclipse.jetty.http.HttpTokens.EndOfContent; import org.eclipse.jetty.util.BufferUtil; @@ -627,23 +625,58 @@ else if (contentLength != field.getLongValue()) case CONNECTION: { - boolean keepAlive = field.contains(HttpHeaderValue.KEEP_ALIVE.asString()); - if (keepAlive && _info.getHttpVersion() == HttpVersion.HTTP_1_0 && _persistent == null) + String value = field.getValue(); + + // Handle simple case of close value only + if (HttpHeaderValue.CLOSE.is(value)) { - _persistent = true; + if (!close) + header.put(CONNECTION_CLOSE); + close = true; + _persistent = false; } - if (field.contains(HttpHeaderValue.CLOSE.asString())) + // Handle close with other values + else if (field.contains(HttpHeaderValue.CLOSE.asString())) { close = true; _persistent = false; + + // Add the field, but without keep-alive + putTo(field.withoutValue(HttpHeaderValue.KEEP_ALIVE.asString()), header); } - if (keepAlive && _persistent == Boolean.FALSE) + // Handle Keep-Alive value only + else if (HttpHeaderValue.KEEP_ALIVE.is(value)) { - field = new HttpField(HttpHeader.CONNECTION, - Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)) - .collect(Collectors.joining(", "))); + // If we can persist for HTTP/1.0 + if (_persistent != Boolean.FALSE && _info.getHttpVersion() == HttpVersion.HTTP_1_0) + { + // then do so + _persistent = true; + header.put(CONNECTION_KEEP_ALIVE); + } + // otherwise we just ignore the keep-alive + } + // Handle Keep-Alive with other values, but no close + else if (field.contains(HttpHeaderValue.KEEP_ALIVE.asString())) + { + // If we can persist for HTTP/1.0 + if (_persistent != Boolean.FALSE && _info.getHttpVersion() == HttpVersion.HTTP_1_0) + { + // then do so + _persistent = true; + putTo(field, header); + } + else + { + // otherwise we add the field, but without keep-alive + putTo(field.withoutValue(HttpHeaderValue.KEEP_ALIVE.asString()), header); + } + } + // Handle connection header without either close nor keep-alive + else + { + putTo(field, header); } - putTo(field, header); break; } @@ -799,6 +832,7 @@ public String toString() private static final byte[] LAST_CHUNK = {(byte)'0', (byte)'\r', (byte)'\n', (byte)'\r', (byte)'\n'}; private static final byte[] CONTENT_LENGTH_0 = StringUtil.getBytes("Content-Length: 0\r\n"); private static final byte[] CONNECTION_CLOSE = StringUtil.getBytes("Connection: close\r\n"); + private static final byte[] CONNECTION_KEEP_ALIVE = StringUtil.getBytes("Connection: keep-alive\r\n"); private static final byte[] HTTP_1_1_SPACE = StringUtil.getBytes(HttpVersion.HTTP_1_1 + " "); private static final byte[] TRANSFER_ENCODING_CHUNKED = StringUtil.getBytes("Transfer-Encoding: chunked\r\n"); diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java index a90084e0068d..5d98a2e42020 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSV.java @@ -144,6 +144,18 @@ public Iterator iterator() return _values.iterator(); } + public String asString() + { + if (_values.isEmpty()) + return null; + if (_values.size() == 1) + return _values.get(0); + + StringBuilder builder = new StringBuilder(); + join(builder, _values); + return builder.toString(); + } + @Override public String toString() { diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldTest.java index 98385dfef823..efaa7d981d41 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldTest.java @@ -21,7 +21,10 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -203,4 +206,19 @@ public void testGetValueParameters() assertThat(map.get("p2"), is("v2")); assertThat(map.get("p3"), is(" v ; 3=three")); } + + @Test + public void testWithoutValue() + { + HttpField field = new HttpField("name", "value"); + assertThat(field.withoutValue("value"), nullValue()); + assertThat(field.withoutValue("other"), sameInstance(field)); + assertThat(field.withoutValue("val"), sameInstance(field)); + + + field = new HttpField("name", "list, of, values"); + assertThat(field.withoutValue("value"), sameInstance(field)); + assertThat(field.withoutValue("often"), sameInstance(field)); + assertThat(field.withoutValue("of"), equalTo(new HttpField("name", "list, values"))); + } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index cef0156fddca..b93a6daa100f 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -23,6 +23,7 @@ import java.io.BufferedReader; import java.io.StringReader; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; import java.util.HashSet; @@ -39,6 +40,7 @@ import org.eclipse.jetty.http.HttpParser; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.io.Content; @@ -67,6 +69,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; public class HttpConnectionTest @@ -90,7 +93,6 @@ public void init() throws Exception _connector.setIdleTimeout(5000); _server.addConnector(_connector); _server.setHandler(new DumpHandler()); - _server.start(); } @AfterEach @@ -103,6 +105,7 @@ public void destroy() throws Exception @Test public void testFragmentedChunk() throws Exception { + _server.start(); String response = null; try { @@ -152,6 +155,7 @@ public void testFragmentedChunk() throws Exception @Test public void testHttp09NoVersion() throws Exception { + _server.start(); _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); String request = "GET / HTTP/0.9\r\n\r\n"; String response = _connector.getResponse(request); @@ -171,6 +175,7 @@ public void testHttp09NoVersion() throws Exception @Test public void testHttp09NoHeaders() throws Exception { + _server.start(); _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); // header looking like another request is ignored String request = "GET /one\r\nGET :/two\r\n\r\n"; @@ -185,6 +190,7 @@ public void testHttp09NoHeaders() throws Exception @Test public void testHttp09MultipleRequests() throws Exception { + _server.start(); _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); // Verify that pipelining does not work with HTTP/0.9. @@ -202,6 +208,7 @@ public void testHttp09MultipleRequests() throws Exception @Test public void testHttp11ChunkedBodyTruncation() throws Exception { + _server.start(); String request = "POST /?id=123 HTTP/1.1\r\n" + "Host: local\r\n" + "Transfer-Encoding: chunked\r\n" + @@ -251,6 +258,7 @@ public static Stream contentLengths() @MethodSource("contentLengths") public void testHttp11MultipleContentLength(int[] clen) throws Exception { + _server.start(); LOG.info("badMessage: 400 Bad messages EXPECTED..."); StringBuilder request = new StringBuilder(); request.append("POST / HTTP/1.1\r\n"); @@ -298,6 +306,7 @@ public static Stream http11ContentLengthAndChunkedData() @MethodSource("http11ContentLengthAndChunkedData") public void testHttp11ContentLengthAndChunk(int[] contentLengths) throws Exception { + _server.start(); LOG.info("badMessage: 400 Bad messages EXPECTED..."); StringBuilder request = new StringBuilder(); @@ -354,6 +363,7 @@ public static Stream http11TransferEncodingChunked() @MethodSource("http11TransferEncodingChunked") public void testHttp11TransferEncodingChunked(List tokens) throws Exception { + _server.start(); StringBuilder request = new StringBuilder(); request.append("POST / HTTP/1.1\r\n"); request.append("Host: local\r\n"); @@ -405,6 +415,7 @@ public static Stream http11TransferEncodingInvalidChunked() @MethodSource("http11TransferEncodingInvalidChunked") public void testHttp11TransferEncodingInvalidChunked(List tokens) throws Exception { + _server.start(); LOG.info("badMessage: 400 Bad messages EXPECTED..."); StringBuilder request = new StringBuilder(); request.append("POST / HTTP/1.1\r\n"); @@ -424,6 +435,7 @@ public void testHttp11TransferEncodingInvalidChunked(List tokens) throws @Test public void testNoPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET http://localhost:80 HTTP/1.1\r\n" + "Host: localhost:80\r\n" + "Connection: close\r\n" + @@ -437,6 +449,7 @@ public void testNoPath() throws Exception @Test public void testDate() throws Exception { + _server.start(); String response = _connector.getResponse("GET / HTTP/1.1\r\n" + "Host: localhost:80\r\n" + "Connection: close\r\n" + @@ -451,6 +464,7 @@ public void testDate() throws Exception @Test public void testSetDate() throws Exception { + _server.start(); String response = _connector.getResponse("GET /?date=1+Jan+1970 HTTP/1.1\r\n" + "Host: localhost:80\r\n" + "Connection: close\r\n" + @@ -465,6 +479,7 @@ public void testSetDate() throws Exception @Test public void testBadNoPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET http://localhost:80/../cheat HTTP/1.1\r\n" + "Host: localhost:80\r\n" + "\r\n"); @@ -474,6 +489,7 @@ public void testBadNoPath() throws Exception @Test public void testOKPathDotDotPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET /ooops/../path HTTP/1.0\r\nHost: localhost:80\r\n\n"); checkContains(response, 0, "HTTP/1.1 200 OK"); checkContains(response, 0, "pathInContext=/path"); @@ -482,6 +498,7 @@ public void testOKPathDotDotPath() throws Exception @Test public void testBadPathDotDotPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET /ooops/../../path HTTP/1.0\r\nHost: localhost:80\r\n\n"); checkContains(response, 0, "HTTP/1.1 400 "); checkContains(response, 0, "MESSAGE:Bad Request"); @@ -490,6 +507,7 @@ public void testBadPathDotDotPath() throws Exception @Test public void testBadDotDotPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET ../path HTTP/1.0\r\nHost: localhost:80\r\n\n"); checkContains(response, 0, "HTTP/1.1 400 "); checkContains(response, 0, "MESSAGE:Bad Request"); @@ -498,14 +516,16 @@ public void testBadDotDotPath() throws Exception @Test public void testBadSlashDotDotPath() throws Exception { + _server.start(); String response = _connector.getResponse("GET /../path HTTP/1.0\r\nHost: localhost:80\r\n\n"); checkContains(response, 0, "HTTP/1.1 400 "); checkContains(response, 0, "MESSAGE:Bad Request"); } @Test - public void test09() + public void test09() throws Exception { + _server.start(); _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616_LEGACY); LocalConnector.LocalEndPoint endp = _connector.executeRequest("GET /R1\n"); endp.waitUntilClosed(); @@ -521,6 +541,7 @@ public void test09() @Test public void testSimple() throws Exception { + _server.start(); String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + @@ -535,6 +556,7 @@ public void testSimple() throws Exception @Test public void testEmptyNotPersistent() throws Exception { + _server.start(); String response = _connector.getResponse(""" GET /R1?empty=true HTTP/1.0\r Host: localhost\r @@ -558,6 +580,7 @@ public void testEmptyNotPersistent() throws Exception @Test public void testEmptyPersistent() throws Exception { + _server.start(); String response = _connector.getResponse(""" GET /R1?empty=true HTTP/1.0\r Host: localhost\r @@ -585,6 +608,7 @@ public void testEmptyPersistent() throws Exception @Test public void testEmptyChunk() throws Exception { + _server.start(); String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + @@ -602,6 +626,7 @@ public void testEmptyChunk() throws Exception @Test public void testChunk() throws Exception { + _server.start(); String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + @@ -622,6 +647,7 @@ public void testChunk() throws Exception @Test public void testChunkTrailer() throws Exception { + _server.start(); String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + @@ -643,6 +669,7 @@ public void testChunkTrailer() throws Exception @Test public void testChunkNoTrailer() throws Exception { + _server.start(); // Expect TimeoutException logged _connector.setIdleTimeout(1000); String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + @@ -663,6 +690,7 @@ public void testChunkNoTrailer() throws Exception @Test public void testHead() throws Exception { + _server.start(); String responsePOST = _connector.getResponse("POST /R1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + @@ -714,6 +742,7 @@ public void testHead() throws Exception @Test public void testHeadChunked() throws Exception { + _server.start(); String responsePOST = _connector.getResponse("POST /R1?no-content-length=true HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n", false, 1, TimeUnit.SECONDS); @@ -763,6 +792,7 @@ public void testHeadChunked() throws Exception @Test public void testHostOnlyPort() throws Exception { + _server.start(); String response; response = _connector.getResponse(""" @@ -777,6 +807,7 @@ public void testHostOnlyPort() throws Exception @Test public void testBadHostPort() throws Exception { + _server.start(); String response; response = _connector.getResponse("GET http://localhost:EXPECTED_NUMBER_FORMAT_EXCEPTION/ HTTP/1.1\r\n" + @@ -789,6 +820,7 @@ public void testBadHostPort() throws Exception @Test public void testNoHost() throws Exception { + _server.start(); String response; response = _connector.getResponse(""" @@ -801,6 +833,7 @@ public void testNoHost() throws Exception @Test public void testEmptyHost() throws Exception { + _server.start(); String response; response = _connector.getResponse("GET / HTTP/1.1\r\n" + @@ -812,6 +845,7 @@ public void testEmptyHost() throws Exception @Test public void testEmptyHostAbsolute() throws Exception { + _server.start(); String response; response = _connector.getResponse("GET scheme:/// HTTP/1.1\r\n" + @@ -823,6 +857,7 @@ public void testEmptyHostAbsolute() throws Exception @Test public void testBadURIencoding() throws Exception { + _server.start(); String response = _connector.getResponse("GET /bad/encoding%x HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + @@ -834,6 +869,7 @@ public void testBadURIencoding() throws Exception @Disabled("review this test. seems a security issue to fallback from utf-8 to iso-1, was there a reason to do that?") public void testBadUTF8FallsbackTo8859() throws Exception { + _server.start(); LOG.info("badMessage: bad encoding expected ..."); String response; @@ -853,6 +889,7 @@ public void testBadUTF8FallsbackTo8859() throws Exception @Test public void testAutoFlush() throws Exception { + _server.start(); int offset = 0; String response = _connector.getResponse("GET /R1 HTTP/1.1\r\n" + @@ -874,7 +911,6 @@ public void testAutoFlush() throws Exception @Test public void testEmptyFlush() throws Exception { - _server.stop(); _server.setHandler(new Handler.Abstract.NonBlocking() { @Override @@ -898,6 +934,7 @@ public boolean handle(Request request, Response response, Callback callback) @Test public void testUnconsumed() throws Exception { + _server.start(); int offset = 0; String requests = "GET /R1?read=4 HTTP/1.1\r\n" + @@ -935,6 +972,7 @@ public void testUnconsumed() throws Exception @Test public void testUnconsumedTimeout() throws Exception { + _server.start(); _connector.setIdleTimeout(500); int offset = 0; String requests = @@ -959,6 +997,7 @@ public void testUnconsumedTimeout() throws Exception @Test public void testUnconsumedErrorRead() throws Exception { + _server.start(); int offset = 0; String requests = "GET /R1?read=1&error=499 HTTP/1.1\r\n" + @@ -992,6 +1031,7 @@ public void testUnconsumedErrorRead() throws Exception @Test public void testUnconsumedErrorStream() throws Exception { + _server.start(); int offset = 0; String requests = "GET /R1?error=599 HTTP/1.1\r\n" + @@ -1028,6 +1068,7 @@ public void testUnconsumedErrorStream() throws Exception @Test public void testUnconsumedException() throws Exception { + _server.start(); int offset = 0; String requests = "GET /R1?read=1&ISE=true HTTP/1.1\r\n" + "Host: localhost\r\n" + @@ -1059,6 +1100,7 @@ public void testUnconsumedException() throws Exception @Test public void testConnection() throws Exception { + _server.start(); String response = null; try { @@ -1091,6 +1133,7 @@ public void testConnection() throws Exception @Test public void testOversizedBuffer() throws Exception { + _server.start(); String response = null; try { @@ -1123,6 +1166,7 @@ public void testOversizedBuffer() throws Exception @Test public void testExcessiveHeader() throws Exception { + _server.start(); int offset = 0; StringBuilder request = new StringBuilder(); @@ -1150,7 +1194,6 @@ public void testOversizedResponse() throws Exception } final String longstr = str; final CountDownLatch checkError = new CountDownLatch(1); - _server.stop(); _server.setHandler(new Handler.Abstract.NonBlocking() { @Override @@ -1189,7 +1232,6 @@ public void testAllowedLargeResponse() throws Exception Arrays.fill(bytes, (byte)'X'); final String longstr = "thisisastringthatshouldreachover12kbytes-" + new String(bytes, StandardCharsets.ISO_8859_1) + "_Z_"; final CountDownLatch checkError = new CountDownLatch(1); - _server.stop(); _server.setHandler(new Handler.Abstract.NonBlocking() { @Override @@ -1224,6 +1266,7 @@ public boolean handle(Request request, Response response, Callback callback) @Test public void testAsterisk() throws Exception { + _server.start(); String response = null; try (StacklessLogging stackless = new StacklessLogging(HttpParser.class)) { @@ -1278,6 +1321,7 @@ public void testAsterisk() throws Exception @Test public void testCONNECT() throws Exception { + _server.start(); String response = null; try { @@ -1302,7 +1346,6 @@ public void testBytesIn() throws Exception String chunk1 = "0123456789ABCDEF"; String chunk2 = IntStream.range(0, 64).mapToObj(i -> chunk1).collect(Collectors.joining()); long dataLength = chunk1.length() + chunk2.length(); - _server.stop(); _server.setHandler(new Handler.Abstract() { @Override @@ -1386,6 +1429,7 @@ public boolean handle(Request request, Response response, Callback callback) @Test public void testBadURI() throws Exception { + _server.start(); String request = """ GET /ambiguous/doubleSlash// HTTP/1.0 Host: whatever @@ -1400,6 +1444,7 @@ public void testBadURI() throws Exception @Test public void testAmbiguousParameters() throws Exception { + _server.start(); String request = """ GET /ambiguous/..;/path HTTP/1.0\r Host: whatever\r @@ -1420,6 +1465,7 @@ public void testAmbiguousParameters() throws Exception @Test public void testAmbiguousSegments() throws Exception { + _server.start(); String request = """ GET /ambiguous/%2e%2e/path HTTP/1.0\r Host: whatever\r @@ -1436,6 +1482,7 @@ public void testAmbiguousSegments() throws Exception @Test public void testAmbiguousSeparators() throws Exception { + _server.start(); String request = """ GET /ambiguous/%2f/path HTTP/1.0\r Host: whatever\r @@ -1454,6 +1501,7 @@ public void testAmbiguousSeparators() throws Exception @Test public void testAmbiguousPaths() throws Exception { + _server.start(); String request = """ GET /unnormal/.././path/ambiguous%2f%2e%2e/%2e;/info HTTP/1.0\r Host: whatever\r @@ -1475,6 +1523,7 @@ public void testAmbiguousPaths() throws Exception @Test public void testAmbiguousEncoding() throws Exception { + _server.start(); String request = """ GET /ambiguous/encoded/%25/path HTTP/1.0\r Host: whatever\r @@ -1500,6 +1549,7 @@ public void testAmbiguousEncoding() throws Exception @Test public void testAmbiguousDoubleSlash() throws Exception { + _server.start(); String request = """ GET /ambiguous/doubleSlash// HTTP/1.0 Host: whatever @@ -1518,6 +1568,7 @@ public void testAmbiguousDoubleSlash() throws Exception @Test public void testRelativePath() throws Exception { + _server.start(); String request = """ GET foo/bar HTTP/1.0\r Host: whatever\r @@ -1538,4 +1589,143 @@ private void checkNotContained(String s, int offset, String c) { assertThat(s.substring(offset), Matchers.not(Matchers.containsString(c))); } + + public static Stream connectionBehavior() + { + List cases = new ArrayList<>(); + + // --- VALID HTTP/1.0 SPEC REQUESTS --- + + // Request does not send connection header. + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, null, null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "keep-alive", null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Keep-Alive", null)); + + // Request sends "keep-alive" + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", null, "keep-alive")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", "keep-alive", "keep-alive")); + + // --- INVALID HTTP/1.0 SPEC REQUESTS --- + + // Request sends invalid value "close" (on HTTP/1.0) + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "close", null, null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "close", "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "close", "keep-alive", null)); + + // --- INVALID HTTP RESPONSE HEADERS --- + // this is when the servlet is setting headers that are in conflict with + // the spec and each other. + + // Request does not set "connection" header. + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "keep-alive, close", "close")); + + // Request sends invalid value "close" (on HTTP/1.0) + // keep-alive is forbidden when not persistent + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "close", "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "close", "keep-alive, close", "close")); + + // Request sends "keep-alive" + // connection lists are preserved per hop-by-hop rules + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", "keep-alive, close", "close")); + + // Other connection values + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Other, Fields", "Other, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Other, close, Fields", "Other, close, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Other, keep-alive, Fields", "Other, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, null, "Other, close, keep-alive, Fields", "Other, close, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_0, "keep-alive", "Other, keep-alive, Fields", "Other, keep-alive, Fields")); + + + // --- VALID HTTP/1.1 SPEC REQUESTS --- + + // Request does not send connection header. + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, null, null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "keep-alive", null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Keep-Alive", null)); + + // Request sends value "close" + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", null, "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", "keep-alive", "close")); + + // --- INVALID HTTP/1.0 SPEC REQUESTS --- + + // Request sends invalid "keep-alive" header value (on HTTP/1.1) + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "keep-alive", null, null)); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "keep-alive", "close", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "keep-alive", "keep-alive", null)); + + // --- INVALID HTTP RESPONSE HEADERS --- + // this is when the servlet is setting headers that are in conflict with + // the spec and each other. + + // Request does not set "connection" header. + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "keep-alive, close", "close")); + + // Request sends value "close" + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", "keep-alive, close", "close")); + + // Request sends invalid "keep-alive" header value (on HTTP/1.1) + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "keep-alive", "close, keep-alive", "close")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "keep-alive", "keep-alive, close", "close")); + + // Other connection values + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Other, Fields", "Other, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Other, close, Fields", "Other, close, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Other, keep-alive, Fields", "Other, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, null, "Other, close, keep-alive, Fields", "Other, close, Fields")); + cases.add(Arguments.of(HttpVersion.HTTP_1_1, "close", "Other, Fields", "Other, Fields|close")); + + return cases.stream(); + } + + @ParameterizedTest + @MethodSource("connectionBehavior") + public void testConnectionBehavior(HttpVersion version, String requestConnectionHeader, String responseConnectionHeader, String expectedConnectionHeader) throws Exception + { + _server.setHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) throws Exception + { + if (responseConnectionHeader != null) + response.getHeaders().put(HttpHeader.CONNECTION, responseConnectionHeader); + + callback.succeeded(); + return true; + } + }); + + _server.start(); + + StringBuilder rawRequest = new StringBuilder(); + rawRequest.append("GET /nothing %s\r\n".formatted(version.asString())); + rawRequest.append("Host: test\r\n"); + if (requestConnectionHeader != null) + rawRequest.append("Connection: ").append(requestConnectionHeader).append("\r\n"); + rawRequest.append("\r\n"); + + String rawResponse = _connector.getResponse(rawRequest.toString()); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat(response.getStatus(), is(200)); + assertThat(response.getVersion(), is(HttpVersion.HTTP_1_1)); + if (expectedConnectionHeader == null) + assertFalse(response.getMetaData().getHttpFields().contains(HttpHeader.CONNECTION)); + else + { + List actual = response.getValuesList(HttpHeader.CONNECTION); + for (String expected : expectedConnectionHeader.split("\\|")) + assertThat(actual.remove(0), is(expected)); + } + } }