Repository: netty/netty
Issue: Resolve charset from ContentType header for SOAP 1.2 requests

Hello, there is a problem with charset resolution from `ContentType` header for SOAP 1.2 using `io.netty.handler.codec.http.HttpUtils` class. SOAP 1.2 could have action parameter in `ContentType` header and method `getCharsetAsSequence(CharSequence contentTypeValue)` from `io.netty.handler.codec.http.HttpUtils` class just finds charset word and take whole string from this word end to the end.

### Expected behavior
Resolve charset from `ContentType` header with value `text/xml; charset=utf-8; action="someaction"` as UTF-8

### Actual behavior
Resolve charset substring from `ContentType` header with value `text/xml; charset=utf-8; action="someaction"` as `utf-8; action="someaction"`. And then you will get an exception `java.nio.charset.IllegalCharsetNameException`

### Steps to reproduce

Resolve charset from any request which contains action parameter in `ContentType` header value

### Netty version

4.1.19

### JVM version (e.g. `java -version`)

java version "1.8.0_181"
Java(TM) SE Runtime Environment (build 1.8.0_181-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.181-b13, mixed mode)

### OS version (e.g. `uname -a`)

Linux 4.15.0-33-generic Ubuntu 18 

Issue comments:
@normanmaurer, please verify it or somebody else.
@amizurov merged... thanks

Pull request review comments:
nit: You can remove the `else` here as you return in the if block.
@amizurov can you add comment that passing null into `contentEqualsIngoreCase` is fine ?
Thanks, done
If we passed null to ```contentEqulasIgnoreCase(...)``` this always return false, null safe method.

Patch that resolved the issue:
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 5b6e90e9abd..94af7901fc4 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
@@ -15,18 +15,18 @@
  */
 package io.netty.handler.codec.http;
 
-import io.netty.util.AsciiString;
-import io.netty.util.CharsetUtil;
-import io.netty.util.NetUtil;
-
 import java.net.InetSocketAddress;
 import java.net.URI;
-import java.util.ArrayList;
 import java.nio.charset.Charset;
 import java.nio.charset.UnsupportedCharsetException;
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import io.netty.util.AsciiString;
+import io.netty.util.CharsetUtil;
+import io.netty.util.NetUtil;
+
 /**
  * Utility methods useful in the HTTP context.
  */
@@ -60,12 +60,13 @@ public static boolean isAsteriskForm(URI uri) {
     /**
      * Returns {@code true} if and only if the connection can remain open and
      * thus 'kept alive'.  This methods respects the value of the.
+     *
      * {@code "Connection"} header first and then the return value of
      * {@link HttpVersion#isKeepAliveDefault()}.
      */
     public static boolean isKeepAlive(HttpMessage message) {
         CharSequence connection = message.headers().get(HttpHeaderNames.CONNECTION);
-        if (connection != null && HttpHeaderValues.CLOSE.contentEqualsIgnoreCase(connection)) {
+        if (HttpHeaderValues.CLOSE.contentEqualsIgnoreCase(connection)) {
             return false;
         }
 
@@ -193,6 +194,7 @@ public static long getContentLength(HttpMessage message, long defaultValue) {
 
     /**
      * Get an {@code int} representation of {@link #getContentLength(HttpMessage, long)}.
+     *
      * @return the content length or {@code defaultValue} if this message does
      *         not have the {@code "Content-Length"} header or its value is not
      *         a number. Not to exceed the boundaries of integer.
@@ -313,6 +315,7 @@ public static boolean isTransferEncodingChunked(HttpMessage message) {
     /**
      * Set the {@link HttpHeaderNames#TRANSFER_ENCODING} to either include {@link HttpHeaderValues#CHUNKED} if
      * {@code chunked} is {@code true}, or remove {@link HttpHeaderValues#CHUNKED} if {@code chunked} is {@code false}.
+     *
      * @param m The message which contains the headers to modify.
      * @param chunked if {@code true} then include {@link HttpHeaderValues#CHUNKED} in the headers. otherwise remove
      * {@link HttpHeaderValues#CHUNKED} from the headers.
@@ -371,7 +374,7 @@ public static Charset getCharset(CharSequence contentTypeValue) {
     /**
      * Fetch charset from message's Content-Type header.
      *
-     * @param message entity to fetch Content-Type header from
+     * @param message        entity to fetch Content-Type header from
      * @param defaultCharset result to use in case of empty, incorrect or doesn't contain required part header value
      * @return the charset from message's Content-Type header or {@code defaultCharset}
      * if charset is not presented or unparsable
@@ -389,7 +392,7 @@ public static Charset getCharset(HttpMessage message, Charset defaultCharset) {
      * Fetch charset from Content-Type header value.
      *
      * @param contentTypeValue Content-Type header value to parse
-     * @param defaultCharset result to use in case of empty, incorrect or doesn't contain required part header value
+     * @param defaultCharset   result to use in case of empty, incorrect or doesn't contain required part header value
      * @return the charset from message's Content-Type header or {@code defaultCharset}
      * if charset is not presented or unparsable
      */
@@ -459,13 +462,23 @@ public static CharSequence getCharsetAsSequence(CharSequence contentTypeValue) {
         if (contentTypeValue == null) {
             throw new NullPointerException("contentTypeValue");
         }
+
         int indexOfCharset = AsciiString.indexOfIgnoreCaseAscii(contentTypeValue, CHARSET_EQUALS, 0);
-        if (indexOfCharset != AsciiString.INDEX_NOT_FOUND) {
-            int indexOfEncoding = indexOfCharset + CHARSET_EQUALS.length();
-            if (indexOfEncoding < contentTypeValue.length()) {
-                return contentTypeValue.subSequence(indexOfEncoding, contentTypeValue.length());
+        if (indexOfCharset == AsciiString.INDEX_NOT_FOUND) {
+            return null;
+        }
+
+        int indexOfEncoding = indexOfCharset + CHARSET_EQUALS.length();
+        if (indexOfEncoding < contentTypeValue.length()) {
+            CharSequence charsetCandidate = contentTypeValue.subSequence(indexOfEncoding, contentTypeValue.length());
+            int indexOfSemicolon = AsciiString.indexOfIgnoreCaseAscii(charsetCandidate, SEMICOLON, 0);
+            if (indexOfSemicolon == AsciiString.INDEX_NOT_FOUND) {
+                return charsetCandidate;
             }
+
+            return charsetCandidate.subSequence(0, indexOfSemicolon);
         }
+
         return null;
     }
 
@@ -517,6 +530,7 @@ public static CharSequence getMimeType(CharSequence contentTypeValue) {
     /**
      * Formats the host string of an address so it can be used for computing an HTTP component
      * such as an URL or a Host header
+     *
      * @param addr the address
      * @return the formatted String
      */
@@ -526,7 +540,7 @@ public static String formatHostnameForHttp(InetSocketAddress addr) {
             if (!addr.isUnresolved()) {
                 hostString = NetUtil.toAddressString(addr.getAddress());
             }
-            return "[" + hostString + "]";
+            return '[' + hostString + ']';
         }
         return hostString;
     }


Test patch:
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 6ad08e6b4c8..31596067aef 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,10 +15,6 @@
  */
 package io.netty.handler.codec.http;
 
-import io.netty.util.CharsetUtil;
-import io.netty.util.ReferenceCountUtil;
-import org.junit.Test;
-
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
@@ -26,12 +22,14 @@
 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 org.hamcrest.Matchers.hasToString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -91,6 +89,22 @@ public void testGetCharset() {
         assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(UPPER_CASE_NORMAL_CONTENT_TYPE));
     }
 
+    @Test
+    public void testGetCharsetIfNotLastParameter() {
+        String NORMAL_CONTENT_TYPE_WITH_PARAMETERS = "application/soap-xml; charset=utf-8; "
+            + "action=\"http://www.soap-service.by/foo/add\"";
+
+        HttpMessage message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST,
+            "http://localhost:7788/foo");
+        message.headers().set(HttpHeaderNames.CONTENT_TYPE, NORMAL_CONTENT_TYPE_WITH_PARAMETERS);
+
+        assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(message));
+        assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(NORMAL_CONTENT_TYPE_WITH_PARAMETERS));
+
+        assertEquals("utf-8", HttpUtil.getCharsetAsSequence(message));
+        assertEquals("utf-8", HttpUtil.getCharsetAsSequence(NORMAL_CONTENT_TYPE_WITH_PARAMETERS));
+    }
+
     @Test
     public void testGetCharset_defaultValue() {
         final String SIMPLE_CONTENT_TYPE = "text/html";
@@ -292,4 +306,15 @@ public void testIpv4Unresolved()  {
         InetSocketAddress socketAddress = InetSocketAddress.createUnresolved("10.0.0.1", 8080);
         assertEquals("10.0.0.1", HttpUtil.formatHostnameForHttp(socketAddress));
     }
+
+    @Test
+    public void testKeepAliveIfConnectionHeaderAbsent() {
+        HttpMessage http11Message = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
+            "http:localhost/http_1_1");
+        assertTrue(HttpUtil.isKeepAlive(http11Message));
+
+        HttpMessage http10Message = new DefaultHttpRequest(HttpVersion.HTTP_1_0, HttpMethod.GET,
+            "http:localhost/http_1_0");
+        assertFalse(HttpUtil.isKeepAlive(http10Message));
+    }
 }