From 5a658bb8875e5e13cbcf47cde417d88d9851e1ae Mon Sep 17 00:00:00 2001
From: Nitesh Kant <nitesh_kant@apple.com>
Date: Wed, 23 Jun 2021 03:07:16 -0700
Subject: [PATCH] `HttpUtil#normalizeAndGetContentLength()` should handle empty
 value (#11409)

__Motivation__

`HttpUtil#normalizeAndGetContentLength()` throws `StringIndexOutOfBoundsException` for empty `content-length` values, it should instead throw `IllegalArgumentException` for all invalid values.

__Modification__

- Throw `IllegalArgumentException` if the `content-length` value is empty.
- Add tests

__Result__

Fixes https://github.com/netty/netty/issues/11408
---
 .../io/netty/handler/codec/http/HttpUtil.java |  2 +-
 .../handler/codec/http/HttpUtilTest.java      | 39 ++++++++++++++++---
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
index 3e50f3d448..3cf648e612 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpUtil.java
@@ -602,7 +602,7 @@ public final class HttpUtil {
         }
         // Ensure we not allow sign as part of the content-length:
         // See https://github.com/squid-cache/squid/security/advisories/GHSA-qf3v-rc95-96j5
-        if (!Character.isDigit(firstField.charAt(0))) {
+        if (firstField.isEmpty() || !Character.isDigit(firstField.charAt(0))) {
             // Reject the message as invalid
             throw new IllegalArgumentException(
                     "Content-Length value is not a number: " + firstField);
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java
index 6b05fa53e5..abe05f1988 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpUtilTest.java
@@ -15,21 +15,24 @@
  */
 package io.netty.handler.codec.http;
 
+import io.netty.util.CharsetUtil;
+import io.netty.util.ReferenceCountUtil;
+import org.junit.Test;
+import org.junit.jupiter.api.function.Executable;
+
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
-import io.netty.util.CharsetUtil;
-import io.netty.util.ReferenceCountUtil;
-import org.junit.Test;
-
 import static io.netty.handler.codec.http.HttpHeadersTestUtils.of;
+import static io.netty.handler.codec.http.HttpUtil.normalizeAndGetContentLength;
+import static java.util.Collections.singletonList;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -246,7 +249,7 @@ public class HttpUtilTest {
         HttpMessage message = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
         message.headers().add(HttpHeaderNames.TRANSFER_ENCODING, "chunked");
         HttpUtil.setTransferEncodingChunked(message, true);
-        List<String> expected = Collections.singletonList("chunked");
+        List<String> expected = singletonList("chunked");
         assertEquals(expected, message.headers().getAll(HttpHeaderNames.TRANSFER_ENCODING));
     }
 
@@ -395,4 +398,28 @@ public class HttpUtilTest {
                 HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE + ", " + HttpHeaderValues.KEEP_ALIVE);
         assertTrue(HttpUtil.isKeepAlive(http11Message));
     }
+
+    @Test
+    public void normalizeAndGetContentLengthEmpty() {
+        testNormalizeAndGetContentLengthInvalidContentLength("");
+    }
+
+    @Test
+    public void normalizeAndGetContentLengthNotANumber() {
+        testNormalizeAndGetContentLengthInvalidContentLength("foo");
+    }
+
+    @Test
+    public void normalizeAndGetContentLengthNegative() {
+        testNormalizeAndGetContentLengthInvalidContentLength("-1");
+    }
+
+    private static void testNormalizeAndGetContentLengthInvalidContentLength(final String contentLengthField) {
+        assertThrows(IllegalArgumentException.class, new Executable() {
+            @Override
+            public void execute() {
+                normalizeAndGetContentLength(singletonList(contentLengthField), false, false);
+            }
+        });
+    }
 }