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..db69583 --- /dev/null +++ b/src/main/java/net/uiqui/embedhttp/server/io/HttpConnectionReader.java @@ -0,0 +1,97 @@ +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 static final int MAX_LINE_LENGTH = 8192; // 8KB — request line and header lines + + 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); + + // 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) { + 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) throws ProtocolException { + var bytes = buffer.toByteArray(); + var length = bytes.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 - 1, 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..7a8b942 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; @@ -22,19 +21,29 @@ 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 { - 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); + 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 RequestLine decodeRequestLine(BufferedReader reader) throws IOException { + 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); var parts = line.split(" ", 3); @@ -57,7 +66,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,16 +83,12 @@ 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; 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); @@ -96,20 +101,34 @@ private InsensitiveMap decodeRequestHeaders(BufferedReader reader) throws IOExce 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); } return headers; } - private String decodeRequestBody(BufferedReader reader, Map headers) throws IOException { - if (headers.containsKey(HttpHeader.CONTENT_LENGTH.getValue())) { - var contentLength = Integer.parseInt(headers.get(HttpHeader.CONTENT_LENGTH.getValue())); + private String decodeRequestBody(HttpConnectionReader reader, Map headers) throws IOException { + 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); } - return readFixedSizeBodyChunk(reader, contentLength); + return new String(reader.readBody(contentLength), StandardCharsets.UTF_8); } if (TRANSFER_ENCODING_CHUNKED.equalsIgnoreCase(headers.get(HttpHeader.TRANSFER_ENCODING.getValue()))) { @@ -119,21 +138,38 @@ private String decodeRequestBody(BufferedReader reader, Map head return ""; // No body or unsupported format } - private boolean decodeKeepAlive(InsensitiveMap headers) { - var connectionHeader = headers.get(HttpHeader.CONNECTION.getValue()); - if (connectionHeader == null) { - return true; // Default to keep-alive if no connection header is present + private int parseContentLength(String value) throws ProtocolException { + int contentLength; + try { + contentLength = Integer.parseInt(value.trim()); + } catch (NumberFormatException e) { + throw new ProtocolException("Invalid Content-Length: " + value); } + if (contentLength < 0) { + throw new ProtocolException("Invalid Content-Length: " + value); + } + + return contentLength; + } + + private boolean decodeKeepAlive(HttpVersion version, InsensitiveMap headers) { + var connectionHeader = headers.get(HttpHeader.CONNECTION.getValue()); + if (KEEP_ALIVE.getValue().equalsIgnoreCase(connectionHeader)) { return true; } - return !CLOSE.getValue().equalsIgnoreCase(connectionHeader); + if (CLOSE.getValue().equalsIgnoreCase(connectionHeader)) { + return false; + } + + // No explicit directive: HTTP/1.1 defaults to keep-alive, HTTP/1.0 to close. + return version == HttpVersion.VERSION_1_1; } - 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,44 +178,50 @@ private String readChunkedBody(BufferedReader reader) throws IOException { break; } - body.append(readFixedSizeBodyChunk(reader, chunkSize)); + // Cap the aggregate body size; per-chunk limits alone leave chunked transfers unbounded. + if (body.size() + chunkSize > 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 } - 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"); } - int chunkSize = Integer.parseInt(line.trim(), 16); - if (chunkSize > MAX_CHUNK_SIZE) { - throw new ProtocolException("Chunk size too large: " + chunkSize); + // 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(); } - return chunkSize; - } - - private String readFixedSizeBodyChunk(BufferedReader reader, int chunkSize) throws IOException { - var chunk = new char[chunkSize]; - int read = 0; + int chunkSize; + try { + chunkSize = Integer.parseInt(sizeToken, 16); + } catch (NumberFormatException e) { + throw new ProtocolException("Invalid chunk size: " + line); + } - while (read < chunkSize) { - var readCount = reader.read(chunk, read, chunkSize - read); - if (readCount == -1) { - throw new ProtocolException("Unexpected end of stream while reading body"); - } + if (chunkSize < 0) { + throw new ProtocolException("Invalid chunk size: " + line); + } - read += readCount; + if (chunkSize > MAX_CHUNK_SIZE) { + throw new ProtocolException("Chunk size too large: " + chunkSize); } - return new String(chunk); + return chunkSize; } - 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 +230,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..a29aaaa 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() @@ -74,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 c888058..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"; @@ -32,11 +33,23 @@ public void writeResponse(OutputStream outputStream, HttpResponseImpl response) .append(CRLF); } - // Write the Date header - builder.append(HttpHeader.DATE.getValue()) - .append(": ") - .append(dateHeader.getDateHeaderValue()) - .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, 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); @@ -47,7 +60,16 @@ public void writeResponse(OutputStream outputStream, HttpResponseImpl response) } // 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(); } + + 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..38e4057 --- /dev/null +++ b/src/test/java/net/uiqui/embedhttp/server/io/HttpConnectionReaderTest.java @@ -0,0 +1,104 @@ +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 testReadLineRejectsBareLf() { + // given — HTTP framing requires CRLF; a bare LF is rejected (smuggling defense) + var reader = readerFor("line-one\nline-two\n"); + // 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 + 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..f3d27ae 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 @@ -259,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"); } @@ -286,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"); } @@ -294,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)); @@ -447,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 37a6d80..19e8a81 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,11 +213,60 @@ 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; + @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 + 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) { 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..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; @@ -69,6 +70,64 @@ 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); + } + + @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);