diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java index 386c7852434..44314749ee8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.client.HttpRequest; import org.eclipse.jetty.client.HttpRequestException; import org.eclipse.jetty.client.HttpSender; import org.eclipse.jetty.client.api.ContentProvider; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpGenerator; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MetaData; @@ -36,6 +37,8 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingCallback; +import static org.eclipse.jetty.http.HttpStatus.INTERNAL_SERVER_ERROR_500; + public class HttpSenderOverHTTP extends HttpSender { private final HttpGenerator generator = new HttpGenerator(); @@ -225,6 +228,10 @@ public class HttpSenderOverHTTP extends HttpSender headerBuffer = httpClient.getByteBufferPool().acquire(httpClient.getRequestBufferSize(), false); break; } + case HEADER_OVERFLOW: + { + throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Request header too large"); + } case NEED_CHUNK: { chunkBuffer = httpClient.getByteBufferPool().acquire(HttpGenerator.CHUNK_SIZE, false); diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index 20b73f39fb8..8be21bff0d3 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -74,6 +74,7 @@ public class HttpGenerator NEED_CHUNK, // Need a small chunk buffer of CHUNK_SIZE NEED_INFO, // Need the request/response metadata info NEED_HEADER, // Need a buffer to build HTTP headers into + HEADER_OVERFLOW, // The header buffer overflowed NEED_CHUNK_TRAILER, // Need a large chunk buffer for last chunk and trailers FLUSH, // The buffers previously generated should be flushed CONTINUE, // Continue generating the message @@ -262,7 +263,8 @@ public class HttpGenerator } catch (BufferOverflowException e) { - throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Request header too large", e); + LOG.ignore(e); + return Result.HEADER_OVERFLOW; } catch (Exception e) { @@ -446,7 +448,7 @@ public class HttpGenerator catch (BufferOverflowException e) { LOG.ignore(e); - return Result.NEED_HEADER; + return Result.HEADER_OVERFLOW; } catch (Exception e) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorClientTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorClientTest.java index 492a77de0f2..3a1437a293f 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorClientTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorClientTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; 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 HttpGeneratorClientTest @@ -122,6 +123,42 @@ public class HttpGeneratorClientTest assertThat(out, Matchers.not(Matchers.containsString("Null:"))); } + @Test + public void testHeaderOverflow() throws Exception + { + HttpGenerator gen = new HttpGenerator(); + + Info info = new Info("GET", "/index.html"); + info.getFields().add("Host", "localhost"); + info.getFields().add("Field", "SomeWhatLongValue"); + info.setHttpVersion(HttpVersion.HTTP_1_0); + + HttpGenerator.Result result = gen.generateRequest(info, null, null, null, true); + assertEquals(HttpGenerator.Result.NEED_HEADER, result); + + ByteBuffer header = BufferUtil.allocate(16); + result = gen.generateRequest(info, header, null, null, true); + assertEquals(HttpGenerator.Result.HEADER_OVERFLOW, result); + + header = BufferUtil.allocate(2048); + result = gen.generateRequest(info, header, null, null, true); + assertEquals(HttpGenerator.Result.FLUSH, result); + assertEquals(HttpGenerator.State.COMPLETING, gen.getState()); + assertFalse(gen.isChunking()); + String out = BufferUtil.toString(header); + BufferUtil.clear(header); + + result = gen.generateResponse(null, false, null, null, null, false); + assertEquals(HttpGenerator.Result.SHUTDOWN_OUT, result); + assertEquals(HttpGenerator.State.END, gen.getState()); + assertFalse(gen.isChunking()); + + assertEquals(0, gen.getContentPrepared()); + assertThat(out, Matchers.containsString("GET /index.html HTTP/1.0")); + assertThat(out, Matchers.not(Matchers.containsString("Content-Length"))); + assertThat(out, Matchers.containsString("Field: SomeWhatLongValue")); + } + @Test public void testPOSTRequestNoContent() throws Exception { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java index c42f15e4aff..cfe3cd9442b 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerHTTPTest.java @@ -155,6 +155,12 @@ public class HttpGeneratorServerHTTPTest header = BufferUtil.allocate(2048); continue; + case HEADER_OVERFLOW: + if (header.capacity() >= 8192) + throw new BadMessageException(500, "Header too large"); + header = BufferUtil.allocate(8192); + continue; + case NEED_CHUNK: chunk = BufferUtil.allocate(HttpGenerator.CHUNK_SIZE); continue; diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java index 5574bcf5b9f..bb32ae7331a 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java @@ -110,6 +110,38 @@ public class HttpGeneratorServerTest assertThat(response, containsString("\r\n0123456789")); } + @Test + public void testHeaderOverflow() throws Exception + { + HttpGenerator gen = new HttpGenerator(); + + MetaData.Response info = new MetaData.Response(HttpVersion.HTTP_1_1, 302, null, new HttpFields(), 0); + info.getFields().add("Location", "http://somewhere/else"); + + HttpGenerator.Result result = gen.generateResponse(info, false, null, null, null, true); + assertEquals(HttpGenerator.Result.NEED_HEADER, result); + + ByteBuffer header = BufferUtil.allocate(16); + result = gen.generateResponse(info, false, header, null, null, true); + assertEquals(HttpGenerator.Result.HEADER_OVERFLOW, result); + + header = BufferUtil.allocate(8096); + result = gen.generateResponse(info, false, header, null, null, true); + assertEquals(HttpGenerator.Result.FLUSH, result); + assertEquals(HttpGenerator.State.COMPLETING, gen.getState()); + String response = BufferUtil.toString(header); + BufferUtil.clear(header); + + result = gen.generateResponse(null, false, null, null, null, false); + assertEquals(HttpGenerator.Result.DONE, result); + assertEquals(HttpGenerator.State.END, gen.getState()); + + assertEquals(0, gen.getContentPrepared()); + + assertThat(response, containsString("HTTP/1.1 302 Found")); + assertThat(response, containsString("Location: http://somewhere/else")); + } + @Test public void test204() throws Exception { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java index 96a59bc446b..06b9d005a44 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpTester.java @@ -451,6 +451,12 @@ public class HttpTester header = BufferUtil.allocate(8192); continue; + case HEADER_OVERFLOW: + if (header.capacity() >= 32 * 1024) + throw new BadMessageException(500, "Header too large"); + header = BufferUtil.allocate(32 * 1024); + continue; + case NEED_CHUNK: chunk = BufferUtil.allocate(HttpGenerator.CHUNK_SIZE); continue; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index 15c357703f3..26f7cb550db 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -757,19 +757,16 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http case NEED_HEADER: { - int size; - if (_header == null) - { - size = Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize()); - } - else - { - if (_header.capacity() >= _config.getResponseHeaderSize()) - throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large"); - size = _config.getResponseHeaderSize(); - _bufferPool.release(_header); - } - _header = _bufferPool.acquire(size, HEADER_BUFFER_DIRECT); + _header = _bufferPool.acquire(Math.min(_config.getResponseHeaderSize(), _config.getOutputBufferSize()), HEADER_BUFFER_DIRECT); + continue; + } + + case HEADER_OVERFLOW: + { + if (_header.capacity() >= _config.getResponseHeaderSize()) + throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large"); + _bufferPool.release(_header); + _header = _bufferPool.acquire(_config.getResponseHeaderSize(), HEADER_BUFFER_DIRECT); continue; } case NEED_CHUNK: