Repository: netty/netty
Issue: NPE in CipherSuiteConverter.toJava(CipherSuiteConverter.java:281)
Labels: defect

Providing repro shortly but it's triggered by an Unit Test for the OCSP Stapling PR.

Basically... OpenSSL (native) is calling my OCSP callback (java) which throws for the sake of the test an Exception. Instead of bubbling up the Exceptions it's running into an unhandled (?) NPE and my purposely thrown Exception gets swallowed somewhere.

Netty 4.1.9.Final-SNAPSHOT

The NPE's stack looks like this:

```java
io.netty.handler.codec.DecoderException: java.lang.NullPointerException
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:442)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:248)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1334)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:926)
	at io.netty.channel.local.LocalChannel.finishPeerRead0(LocalChannel.java:443)
	at io.netty.channel.local.LocalChannel.access$11(LocalChannel.java:424)
	at io.netty.channel.local.LocalChannel$5.run(LocalChannel.java:397)
	at io.netty.channel.DefaultEventLoop.run(DefaultEventLoop.java:54)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
	at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:144)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
	at io.netty.handler.ssl.CipherSuiteConverter.toJava(CipherSuiteConverter.java:281)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.toJavaCipherSuite(ReferenceCountedOpenSslEngine.java:1539)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.access$4(ReferenceCountedOpenSslEngine.java:1533)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine$OpenSslSession.handshakeFinished(ReferenceCountedOpenSslEngine.java:1862)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.handshake(ReferenceCountedOpenSslEngine.java:1499)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.wrap(ReferenceCountedOpenSslEngine.java:640)
	at javax.net.ssl.SSLEngine.wrap(SSLEngine.java:509)
	at io.netty.handler.ssl.SslHandler.wrap(SslHandler.java:836)
	at io.netty.handler.ssl.SslHandler.wrap(SslHandler.java:659)
	at io.netty.handler.ssl.SslHandler.wrapAndFlush(SslHandler.java:631)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1050)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:411)
	... 15 more
```

The throw stack looks like this (OcspTest.java:314 throws a IllegalStateException)

```java
java.lang.Exception: Stack trace
	at java.lang.Thread.dumpStack(Thread.java:1333)
	at io.netty.handler.ssl.ocsp.OcspTest$13.staple(OcspTest.java:314)
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext$ServerOcspCallback.callback(ReferenceCountedOpenSslContext.java:893)
	at io.netty.tcnative.jni.SSL.readFromSSL(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.readPlaintextData(ReferenceCountedOpenSslEngine.java:479)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:936)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1042)
	at io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1085)
	at io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:206)
	at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1123)
	at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1045)
	at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:411)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:248)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1334)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:926)
	at io.netty.channel.local.LocalChannel.finishPeerRead0(LocalChannel.java:443)
	at io.netty.channel.local.LocalChannel.access$11(LocalChannel.java:424)
	at io.netty.channel.local.LocalChannel$5.run(LocalChannel.java:397)
	at io.netty.channel.DefaultEventLoop.run(DefaultEventLoop.java:54)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
	at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:144)
	at java.lang.Thread.run(Thread.java:745)
```

Issue comments:
any idea what the parameter values are `static String toJava(String openSslCipherSuite, String protocol)`?
Will find out tomorrow morning. 
@rkapsi - Just realized you pushed your code so I can do it my self too :)

```
openSslCipherSuite: "(NONE)"
protocol: "TLS"
```

Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
index a3afeda3fe4..1737ad1e1d5 100644
--- a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
+++ b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
@@ -276,6 +276,11 @@ static String toJava(String openSslCipherSuite, String protocol) {
         Map<String, String> p2j = o2j.get(openSslCipherSuite);
         if (p2j == null) {
             p2j = cacheFromOpenSsl(openSslCipherSuite);
+            // This may happen if this method is queried when OpenSSL doesn't yet have a cipher setup. It will return
+            // "(NONE)" in this case.
+            if (p2j == null) {
+                return null;
+            }
         }
 
         String javaCipherSuite = p2j.get(protocol);


Test patch:
diff --git a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java
index 92e1aad49c6..11c6f2d2295 100644
--- a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java
@@ -20,8 +20,10 @@
 import io.netty.util.internal.logging.InternalLoggerFactory;
 import org.junit.Test;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 
 public class CipherSuiteConverterTest {
 
@@ -271,6 +273,34 @@ public void testCachedJ2OMappings() {
         testCachedJ2OMapping("TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", "ECDHE-ECDSA-AES128-SHA256");
     }
 
+    @Test
+    public void testUnknownOpenSSLCiphersToJava() {
+        testUnknownOpenSSLCiphersToJava("(NONE)");
+        testUnknownOpenSSLCiphersToJava("unknown");
+        testUnknownOpenSSLCiphersToJava("");
+    }
+
+    @Test
+    public void testUnknownJavaCiphersToOpenSSL() {
+        testUnknownJavaCiphersToOpenSSL("(NONE)");
+        testUnknownJavaCiphersToOpenSSL("unknown");
+        testUnknownJavaCiphersToOpenSSL("");
+    }
+
+    private void testUnknownOpenSSLCiphersToJava(String openSslCipherSuite) {
+        CipherSuiteConverter.clearCache();
+
+        assertNull(CipherSuiteConverter.toJava(openSslCipherSuite, "TLS"));
+        assertNull(CipherSuiteConverter.toJava(openSslCipherSuite, "SSL"));
+    }
+
+    private void testUnknownJavaCiphersToOpenSSL(String javaCipherSuite) {
+        CipherSuiteConverter.clearCache();
+
+        assertNull(CipherSuiteConverter.toOpenSsl(javaCipherSuite));
+        assertNull(CipherSuiteConverter.toOpenSsl(javaCipherSuite));
+    }
+
     private static void testCachedJ2OMapping(String javaCipherSuite, String openSslCipherSuite) {
         CipherSuiteConverter.clearCache();