From 2d55c121233fee797ef933cba29d31ab3ad305ba Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 9 Jul 2020 11:43:57 +0200 Subject: [PATCH] Polishing #1326 Rename option to handshakeTimeout. Add unit test. Original pull request: #1338 --- .../io/lettuce/core/SslConnectionBuilder.java | 8 +-- src/main/java/io/lettuce/core/SslOptions.java | 63 +++++++++---------- .../io/lettuce/core/SslOptionsUnitTests.java | 9 +++ 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/main/java/io/lettuce/core/SslConnectionBuilder.java b/src/main/java/io/lettuce/core/SslConnectionBuilder.java index 42cada3a28..03c916de96 100644 --- a/src/main/java/io/lettuce/core/SslConnectionBuilder.java +++ b/src/main/java/io/lettuce/core/SslConnectionBuilder.java @@ -56,6 +56,7 @@ * * @author Mark Paluch * @author Amin Mohtashami + * @author Felipe Ruiz */ public class SslConnectionBuilder extends ConnectionBuilder { @@ -181,10 +182,9 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception { } SslHandler sslHandler = new SslHandler(sslEngine, startTls); - Duration sslHandshakeTimeout = sslOptions.getSslHandshakeTimeout(); - if (sslHandshakeTimeout != null) { - sslHandler.setHandshakeTimeoutMillis(sslHandshakeTimeout.toMillis()); - } + Duration sslHandshakeTimeout = sslOptions.getHandshakeTimeout(); + sslHandler.setHandshakeTimeoutMillis(sslHandshakeTimeout.toMillis()); + channel.pipeline().addLast(sslHandler); if (channel.pipeline().get("channelActivator") == null) { diff --git a/src/main/java/io/lettuce/core/SslOptions.java b/src/main/java/io/lettuce/core/SslOptions.java index 006e85f284..dd957397aa 100644 --- a/src/main/java/io/lettuce/core/SslOptions.java +++ b/src/main/java/io/lettuce/core/SslOptions.java @@ -44,6 +44,7 @@ * * @author Mark Paluch * @author Amin Mohtashami + * @author Felipe Ruiz * @since 4.3 */ public class SslOptions { @@ -74,11 +75,12 @@ public class SslOptions { private final KeystoreAction trustmanager; - private final Duration sslHandshakeTimeout; + private final Duration handshakeTimeout; protected SslOptions(Builder builder) { this.keyStoreType = builder.keyStoreType; this.sslProvider = builder.sslProvider; + this.handshakeTimeout = builder.sslHandshakeTimeout; this.keystore = builder.keystore; this.keystorePassword = builder.keystorePassword; this.truststore = builder.truststore; @@ -91,12 +93,12 @@ protected SslOptions(Builder builder) { this.sslParametersSupplier = builder.sslParametersSupplier; this.keymanager = builder.keymanager; this.trustmanager = builder.trustmanager; - this.sslHandshakeTimeout = builder.sslHandshakeTimeout; } protected SslOptions(SslOptions original) { this.keyStoreType = original.keyStoreType; this.sslProvider = original.getSslProvider(); + this.handshakeTimeout = original.handshakeTimeout; this.keystore = original.keystore; this.keystorePassword = original.keystorePassword; this.truststore = original.getTruststore(); @@ -109,7 +111,6 @@ protected SslOptions(SslOptions original) { this.sslParametersSupplier = original.sslParametersSupplier; this.keymanager = original.keymanager; this.trustmanager = original.trustmanager; - this.sslHandshakeTimeout = original.sslHandshakeTimeout; } /** @@ -171,7 +172,7 @@ public static class Builder { private KeystoreAction trustmanager = KeystoreAction.NO_OP; - private Duration sslHandshakeTimeout = null; + private Duration sslHandshakeTimeout = Duration.ofSeconds(10); private Builder() { } @@ -225,6 +226,21 @@ private Builder sslProvider(SslProvider sslProvider) { return this; } + /** + * Sets a timeout for the SSL handshake. + * + * @param timeout {@link Duration}. + * @return {@code this} + * @since 5.3.2 + */ + public Builder handshakeTimeout(Duration timeout) { + + LettuceAssert.notNull(timeout, "SSL Handshake Timeout must not be null"); + + this.sslHandshakeTimeout = timeout; + return this; + } + /** * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set. * @@ -585,18 +601,6 @@ public Builder sslParameters(Supplier sslParametersSupplier) { return this; } - /** - * Sets a timeout for the SSL handshake. - * - * @param timeout {@link Duration}. - * @return {@code this} - */ - public Builder sslHandshakeTimeout(Duration timeout) { - - this.sslHandshakeTimeout = timeout; - return this; - } - /** * Create a new instance of {@link SslOptions} * @@ -681,13 +685,12 @@ public SslOptions.Builder mutate() { builder.sslParametersSupplier = this.sslParametersSupplier; builder.keymanager = this.keymanager; builder.trustmanager = this.trustmanager; - builder.sslHandshakeTimeout = this.sslHandshakeTimeout; + builder.sslHandshakeTimeout = this.handshakeTimeout; return builder; } /** - * * @return the configured {@link SslProvider}. */ @Deprecated @@ -696,7 +699,6 @@ public SslProvider getSslProvider() { } /** - * * @return the keystore {@link URL}. * @deprecated since 5.3, {@link javax.net.ssl.KeyManager} is configured via {@link #createSslContextBuilder()}. */ @@ -706,7 +708,6 @@ public URL getKeystore() { } /** - * * @return the set of protocols. */ public String[] getProtocols() { @@ -714,7 +715,6 @@ public String[] getProtocols() { } /** - * * @return the set of cipher suites. */ public String[] getCipherSuites() { @@ -722,7 +722,14 @@ public String[] getCipherSuites() { } /** - * + * @return the SSL handshake timeout + * @since 5.3.2 + */ + public Duration getHandshakeTimeout() { + return handshakeTimeout; + } + + /** * @return the password for the keystore. May be empty. * @deprecated since 5.3, {@link javax.net.ssl.KeyManager} is configured via {@link #createSslContextBuilder()}. */ @@ -732,7 +739,6 @@ public char[] getKeystorePassword() { } /** - * * @return the truststore {@link URL}. * @deprecated since 5.3, {@link javax.net.ssl.TrustManager} is configured via {@link #createSslContextBuilder()}. */ @@ -751,13 +757,6 @@ public char[] getTruststorePassword() { return Arrays.copyOf(truststorePassword, truststorePassword.length); } - /** - * @return the SSL handshake timeout - */ - public Duration getSslHandshakeTimeout() { - return sslHandshakeTimeout; - } - private static KeyManagerFactory createKeyManagerFactory(InputStream inputStream, char[] storePassword, String keyStoreType) throws GeneralSecurityException, IOException { @@ -827,7 +826,7 @@ public interface Resource { * @param url the URL to obtain the {@link InputStream} from. * @return a {@link Resource} that opens a connection to the URL and obtains the {@link InputStream} for it. */ - public static Resource from(URL url) { + static Resource from(URL url) { LettuceAssert.notNull(url, "URL must not be null"); @@ -840,7 +839,7 @@ public static Resource from(URL url) { * @param file the File to obtain the {@link InputStream} from. * @return a {@link Resource} that obtains the {@link FileInputStream} for the given {@link File}. */ - public static Resource from(File file) { + static Resource from(File file) { LettuceAssert.notNull(file, "File must not be null"); diff --git a/src/test/java/io/lettuce/core/SslOptionsUnitTests.java b/src/test/java/io/lettuce/core/SslOptionsUnitTests.java index bbbdd98cf9..23c5c06bd8 100644 --- a/src/test/java/io/lettuce/core/SslOptionsUnitTests.java +++ b/src/test/java/io/lettuce/core/SslOptionsUnitTests.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import java.util.Collections; import javax.net.ssl.SSLParameters; @@ -41,6 +42,14 @@ void shouldCreateEmptySslOptions() throws Exception { assertThat(options.createSslContextBuilder()).isNotNull(); } + @Test + void shouldConfigureSslHandshakeTimeout() { + + SslOptions options = SslOptions.builder().handshakeTimeout(Duration.ofSeconds(1)).build(); + + assertThat(options.getHandshakeTimeout()).isEqualTo(Duration.ofSeconds(1)); + } + @Test void shouldConfigureCipherSuiteAndProtocol() {