Fixing issue with content-length and compression where the length was being sent back but was incorrect. This now chunks when compression is handled inline and not by the HTTPHandler.

Author: voidmain

Diff for: README.md

@@ -14,20 +14,20 @@ To add this library to your project, you can include this dependency in your Mav
 <dependency>
   <groupId>io.fusionauth</groupId>
   <artifactId>java-http</artifactId>
-  <version>0.4.0-RC.2</version>
+  <version>0.4.0-RC.3</version>
 </dependency>
 ```
 
 If you are using Gradle, you can add this to your build file:
 
 ```groovy
-implementation 'io.fusionauth:java-http:0.4.0-RC.2'
+implementation 'io.fusionauth:java-http:0.4.0-RC.3'
 ```
 
 If you are using Savant, you can add this to your build file:
 
 ```groovy
-dependency(id: "io.fusionauth:java-http:0.4.0-RC.2")
+dependency(id: "io.fusionauth:java-http:0.4.0-RC.3")
 ```
 
 ## Examples Usages:

Diff for: build.savant

@@ -17,7 +17,7 @@ jackson5Version = "3.0.1"
 restifyVersion = "4.2.1"
 testngVersion = "7.10.2"
 
-project(group: "io.fusionauth", name: "java-http", version: "0.4.0-RC.2", licenses: ["ApacheV2_0"]) {
+project(group: "io.fusionauth", name: "java-http", version: "0.4.0-RC.3", licenses: ["ApacheV2_0"]) {
   workflow {
     fetch {
       // Dependency resolution order:

Diff for: java-http.iml

@@ -43,7 +43,7 @@
         </CLASSES>
         <JAVADOC />
         <SOURCES>
-          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-databind/2.15.4/jackson-databind-2.15.4-sources.jar!/" />
+          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-databind/2.15.4/jackson-databind-2.15.4-src.jar!/" />
         </SOURCES>
       </library>
     </orderEntry>
@@ -54,7 +54,7 @@
         </CLASSES>
         <JAVADOC />
         <SOURCES>
-          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-annotations/2.15.4/jackson-annotations-2.15.4-sources.jar!/" />
+          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-annotations/2.15.4/jackson-annotations-2.15.4-src.jar!/" />
         </SOURCES>
       </library>
     </orderEntry>
@@ -65,7 +65,7 @@
         </CLASSES>
         <JAVADOC />
         <SOURCES>
-          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-core/2.15.4/jackson-core-2.15.4-sources.jar!/" />
+          <root url="jar://$MODULE_DIR$/.savant/cache/com/fasterxml/jackson/core/jackson-core/2.15.4/jackson-core-2.15.4-src.jar!/" />
         </SOURCES>
       </library>
     </orderEntry>
@@ -76,7 +76,7 @@
         </CLASSES>
         <JAVADOC />
         <SOURCES>
-          <root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.10.2/testng-7.10.2-sources.jar!/" />
+          <root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.10.2/testng-7.10.2-src.jar!/" />
         </SOURCES>
       </library>
     </orderEntry>
@@ -109,7 +109,7 @@
         </CLASSES>
         <JAVADOC />
         <SOURCES>
-          <root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.7.1/jquery-3.7.1-sources.jar!/" />
+          <root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.7.1/jquery-3.7.1-src.jar!/" />
         </SOURCES>
       </library>
     </orderEntry>

Diff for: load-tests/self/build.savant

@@ -32,7 +32,7 @@ project(group: "io.fusionauth", name: "self", version: "0.1.0", licenses: ["Apac
 
   dependencies {
     group(name: "compile") {
-      dependency(id: "io.fusionauth:java-http:0.4.0-RC.2.{integration}")
+      dependency(id: "io.fusionauth:java-http:0.4.0-RC.3.{integration}")
     }
   }
 

Diff for: load-tests/self/src/main/java/io/fusionauth/http/load/LoadHandler.java

@@ -26,17 +26,32 @@
 public class LoadHandler implements HTTPHandler {
   @Override
   public void handle(HTTPRequest req, HTTPResponse res) {
-    try (InputStream is = req.getInputStream()) {
-      byte[] body = is.readAllBytes();
-      String result = Base64.getEncoder().encodeToString(body);
+    if (req.getPath().equals("/text")) {
+      System.out.println("Text");
       res.setStatus(200);
+      res.setContentType("text/plain");
 
-      OutputStream os = res.getOutputStream();
-      os.write(result.getBytes());
-      os.flush();
-      os.close();
-    } catch (Exception e) {
-      res.setStatus(500);
+      try (OutputStream os = res.getOutputStream()) {
+        System.out.println("Wrote");
+        os.write("Hello world".getBytes());
+        os.flush();
+      } catch (Exception e) {
+        System.out.println("Failed");
+        res.setStatus(500);
+      }
+    } else {
+      try (InputStream is = req.getInputStream()) {
+        byte[] body = is.readAllBytes();
+        String result = Base64.getEncoder().encodeToString(body);
+        res.setStatus(200);
+
+        try (OutputStream os = res.getOutputStream()) {
+          os.write(result.getBytes());
+          os.flush();
+        }
+      } catch (Exception e) {
+        res.setStatus(500);
+      }
     }
   }
 }

Diff for: pom.xml

@@ -2,7 +2,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>io.fusionauth</groupId>
   <artifactId>java-http</artifactId>
-  <version>0.4.0-RC.2</version>
+  <version>0.4.0-RC.3</version>
   <packaging>jar</packaging>
 
   <name>Java HTTP library (client and server)</name>

Diff for: src/main/java/io/fusionauth/http/server/internal/HTTPServerThread.java

@@ -113,7 +113,7 @@ public void run() {
         clients.add(new ClientInfo(client, runnable, throughput));
       } catch (SocketTimeoutException ignore) {
         // Completely smother since this is expected with the SO_TIMEOUT setting in the constructor
-        logger.trace("Nothing accepted. Cleaning up existing connections.");
+        logger.debug("Nothing accepted. Cleaning up existing connections.");
       } catch (SocketException e) {
         // This should only happen when the server is shutdown
         if (socket.isClosed()) {
@@ -164,7 +164,7 @@ public void run() {
           ClientInfo client = iterator.next();
           Thread thread = client.thread();
           if (!thread.isAlive()) {
-            logger.info("Thread is dead. Removing.");
+            logger.debug("Thread is dead. Removing.");
             iterator.remove();
             continue;
           }

Diff for: src/main/java/io/fusionauth/http/server/internal/HTTPWorker.java

@@ -95,7 +95,6 @@ public void run() {
         var request = new HTTPRequest(configuration.getContextPath(), configuration.getMultipartBufferSize(),
             listener.getCertificate() != null ? "https" : "http", listener.getPort(), socket.getInetAddress().getHostAddress());
 
-        logger.debug("Reading preamble");
         var inputStream = new ThroughputInputStream(socket.getInputStream(), throughput);
         var bodyBytes = HTTPTools.parseRequestPreamble(inputStream, request, buffers.requestBuffer(), instrumenter, () -> state = State.Read);
         var httpInputStream = new HTTPInputStream(configuration, request, inputStream, bodyBytes);

Diff for: src/main/java/io/fusionauth/http/server/io/HTTPOutputStream.java

@@ -179,36 +179,40 @@ private void commit(boolean closing) throws IOException {
 
     committed = true;
 
-    // +++++++++++ Step 1: Determine the extra headers first and add them +++++++++++
+    // +++++++++++ Step 1: Determine if there is content and set up the compression, encoding, chunking, and length headers +++++++++++
     boolean twoOhFour = response.getStatus() == 204;
+    boolean gzip = false;
+    boolean deflate = false;
     boolean chunked = false;
-    if (response.getContentLength() == null && !twoOhFour) { // 204 status is specifically "No Content" so we shouldn't write the transfer-encoding header if the status is 204
-      response.setHeader(Headers.TransferEncoding, TransferEncodings.Chunked);
-      chunked = true;
-    }
 
     // If the output stream is closing, but nothing has been written yet, we can safely set the Content-Length header to 0 to let the client
     // know nothing more is coming beyond the preamble
     if (closing && !twoOhFour) { // 204 status is specifically "No Content" so we shouldn't write the content-length header if the status is 204
       response.setContentLength(0L);
-    }
-
-    boolean gzip = false;
-    boolean deflate = false;
-    if (compress && !twoOhFour) { // 204 status is specifically "No Content" so we shouldn't write the content-encoding and vary headers if the status is 204
-      for (String encoding : acceptEncodings) {
-        if (encoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
-          response.setHeader(Headers.ContentEncoding, ContentEncodings.Gzip);
-          response.setHeader(Headers.Vary, Headers.AcceptEncoding);
-          gzip = true;
-          break;
-        } else if (encoding.equalsIgnoreCase(ContentEncodings.Deflate)) {
-          response.setHeader(Headers.ContentEncoding, ContentEncodings.Deflate);
-          response.setHeader(Headers.Vary, Headers.AcceptEncoding);
-          deflate = true;
-          break;
+    } else {
+      // 204 status is specifically "No Content" so we shouldn't write the content-encoding and vary headers if the status is 204
+      if (compress && !twoOhFour) {
+        for (String encoding : acceptEncodings) {
+          if (encoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
+            response.setHeader(Headers.ContentEncoding, ContentEncodings.Gzip);
+            response.setHeader(Headers.Vary, Headers.AcceptEncoding);
+            response.removeHeader(Headers.ContentLength); // Compression will change the length, so we'll chunk instead
+            gzip = true;
+            break;
+          } else if (encoding.equalsIgnoreCase(ContentEncodings.Deflate)) {
+            response.setHeader(Headers.ContentEncoding, ContentEncodings.Deflate);
+            response.setHeader(Headers.Vary, Headers.AcceptEncoding);
+            response.removeHeader(Headers.ContentLength); // Compression will change the length, so we'll chunk instead
+            deflate = true;
+            break;
+          }
         }
       }
+
+      if (response.getContentLength() == null && !twoOhFour) { // 204 status is specifically "No Content" so we shouldn't write the transfer-encoding header if the status is 204
+        response.setHeader(Headers.TransferEncoding, TransferEncodings.Chunked);
+        chunked = true;
+      }
     }
 
     // +++++++++++ Step 2: Write the preamble. This must be first without any other output stream interference +++++++++++

Diff for: src/test/java/io/fusionauth/http/CompressionTest.java

@@ -72,6 +72,7 @@ public void compress(String encoding, boolean chunked, String scheme) throws Exc
       res.setHeader(Headers.ContentType, "text/plain");
       res.setStatus(200);
 
+      // Technically, this is ignored anytime compression is used, but we are testing if folks don't set it here
       if (!chunked) {
         res.setContentLength(Files.size(file));
       }
@@ -112,17 +113,14 @@ public void compress(String encoding, boolean chunked, String scheme) throws Exc
     }
   }
 
-  @Test(dataProvider = "compressedChunkedSchemes")
-  public void compress_onByDefault(String encoding, boolean chunked, String scheme) throws Exception {
+  @Test
+  public void compressBadContentLength() throws Exception {
     HTTPHandler handler = (req, res) -> {
-      // Use case, do not call response.setCompress(true)
+      res.setCompress(true);
+      res.setContentLength(1_000_000); // Ignored
       res.setHeader(Headers.ContentType, "text/plain");
       res.setStatus(200);
 
-      if (!chunked) {
-        res.setContentLength(Files.size(file));
-      }
-
       try (InputStream is = Files.newInputStream(file)) {
         OutputStream outputStream = res.getOutputStream();
         is.transferTo(outputStream);
@@ -132,27 +130,22 @@ public void compress_onByDefault(String encoding, boolean chunked, String scheme
       }
     };
 
-    CountingInstrumenter instrumenter = new CountingInstrumenter();
-    try (var client = makeClient(scheme, null); var ignore = makeServer(scheme, handler, instrumenter).start()) {
-      URI uri = makeURI(scheme, "");
-      var response = client.send(
-          HttpRequest.newBuilder().header(Headers.AcceptEncoding, encoding).uri(uri).GET().build(),
-          r -> BodySubscribers.ofInputStream()
-      );
-
-      var result = new String(
-          encoding.equals(ContentEncodings.Deflate)
-              ? new InflaterInputStream(response.body()).readAllBytes()
-              : new GZIPInputStream(response.body()).readAllBytes(), StandardCharsets.UTF_8);
-
-      assertEquals(response.headers().firstValue(Headers.ContentEncoding).orElse(null), encoding);
+    try (var client = makeClient("http", null); var ignore = makeServer("http", handler).withCompressByDefault(false).start()) {
+      URI uri = makeURI("http", "");
+      var response = client.send(HttpRequest.newBuilder()
+                                            .header(Headers.AcceptEncoding, ContentEncodings.Gzip)
+                                            .uri(uri)
+                                            .GET()
+                                            .build(),
+          r -> BodySubscribers.ofInputStream());
+      assertEquals(response.headers().firstValue(Headers.ContentEncoding).orElse(null), ContentEncodings.Gzip);
       assertEquals(response.statusCode(), 200);
-      assertEquals(result, Files.readString(file));
+      assertEquals(new String(new GZIPInputStream(response.body()).readAllBytes(), StandardCharsets.UTF_8), Files.readString(file));
     }
   }
 
-  @Test(enabled = false, groups = "performance")
-  public void compress_performance() throws Exception {
+  @Test(groups = "performance")
+  public void compressPerformance() throws Exception {
     HTTPHandler handler = (req, res) -> {
       // Use case, do not call response.setCompress(true)
       res.setHeader(Headers.ContentType, "text/plain");
@@ -202,7 +195,46 @@ public void compress_performance() throws Exception {
         System.out.println("[compression: " + mode + "][" + formatter.format(counter) + "] requests. Total time [" + (formatter.format(total)) + "] ms, Avg [" + avg + "] ms");
       }
     }
+  }
+
+  @Test(dataProvider = "compressedChunkedSchemes")
+  public void compress_onByDefault(String encoding, boolean chunked, String scheme) throws Exception {
+    HTTPHandler handler = (req, res) -> {
+      // Use case, do not call response.setCompress(true)
+      res.setHeader(Headers.ContentType, "text/plain");
+      res.setStatus(200);
 
+      // Technically, this is ignored anytime compression is used, but we are testing if folks don't set it here
+      if (!chunked) {
+        res.setContentLength(Files.size(file));
+      }
+
+      try (InputStream is = Files.newInputStream(file)) {
+        OutputStream outputStream = res.getOutputStream();
+        is.transferTo(outputStream);
+        outputStream.close();
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+    };
+
+    CountingInstrumenter instrumenter = new CountingInstrumenter();
+    try (var client = makeClient(scheme, null); var ignore = makeServer(scheme, handler, instrumenter).start()) {
+      URI uri = makeURI(scheme, "");
+      var response = client.send(
+          HttpRequest.newBuilder().header(Headers.AcceptEncoding, encoding).uri(uri).GET().build(),
+          r -> BodySubscribers.ofInputStream()
+      );
+
+      var result = new String(
+          encoding.equals(ContentEncodings.Deflate)
+              ? new InflaterInputStream(response.body()).readAllBytes()
+              : new GZIPInputStream(response.body()).readAllBytes(), StandardCharsets.UTF_8);
+
+      assertEquals(response.headers().firstValue(Headers.ContentEncoding).orElse(null), encoding);
+      assertEquals(response.statusCode(), 200);
+      assertEquals(result, Files.readString(file));
+    }
   }
 
   @DataProvider(name = "compressedChunkedSchemes")
@@ -237,6 +269,7 @@ public void requestedButNotAccepted(boolean chunked, String scheme) throws Excep
       res.setHeader(Headers.ContentType, "text/plain");
       res.setStatus(200);
 
+      // Technically, this is ignored anytime compression is used, but we are testing if folks don't set it here
       if (!chunked) {
         res.setContentLength(Files.size(file));
       }
@@ -273,6 +306,7 @@ public void requestedButNotAccepted_unSupportedEncoding(boolean chunked, String
       res.setHeader(Headers.ContentType, "text/plain");
       res.setStatus(200);
 
+      // Technically, this is ignored anytime compression is used, but we are testing if folks don't set it here
       if (!chunked) {
         res.setContentLength(Files.size(file));
       }