Working

Author: robotdan

src/main/java/io/fusionauth/http/HTTPValues.java

@@ -51,6 +51,8 @@ public static final class ContentEncodings {
 
     public static final String Gzip = "gzip";
 
+    public static final String XGzip = "x-gzip";
+
     private ContentEncodings() {
     }
   }

src/main/java/io/fusionauth/http/io/PushbackInputStream.java

@@ -38,7 +38,6 @@ public class PushbackInputStream extends InputStream {
 
   private int bufferPosition;
 
-
   public PushbackInputStream(InputStream delegate, Instrumenter instrumenter) {
     this.delegate = delegate;
     this.instrumenter = instrumenter;

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

@@ -44,6 +44,7 @@
 import io.fusionauth.http.FileInfo;
 import io.fusionauth.http.HTTPMethod;
 import io.fusionauth.http.HTTPValues.Connections;
+import io.fusionauth.http.HTTPValues.ContentEncodings;
 import io.fusionauth.http.HTTPValues.ContentTypes;
 import io.fusionauth.http.HTTPValues.Headers;
 import io.fusionauth.http.HTTPValues.Protocols;
@@ -231,7 +232,10 @@ public List<String> getAcceptEncodings() {
 
   public void setAcceptEncodings(List<String> encodings) {
     this.acceptEncodings.clear();
-    this.acceptEncodings.addAll(encodings);
+    // TODO : ? Maybe not worth being defensive here since we likely have many methods on this object that are not null safe?
+    if (encodings != null) {
+      this.acceptEncodings.addAll(encodings);
+    }
   }
 
   /**
@@ -302,9 +306,13 @@ public List<String> getContentEncodings() {
     return contentEncodings;
   }
 
+  // TODO : Note : Should I add 'addContentEncodings' and 'addContentEncoding' to match 'accept*' ?
   public void setContentEncodings(List<String> encodings) {
     this.contentEncodings.clear();
-    this.contentEncodings.addAll(encodings);
+    // TODO : ? Maybe not worth being defensive here since we likely have many methods on this object that are not null safe?
+    if (encodings != null) {
+      this.contentEncodings.addAll(encodings);
+    }
   }
 
   public Long getContentLength() {
@@ -726,7 +734,7 @@ public void setURLParameters(String name, Collection<String> values) {
 
   private void decodeHeader(String name, String value) {
     switch (name) {
-      case Headers.AcceptEncodingLower, Headers.ContentEncodingLower:
+      case Headers.AcceptEncodingLower:
         SortedSet<WeightedString> weightedStrings = new TreeSet<>();
         String[] parts = value.split(",");
         int index = 0;
@@ -743,21 +751,19 @@ private void decodeHeader(String name, String value) {
             weight = Double.parseDouble(weightText);
           }
 
-          WeightedString ws = new WeightedString(parsed.value(), weight, index);
+          // Content-Encoding values are not case-sensitive
+          // TODO : Ok to lc here? We are currently just always using equalsIgnoreCase
+          WeightedString ws = new WeightedString(parsed.value().toLowerCase(), weight, index);
           weightedStrings.add(ws);
           index++;
         }
 
         // Transfer the Strings in weighted-position order
-        var result = weightedStrings.stream()
-                                    .map(WeightedString::value)
-                                    .toList();
-
-        if (name.equals(Headers.AcceptEncodingLower)) {
-          setAcceptEncodings(result);
-        } else {
-          setContentEncodings(result);
-        }
+        setAcceptEncodings(
+            weightedStrings.stream()
+                           .map(WeightedString::value)
+                           .toList()
+        );
         break;
       case Headers.AcceptLanguageLower:
         try {
@@ -771,6 +777,31 @@ private void decodeHeader(String name, String value) {
           // Ignore the exception and keep the value null
         }
         break;
+      case Headers.ContentEncodingLower:
+        // TODO : Note that we don't expect more than one Content-Encoding header. We could take the last one we find,
+        //        or try and combine the values. MDN and other places indicate combining may be preferred even though
+        //        it isn't ideal to send multiple headers like this. I tend to think we should just accept the last one.
+        String[] encodings = value.split(",");
+        List<String> contentEncodings = new ArrayList<>(1);
+        int encodingIndex = 0;
+        for (String encoding : encodings) {
+          // TODO : Ok to lc here? We are currently just always using equalsIgnoreCase
+          encoding = encoding.trim().toLowerCase();
+          if (encoding.isEmpty()) {
+            continue;
+          }
+
+          // The HTTP/1.1 standard recommends that the servers supporting gzip also recognize x-gzip as an alias, for compatibility purposes.
+          if (encoding.equals(ContentEncodings.XGzip)) {
+            encoding = ContentEncodings.Gzip;
+          }
+
+          contentEncodings.add(encoding);
+          encodingIndex++;
+        }
+
+        setContentEncodings(contentEncodings);
+        break;
       case Headers.ContentTypeLower:
         this.encoding = null;
         this.multipart = false;

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

@@ -25,6 +25,7 @@
 import io.fusionauth.http.HTTPProcessingException;
 import io.fusionauth.http.HTTPValues;
 import io.fusionauth.http.HTTPValues.Connections;
+import io.fusionauth.http.HTTPValues.ContentEncodings;
 import io.fusionauth.http.HTTPValues.Headers;
 import io.fusionauth.http.HTTPValues.Protocols;
 import io.fusionauth.http.ParseException;
@@ -149,6 +150,8 @@ public void run() {
         //        Also... with Pushback bytes... the bytes may be compressed for the next request. So we'll need to be able to
         //        read handle this. So perhaps this should all be handled at a high level?
 
+        // HTTPInputStream > Pushback > Decompression > Throughput > Socket
+
         httpInputStream = new HTTPInputStream(configuration, request, inputStream);
         request.setInputStream(httpInputStream);
 
@@ -449,6 +452,25 @@ private Integer validatePreamble(HTTPRequest request) {
       request.removeHeader(Headers.ContentLength);
     }
 
+    // TODO : Note that some indicate you can return a 415 based upon the Accept-Encoding header as well if you do not support the request.
+    //        But I don't know that I agree- to me that is just telling the server here are encoding values I support, and you can tell me what you did.
+    //        The spec even says you can ignore the Accept-Encoding header if you want based upon CPU load, or other reasons.
+    //        Not planning to add any validation for Accept-Encoding.
+
+    // Content-Encoding
+    var contentEncodings = request.getContentEncodings();
+    // TODO : If provided, I think we can safely remove the Content-Length header if present?
+    //        Although, I suppose as long as we decompress early, we should still be able to use the Content-Length header as long as
+    //        it represents the un-compressed payload.
+    for (var encoding : contentEncodings) {
+      // We only support gzip and deflate.
+      // TODO : Ensure we use the same equals check here as other places, I think we should lowercase during parse, and then expect these to be lc.
+      if (!encoding.equals(ContentEncodings.Gzip) && !encoding.equals(ContentEncodings.Deflate)) {
+        // TODO : Add log statement
+        return Status.UnsupportedMediaType;
+      }
+    }
+
     return null;
   }
 
@@ -467,6 +489,8 @@ private enum CloseSocketReason {
   private static class Status {
     public static final int BadRequest = 400;
 
+    public static final int UnsupportedMediaType = 415;
+
     public static final int HTTPVersionNotSupported = 505;
 
     public static final int InternalServerError = 500;

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

@@ -17,8 +17,8 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.zip.DeflaterInputStream;
 import java.util.zip.GZIPInputStream;
+import java.util.zip.InflaterInputStream;
 
 import io.fusionauth.http.HTTPValues.ContentEncodings;
 import io.fusionauth.http.io.ChunkedInputStream;
@@ -170,18 +170,20 @@ private void commit() throws IOException {
     Long contentLength = request.getContentLength();
     boolean hasBody = (contentLength != null && contentLength > 0) || request.isChunked();
 
-//    if (hasBody) {
-//      // The request may contain more than one value, apply in reverse order.
-//      for (String contentEncoding : request.getContentEncodings().reversed()) {
-//        if (contentEncoding.equalsIgnoreCase(ContentEncodings.Deflate)) {
-//          // Use the default buffer size of 512
-//          delegate = new DeflaterInputStream(delegate);
-//        } else if (contentEncoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
-//          // Use the default buffer size of 512
-//          delegate = new GZIPInputStream(delegate);
-//        }
-//      }
-//    }
+    // Request order of operations:
+    //  - "hello world" -> compress -> chunked.
+
+    // Current:
+    //   Chunked:
+    //     HTTPInputStream > Chunked > Pushback > Throughput > Socket
+    //   Fixed:
+    //     HTTPInputStream > Pushback > Throughput > Socket
+
+    // New:
+    //   Chunked
+    //     HTTPInputStream > Chunked > Pushback > Decompress > Throughput > Socket
+    //   Fixed
+    //     HTTPInputStream > Pushback > Decompress > Throughput > Socket
 
     // 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
@@ -198,5 +200,19 @@ private void commit() throws IOException {
     } else {
       logger.trace("Client indicated it was NOT sending an entity-body in the request");
     }
+
+    // TODO : Note I could leave this alone, but when we parse the header we can lower case these values and then remove the equalsIgnoreCase here?
+    //        Seems like ideally we would normalize them to lowercase earlier.
+    if (hasBody) {
+      // The request may contain more than one value, apply in reverse order.
+      // - These are both using the default 512 buffer size.
+      for (String contentEncoding : request.getContentEncodings().reversed()) {
+        if (contentEncoding.equalsIgnoreCase(ContentEncodings.Deflate)) {
+          delegate = new InflaterInputStream(delegate);
+        } else if (contentEncoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
+          delegate = new GZIPInputStream(delegate);
+        }
+      }
+    }
   }
 }

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

@@ -128,6 +128,9 @@ public void reset() {
    */
   public boolean willCompress() {
     if (compress) {
+      // TODO : Note I could leave this alone, but when we parse the header we can lower case these values and then remove the equalsIgnoreCase here?
+      //        Seems like ideally we would normalize them to lowercase earlier.
+      //        Hmm.. sems like we have to in theory since someone could call setAcceptEncodings later, or addAcceptEncodings?
       for (String encoding : acceptEncodings) {
         if (encoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
           return true;
@@ -191,7 +194,11 @@ private void commit(boolean closing) throws IOException {
       response.setContentLength(0L);
     } else {
       // 204 status is specifically "No Content" so we shouldn't write the content-encoding and vary headers if the status is 204
+      // TODO : Compress by default is on by default. But it looks like we don't actually compress unless you also send in an Accept-Encoding header?
       if (compress && !twoOhFour) {
+        // TODO : Note I could leave this alone, but when we parse the header we can lower case these values and then remove the equalsIgnoreCase here?
+        //        Seems like ideally we would normalize them to lowercase earlier.
+        //        Hmm.. sems like we have to in theory since someone could call setAcceptEncodings later, or addAcceptEncodings?
         for (String encoding : acceptEncodings) {
           if (encoding.equalsIgnoreCase(ContentEncodings.Gzip)) {
             response.setHeader(Headers.ContentEncoding, ContentEncodings.Gzip);

src/main/java/io/fusionauth/http/util/HTTPTools.java

@@ -92,8 +92,8 @@ public static boolean isHexadecimalCharacter(byte ch) {
    */
   public static boolean isTokenCharacter(byte ch) {
     return ch == '!' || ch == '#' || ch == '$' || ch == '%' || ch == '&' || ch == '\'' || ch == '*' || ch == '+' || ch == '-' || ch == '.' ||
-        ch == '^' || ch == '_' || ch == '`' || ch == '|' || ch == '~' || (ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z') ||
-        (ch >= '0' && ch <= '9');
+           ch == '^' || ch == '_' || ch == '`' || ch == '|' || ch == '~' || (ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z') ||
+           (ch >= '0' && ch <= '9');
   }
 
   /**

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

@@ -15,6 +15,8 @@
  */
 package io.fusionauth.http;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -51,6 +53,10 @@
 import java.util.Map;
 import java.util.UUID;
 import java.util.stream.Collectors;
+import java.util.zip.DeflaterOutputStream;
+import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
+import java.util.zip.InflaterInputStream;
 
 import io.fusionauth.http.HTTPValues.Connections;
 import io.fusionauth.http.log.FileLogger;
@@ -407,6 +413,33 @@ protected void assertHTTPResponseEquals(Socket socket, String expectedResponse)
     }
   }
 
+  protected byte[] deflate(byte[] bytes) throws Exception {
+    ByteArrayOutputStream baseOutputStream = new ByteArrayOutputStream();
+    try (DeflaterOutputStream out = new DeflaterOutputStream(baseOutputStream, true)) {
+      out.write(bytes);
+      out.flush();
+      out.finish();
+      return baseOutputStream.toByteArray();
+    }
+  }
+
+  protected byte[] gzip(byte[] bytes) throws Exception {
+    ByteArrayOutputStream baseOutputStream = new ByteArrayOutputStream();
+    try (DeflaterOutputStream out = new GZIPOutputStream(baseOutputStream, true)) {
+      out.write(bytes);
+      out.flush();
+      out.finish();
+      return baseOutputStream.toByteArray();
+    }
+  }
+
+  protected byte[] inflate(byte[] bytes) throws Exception {
+    ByteArrayInputStream baseInputStream = new ByteArrayInputStream(bytes);
+    try (InflaterInputStream in = new InflaterInputStream(baseInputStream)) {
+      return in.readAllBytes();
+    }
+  }
+
   protected void printf(String format, Object... args) {
     if (verbose) {
       System.out.printf(SystemOutPrefix + format, args);
@@ -426,6 +459,13 @@ protected void sleep(long millis) {
     }
   }
 
+  protected byte[] ungzip(byte[] bytes) throws Exception {
+    ByteArrayInputStream baseInputStream = new ByteArrayInputStream(bytes);
+    try (InflaterInputStream in = new GZIPInputStream(baseInputStream)) {
+      return in.readAllBytes();
+    }
+  }
+
   /**
    * Verifies that the chain certificates can be validated up to the supplied root certificate. See
    * {@link CertPathValidator#validate(CertPath, CertPathParameters)} for details.