Skip to content

Netty factory: Close TCP connection when receive TLS close notify #1220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KostyaSha merged 1 commit into from
Jul 18, 2019
Merged

Netty factory: Close TCP connection when receive TLS close notify #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
66 changes: 66 additions & 0 deletions src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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();

Expand Down