Repository: netty/netty
Issue: The value of the SNI extension is ASCII encoded but Netty uses UTF-8

The value of the SNI extension is ASCII encoded but Netty's SNI code tries to decode the `byte[]` as UTF-8.

The was flagged by @fenik17 in this https://github.com/netty/netty/pull/6623#discussion_r111070719

https://github.com/netty/netty/blob/4.1/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java#L188

https://tools.ietf.org/html/rfc6066#page-6

> "HostName" contains the fully qualified DNS hostname of the server,
   as understood by the client.  The hostname is represented as a byte
   string using ASCII encoding without a trailing dot.  This allows the
   support of internationalized domain names through the use of A-labels
   defined in [RFC5890].  DNS hostnames are case-insensitive.  The
   algorithm to compare hostnames is described in [RFC5890], Section
   2.3.2.4.

### Expected behavior

Do as the RFC says?

### Actual behavior

Not doing as the RFC says!

### Steps to reproduce

### Minimal yet complete reproducer code (or URL to code)

### Netty version

### JVM version (e.g. `java -version`)

### OS version (e.g. `uname -a`)


Issue comments:
@rkapsi sounds good (do it as the RFC). You want to do a PR or should I put it on my back burner ?
Sounds good, inbound PR shortly
<3

Pull request review comments:
please also call `ch.finishAndReleaseAll();` if `ch != null`

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
index e75c0554f13..86e7eb25be8 100644
--- a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java
@@ -29,7 +29,6 @@
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
-import java.net.IDN;
 import java.net.SocketAddress;
 import java.util.List;
 import java.util.Locale;
@@ -186,11 +185,10 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
                                             }
 
                                             final String hostname = in.toString(offset, serverNameLength,
-                                                                                CharsetUtil.UTF_8);
+                                                                                CharsetUtil.US_ASCII);
 
                                             try {
-                                                select(ctx, IDN.toASCII(hostname,
-                                                                        IDN.ALLOW_UNASSIGNED).toLowerCase(Locale.US));
+                                                select(ctx, hostname.toLowerCase(Locale.US));
                                             } catch (Throwable t) {
                                                 PlatformDependent.throwException(t);
                                             }


Test patch:
diff --git a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
index d49bdeaeebe..44cfbe9df93 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SniHandlerTest.java
@@ -160,38 +160,71 @@ public void testServerNameParsing() throws Exception {
             SniHandler handler = new SniHandler(mapping);
             EmbeddedChannel ch = new EmbeddedChannel(handler);
 
-            // hex dump of a client hello packet, which contains hostname "CHAT4。LEANCLOUD。CN"
-            String tlsHandshakeMessageHex1 = "16030100";
-            // part 2
-            String tlsHandshakeMessageHex = "bd010000b90303a74225676d1814ba57faff3b366" +
-                    "3656ed05ee9dbb2a4dbb1bb1c32d2ea5fc39e0000000100008c0000001700150000164348" +
-                    "415434E380824C45414E434C4F5544E38082434E000b000403000102000a00340032000e0" +
-                    "00d0019000b000c00180009000a0016001700080006000700140015000400050012001300" +
-                    "0100020003000f0010001100230000000d0020001e0601060206030501050205030401040" +
-                    "20403030103020303020102020203000f00010133740000";
-
             try {
-                // Push the handshake message.
-                // Decode should fail because SNI error
+                // hex dump of a client hello packet, which contains hostname "CHAT4.LEANCLOUD.CN"
+                String tlsHandshakeMessageHex1 = "16030100";
+                // part 2
+                String tlsHandshakeMessageHex = "c6010000c20303bb0855d66532c05a0ef784f7c384feeafa68b3" +
+                        "b655ac7288650d5eed4aa3fb52000038c02cc030009fcca9cca8ccaac02b" +
+                        "c02f009ec024c028006bc023c0270067c00ac0140039c009c0130033009d" +
+                        "009c003d003c0035002f00ff010000610000001700150000124348415434" +
+                        "2e4c45414e434c4f55442e434e000b000403000102000a000a0008001d00" +
+                        "170019001800230000000d0020001e060106020603050105020503040104" +
+                        "0204030301030203030201020202030016000000170000";
+
                 ch.writeInbound(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex1)));
                 ch.writeInbound(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex)));
-                fail();
-            } catch (DecoderException e) {
-                // expected
+
+                // This should produce an alert
+                assertTrue(ch.finish());
+
+                assertThat(handler.hostname(), is("chat4.leancloud.cn"));
+                assertThat(handler.sslContext(), is(leanContext));
+            } finally {
+                ch.finishAndReleaseAll();
             }
+        } finally {
+            releaseAll(leanContext, leanContext2, nettyContext);
+        }
+    }
 
-            // This should produce an alert
-            assertTrue(ch.finish());
+    @Test(expected = DecoderException.class)
+    public void testNonAsciiServerNameParsing() throws Exception {
+        SslContext nettyContext = makeSslContext(provider, false);
+        SslContext leanContext = makeSslContext(provider, false);
+        SslContext leanContext2 = makeSslContext(provider, false);
 
-            assertThat(handler.hostname(), is("chat4.leancloud.cn"));
-            assertThat(handler.sslContext(), is(leanContext));
+        try {
+            DomainNameMapping<SslContext> mapping = new DomainNameMappingBuilder<SslContext>(nettyContext)
+                    .add("*.netty.io", nettyContext)
+                    // input with custom cases
+                    .add("*.LEANCLOUD.CN", leanContext)
+                    // a hostname conflict with previous one, since we are using order-sensitive config,
+                    // the engine won't be used with the handler.
+                    .add("chat4.leancloud.cn", leanContext2)
+                    .build();
 
-            for (;;) {
-                Object msg = ch.readOutbound();
-                if (msg == null) {
-                    break;
-                }
-                ReferenceCountUtil.release(msg);
+            SniHandler handler = new SniHandler(mapping);
+            EmbeddedChannel ch = new EmbeddedChannel(handler);
+
+            try {
+                // hex dump of a client hello packet, which contains an invalid hostname "CHAT4。LEANCLOUD。CN"
+                String tlsHandshakeMessageHex1 = "16030100";
+                // part 2
+                String tlsHandshakeMessageHex = "bd010000b90303a74225676d1814ba57faff3b366" +
+                        "3656ed05ee9dbb2a4dbb1bb1c32d2ea5fc39e0000000100008c0000001700150000164348" +
+                        "415434E380824C45414E434C4F5544E38082434E000b000403000102000a00340032000e0" +
+                        "00d0019000b000c00180009000a0016001700080006000700140015000400050012001300" +
+                        "0100020003000f0010001100230000000d0020001e0601060206030501050205030401040" +
+                        "20403030103020303020102020203000f00010133740000";
+
+                // Push the handshake message.
+                // Decode should fail because of the badly encoded "HostName" string in the SNI extension
+                // that isn't ASCII as per RFC 6066 - https://tools.ietf.org/html/rfc6066#page-6
+                ch.writeInbound(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex1)));
+                ch.writeInbound(Unpooled.wrappedBuffer(DatatypeConverter.parseHexBinary(tlsHandshakeMessageHex)));
+            } finally {
+                ch.finishAndReleaseAll();
             }
         } finally {
             releaseAll(leanContext, leanContext2, nettyContext);