From ed90e35ee6b8db1fea23d1c0c80c895371cdd86d Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Mon, 8 Jun 2026 23:47:56 +0100 Subject: [PATCH 1/3] Add HttpConnectionReader and byte-accurate parsing Introduce HttpConnectionReader to buffer per-connection input (supports pipelining and prevents read-ahead loss) and provide byte-accurate line/body reads (ISO-8859-1 for lines, raw bytes for bodies). Update RequestParser to use the new reader, correctly handle Content-Length as octet counts, read chunked bodies into byte buffers, and avoid character-based over-reading. Change RequestProcessor.process to accept HttpConnectionReader and OutputStream and adapt IOServer to create/pass the reader per client socket. Enhance ResponseWriter to emit Content-Length: 0 for bodyless responses when a body is allowed to ensure definite framing on keep-alive connections. Add and update tests (HttpConnectionReaderTest, RequestParserTest, RequestProcessorTest, IOServerTest, ResponseWriterTest) and adjust mocks accordingly. Also add /.serena to .gitignore. Note: this includes an API change to RequestProcessor.process (signature change). --- .gitignore | 1 + .../server/io/HttpConnectionReader.java | 90 ++++++++++++++++++ .../uiqui/embedhttp/server/io/IOServer.java | 4 +- .../embedhttp/server/io/RequestParser.java | 48 ++++------ .../embedhttp/server/io/RequestProcessor.java | 12 +-- .../embedhttp/server/io/ResponseWriter.java | 19 ++++ .../server/io/HttpConnectionReaderTest.java | 93 +++++++++++++++++++ .../embedhttp/server/io/IOServerTest.java | 46 ++++++++- .../server/io/RequestParserTest.java | 41 ++++++++ .../server/io/RequestProcessorTest.java | 34 +++---- .../server/io/ResponseWriterTest.java | 18 ++++ 11 files changed, 341 insertions(+), 65 deletions(-) create mode 100644 src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java create mode 100644 src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java diff --git a/.gitignore b/.gitignore index 19942cc..d70be49 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,4 @@ replay_pid* build/ .DS_Store examples/readme/readme.iml +/.serena diff --git a/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java b/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java new file mode 100644 index 0000000..c3387a1 --- /dev/null +++ b/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java @@ -0,0 +1,90 @@ +package net.uiqui.embedhttp.server.io; + +import java.io.BufferedInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.ProtocolException; +import java.nio.charset.StandardCharsets; + +/** + * Byte-oriented reader over a connection's input stream. + *

+ * A single instance is created per connection and reused across keep-alive requests, so bytes read + * ahead while parsing one request remain buffered for the next. This is what makes pipelining work + * and prevents the buffered-read-ahead loss of the previous per-request reader approach. + *

+ *

+ * Lines (request line and headers) are decoded as ISO-8859-1 so that one byte maps to one char, + * keeping size limits byte-accurate. Bodies are returned as raw bytes for the caller to decode. + *

+ */ +public class HttpConnectionReader { + private static final int CR = '\r'; + private static final int LF = '\n'; + + private final InputStream inputStream; + + public HttpConnectionReader(InputStream inputStream) { + this.inputStream = new BufferedInputStream(inputStream); + } + + /** + * Reads a single line terminated by LF (an optional preceding CR is stripped) and returns it + * without the terminator. Bytes after the terminator stay buffered for the next read. + * + * @return the line, or null if the end of the stream is reached before any byte is read. + */ + public String readLine() throws IOException { + var buffer = new ByteArrayOutputStream(); + int read; + + while ((read = inputStream.read()) != -1) { + if (read == LF) { + return toLine(buffer); + } + + buffer.write(read); + } + + if (buffer.size() == 0) { + return null; + } + + return buffer.toString(StandardCharsets.ISO_8859_1); + } + + /** + * Reads exactly {@code length} bytes from the stream. + * + * @param length the number of bytes to read. + * @return a byte array of exactly {@code length} bytes. + * @throws ProtocolException if the stream ends before {@code length} bytes are read. + */ + public byte[] readBody(int length) throws IOException { + var bytes = new byte[length]; + int read = 0; + + while (read < length) { + var count = inputStream.read(bytes, read, length - read); + if (count == -1) { + throw new ProtocolException("Unexpected end of stream while reading body"); + } + + read += count; + } + + return bytes; + } + + private static String toLine(ByteArrayOutputStream buffer) { + var bytes = buffer.toByteArray(); + var length = bytes.length; + + if (length > 0 && bytes[length - 1] == CR) { + length--; + } + + return new String(bytes, 0, length, StandardCharsets.ISO_8859_1); + } +} diff --git a/src/main/java/net/uiqui/embedhttp/server/io/IOServer.java b/src/main/java/net/uiqui/embedhttp/server/io/IOServer.java index 54eae4f..d34ecab 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/IOServer.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/IOServer.java @@ -85,10 +85,12 @@ private void handleRequest(Socket clientSocket, Counter counter, RequestProcesso try (clientSocket) { counter.addOne(); clientSocket.setSoTimeout(SO_TIMEOUT); + var reader = new HttpConnectionReader(clientSocket.getInputStream()); + var outputStream = clientSocket.getOutputStream(); var keepAlive = true; while (keepAlive && stateMachine.getCurrentState() == ServerState.RUNNING) { - keepAlive = requestProcessor.process(clientSocket); + keepAlive = requestProcessor.process(reader, outputStream); } logger.log(DEBUG, () -> serverLogMessage("Client(%s:%d): Connection closed", clientAddress, clientPort)); diff --git a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java index cc7fd70..fbcb905 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java @@ -5,10 +5,9 @@ import net.uiqui.embedhttp.server.InsensitiveMap; import net.uiqui.embedhttp.server.Request; -import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; import java.net.ProtocolException; import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; @@ -25,7 +24,10 @@ public class RequestParser { private static final int MAX_HEADER_SIZE = 8192; // 8KB public Request parseRequest(InputStream inputStream) throws IOException { - var reader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8)); + return parseRequest(new HttpConnectionReader(inputStream)); + } + + public Request parseRequest(HttpConnectionReader reader) throws IOException { var requestLine = decodeRequestLine(reader); var headers = decodeRequestHeaders(reader); var body = decodeRequestBody(reader, headers); @@ -34,7 +36,7 @@ public Request parseRequest(InputStream inputStream) throws IOException { return new Request(requestLine.method(), requestLine.url(), headers, body, keepAlive); } - private RequestLine decodeRequestLine(BufferedReader reader) throws IOException { + private RequestLine decodeRequestLine(HttpConnectionReader reader) throws IOException { var line = readRequestLine(reader); var parts = line.split(" ", 3); @@ -57,7 +59,7 @@ private RequestLine decodeRequestLine(BufferedReader reader) throws IOException return new RequestLine(method, url, version); } - private static String readRequestLine(BufferedReader reader) throws IOException { + private static String readRequestLine(HttpConnectionReader reader) throws IOException { try { var line = reader.readLine(); if (line == null) { @@ -74,7 +76,7 @@ private static String readRequestLine(BufferedReader reader) throws IOException } } - private InsensitiveMap decodeRequestHeaders(BufferedReader reader) throws IOException { + private InsensitiveMap decodeRequestHeaders(HttpConnectionReader reader) throws IOException { var headers = new InsensitiveMap(); String line; int headerCount = 0; @@ -102,14 +104,14 @@ private InsensitiveMap decodeRequestHeaders(BufferedReader reader) throws IOExce return headers; } - private String decodeRequestBody(BufferedReader reader, Map headers) throws IOException { + private String decodeRequestBody(HttpConnectionReader reader, Map headers) throws IOException { if (headers.containsKey(HttpHeader.CONTENT_LENGTH.getValue())) { var contentLength = Integer.parseInt(headers.get(HttpHeader.CONTENT_LENGTH.getValue())); if (contentLength > MAX_BODY_SIZE) { throw new ProtocolException("Request body too large: " + contentLength); } - return readFixedSizeBodyChunk(reader, contentLength); + return new String(reader.readBody(contentLength), StandardCharsets.UTF_8); } if (TRANSFER_ENCODING_CHUNKED.equalsIgnoreCase(headers.get(HttpHeader.TRANSFER_ENCODING.getValue()))) { @@ -132,8 +134,8 @@ private boolean decodeKeepAlive(InsensitiveMap headers) { return !CLOSE.getValue().equalsIgnoreCase(connectionHeader); } - private String readChunkedBody(BufferedReader reader) throws IOException { - var body = new StringBuilder(); + private String readChunkedBody(HttpConnectionReader reader) throws IOException { + var body = new ByteArrayOutputStream(); while (true) { int chunkSize = readChunkSize(reader); @@ -142,14 +144,14 @@ private String readChunkedBody(BufferedReader reader) throws IOException { break; } - body.append(readFixedSizeBodyChunk(reader, chunkSize)); + body.writeBytes(reader.readBody(chunkSize)); consumeTrailingLine(reader); // Consume trailing \r\n } - return body.toString(); + return body.toString(StandardCharsets.UTF_8); } - private int readChunkSize(BufferedReader reader) throws IOException { + private int readChunkSize(HttpConnectionReader reader) throws IOException { var line = reader.readLine(); if (line == null) { throw new ProtocolException("Unexpected end of stream while reading chunk size"); @@ -163,23 +165,7 @@ private int readChunkSize(BufferedReader reader) throws IOException { return chunkSize; } - private String readFixedSizeBodyChunk(BufferedReader reader, int chunkSize) throws IOException { - var chunk = new char[chunkSize]; - int read = 0; - - while (read < chunkSize) { - var readCount = reader.read(chunk, read, chunkSize - read); - if (readCount == -1) { - throw new ProtocolException("Unexpected end of stream while reading body"); - } - - read += readCount; - } - - return new String(chunk); - } - - private void consumeTrailingLine(BufferedReader reader) throws IOException { + private void consumeTrailingLine(HttpConnectionReader reader) throws IOException { var line = reader.readLine(); if (line == null) { throw new ProtocolException("Unexpected end of stream while consuming trailing line"); @@ -188,4 +174,4 @@ private void consumeTrailingLine(BufferedReader reader) throws IOException { protected record RequestLine(HttpMethod method, String url, HttpVersion version) { } -} \ No newline at end of file +} diff --git a/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java b/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java index e91f0d7..953b388 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java @@ -11,10 +11,8 @@ import net.uiqui.embedhttp.server.RequestPipeline; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.net.ProtocolException; -import java.net.Socket; public class RequestProcessor { private final RequestParser requestParser; @@ -27,12 +25,12 @@ public RequestProcessor(RequestParser requestParser, ResponseWriter responseWrit this.router = router; } - public boolean process(Socket clientSocket) throws IOException { - var response = RequestPipeline.value(clientSocket.getInputStream()) + public boolean process(HttpConnectionReader reader, OutputStream outputStream) throws IOException { + var response = RequestPipeline.value(reader) .map(this::parse) .map(this::route) .then(this::execute); - write(response, clientSocket.getOutputStream()); + write(response, outputStream); return shouldKeepAliveConnection(response); } @@ -42,9 +40,9 @@ private boolean shouldKeepAliveConnection(HttpResponse response) { return !castedResponse.closeConnection(); } - private RequestPipeline parse(InputStream inputStream) throws ClientDisconnectedException { + private RequestPipeline parse(HttpConnectionReader reader) throws ClientDisconnectedException { try { - var request = requestParser.parseRequest(inputStream); + var request = requestParser.parseRequest(reader); return RequestPipeline.value(request); } catch (ProtocolException e) { var response = HttpResponse.badRequest() diff --git a/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java b/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java index c888058..e2ba68f 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java @@ -32,6 +32,16 @@ public void writeResponse(OutputStream outputStream, HttpResponseImpl response) .append(CRLF); } + // Ensure a definite message framing on keep-alive connections: a response with no body + // still needs Content-Length (except where the status forbids a body). + if (response.getBody() == null + && statusAllowsBody(response.getStatusCode()) + && !response.getHeaders().containsKey(HttpHeader.CONTENT_LENGTH.getValue())) { + builder.append(HttpHeader.CONTENT_LENGTH.getValue()) + .append(": 0") + .append(CRLF); + } + // Write the Date header builder.append(HttpHeader.DATE.getValue()) .append(": ") @@ -50,4 +60,13 @@ public void writeResponse(OutputStream outputStream, HttpResponseImpl response) outputStream.write(builder.toString().getBytes()); outputStream.flush(); } + + private boolean statusAllowsBody(int statusCode) { + // 1xx informational, 204 No Content and 304 Not Modified must not carry a message body. + if (statusCode >= 100 && statusCode < 200) { + return false; + } + + return statusCode != 204 && statusCode != 304; + } } diff --git a/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java b/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java new file mode 100644 index 0000000..b96e5d8 --- /dev/null +++ b/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java @@ -0,0 +1,93 @@ +package net.uiqui.embedhttp.server.io; + +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.net.ProtocolException; +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable; + +class HttpConnectionReaderTest { + private HttpConnectionReader readerFor(String content) { + var inputStream = new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)); + return new HttpConnectionReader(inputStream); + } + + private HttpConnectionReader readerFor(byte[] content) { + return new HttpConnectionReader(new ByteArrayInputStream(content)); + } + + @Test + void testReadLineStripsCrlf() throws Exception { + // given + var reader = readerFor("GET /test HTTP/1.1\r\nHost: localhost\r\n"); + // when / then + assertThat(reader.readLine()).isEqualTo("GET /test HTTP/1.1"); + assertThat(reader.readLine()).isEqualTo("Host: localhost"); + } + + @Test + void testReadLineAcceptsBareLf() throws Exception { + // given + var reader = readerFor("line-one\nline-two\n"); + // when / then + assertThat(reader.readLine()).isEqualTo("line-one"); + assertThat(reader.readLine()).isEqualTo("line-two"); + } + + @Test + void testReadLineReturnsNullAtEndOfStream() throws Exception { + // given + var reader = readerFor(""); + // when / then + assertThat(reader.readLine()).isNull(); + } + + @Test + void testReadBodyReturnsExactByteCount() throws Exception { + // given + var reader = readerFor("HelloWorld"); + // when + var body = reader.readBody(5); + // then + assertThat(new String(body, StandardCharsets.UTF_8)).isEqualTo("Hello"); + } + + @Test + void testReadLineThenBodyThenLineDoesNotOverRead() throws Exception { + // given — a header line, then a 5-byte body, then a second header line + var reader = readerFor("Header: one\r\nHELLOTrailer: two\r\n"); + // when + var firstLine = reader.readLine(); + var body = reader.readBody(5); + var secondLine = reader.readLine(); + // then + assertThat(firstLine).isEqualTo("Header: one"); + assertThat(new String(body, StandardCharsets.UTF_8)).isEqualTo("HELLO"); + assertThat(secondLine).isEqualTo("Trailer: two"); + } + + @Test + void testReadBodyReadsRawBytesNotCharacters() throws Exception { + // given — "héllo" is 5 chars but 6 bytes in UTF-8 + var bytes = "héllo".getBytes(StandardCharsets.UTF_8); + var reader = readerFor(bytes); + // when + var body = reader.readBody(bytes.length); + // then + assertThat(body).hasSize(6); + assertThat(new String(body, StandardCharsets.UTF_8)).isEqualTo("héllo"); + } + + @Test + void testReadBodyThrowsOnPrematureEndOfStream() { + // given + var reader = readerFor("abc"); + // when + var thrown = catchThrowable(() -> reader.readBody(10)); + // then + assertThat(thrown).isInstanceOf(ProtocolException.class); + } +} diff --git a/src/test/java/net/uiqui/embedhttp/server/io/IOServerTest.java b/src/test/java/net/uiqui/embedhttp/server/io/IOServerTest.java index 5d5a531..86864a4 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/IOServerTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/IOServerTest.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.net.ConnectException; import java.net.http.HttpClient; +import java.nio.charset.StandardCharsets; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.AssertionsForClassTypes.catchThrowable; @@ -74,12 +75,49 @@ void testStop() throws Exception { assertThat(classUnderTest.getInstancePort()).isEqualTo(-1); } + @Test + void testKeepAliveServesMultipleRequestsAndUtf8Body() throws Exception { + // given + var classUnderTest = new IOServer(0, 10); + var router = Router.newRouter() + .get("/ping", req -> HttpResponse.ok().setBody(ContentType.TEXT_PLAIN, "pong")) + .post("/echo", req -> HttpResponse.ok().setBody(ContentType.TEXT_PLAIN, req.getBody())); + classUnderTest.start(router); + var base = "http://localhost:" + classUnderTest.getInstancePort(); + // when — a single client reuses one HTTP/1.1 keep-alive connection across all requests + try (var client = HttpClient.newHttpClient()) { + var first = client.send(get(base + "/ping"), java.net.http.HttpResponse.BodyHandlers.ofString()); + var second = client.send(get(base + "/ping"), java.net.http.HttpResponse.BodyHandlers.ofString()); + var echo = client.send(post(base + "/echo", "héllo"), java.net.http.HttpResponse.BodyHandlers.ofString()); + // then + assertThat(first.statusCode()).isEqualTo(200); + assertThat(first.body()).isEqualTo("pong"); + assertThat(second.statusCode()).isEqualTo(200); + assertThat(second.body()).isEqualTo("pong"); + assertThat(echo.statusCode()).isEqualTo(200); + assertThat(echo.body()).isEqualTo("héllo"); + } finally { + classUnderTest.stop(); + } + } + private java.net.http.HttpResponse callEndpoint(String url) throws IOException, InterruptedException { try (var client = HttpClient.newHttpClient()) { - var request = java.net.http.HttpRequest.newBuilder() - .uri(java.net.URI.create(url)) - .build(); - return client.send(request, java.net.http.HttpResponse.BodyHandlers.ofString()); + return client.send(get(url), java.net.http.HttpResponse.BodyHandlers.ofString()); } } + + private static java.net.http.HttpRequest get(String url) { + return java.net.http.HttpRequest.newBuilder() + .uri(java.net.URI.create(url)) + .build(); + } + + private static java.net.http.HttpRequest post(String url, String body) { + return java.net.http.HttpRequest.newBuilder() + .uri(java.net.URI.create(url)) + .POST(java.net.http.HttpRequest.BodyPublishers.ofString(body, StandardCharsets.UTF_8)) + .header("Content-Type", "text/plain") + .build(); + } } \ No newline at end of file diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java index 1751851..42422aa 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java @@ -83,6 +83,47 @@ void testParseChunkedRequestBody() throws Exception { assertThat(result.isKeepAlive()).isTrue(); } + @Test + void testParseBodyByByteCountNotCharCount() throws Exception { + // given — "héllo" is 5 characters but 6 bytes in UTF-8; Content-Length counts octets + var body = "héllo"; + var contentLength = body.getBytes(StandardCharsets.UTF_8).length; // 6 + var rawRequest = "POST /submit HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: " + contentLength + "\r\n" + + "\r\n" + + body; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = classUnderTest.parseRequest(inputStream); + // then + assertThat(result.getBody()).isEqualTo("héllo"); + } + + @Test + void testParseTwoRequestsOnSameReaderWithoutOverReading() throws Exception { + // given — two back-to-back requests on one connection; the first has a body + var rawRequests = "POST /first HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: 5\r\n" + + "\r\n" + + "HELLO" + + "GET /second HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "\r\n"; + var reader = new HttpConnectionReader( + new ByteArrayInputStream(rawRequests.getBytes(StandardCharsets.UTF_8)) + ); + // when + var first = classUnderTest.parseRequest(reader); + var second = classUnderTest.parseRequest(reader); + // then + assertThat(first.getUrl()).isEqualTo("/first"); + assertThat(first.getBody()).isEqualTo("HELLO"); + assertThat(second.getMethod()).isEqualTo(HttpMethod.GET); + assertThat(second.getUrl()).isEqualTo("/second"); + } + @Test void testKeepAliveHeader() throws Exception { // given diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java index 37a6d80..65894fb 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java @@ -14,13 +14,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.net.Socket; import java.nio.charset.StandardCharsets; import java.time.ZonedDateTime; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; class RequestProcessorTest { @@ -65,9 +62,9 @@ void testProcessValidRequest() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 200 OK\r @@ -89,9 +86,9 @@ void testProcessWithInvalidRequest() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 400 Bad Request\r @@ -117,9 +114,9 @@ void testProcessWithNotFoundRequest() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 404 Not Found\r @@ -144,9 +141,9 @@ void testProcessWithError() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 500 Internal Server Error\r @@ -172,9 +169,9 @@ void testWithConnectionClose() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 200 OK\r @@ -201,9 +198,9 @@ void testWithConnectionKeepAlive() throws IOException { """ ); var outputStream = new ByteArrayOutputStream(); - var clientSocket = buildClientSocket(inputStream, outputStream); + var reader = new HttpConnectionReader(inputStream); // when - classUnderTest.process(clientSocket); + classUnderTest.process(reader, outputStream); // then var expected = """ HTTP/1.1 200 OK\r @@ -216,13 +213,6 @@ void testWithConnectionKeepAlive() throws IOException { assertThat(result).isEqualTo(expected); } - private static Socket buildClientSocket(InputStream inputStream, ByteArrayOutputStream outputStream) throws IOException { - var clientSocket = mock(Socket.class); - given(clientSocket.getInputStream()).willReturn(inputStream); - given(clientSocket.getOutputStream()).willReturn(outputStream); - return clientSocket; - } - private static InputStream buildInputStream(String rawRequest) { return new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); } diff --git a/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java b/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java index 6991680..762ff6c 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java @@ -69,6 +69,24 @@ void testWriteResponseWithBody() throws IOException { assertThat(result).isEqualTo(expected); } + @Test + void testWriteBodylessOkResponseHasContentLengthZero() throws IOException { + // given + var response = buildResponse(HttpStatusCode.OK, null); + var outputStream = new ByteArrayOutputStream(); + // when + classUnderTest.writeResponse(outputStream, response); + // then + var expected = """ + HTTP/1.1 200 OK\r + Content-Length: 0\r + Date: Sun, 01 Oct 2023 12:00:00 GMT\r + \r + """; + var result = outputStream.toString(); + assertThat(result).isEqualTo(expected); + } + private HttpResponseImpl buildResponse(HttpStatusCode status, String body) { var response = new HttpResponseImpl(status); From ad0bcefc33529c72720e2b4148303ba70cfbb10b Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Tue, 9 Jun 2026 00:00:01 +0100 Subject: [PATCH 2/3] Harden HTTP request parsing and validation Add stricter protocol checks and defensive parsing to RequestParser and update tests accordingly. Changes include: enforce Host header for HTTP/1.1 and reject duplicate Host headers; treat presence of both Content-Length and Transfer-Encoding as a protocol error; validate Content-Length is numeric and non-negative; parse chunk-size lines while ignoring extensions and validate chunk sizes; enforce an aggregate cap on chunked bodies to prevent unbounded growth; correct keep-alive semantics to depend on HTTP version and explicit Connection header. Tests were adjusted (some header-limit tests use HTTP/1.0) and several new tests added for invalid Content-Length, invalid chunk sizes, chunk extensions, aggregate chunk size limit, Host header requirements, and ensuring RequestProcessor returns 400 for malformed Content-Length. --- .../embedhttp/server/io/RequestParser.java | 79 ++++++++- .../server/io/RequestParserTest.java | 165 +++++++++++++++++- .../server/io/RequestProcessorTest.java | 28 +++ 3 files changed, 261 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java index fbcb905..aa0104e 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java @@ -30,12 +30,20 @@ public Request parseRequest(InputStream inputStream) throws IOException { public Request parseRequest(HttpConnectionReader reader) throws IOException { var requestLine = decodeRequestLine(reader); var headers = decodeRequestHeaders(reader); + validateHost(requestLine.version(), headers); var body = decodeRequestBody(reader, headers); - var keepAlive = decodeKeepAlive(headers); + var keepAlive = decodeKeepAlive(requestLine.version(), headers); return new Request(requestLine.method(), requestLine.url(), headers, body, keepAlive); } + private void validateHost(HttpVersion version, InsensitiveMap headers) throws ProtocolException { + // RFC 9112 §3.2: an HTTP/1.1 request MUST carry a Host header (duplicates are rejected during header parsing). + if (version == HttpVersion.VERSION_1_1 && !headers.containsKey(HttpHeader.HOST.getValue())) { + throw new ProtocolException("Missing Host header"); + } + } + private RequestLine decodeRequestLine(HttpConnectionReader reader) throws IOException { var line = readRequestLine(reader); @@ -98,6 +106,12 @@ private InsensitiveMap decodeRequestHeaders(HttpConnectionReader reader) throws var headerName = line.substring(0, colonIndex).trim(); var headerValue = line.substring(colonIndex + 1).trim(); + + // RFC 9112 §3.2: more than one Host header is a request-smuggling signal and must be rejected. + if (HttpHeader.HOST.getValue().equalsIgnoreCase(headerName) && headers.containsKey(headerName)) { + throw new ProtocolException("Duplicate Host header"); + } + headers.put(headerName, headerValue); } @@ -105,8 +119,16 @@ private InsensitiveMap decodeRequestHeaders(HttpConnectionReader reader) throws } private String decodeRequestBody(HttpConnectionReader reader, Map headers) throws IOException { - if (headers.containsKey(HttpHeader.CONTENT_LENGTH.getValue())) { - var contentLength = Integer.parseInt(headers.get(HttpHeader.CONTENT_LENGTH.getValue())); + var hasContentLength = headers.containsKey(HttpHeader.CONTENT_LENGTH.getValue()); + var hasTransferEncoding = headers.containsKey(HttpHeader.TRANSFER_ENCODING.getValue()); + + // RFC 9112 §6.1: a message with both framing headers is a request-smuggling vector. + if (hasContentLength && hasTransferEncoding) { + throw new ProtocolException("Content-Length and Transfer-Encoding must not both be present"); + } + + if (hasContentLength) { + var contentLength = parseContentLength(headers.get(HttpHeader.CONTENT_LENGTH.getValue())); if (contentLength > MAX_BODY_SIZE) { throw new ProtocolException("Request body too large: " + contentLength); } @@ -121,17 +143,34 @@ private String decodeRequestBody(HttpConnectionReader reader, Map MAX_BODY_SIZE) { + throw new ProtocolException("Request body too large: exceeds " + MAX_BODY_SIZE + " bytes"); + } + body.writeBytes(reader.readBody(chunkSize)); consumeTrailingLine(reader); // Consume trailing \r\n } @@ -157,7 +201,24 @@ private int readChunkSize(HttpConnectionReader reader) throws IOException { throw new ProtocolException("Unexpected end of stream while reading chunk size"); } - int chunkSize = Integer.parseInt(line.trim(), 16); + // A chunk-size line may carry extensions after a ';' (RFC 9112 §7.1.1); ignore them. + var sizeToken = line.trim(); + var extensionIndex = sizeToken.indexOf(';'); + if (extensionIndex != -1) { + sizeToken = sizeToken.substring(0, extensionIndex).trim(); + } + + int chunkSize; + try { + chunkSize = Integer.parseInt(sizeToken, 16); + } catch (NumberFormatException e) { + throw new ProtocolException("Invalid chunk size: " + line); + } + + if (chunkSize < 0) { + throw new ProtocolException("Invalid chunk size: " + line); + } + if (chunkSize > MAX_CHUNK_SIZE) { throw new ProtocolException("Chunk size too large: " + chunkSize); } diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java index 42422aa..52e3c46 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java @@ -300,7 +300,7 @@ void testRejectTooManyHeaders() { void testAcceptMaxHeaderCount() throws Exception { // given var builder = new StringBuilder(); - builder.append("GET /test HTTP/1.1\r\n"); + builder.append("GET /test HTTP/1.0\r\n"); // 1.0 so the Host requirement does not apply; header limits are version-independent for (int i = 0; i < 100; i++) { // Exactly 100 headers at the limit builder.append("Header-").append(i).append(": value\r\n"); } @@ -335,7 +335,7 @@ void testRejectHeaderTooLarge() { void testAcceptHeaderAtMaxSize() throws Exception { // given var headerValue = "X".repeat(8178); // Total header line is exactly 8192 bytes - var rawRequest = "GET /test HTTP/1.1\r\n" + + var rawRequest = "GET /test HTTP/1.0\r\n" + // 1.0 so the Host requirement does not apply; header limits are version-independent "Large-Header: " + headerValue + "\r\n" + "\r\n"; var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); @@ -488,4 +488,165 @@ void testRejectLowercaseHttpVersion() { assertThat(result).hasMessageContaining("Unsupported HTTP version"); assertThat(result).hasMessageContaining("http/1.1"); } + + // --- M3: protocol hardening --- + + @Test + void testRejectNonNumericContentLength() { + // given + var rawRequest = "POST /submit HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: abc\r\n" + + "\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("Content-Length"); + } + + @Test + void testRejectNegativeContentLength() { + // given + var rawRequest = "POST /submit HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: -5\r\n" + + "\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("Content-Length"); + } + + @Test + void testRejectInvalidChunkSize() { + // given + var rawRequest = "POST /upload HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + "xyz\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("chunk size"); + } + + @Test + void testParseChunkedBodyWithChunkExtension() throws Exception { + // given — a chunk-size line may carry extensions: "5;name=value" + var rawRequest = "POST /upload HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + "5;name=value\r\n" + + "Hello\r\n" + + "0\r\n" + + "\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = classUnderTest.parseRequest(inputStream); + // then + assertThat(result.getBody()).isEqualTo("Hello"); + } + + @Test + void testRejectChunkedBodyExceedingMaxAggregateSize() { + // given — ten 1MB chunks reach the 10MB cap; the eleventh must be rejected before reading + var oneMegabyte = "X".repeat(1024 * 1024); + var builder = new StringBuilder(); + builder.append("POST /upload HTTP/1.1\r\n") + .append("Host: localhost\r\n") + .append("Transfer-Encoding: chunked\r\n") + .append("\r\n"); + for (int i = 0; i < 10; i++) { + builder.append("100000\r\n").append(oneMegabyte).append("\r\n"); + } + builder.append("100000\r\n"); // eleventh chunk size — would push past 10MB + var inputStream = new ByteArrayInputStream(builder.toString().getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("too large"); + } + + @Test + void testRejectContentLengthAndTransferEncoding() { + // given — both framing headers present is a request-smuggling vector (RFC 9112 §6.1) + var rawRequest = "POST /submit HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: 5\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + "HELLO"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + } + + @Test + void testHttp10DefaultsToCloseWithoutConnectionHeader() throws Exception { + // given + var rawRequest = """ + GET /test HTTP/1.0\r + Host: localhost\r + \r + """; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = classUnderTest.parseRequest(inputStream); + // then + assertThat(result.isKeepAlive()).isFalse(); + } + + @Test + void testHttp10KeepAliveWithExplicitHeader() throws Exception { + // given + var rawRequest = """ + GET /test HTTP/1.0\r + Host: localhost\r + Connection: keep-alive\r + \r + """; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = classUnderTest.parseRequest(inputStream); + // then + assertThat(result.isKeepAlive()).isTrue(); + } + + @Test + void testRejectHttp11RequestWithoutHost() { + // given + var rawRequest = "GET /test HTTP/1.1\r\n\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("Host"); + } + + @Test + void testRejectDuplicateHostHeader() { + // given + var rawRequest = "GET /test HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Host: evil.example\r\n" + + "\r\n"; + var inputStream = new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); + // when + var result = catchThrowable(() -> classUnderTest.parseRequest(inputStream)); + // then + assertThat(result).isInstanceOf(ProtocolException.class); + assertThat(result).hasMessageContaining("Host"); + } } \ No newline at end of file diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java index 65894fb..ab57b22 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java @@ -213,6 +213,34 @@ void testWithConnectionKeepAlive() throws IOException { assertThat(result).isEqualTo(expected); } + @Test + void testProcessRejectsMalformedContentLengthWith400() throws IOException { + // given — a non-numeric Content-Length must yield a 400 response, not a dropped connection + var inputStream = buildInputStream( + """ + POST /test HTTP/1.1\r + Host: localhost\r + Content-Length: abc\r + \r + """ + ); + var outputStream = new ByteArrayOutputStream(); + var reader = new HttpConnectionReader(inputStream); + // when + classUnderTest.process(reader, outputStream); + // then + var expected = """ + HTTP/1.1 400 Bad Request\r + Connection: close\r + Content-Length: 40\r + Content-Type: text/plain\r + Date: Sun, 01 Oct 2023 12:00:00 GMT\r + \r + Bad Request: Invalid Content-Length: abc"""; + var result = outputStream.toString(); + assertThat(result).isEqualTo(expected); + } + private static InputStream buildInputStream(String rawRequest) { return new ByteArrayInputStream(rawRequest.getBytes(StandardCharsets.UTF_8)); } From e0b081d1dc8d06300c3b7f10d31a8397aec89041 Mon Sep 17 00:00:00 2001 From: Joaquim Rocha Date: Tue, 9 Jun 2026 00:14:34 +0100 Subject: [PATCH 3/3] Enforce CRLF/line size and improve responses Add strict per-line framing and limits to HttpConnectionReader (CRLF required, 8KB max line length) to prevent request-smuggling and unbounded memory growth; invalid terminators or overlong lines now throw ProtocolException. Remove redundant header-size check from RequestParser since line limits are enforced by the reader. Refactor RequestProcessor to extract handler invocation and ensure Connection: close is honored even when handlers throw (500 responses also close). Update ResponseWriter to avoid duplicating the Date header when already set and to write headers/body using UTF-8. Add and adjust unit tests to cover bare-LF rejection, overlong lines, connection-close propagation on errors, non-duplicated Date header, and UTF-8 body writing. --- .../server/io/HttpConnectionReader.java | 15 +++++-- .../embedhttp/server/io/RequestParser.java | 5 --- .../embedhttp/server/io/RequestProcessor.java | 19 +++++---- .../embedhttp/server/io/ResponseWriter.java | 15 ++++--- .../server/io/HttpConnectionReaderTest.java | 21 +++++++--- .../server/io/RequestParserTest.java | 2 +- .../server/io/RequestProcessorTest.java | 28 +++++++++++++ .../server/io/ResponseWriterTest.java | 41 +++++++++++++++++++ 8 files changed, 118 insertions(+), 28 deletions(-) diff --git a/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java b/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java index c3387a1..db69583 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java @@ -22,6 +22,7 @@ public class HttpConnectionReader { private static final int CR = '\r'; private static final int LF = '\n'; + private static final int MAX_LINE_LENGTH = 8192; // 8KB — request line and header lines private final InputStream inputStream; @@ -45,6 +46,11 @@ public String readLine() throws IOException { } buffer.write(read); + + // Bound per-line memory; allow one extra byte for the trailing CR of a max-length line. + if (buffer.size() > MAX_LINE_LENGTH + 1) { + throw new ProtocolException("Line exceeds maximum length of " + MAX_LINE_LENGTH + " bytes"); + } } if (buffer.size() == 0) { @@ -77,14 +83,15 @@ public byte[] readBody(int length) throws IOException { return bytes; } - private static String toLine(ByteArrayOutputStream buffer) { + private static String toLine(ByteArrayOutputStream buffer) throws ProtocolException { var bytes = buffer.toByteArray(); var length = bytes.length; - if (length > 0 && bytes[length - 1] == CR) { - length--; + // HTTP framing requires CRLF: the LF just consumed must be preceded by CR. + if (length == 0 || bytes[length - 1] != CR) { + throw new ProtocolException("Invalid line terminator: expected CRLF"); } - return new String(bytes, 0, length, StandardCharsets.ISO_8859_1); + return new String(bytes, 0, length - 1, StandardCharsets.ISO_8859_1); } } diff --git a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java index aa0104e..7a8b942 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/RequestParser.java @@ -21,7 +21,6 @@ public class RequestParser { private static final int MAX_BODY_SIZE = 10 * 1024 * 1024; // 10MB private static final int MAX_CHUNK_SIZE = 1024 * 1024; // 1MB private static final int MAX_HEADER_COUNT = 100; - private static final int MAX_HEADER_SIZE = 8192; // 8KB public Request parseRequest(InputStream inputStream) throws IOException { return parseRequest(new HttpConnectionReader(inputStream)); @@ -90,10 +89,6 @@ private InsensitiveMap decodeRequestHeaders(HttpConnectionReader reader) throws int headerCount = 0; while ((line = reader.readLine()) != null && !line.isEmpty()) { - if (line.length() > MAX_HEADER_SIZE) { - throw new ProtocolException("Header too large: maximum " + MAX_HEADER_SIZE + " bytes allowed"); - } - var colonIndex = line.indexOf(':'); if (colonIndex == -1) { throw new ProtocolException("Invalid header line: " + line); diff --git a/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java b/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java index 953b388..a29aaaa 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/RequestProcessor.java @@ -72,16 +72,21 @@ private RequestPipeline route(Request request) { } private HttpResponse execute(HttpRequestImpl httpRequest) { - var handler = httpRequest.getRoute().getHandler(); + var response = invokeHandler(httpRequest); - try { - var response = handler.handle(httpRequest); + // Honour the request's close intent regardless of whether the handler succeeded or threw. + if (!httpRequest.getRequest().isKeepAlive()) { + response.setHeader(HttpHeader.CONNECTION, ConnectionHeader.CLOSE.getValue()); + } - if (!httpRequest.getRequest().isKeepAlive()) { - response.setHeader(HttpHeader.CONNECTION, ConnectionHeader.CLOSE.getValue()); - } + return response; + } - return response; + private HttpResponse invokeHandler(HttpRequestImpl httpRequest) { + var handler = httpRequest.getRoute().getHandler(); + + try { + return handler.handle(httpRequest); } catch (Exception e) { return HttpResponse.unexpectedError() .setBody(ContentType.TEXT_PLAIN, "Unexpected error executing request"); diff --git a/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java b/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java index e2ba68f..afad138 100644 --- a/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java +++ b/src/main/java/net/uiqui/embedhttp/server/io/ResponseWriter.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; public class ResponseWriter { public static final String HTTP_VERSION_1_1 = "HTTP/1.1"; @@ -42,11 +43,13 @@ && statusAllowsBody(response.getStatusCode()) .append(CRLF); } - // Write the Date header - builder.append(HttpHeader.DATE.getValue()) - .append(": ") - .append(dateHeader.getDateHeaderValue()) - .append(CRLF); + // Write the Date header, unless the handler already supplied one (avoid duplicate Date). + if (!response.getHeaders().containsKey(HttpHeader.DATE.getValue())) { + builder.append(HttpHeader.DATE.getValue()) + .append(": ") + .append(dateHeader.getDateHeaderValue()) + .append(CRLF); + } // End of headers builder.append(CRLF); @@ -57,7 +60,7 @@ && statusAllowsBody(response.getStatusCode()) } // Write and flush the output stream to ensure all data is sent - outputStream.write(builder.toString().getBytes()); + outputStream.write(builder.toString().getBytes(StandardCharsets.UTF_8)); outputStream.flush(); } diff --git a/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java b/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java index b96e5d8..38e4057 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java @@ -29,12 +29,23 @@ void testReadLineStripsCrlf() throws Exception { } @Test - void testReadLineAcceptsBareLf() throws Exception { - // given + void testReadLineRejectsBareLf() { + // given — HTTP framing requires CRLF; a bare LF is rejected (smuggling defense) var reader = readerFor("line-one\nline-two\n"); - // when / then - assertThat(reader.readLine()).isEqualTo("line-one"); - assertThat(reader.readLine()).isEqualTo("line-two"); + // when + var thrown = catchThrowable(reader::readLine); + // then + assertThat(thrown).isInstanceOf(ProtocolException.class); + } + + @Test + void testReadLineRejectsOverlongLine() { + // given — a line with no terminator must not grow memory without bound + var reader = readerFor("A".repeat(9000) + "\r\n"); + // when + var thrown = catchThrowable(reader::readLine); + // then + assertThat(thrown).isInstanceOf(ProtocolException.class); } @Test diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java index 52e3c46..f3d27ae 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestParserTest.java @@ -327,7 +327,7 @@ void testRejectHeaderTooLarge() { ); // then assertThat(result).isInstanceOf(ProtocolException.class); - assertThat(result).hasMessageContaining("Header too large"); + assertThat(result).hasMessageContaining("maximum length"); assertThat(result).hasMessageContaining("8192"); } diff --git a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java index ab57b22..19e8a81 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/RequestProcessorTest.java @@ -213,6 +213,34 @@ void testWithConnectionKeepAlive() throws IOException { assertThat(result).isEqualTo(expected); } + @Test + void testProcessErrorPropagatesConnectionClose() throws IOException { + // given — a handler that throws, on a Connection: close request; the 500 must also close + var inputStream = buildInputStream( + """ + PUT /error HTTP/1.1\r + Host: localhost\r + Connection: close\r + \r + """ + ); + var outputStream = new ByteArrayOutputStream(); + var reader = new HttpConnectionReader(inputStream); + // when + classUnderTest.process(reader, outputStream); + // then + var expected = """ + HTTP/1.1 500 Internal Server Error\r + Connection: close\r + Content-Length: 34\r + Content-Type: text/plain\r + Date: Sun, 01 Oct 2023 12:00:00 GMT\r + \r + Unexpected error executing request"""; + var result = outputStream.toString(); + assertThat(result).isEqualTo(expected); + } + @Test void testProcessRejectsMalformedContentLengthWith400() throws IOException { // given — a non-numeric Content-Length must yield a 400 response, not a dropped connection diff --git a/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java b/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java index 762ff6c..d282d6d 100644 --- a/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java +++ b/src/test/java/net/uiqui/embedhttp/server/io/ResponseWriterTest.java @@ -1,6 +1,7 @@ package net.uiqui.embedhttp.server.io; import net.uiqui.embedhttp.api.ContentType; +import net.uiqui.embedhttp.api.HttpHeader; import net.uiqui.embedhttp.api.HttpStatusCode; import net.uiqui.embedhttp.api.impl.HttpResponseImpl; import net.uiqui.embedhttp.server.Now; @@ -87,6 +88,46 @@ void testWriteBodylessOkResponseHasContentLengthZero() throws IOException { assertThat(result).isEqualTo(expected); } + @Test + void testDoesNotDuplicateDateHeaderWhenAlreadySet() throws IOException { + // given — a handler set its own Date; the writer must not append a second one + var response = new HttpResponseImpl(HttpStatusCode.OK); + response.setHeader(HttpHeader.DATE, "Mon, 02 Oct 2023 10:00:00 GMT"); + response.setBody(ContentType.TEXT_PLAIN, "Hello World"); + var outputStream = new ByteArrayOutputStream(); + // when + classUnderTest.writeResponse(outputStream, response); + // then + var expected = """ + HTTP/1.1 200 OK\r + Content-Length: 11\r + Content-Type: text/plain\r + Date: Mon, 02 Oct 2023 10:00:00 GMT\r + \r + Hello World"""; + var result = outputStream.toString(); + assertThat(result).isEqualTo(expected); + } + + @Test + void testWritesBodyAsUtf8Bytes() throws IOException { + // given — a multi-byte body must be written as UTF-8 matching the Content-Length byte count + var response = buildResponse(HttpStatusCode.OK, "héllo"); + var outputStream = new ByteArrayOutputStream(); + // when + classUnderTest.writeResponse(outputStream, response); + // then + var result = new String(outputStream.toByteArray(), java.nio.charset.StandardCharsets.UTF_8); + var expected = """ + HTTP/1.1 200 OK\r + Content-Length: 6\r + Content-Type: text/plain\r + Date: Sun, 01 Oct 2023 12:00:00 GMT\r + \r + héllo"""; + assertThat(result).isEqualTo(expected); + } + private HttpResponseImpl buildResponse(HttpStatusCode status, String body) { var response = new HttpResponseImpl(status);