Change SslOptions to use null for defaults rather than empty sets

Update `SslOptions` so that `null` is used for default values rather
than empty sets. Most libraries use `null` to indicate defaults so
aligning our class makes things easier.

See gh-34814
This commit is contained in:
Phillip Webb 2023-04-21 16:18:44 -07:00
parent 77c468c956
commit c59c8cc674
8 changed files with 31 additions and 39 deletions

View File

@ -58,7 +58,6 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Lazy; import org.springframework.context.annotation.Lazy;
import org.springframework.context.annotation.Scope; import org.springframework.context.annotation.Scope;
import org.springframework.core.io.Resource; import org.springframework.core.io.Resource;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
/** /**
@ -159,8 +158,7 @@ public class CassandraAutoConfiguration {
private void configureSsl(CqlSessionBuilder builder, SslBundle sslBundle) { private void configureSsl(CqlSessionBuilder builder, SslBundle sslBundle) {
SslOptions options = sslBundle.getOptions(); SslOptions options = sslBundle.getOptions();
String[] ciphers = (!CollectionUtils.isEmpty(options.getCiphers()) ? null String[] ciphers = (options.getCiphers() != null) ? options.getCiphers().toArray(String[]::new) : null;
: options.getCiphers().toArray(String[]::new));
builder.withSslEngineFactory(new ProgrammaticSslEngineFactory(sslBundle.createSslContext(), ciphers)); builder.withSslEngineFactory(new ProgrammaticSslEngineFactory(sslBundle.createSslContext(), ciphers));
} }

View File

@ -33,15 +33,15 @@ import javax.net.ssl.SSLEngine;
public interface SslOptions { public interface SslOptions {
/** /**
* {@link SslOptions} that returns no values. * {@link SslOptions} that returns {@code null} results.
*/ */
SslOptions NONE = of(Collections.emptySet(), Collections.emptySet()); SslOptions NONE = of((Set<String>) null, (Set<String>) null);
/** /**
* Return the ciphers that can be used or an empty set. The cipher names in this set * Return the ciphers that can be used or an empty set. The cipher names in this set
* should be compatible with those supported by * should be compatible with those supported by
* {@link SSLEngine#getSupportedCipherSuites()}. * {@link SSLEngine#getSupportedCipherSuites()}.
* @return the ciphers that can be used * @return the ciphers that can be used or {@code null}
*/ */
Set<String> getCiphers(); Set<String> getCiphers();
@ -49,7 +49,7 @@ public interface SslOptions {
* Return the protocols that should be enabled or an empty set. The protocols names in * Return the protocols that should be enabled or an empty set. The protocols names in
* this set should be compatible with those supported by * this set should be compatible with those supported by
* {@link SSLEngine#getSupportedProtocols()}. * {@link SSLEngine#getSupportedProtocols()}.
* @return the protocols to enable * @return the protocols to enable or {@code null}
*/ */
Set<String> getEnabledProtocols(); Set<String> getEnabledProtocols();
@ -74,12 +74,12 @@ public interface SslOptions {
@Override @Override
public Set<String> getCiphers() { public Set<String> getCiphers() {
return (ciphers != null) ? ciphers : Collections.emptySet(); return ciphers;
} }
@Override @Override
public Set<String> getEnabledProtocols() { public Set<String> getEnabledProtocols() {
return (enabledProtocols != null) ? enabledProtocols : Collections.emptySet(); return enabledProtocols;
} }
}; };

View File

@ -22,6 +22,7 @@ import java.lang.reflect.Field;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.net.HttpURLConnection; import java.net.HttpURLConnection;
import java.time.Duration; import java.time.Duration;
import java.util.Set;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.function.Supplier; import java.util.function.Supplier;
@ -49,7 +50,6 @@ import org.springframework.http.client.OkHttp3ClientHttpRequestFactory;
import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.http.client.SimpleClientHttpRequestFactory;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils;
/** /**
@ -172,10 +172,8 @@ public final class ClientHttpRequestFactories {
} }
if (sslBundle != null) { if (sslBundle != null) {
SslOptions options = sslBundle.getOptions(); SslOptions options = sslBundle.getOptions();
String[] enabledProtocols = (!CollectionUtils.isEmpty(options.getEnabledProtocols())) String[] enabledProtocols = toArray(options.getEnabledProtocols());
? options.getEnabledProtocols().toArray(String[]::new) : null; String[] ciphers = toArray(options.getCiphers());
String[] ciphers = (!CollectionUtils.isEmpty(options.getCiphers()))
? options.getCiphers().toArray(String[]::new) : null;
SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslBundle.createSslContext(), SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslBundle.createSslContext(),
enabledProtocols, ciphers, new DefaultHostnameVerifier()); enabledProtocols, ciphers, new DefaultHostnameVerifier());
connectionManagerBuilder.setSSLSocketFactory(socketFactory); connectionManagerBuilder.setSSLSocketFactory(socketFactory);
@ -184,6 +182,10 @@ public final class ClientHttpRequestFactories {
return HttpClientBuilder.create().setConnectionManager(connectionManager).build(); return HttpClientBuilder.create().setConnectionManager(connectionManager).build();
} }
private static String[] toArray(Set<String> set) {
return (set != null) ? set.toArray(String[]::new) : null;
}
} }
/** /**

View File

@ -40,8 +40,6 @@ import org.springframework.boot.web.server.Http2;
import org.springframework.boot.web.server.Ssl.ClientAuth; import org.springframework.boot.web.server.Ssl.ClientAuth;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.ClassUtils; import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ObjectUtils;
/** /**
* {@link JettyServerCustomizer} that configures SSL on the given Jetty server instance. * {@link JettyServerCustomizer} that configures SSL on the given Jetty server instance.
@ -180,11 +178,11 @@ class SslServerCustomizer implements JettyServerCustomizer {
factory.setKeyStorePassword(stores.getKeyStorePassword()); factory.setKeyStorePassword(stores.getKeyStorePassword());
} }
factory.setCertAlias(key.getAlias()); factory.setCertAlias(key.getAlias());
if (!ObjectUtils.isEmpty(options.getCiphers())) { if (options.getCiphers() != null) {
factory.setIncludeCipherSuites(options.getCiphers().toArray(String[]::new)); factory.setIncludeCipherSuites(options.getCiphers().toArray(String[]::new));
factory.setExcludeCipherSuites(); factory.setExcludeCipherSuites();
} }
if (!CollectionUtils.isEmpty(options.getEnabledProtocols())) { if (options.getEnabledProtocols() != null) {
factory.setIncludeProtocols(options.getEnabledProtocols().toArray(String[]::new)); factory.setIncludeProtocols(options.getEnabledProtocols().toArray(String[]::new));
} }
try { try {

View File

@ -26,7 +26,6 @@ import org.springframework.boot.ssl.SslBundle;
import org.springframework.boot.ssl.SslOptions; import org.springframework.boot.ssl.SslOptions;
import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Http2;
import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.Ssl;
import org.springframework.util.CollectionUtils;
/** /**
* {@link NettyServerCustomizer} that configures SSL for the given Reactor Netty server * {@link NettyServerCustomizer} that configures SSL for the given Reactor Netty server
@ -66,12 +65,8 @@ public class SslServerCustomizer implements NettyServerCustomizer {
sslContextSpec.configure((builder) -> { sslContextSpec.configure((builder) -> {
builder.trustManager(this.sslBundle.getManagers().getTrustManagerFactory()); builder.trustManager(this.sslBundle.getManagers().getTrustManagerFactory());
SslOptions options = this.sslBundle.getOptions(); SslOptions options = this.sslBundle.getOptions();
if (!CollectionUtils.isEmpty(options.getEnabledProtocols())) { builder.protocols(options.getEnabledProtocols());
builder.protocols(options.getEnabledProtocols()); builder.ciphers(options.getCiphers());
}
if (!CollectionUtils.isEmpty(options.getCiphers())) {
builder.ciphers(options.getCiphers());
}
builder.clientAuth(org.springframework.boot.web.server.Ssl.ClientAuth.map(this.clientAuth, ClientAuth.NONE, builder.clientAuth(org.springframework.boot.web.server.Ssl.ClientAuth.map(this.clientAuth, ClientAuth.NONE,
ClientAuth.OPTIONAL, ClientAuth.REQUIRE)); ClientAuth.OPTIONAL, ClientAuth.REQUIRE));
}); });

View File

@ -30,7 +30,6 @@ import org.springframework.boot.ssl.SslOptions;
import org.springframework.boot.ssl.SslStoreBundle; import org.springframework.boot.ssl.SslStoreBundle;
import org.springframework.boot.web.server.Ssl.ClientAuth; import org.springframework.boot.web.server.Ssl.ClientAuth;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
/** /**
@ -86,7 +85,7 @@ class SslConnectorCustomizer implements TomcatConnectorCustomizer {
certificate.setCertificateKeyAlias(key.getAlias()); certificate.setCertificateKeyAlias(key.getAlias());
} }
sslHostConfig.addCertificate(certificate); sslHostConfig.addCertificate(certificate);
if (!CollectionUtils.isEmpty(options.getCiphers())) { if (options.getCiphers() != null) {
String ciphers = StringUtils.collectionToCommaDelimitedString(options.getCiphers()); String ciphers = StringUtils.collectionToCommaDelimitedString(options.getCiphers());
sslHostConfig.setCiphers(ciphers); sslHostConfig.setCiphers(ciphers);
} }
@ -96,9 +95,10 @@ class SslConnectorCustomizer implements TomcatConnectorCustomizer {
private void configureEnabledProtocols(AbstractHttp11JsseProtocol<?> protocol) { private void configureEnabledProtocols(AbstractHttp11JsseProtocol<?> protocol) {
SslOptions options = this.sslBundle.getOptions(); SslOptions options = this.sslBundle.getOptions();
if (!CollectionUtils.isEmpty(options.getEnabledProtocols())) { if (options.getEnabledProtocols() != null) {
String enabledProtocols = StringUtils.collectionToCommaDelimitedString(options.getEnabledProtocols());
for (SSLHostConfig sslHostConfig : protocol.findSslHostConfigs()) { for (SSLHostConfig sslHostConfig : protocol.findSslHostConfigs()) {
sslHostConfig.setProtocols(StringUtils.collectionToCommaDelimitedString(options.getEnabledProtocols())); sslHostConfig.setProtocols(enabledProtocols);
} }
} }
} }

View File

@ -28,7 +28,6 @@ import org.xnio.SslClientAuthMode;
import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslBundle;
import org.springframework.boot.ssl.SslOptions; import org.springframework.boot.ssl.SslOptions;
import org.springframework.boot.web.server.Ssl.ClientAuth; import org.springframework.boot.web.server.Ssl.ClientAuth;
import org.springframework.util.CollectionUtils;
/** /**
* {@link UndertowBuilderCustomizer} that configures SSL on the given builder instance. * {@link UndertowBuilderCustomizer} that configures SSL on the given builder instance.
@ -62,10 +61,10 @@ class SslBuilderCustomizer implements UndertowBuilderCustomizer {
builder.addHttpsListener(this.port, getListenAddress(), sslContext); builder.addHttpsListener(this.port, getListenAddress(), sslContext);
builder.setSocketOption(Options.SSL_CLIENT_AUTH_MODE, ClientAuth.map(this.clientAuth, builder.setSocketOption(Options.SSL_CLIENT_AUTH_MODE, ClientAuth.map(this.clientAuth,
SslClientAuthMode.NOT_REQUESTED, SslClientAuthMode.REQUESTED, SslClientAuthMode.REQUIRED)); SslClientAuthMode.NOT_REQUESTED, SslClientAuthMode.REQUESTED, SslClientAuthMode.REQUIRED));
if (!CollectionUtils.isEmpty(options.getEnabledProtocols())) { if (options.getEnabledProtocols() != null) {
builder.setSocketOption(Options.SSL_ENABLED_PROTOCOLS, Sequence.of(options.getEnabledProtocols())); builder.setSocketOption(Options.SSL_ENABLED_PROTOCOLS, Sequence.of(options.getEnabledProtocols()));
} }
if (!CollectionUtils.isEmpty(options.getCiphers())) { if (options.getCiphers() != null) {
builder.setSocketOption(Options.SSL_ENABLED_CIPHER_SUITES, Sequence.of(options.getCiphers())); builder.setSocketOption(Options.SSL_ENABLED_CIPHER_SUITES, Sequence.of(options.getCiphers()));
} }
} }

View File

@ -30,10 +30,10 @@ import static org.assertj.core.api.Assertions.assertThat;
class SslOptionsTests { class SslOptionsTests {
@Test @Test
void noneReturnsEmptyCollections() { void noneReturnsNull() {
SslOptions options = SslOptions.NONE; SslOptions options = SslOptions.NONE;
assertThat(options.getCiphers()).isEmpty(); assertThat(options.getCiphers()).isNull();
assertThat(options.getEnabledProtocols()).isEmpty(); assertThat(options.getEnabledProtocols()).isNull();
} }
@Test @Test
@ -50,8 +50,8 @@ class SslOptionsTests {
String[] ciphers = null; String[] ciphers = null;
String[] enabledProtocols = null; String[] enabledProtocols = null;
SslOptions options = SslOptions.of(ciphers, enabledProtocols); SslOptions options = SslOptions.of(ciphers, enabledProtocols);
assertThat(options.getCiphers()).isEmpty(); assertThat(options.getCiphers()).isNull();
assertThat(options.getEnabledProtocols()).isEmpty(); assertThat(options.getEnabledProtocols()).isNull();
} }
@Test @Test
@ -68,8 +68,8 @@ class SslOptionsTests {
Set<String> ciphers = null; Set<String> ciphers = null;
Set<String> enabledProtocols = null; Set<String> enabledProtocols = null;
SslOptions options = SslOptions.of(ciphers, enabledProtocols); SslOptions options = SslOptions.of(ciphers, enabledProtocols);
assertThat(options.getCiphers()).isEmpty(); assertThat(options.getCiphers()).isNull();
assertThat(options.getEnabledProtocols()).isEmpty(); assertThat(options.getEnabledProtocols()).isNull();
} }
} }