Working

Author: robotdan

src/main/java/io/fusionauth/http/server/internal/HTTPWorker.java

@@ -145,7 +145,6 @@ public void run() {
           instrumenter.acceptedRequest();
         }
 
-        // Configure maximum content length
         int maximumContentLength = getMaximumContentLength(request);
         httpInputStream = new HTTPInputStream(configuration, request, inputStream, maximumContentLength);
         request.setInputStream(httpInputStream);
@@ -258,7 +257,7 @@ public void run() {
       closeSocketOnly(reason);
     } catch (ParseException e) {
       logger.debug("[{}] Closing socket with status [{}]. Bad request, failed to parse request. Reason [{}] Parser state [{}]", Thread.currentThread().threadId(), Status.BadRequest, e.getMessage(), e.getState());
-      closeSocketOnError(response, Status.BadRequest);
+      closeSocketOnError(response, Status.HTTPVersionNotSupported);
     } catch (SocketException e) {
       // When the HTTPServerThread shuts down, we will interrupt each client thread, so debug log it accordingly.
       // - This will cause the socket to throw a SocketException, so log it.

src/main/java/io/fusionauth/http/server/io/HTTPInputStream.java

@@ -151,8 +151,9 @@ public int read(byte[] b, int off, int len) throws IOException {
 
     // When we have a fixed length request, read beyond the remainingBytes if possible.
     // - If we have read past the end of the current request, push those bytes back onto the InputStream.
-    int maxLen = maximumContentLength == -1 ? len : Math.min(len, maximumContentLength - bytesRead + 1);
-    int read = delegate.read(b, off, maxLen);
+    // - When a maximum content length has been specified, read at most one byte past the maximum.
+    int maxReadLen = maximumContentLength == -1 ? len : Math.min(len, maximumContentLength - bytesRead + 1);
+    int read = delegate.read(b, off, maxReadLen);
 
     int reportBytesRead = read;
     if (fixedLength && read > 0) {
@@ -163,16 +164,14 @@ public int read(byte[] b, int off, int len) throws IOException {
       }
     }
 
-    if (read > 0) {
-      if (fixedLength) {
-        bytesRemaining -= reportBytesRead;
-      }
+    if (read > 0 && fixedLength) {
+      bytesRemaining -= reportBytesRead;
     }
 
     bytesRead += reportBytesRead;
 
-    // This won't cause us to fail as fast as we could, but it keeps the code a bit simpler.
-    // - This means we will have read past the maximum by n where n is > 0 && < len. This seems like an acceptable over-read, in practice the buffers will be
+    // Note that when the request is fixed length, we will have failed early during commit().
+    // - This will handle all requests that are not fixed length.
     if (maximumContentLength != -1) {
       if (bytesRead > maximumContentLength) {
         String detailedMessage = "The maximum request size has been exceeded.The maximum request size is [" + maximumContentLength + "] bytes.";
@@ -186,8 +185,6 @@ public int read(byte[] b, int off, int len) throws IOException {
   private void commit() {
     committed = true;
 
-    // TODO : Handle : Content-Encoding
-
     // Note that isChunked() should take precedence over the fact that we have a Content-Length.
     // - The client should not send both, but in the case they are both present we ignore Content-Length
     //   In practice, we will remove the Content-Length header when sent in addition to Transfer-Encoding. See HTTPWorker.validatePreamble.

src/test/java/io/fusionauth/http/BaseSocketTest.java

@@ -21,6 +21,7 @@
 
 import io.fusionauth.http.server.HTTPHandler;
 import io.fusionauth.http.server.HTTPServer;
+import static org.testng.Assert.assertEquals;
 
 /**
  * A base class to provide some helpers for socket based tests. A socket test doesn't use an HTTP client, but instead manually writes to the
@@ -54,6 +55,8 @@ private void assertResponse(String request, String chunkedExtension, String resp
         .start();
          Socket socket = makeClientSocket("http")) {
 
+      socket.setSoTimeout((int) Duration.ofSeconds(30).toMillis());
+
       var bodyString = "These pretzels are making me thirsty. ";
       // Ensure this is larger than the default configured size for the request buffer.
       // - This body is added to each request to ensure we correctly drain the InputStream before we can write the HTTP response.
@@ -85,11 +88,22 @@ private void assertResponse(String request, String chunkedExtension, String resp
       }
 
       request = request.replace("{body}", body);
-      request = request.replace("{contentLength}", body.getBytes(StandardCharsets.UTF_8).length + "");
+      var contentLength = body.getBytes(StandardCharsets.UTF_8).length;
+      request = request.replace("{contentLength}", contentLength + "");
+
+      // Ensure the caller didn't add an extra line return to the request.
+      int bodyStart = request.indexOf("\r\n\r\n") + 4;
+      String payload = request.substring(bodyStart);
+      assertEquals(contentLength, payload.getBytes(StandardCharsets.UTF_8).length, "Check the value you provided for 'withRequest' it looks like you may have a trailing line return or something.\n");
 
       var os = socket.getOutputStream();
       os.write(request.getBytes(StandardCharsets.UTF_8));
 
+      // 1. Write bytes, the content-length is short by one byte.
+      // 2. Next byte \n
+      // 3. Success.
+      // 4. Next keep-alive request reads preamble, and reads the \n and throws an exception
+
       assertHTTPResponseEquals(socket, response);
     }
   }

src/test/java/io/fusionauth/http/BaseTest.java

@@ -387,9 +387,11 @@ protected void assertHTTPResponseEquals(Socket socket, String expectedResponse)
     var is = socket.getInputStream();
     var expectedResponseLength = expectedResponse.getBytes(StandardCharsets.UTF_8).length;
     try {
-      byte[] buffer = new byte[expectedResponse.length() * 2];
+
+      byte[] buffer = new byte[expectedResponseLength * 2];
       int read = is.read(buffer);
       var actualResponse = new String(buffer, 0, read, StandardCharsets.UTF_8);
+
       // Perform an initial equality check, this is fast. If it fails, it may because there are remaining bytes left to read. This is slower.
       if (!actualResponse.equals(expectedResponse)) {
         // Note this is going to block until the socket keep-alive times out.

src/test/java/io/fusionauth/http/HTTP11SocketTest.java

@@ -30,8 +30,7 @@ public void bad_request() throws Exception {
     withRequest("""
         cat /etc/password\r
         \r
-        {body}
-        """
+        {body}"""
     ).expectResponse("""
         HTTP/1.1 400 \r
         connection: close\r
@@ -391,12 +390,18 @@ public void transfer_encoding_content_length() throws Exception {
         Content-Length: {contentLength}\r
         Transfer-Encoding: chunked\r
         \r
-        {body}"""
+        {body}
+        """
     ).expectResponse("""
         HTTP/1.1 200 \r
         connection: keep-alive\r
         content-length: 0\r
         \r
         """);
+
+    // Order of operation:
+    // 1. Write body w/ a trailing line return not accounted for in Content-Length.
+    // 2. Read body, write 200 response. Keep alive.
+    // 3. Read remaining byte, parse error. Write 400 response. Close connection.
   }
 }

src/test/java/io/fusionauth/http/io/HTTPInputStreamTest.java

@@ -33,8 +33,8 @@ public void read_chunked_withPushback() throws Exception {
     // Ensure that when we read a chunked encoded body that the InputStream returns the correct number of bytes read even when
     // we read past the end of the current request and use the PushbackInputStream.
 
-    int contentLength = 113;
     String content = "These pretzels are making me thirsty. These pretzels are making me thirsty. These pretzels are making me thirsty.";
+    int contentLength = content.getBytes(StandardCharsets.UTF_8).length;
 
     // Chunk the content
     byte[] bytes = """
@@ -60,8 +60,8 @@ public void read_fixedLength_withPushback() throws Exception {
     // Ensure that when we read a fixed length body that the InputStream returns the correct number of bytes read even when
     // we read past the end of the current request and use the PushbackInputStream.
 
-    int contentLength = 113;
     String content = "These pretzels are making me thirsty. These pretzels are making me thirsty. These pretzels are making me thirsty.";
+    int contentLength = content.getBytes(StandardCharsets.UTF_8).length;
 
     // Fixed length body with the start of the next request in the buffer
     byte[] bytes = (content + "GET / HTTP/1.1\r\n").getBytes(StandardCharsets.UTF_8);