Merge pull request #1220 from werehuman/ssl-close-notify-to-tcp-fin

KostyaSha · web-flow · commit f760f0cd80e6 · 2019-07-18T18:07:41.000+03:00
Netty factory: Close TCP connection when receive TLS close notify
diff --git a/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java b/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java
@@ -15,6 +15,8 @@
 
 import com.github.dockerjava.core.AbstractDockerCmdExecFactory;
 import com.github.dockerjava.core.WebTarget;
+import io.netty.util.concurrent.Future;
+import io.netty.util.concurrent.GenericFutureListener;
 import org.apache.commons.lang.SystemUtils;
 
 import com.github.dockerjava.api.command.DockerCmdExecFactory;
@@ -232,12 +234,34 @@ public DuplexChannel connect(Bootstrap bootstrap) throws InterruptedException {
                 throw new RuntimeException("no port configured for " + host);
             }
 
-            DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel();
+            final DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel();
 
             final SslHandler ssl = initSsl(dockerClientConfig);
 
             if (ssl != null) {
                 channel.pipeline().addFirst(ssl);
+
+                // https://tools.ietf.org/html/rfc5246#section-7.2.1
+                // TLS has its own special message about connection termination. Because TLS is a
+                // session-level protocol, it can be covered by any transport-level protocol like
+                // TCP, UTP and so on. But we know exactly that data being transferred over TCP and
+                // that other side will never send any byte into this TCP connection, so this
+                // channel should be closed.
+                // RFC says that we must notify opposite side about closing. This could be done only
+                // in sun.security.ssl.SSLEngineImpl and unfortunately it does not send this
+                // message. On the other hand RFC does not enforce the opposite side to wait for
+                // such message.
+                ssl.sslCloseFuture().addListener(new GenericFutureListener<Future<? super Channel>>() {
+                    @Override
+                    public void operationComplete(Future<? super Channel> future) {
+                        channel.eventLoop().execute(new Runnable() {
+                            @Override
+                            public void run() {
+                                channel.close();
+                            }
+                        });
+                    }
+                });
             }
 
             return channel;
diff --git a/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java b/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java
@@ -13,11 +13,14 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.ByteArrayInputStream;
+import java.io.Closeable;
 import java.io.File;
 import java.io.InputStream;
 import java.io.PipedInputStream;
 import java.io.PipedOutputStream;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 
 import static com.github.dockerjava.junit.DockerRule.DEFAULT_IMAGE;
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -28,6 +31,7 @@
 import static org.hamcrest.Matchers.isEmptyString;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeThat;
 
 /**
@@ -205,6 +209,68 @@ public void onNext(Frame frame) {
         callback.close();
     }
 
+    /**
+     * {@link AttachContainerResultCallback#onComplete()} should be called immediately after
+     * container exit. It was broken for Netty and TLS connection.
+     */
+    @Test
+    public void attachContainerClosesStdoutWhenContainerExits() throws Exception {
+        DockerClient dockerClient = dockerRule.getClient();
+
+        CreateContainerResponse container = dockerClient.createContainerCmd(DEFAULT_IMAGE)
+                .withCmd("echo", "hello world")
+                .withTty(false)
+                .exec();
+        LOG.info("Created container: {}", container.toString());
+
+        final CountDownLatch started = new CountDownLatch(1);
+        final AtomicLong startedAtNanos = new AtomicLong();
+        final CountDownLatch gotLine = new CountDownLatch(1);
+        final CountDownLatch completed = new CountDownLatch(1);
+        final AtomicLong gotLineAtNanos = new AtomicLong();
+        AttachContainerTestCallback callback = new AttachContainerTestCallback() {
+            @Override
+            public void onStart(Closeable stream) {
+                startedAtNanos.set(System.nanoTime());
+                started.countDown();
+                super.onStart(stream);
+            }
+
+            @Override
+            public void onNext(Frame item) {
+                if (item.getStreamType() == StreamType.STDOUT) {
+                    gotLineAtNanos.set(System.nanoTime());
+                    gotLine.countDown();
+                }
+                super.onNext(item);
+            }
+
+            @Override
+            public void onComplete() {
+                completed.countDown();
+                super.onComplete();
+            }
+        };
+
+        try (Closeable ignored = callback) {
+            dockerClient.attachContainerCmd(container.getId())
+                    .withStdOut(true)
+                    .withFollowStream(true)
+                    .exec(callback);
+
+            dockerClient.startContainerCmd(container.getId()).exec();
+
+            assertTrue("Should start in a reasonable time", started.await(30, SECONDS));
+            assertTrue("Should get first line quickly after the start", gotLine.await(15, SECONDS));
+
+            long gotLineDurationSeconds = (gotLineAtNanos.get() - startedAtNanos.get()) / 1_000_000_000L;
+            LOG.info("Got line from {} for {} seconds", container.getId(), gotLineDurationSeconds);
+
+            boolean finished = completed.await(1L + gotLineDurationSeconds, SECONDS);
+            assertTrue("Should get EOF in a time close to time of getting the first line", finished);
+        }
+    }
+
     public static class AttachContainerTestCallback extends AttachContainerResultCallback {
         private StringBuffer log = new StringBuffer();
 
