8243246: HTTP Client sometimes gets java.io.IOException -> Invalid chunk header byte 32

The HTTP/1 chunked body parser is updated to discard chunk extensions

Reviewed-by: chegar, alanb

src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -125,6 +125,7 @@ class ResponseContent {
 
 
     static enum ChunkState {READING_LENGTH, READING_DATA, DONE}
+    static final int MAX_CHUNK_HEADER_SIZE = 2050;
     class ChunkedBodyParser implements BodyParser {
         final ByteBuffer READMORE = Utils.EMPTY_BYTEBUFFER;
         final Consumer<Throwable> onComplete;
@@ -136,6 +137,8 @@ class ResponseContent {
         volatile int chunklen = -1;  // number of bytes in chunk
         volatile int bytesremaining;  // number of bytes in chunk left to be read incl CRLF
         volatile boolean cr = false;  // tryReadChunkLength has found CR
+        volatile int chunkext = 0;    // number of bytes already read in the chunk extension
+        volatile int digits = 0;      // number of chunkLength bytes already read
         volatile int bytesToConsume;  // number of bytes that still need to be consumed before proceeding
         volatile ChunkState state = ChunkState.READING_LENGTH; // current state
         volatile AbstractSubscription sub;
@@ -147,6 +150,26 @@ class ResponseContent {
             return dbgTag;
         }
 
+        // best effort - we're assuming UTF-8 text and breaks at character boundaries
+        // for this debug output. Not called.
+        private void debugBuffer(ByteBuffer b) {
+            if (!debug.on()) return;
+            ByteBuffer printable = b.asReadOnlyBuffer();
+            byte[] bytes = new byte[printable.limit() - printable.position()];
+            printable.get(bytes, 0, bytes.length);
+            String msg = "============== accepted ==================\n";
+            try {
+                var str = new String(bytes, "UTF-8");
+                msg += str;
+            } catch (Exception x) {
+                msg += x;
+                x.printStackTrace();
+            }
+            msg += "\n==========================================\n";
+            debug.log(msg);
+
+        }
+
         @Override
         public void onSubscribe(AbstractSubscription sub) {
             if (debug.on())
@@ -158,7 +181,6 @@ class ResponseContent {
         public String currentStateMessage() {
             return format("chunked transfer encoding, state: %s", state);
         }
-
         @Override
         public void accept(ByteBuffer b) {
             if (closedExceptionally != null) {
@@ -166,6 +188,7 @@ class ResponseContent {
                     debug.log("already closed: " + closedExceptionally);
                 return;
             }
+            // debugBuffer(b);
             boolean completed = false;
             try {
                 List<ByteBuffer> out = new ArrayList<>();
@@ -221,6 +244,9 @@ class ResponseContent {
         private int tryReadChunkLen(ByteBuffer chunkbuf) throws IOException {
             assert state == ChunkState.READING_LENGTH;
             while (chunkbuf.hasRemaining()) {
+                if (chunkext + digits >= MAX_CHUNK_HEADER_SIZE) {
+                    throw new IOException("Chunk header size too long: " + (chunkext + digits));
+                }
                 int c = chunkbuf.get();
                 if (cr) {
                     if (c == LF) {
@@ -231,9 +257,34 @@ class ResponseContent {
                 }
                 if (c == CR) {
                     cr = true;
+                    if (digits == 0 && debug.on()) {
+                        debug.log("tryReadChunkLen: invalid chunk header? No digits in chunkLen?");
+                    }
+                } else if (cr == false && chunkext > 0) {
+                    // we have seen a non digit character after the chunk length.
+                    // skip anything until CR is found.
+                    chunkext++;
+                    if (debug.on()) {
+                        debug.log("tryReadChunkLen: More extraneous character after chunk length: " + c);
+                    }
                 } else {
                     int digit = toDigit(c);
-                    partialChunklen = partialChunklen * 16 + digit;
+                    if (digit < 0) {
+                        if (digits > 0) {
+                            // first non-digit character after chunk length.
+                            // skip anything until CR is found.
+                            chunkext++;
+                            if (debug.on()) {
+                                debug.log("tryReadChunkLen: Extraneous character after chunk length: " + c);
+                            }
+                        } else {
+                            // there should be at list one digit in chunk length
+                            throw new IOException("Illegal character in chunk size: " + c);
+                        }
+                    } else {
+                        digits++;
+                        partialChunklen = partialChunklen * 16 + digit;
+                    }
                 }
             }
             return -1;
@@ -286,6 +337,7 @@ class ResponseContent {
                         + " (remaining in buffer:"+chunk.remaining()+")");
                 int clen = chunklen = tryReadChunkLen(chunk);
                 if (clen == -1) return READMORE;
+                digits = chunkext = 0;
                 if (debug.on()) debug.log("Got chunk len %d", clen);
                 cr = false; partialChunklen = 0;
                 unfulfilled = bytesremaining =  clen;
@@ -354,6 +406,7 @@ class ResponseContent {
                     chunklen = -1;
                     partialChunklen = 0;
                     cr = false;
+                    digits = chunkext = 0;
                     state = ChunkState.READING_LENGTH;
                     if (debug.on()) debug.log("Ready to read next chunk");
                 }
@@ -395,7 +448,7 @@ class ResponseContent {
             if (b >= 0x61 && b <= 0x66) {
                 return b - 0x61 + 10;
             }
-            throw new IOException("Invalid chunk header byte " + b);
+            return -1;
         }
 
     }