From b4fb54b63cbbceb4116c062f184848d9497dfff8 Mon Sep 17 00:00:00 2001 From: scottf Date: Sun, 23 Jul 2023 09:14:21 -0400 Subject: [PATCH 1/8] Improving options to allow creation of TLS and JWT based connections via properties file --- src/main/java/io/nats/client/Options.java | 600 ++++++++---------- .../java/io/nats/client/OptionsTests.java | 9 - 2 files changed, 271 insertions(+), 338 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index f6757f3a7..266618147 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -16,10 +16,7 @@ import io.nats.client.impl.DataPort; import io.nats.client.impl.ErrorListenerLoggerImpl; import io.nats.client.impl.SocketDataPort; -import io.nats.client.support.HttpRequest; -import io.nats.client.support.NatsConstants; -import io.nats.client.support.NatsUri; -import io.nats.client.support.SSLUtils; +import io.nats.client.support.*; import javax.net.ssl.SSLContext; import java.lang.reflect.Constructor; @@ -40,7 +37,7 @@ /** * The Options class specifies the connection options for a new NATs connection, including the default options. * Options are created using a {@link Options.Builder Builder}. - * This class, and the builder associated with it, is basically a long list of parameters. The documentation attempts + * This class and the builder associated with it, is basically a long list of parameters. The documentation attempts * to clarify the value of each parameter in place on the builder and here, but it may be easier to read the documentation * starting with the {@link Options.Builder Builder}, since it has a simple list of methods that configure the connection. */ @@ -49,7 +46,8 @@ public class Options { // NOTE TO DEVS!!! To add an option, you have to address: // ---------------------------------------------------------------------------------------------------- // CONSTANTS * optionally add a default value constant - // ENVIRONMENT * most of the time add an environment property + // ENVIRONMENT * most of the time add an environment property, should always be in the form PFX + + // PROTOCOL CONNECT OPTION CONSTANTS * not related to options, but here because Options code uses them // CLASS VARIABLES * add a variable to the class // BUILDER VARIABLES * add a variable in builder // BUILD CONSTRUCTOR PROPS * update build props constructor to read new props @@ -58,143 +56,117 @@ public class Options { // BUILDER COPY CONSTRUCTOR * update builder constructor to ensure new variables are set // CONSTRUCTOR * update constructor to ensure new variables are set from builder // GETTERS * update getter to be able to retrieve class variable value + // HELPER FUNCTIONS * just helpers // ---------------------------------------------------------------------------------------------------- // ---------------------------------------------------------------------------------------------------- // CONSTANTS // ---------------------------------------------------------------------------------------------------- /** - * Default server URL. - * - *

- * This property is defined as {@value} + * Default server URL. This property is defined as {@value} */ public static final String DEFAULT_URL = "nats://localhost:4222"; /** - * Default server port. - * - *

- * This property is defined as {@value} + * Default server port. This property is defined as {@value} */ public static final int DEFAULT_PORT = NatsConstants.DEFAULT_PORT; /** * Default maximum number of reconnect attempts, see {@link #getMaxReconnect() getMaxReconnect()}. - * - *

* This property is defined as {@value} */ public static final int DEFAULT_MAX_RECONNECT = 60; /** * Default wait time before attempting reconnection to the same server, see {@link #getReconnectWait() getReconnectWait()}. - * - *

* This property is defined as 2000 milliseconds (2 seconds). */ public static final Duration DEFAULT_RECONNECT_WAIT = Duration.ofMillis(2000); /** * Default wait time before attempting reconnection to the same server, see {@link #getReconnectJitter() getReconnectJitter()}. - * - *

* This property is defined as 100 milliseconds. */ public static final Duration DEFAULT_RECONNECT_JITTER = Duration.ofMillis(100); /** * Default wait time before attempting reconnection to the same server, see {@link #getReconnectJitterTls() getReconnectJitterTls()}. - * - *

* This property is defined as 1000 milliseconds (1 second). */ public static final Duration DEFAULT_RECONNECT_JITTER_TLS = Duration.ofMillis(1000); /** * Default connection timeout, see {@link #getConnectionTimeout() getConnectionTimeout()}. - * - *

* This property is defined as 2 seconds. */ public static final Duration DEFAULT_CONNECTION_TIMEOUT = Duration.ofSeconds(2); /** * Default server ping interval. The client will send a ping to the server on this interval to insure liveness. - * The server may send pings to the client as well, these are handled automatically by the library - * , see {@link #getPingInterval() getPingInterval()}. - * - *

A value of {@code <=0} means disabled. - * - *

This property is defined as 2 minutes. + * The server may send pings to the client as well, these are handled automatically by the library, + * see {@link #getPingInterval() getPingInterval()}. + *

A value of {@code <=0} means disabled.

+ *

This property is defined as 2 minutes.

*/ - public static final Duration DEFAULT_PING_INTERVAL = Duration.ofMinutes(2); /** * Default interval to clean up cancelled/timed out requests. * A timer is used to clean up futures that were handed out but never completed * via a message, {@link #getRequestCleanupInterval() getRequestCleanupInterval()}. - * - *

This property is defined as 5 seconds. + *

This property is defined as 5 seconds.

*/ public static final Duration DEFAULT_REQUEST_CLEANUP_INTERVAL = Duration.ofSeconds(5); /** - * Default maximum number of pings have not received a response allowed by the + * Default maximum number of pings have not received a response allowed by the * client, {@link #getMaxPingsOut() getMaxPingsOut()}. - * - *

This property is defined as {@value} + *

This property is defined as {@value}

*/ public static final int DEFAULT_MAX_PINGS_OUT = 2; /** * Default SSL protocol used to create an SSLContext if the {@link #PROP_SECURE * secure property} is used. - *

This property is defined as {@value} + *

This property is defined as {@value}

*/ public static final String DEFAULT_SSL_PROTOCOL = "TLSv1.2"; /** * Default of pending message buffer that is used for buffering messages that * are published during a disconnect/reconnect, {@link #getReconnectBufferSize() getReconnectBufferSize()}. - * - *

This property is defined as {@value} bytes, 8 * - * 1024 * 1024. + *

This property is defined as {@value} bytes, 8 * 1024 * 1024.

*/ public static final int DEFAULT_RECONNECT_BUF_SIZE = 8_388_608; /** * The default length, {@value} bytes, the client will allow in an * outgoing protocol control line, {@link #getMaxControlLine() getMaxControlLine()}. - * *

This value is configurable on the server, and should be set here to match.

*/ public static final int DEFAULT_MAX_CONTROL_LINE = 4096; /** * Default dataport class, which will use a TCP socket, {@link #getDataPortType() getDataPortType()}. - * - *

This option is currently provided only for testing, and experimentation, the default - * should be used in almost all cases. + *

This option is currently provided only for testing, and experimentation, the default + * should be used in almost all cases.

*/ public static final String DEFAULT_DATA_PORT_TYPE = SocketDataPort.class.getCanonicalName(); /** - * Default size for buffers in the connection, not as available as other settings, + * Default size for buffers in the connection, not as available as other settings, * this is primarily changed for testing, {@link #getBufferSize() getBufferSize()}. */ public static final int DEFAULT_BUFFER_SIZE = 64 * 1024; /** - * Default thread name prefix. Used by the default exectuor when creating threads. - * - *

+ * Default thread name prefix. Used by the default executor when creating threads. * This property is defined as {@value} */ public static final String DEFAULT_THREAD_NAME_PREFIX = "nats"; - + /** * Default prefix used for inboxes, you can change this to manage authorization of subjects. * See {@link #getInboxPrefix() getInboxPrefix()}, the . is required but will be added if missing. @@ -231,6 +203,7 @@ public class Options { // ENVIRONMENT // ---------------------------------------------------------------------------------------------------- static final String PFX = "io.nats.client."; + static final int PFX_LEN = PFX.length(); /** * Property used to configure a builder from a Properties object. {@value}, see @@ -311,17 +284,6 @@ public class Options { * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#noNoResponders() noNoResponders}. */ public static final String PROP_NO_NORESPONDERS = PFX + "nonoresponders"; - /** - * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. - * @deprecated Client Side Limit checks are no longer performed. - */ - @Deprecated - public static final String PROP_CLIENT_SIDE_LIMIT_CHECKS = PFX + "clientsidelimitchecks"; - /** - * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#connectionName(String) - * connectionName}. - */ - public static final String PROP_CONNECTION_NAME = PFX + "name"; /** * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#noRandomize() noRandomize}. */ @@ -366,7 +328,7 @@ public class Options { public static final String PROP_SECURE = PFX + "secure"; /** - * Property used to configure a builder from a Properties object. + * Property used to configure a builder from a Properties object. * {@value}, see {@link Builder#sslContext(SSLContext) sslContext}. * This property is a boolean flag, but it tells the options parser to use * an SSL context that takes any server TLS certificate and does not provide @@ -394,19 +356,35 @@ public class Options { * maxControlLine}. */ public static final String PROP_MAX_CONTROL_LINE = "max.control.line"; - /** - * @deprecated Plans are to remove allowing utf8mode - * This property is used to enable support for UTF8 subjects. See {@link Builder#supportUTF8Subjects() supportUTF8Subjcts()} + * Property used to set the inbox prefix + */ + public static final String PROP_INBOX_PREFIX = "inbox.prefix"; + /** + * Property used to set whether to ignore discovered servers when connecting + */ + public static final String PROP_IGNORE_DISCOVERED_SERVERS = "ignore_discovered_servers"; + /** + * Property used to set class name for ServerPool implementation + * {@link Builder#serverPool(ServerPool) serverPool}. + */ + public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS = "servers_pool_implementation_class"; + /** + * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. + * @deprecated Client Side Limit checks are no longer performed. */ @Deprecated - public static final String PROP_UTF8_SUBJECTS = "allow.utf8.subjects"; - + public static final String PROP_CONNECTION_NAME = PFX + "name"; /** - * Property used to set the inbox prefix + * This property is used to enable support for UTF8 subjects. See {@link Builder#supportUTF8Subjects() supportUTF8Subjcts()} + * @deprecated only plain ascii subjects are supported */ - public static final String PROP_INBOX_PREFIX = "inbox.prefix"; + @Deprecated + public static final String PROP_UTF8_SUBJECTS = "allow.utf8.subjects"; + // ---------------------------------------------------------------------------------------------------- + // PROTOCOL CONNECT OPTION CONSTANTS + // ---------------------------------------------------------------------------------------------------- /** * Protocol key {@value}, see {@link Builder#verbose() verbose}. */ @@ -480,7 +458,7 @@ public class Options { static final String OPTION_SIG = "sig"; /** - * JWT key {@value #OPTION_SIG}, the user JWT to send to the server. + * JWT key {@value}, the user JWT to send to the server. */ static final String OPTION_JWT = "jwt"; @@ -494,18 +472,6 @@ public class Options { */ static final String OPTION_NORESPONDERS = "no_responders"; - /** - * Property used to set whether to ignore discovered servers when connecting - */ - public static final String PROP_IGNORE_DISCOVERED_SERVERS = "ignore_discovered_servers"; - - /** - * Property used to set class name for ServerPool implementation - * {@link Builder#serverPool(ServerPool) serverPool}. - * IMPORTANT! ServerPool IS CURRENTLY EXPERIMENTAL AND SUBJECT TO CHANGE. - */ - public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS = "servers_pool_implementation_class"; - // ---------------------------------------------------------------------------------------------------- // CLASS VARIABLES // ---------------------------------------------------------------------------------------------------- @@ -579,6 +545,16 @@ public Thread newThread(Runnable r) { } } + /** + * Set old request style. + * @param value true to use the old request style + * @deprecated Use Builder + */ + @Deprecated + public void setOldRequestStyle(boolean value) { + useOldRequestStyle = value; + } + // ---------------------------------------------------------------------------------------------------- // BUILDER // ---------------------------------------------------------------------------------------------------- @@ -650,25 +626,22 @@ public static class Builder { /** * Constructs a new Builder with the default values. - * - *

One tiny clarification is that the builder doesn't have a server url. When {@link #build() build()} - * is called on a default builder it will add the {@link Options#DEFAULT_URL - * default url} to its list of servers before creating the options object. + *

When {@link #build() build()} is called on a default builder it will add the {@link Options#DEFAULT_URL + * default url} to its list of servers if there were no servers defined.

*/ - public Builder() { - } + public Builder() {} // ---------------------------------------------------------------------------------------------------- // BUILD CONSTRUCTOR PROPS // ---------------------------------------------------------------------------------------------------- /** * Constructs a new {@code Builder} from a {@link Properties} object. - * + * *

If {@link Options#PROP_SECURE PROP_SECURE} is set, the builder will * try to get the default context{@link SSLContext#getDefault() getDefault()}. - * If a context can't be found, no context is set and an IllegalArgumentException is thrown. - * - *

Methods called on the builder after construction can override the properties. + * If a context can't be found, no context is set and an IllegalArgumentException is thrown.

+ * + *

Methods called on the builder after construction can override the properties.

* * @param props the {@link Properties} object */ @@ -677,200 +650,71 @@ public Builder(Properties props) throws IllegalArgumentException { throw new IllegalArgumentException("Properties cannot be null"); } - if (props.containsKey(PROP_URL)) { - this.server(props.getProperty(PROP_URL, DEFAULT_URL)); - } - - if (props.containsKey(PROP_USERNAME)) { - this.username = props.getProperty(PROP_USERNAME, null).toCharArray(); - } + stringProperty(props, PROP_URL, this::server); + charArrayProperty(props, PROP_USERNAME, ca -> this.username = ca); + charArrayProperty(props, PROP_PASSWORD, ca -> this.password = ca); + charArrayProperty(props, PROP_TOKEN, ca -> this.token = ca); - if (props.containsKey(PROP_PASSWORD)) { - this.password = props.getProperty(PROP_PASSWORD, null).toCharArray(); - } + stringProperty(props, PROP_SERVERS, str -> { + String[] servers = str.trim().split(",\\s*"); + this.servers(servers); + }); - if (props.containsKey(PROP_TOKEN)) { - this.token = props.getProperty(PROP_TOKEN, null).toCharArray(); - } + booleanIfTrueProperty(props, PROP_NORANDOMIZE, alwaysTrue -> this.noRandomize = true); + booleanIfTrueProperty(props, PROP_NO_RESOLVE_HOSTNAMES, alwaysTrue -> this.noResolveHostnames = true); + booleanIfTrueProperty(props, PROP_REPORT_NO_RESPONDERS, alwaysTrue -> this.reportNoResponders = true); - if (props.containsKey(PROP_SERVERS)) { - String str = props.getProperty(PROP_SERVERS); - if (str.isEmpty()) { - throw new IllegalArgumentException(PROP_SERVERS + " cannot be empty"); - } else { - String[] servers = str.trim().split(",\\s*"); - this.servers(servers); + booleanIfTrueProperty(props, PROP_SECURE, alwaysTrue -> { + try { + this.sslContext = SSLContext.getDefault(); } - } - - if (props.containsKey(PROP_NORANDOMIZE)) { - this.noRandomize = Boolean.parseBoolean(props.getProperty(PROP_NORANDOMIZE)); - } - - if (props.containsKey(PROP_NO_RESOLVE_HOSTNAMES)) { - noResolveHostnames = Boolean.parseBoolean(props.getProperty(PROP_NO_RESOLVE_HOSTNAMES)); - } - - if (props.containsKey(PROP_REPORT_NO_RESPONDERS)) { - reportNoResponders = Boolean.parseBoolean(props.getProperty(PROP_REPORT_NO_RESPONDERS)); - } - - if (props.containsKey(PROP_SECURE)) { - boolean secure = Boolean.parseBoolean(props.getProperty(PROP_SECURE)); - - if (secure) { - try { - this.sslContext = SSLContext.getDefault(); - } catch (NoSuchAlgorithmException e) { - this.sslContext = null; - throw new IllegalArgumentException("Unable to retrieve default SSL context"); - } + catch (NoSuchAlgorithmException e) { + this.sslContext = null; + throw new IllegalArgumentException("Unable to retrieve default SSL context"); } - } - - if (props.containsKey(PROP_OPENTLS)) { - boolean tls = Boolean.parseBoolean(props.getProperty(PROP_OPENTLS)); + }); - if (tls) { - try { - this.sslContext = SSLUtils.createOpenTLSContext(); - } catch (Exception e) { - this.sslContext = null; - throw new IllegalArgumentException("Unable to create open SSL context"); - } + booleanIfTrueProperty(props, PROP_OPENTLS, alwaysTrue -> { + try { + this.sslContext = SSLUtils.createOpenTLSContext(); } - } - - if (props.containsKey(PROP_CONNECTION_NAME)) { - this.connectionName = props.getProperty(PROP_CONNECTION_NAME, null); - } - - if (props.containsKey(PROP_VERBOSE)) { - this.verbose = Boolean.parseBoolean(props.getProperty(PROP_VERBOSE)); - } - - if (props.containsKey(PROP_NO_ECHO)) { - this.noEcho = Boolean.parseBoolean(props.getProperty(PROP_NO_ECHO)); - } - - if (props.containsKey(PROP_NO_HEADERS)) { - this.noHeaders = Boolean.parseBoolean(props.getProperty(PROP_NO_HEADERS)); - } - - if (props.containsKey(PROP_NO_NORESPONDERS)) { - this.noNoResponders = Boolean.parseBoolean(props.getProperty(PROP_NO_NORESPONDERS)); - } - - if (props.containsKey(PROP_UTF8_SUBJECTS)) { - this.utf8Support = Boolean.parseBoolean(props.getProperty(PROP_UTF8_SUBJECTS)); - } - - if (props.containsKey(PROP_PEDANTIC)) { - this.pedantic = Boolean.parseBoolean(props.getProperty(PROP_PEDANTIC)); - } - - if (props.containsKey(PROP_MAX_RECONNECT)) { - this.maxReconnect = Integer - .parseInt(props.getProperty(PROP_MAX_RECONNECT, Integer.toString(DEFAULT_MAX_RECONNECT))); - } - - if (props.containsKey(PROP_RECONNECT_WAIT)) { - int ms = Integer.parseInt(props.getProperty(PROP_RECONNECT_WAIT, "-1")); - this.reconnectWait = (ms < 0) ? DEFAULT_RECONNECT_WAIT : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_RECONNECT_JITTER)) { - int ms = Integer.parseInt(props.getProperty(PROP_RECONNECT_JITTER, "-1")); - this.reconnectJitter = (ms < 0) ? DEFAULT_RECONNECT_JITTER : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_RECONNECT_JITTER_TLS)) { - int ms = Integer.parseInt(props.getProperty(PROP_RECONNECT_JITTER_TLS, "-1")); - this.reconnectJitterTls = (ms < 0) ? DEFAULT_RECONNECT_JITTER_TLS : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_RECONNECT_BUF_SIZE)) { - this.reconnectBufferSize = Long.parseLong( - props.getProperty(PROP_RECONNECT_BUF_SIZE, Long.toString(DEFAULT_RECONNECT_BUF_SIZE))); - } - - if (props.containsKey(PROP_CONNECTION_TIMEOUT)) { - int ms = Integer.parseInt(props.getProperty(PROP_CONNECTION_TIMEOUT, "-1")); - this.connectionTimeout = (ms < 0) ? DEFAULT_CONNECTION_TIMEOUT : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_MAX_CONTROL_LINE)) { - int bytes = Integer.parseInt(props.getProperty(PROP_MAX_CONTROL_LINE, "-1")); - this.maxControlLine = (bytes < 0) ? DEFAULT_MAX_CONTROL_LINE : bytes; - } - - if (props.containsKey(PROP_PING_INTERVAL)) { - int ms = Integer.parseInt(props.getProperty(PROP_PING_INTERVAL, "-1")); - this.pingInterval = (ms < 0) ? DEFAULT_PING_INTERVAL : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_CLEANUP_INTERVAL)) { - int ms = Integer.parseInt(props.getProperty(PROP_CLEANUP_INTERVAL, "-1")); - this.requestCleanupInterval = (ms < 0) ? DEFAULT_REQUEST_CLEANUP_INTERVAL : Duration.ofMillis(ms); - } - - if (props.containsKey(PROP_MAX_PINGS)) { - this.maxPingsOut = Integer - .parseInt(props.getProperty(PROP_MAX_PINGS, Integer.toString(DEFAULT_MAX_PINGS_OUT))); - } - - if (props.containsKey(PROP_USE_OLD_REQUEST_STYLE)) { - this.useOldRequestStyle = Boolean.parseBoolean(props.getProperty(PROP_USE_OLD_REQUEST_STYLE)); - } - - if (props.containsKey(PROP_ERROR_LISTENER)) { - Object instance = createInstanceOf(props.getProperty(PROP_ERROR_LISTENER)); - this.errorListener = (ErrorListener) instance; - } - - if (props.containsKey(PROP_CONNECTION_CB)) { - Object instance = createInstanceOf(props.getProperty(PROP_CONNECTION_CB)); - this.connectionListener = (ConnectionListener) instance; - } - - if (props.containsKey(PROP_DATA_PORT_TYPE)) { - this.dataPortType = props.getProperty(PROP_DATA_PORT_TYPE); - } - - if (props.containsKey(PROP_INBOX_PREFIX)) { - this.inboxPrefix(props.getProperty(PROP_INBOX_PREFIX, DEFAULT_INBOX_PREFIX)); - } - - if (props.containsKey(PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE)) { - int maxMessagesInOutgoingQueue = Integer.parseInt(props.getProperty(PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, "-1")); - this.maxMessagesInOutgoingQueue = (maxMessagesInOutgoingQueue < 0) ? DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE : maxMessagesInOutgoingQueue; - } - - if (props.containsKey(PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL)) { - this.discardMessagesWhenOutgoingQueueFull = Boolean.parseBoolean(props.getProperty( - PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, Boolean.toString(DEFAULT_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL))); - } - - if (props.containsKey(PROP_IGNORE_DISCOVERED_SERVERS)) { - this.ignoreDiscoveredServers = Boolean.parseBoolean(props.getProperty(PROP_IGNORE_DISCOVERED_SERVERS)); - } - - if (props.containsKey(PROP_SERVERS_POOL_IMPLEMENTATION_CLASS)) { - Object instance = createInstanceOf(props.getProperty(PROP_SERVERS_POOL_IMPLEMENTATION_CLASS)); - this.serverPool = (ServerPool) instance; - } - } - - static Object createInstanceOf(String className) { - Object instance; - try { - Class clazz = Class.forName(className); - Constructor constructor = clazz.getConstructor(); - instance = constructor.newInstance(); - } catch (Exception e) { - throw new IllegalArgumentException(e); - } - return instance; + catch (Exception e) { + this.sslContext = null; + throw new IllegalArgumentException("Unable to create open SSL context"); + } + }); + + stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); + booleanIfTrueProperty(props, PROP_VERBOSE, alwaysTrue -> this.verbose = true); + booleanIfTrueProperty(props, PROP_NO_ECHO, alwaysTrue -> this.noEcho = true); + booleanIfTrueProperty(props, PROP_NO_HEADERS, alwaysTrue -> this.noHeaders = true); + booleanIfTrueProperty(props, PROP_NO_NORESPONDERS, alwaysTrue -> this.noNoResponders = true); + booleanIfTrueProperty(props, PROP_UTF8_SUBJECTS, alwaysTrue -> this.utf8Support = true); + booleanIfTrueProperty(props, PROP_PEDANTIC, alwaysTrue -> this.pedantic = true); + + intProperty(props, PROP_MAX_RECONNECT, DEFAULT_MAX_RECONNECT, i -> this.maxReconnect = i); + durationProperty(props, PROP_RECONNECT_WAIT, DEFAULT_RECONNECT_WAIT, d -> this.reconnectWait = d); + durationProperty(props, PROP_RECONNECT_JITTER, DEFAULT_RECONNECT_JITTER, d -> this.reconnectJitter = d); + durationProperty(props, PROP_RECONNECT_JITTER_TLS, DEFAULT_RECONNECT_JITTER_TLS, d -> this.reconnectJitterTls = d); + longProperty(props, PROP_RECONNECT_BUF_SIZE, DEFAULT_RECONNECT_BUF_SIZE, l -> this.reconnectBufferSize = l); + durationProperty(props, PROP_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT, d -> this.connectionTimeout = d); + + intGtEqZeroProperty(props, PROP_MAX_CONTROL_LINE, DEFAULT_MAX_CONTROL_LINE, i -> this.maxControlLine = i); + durationProperty(props, PROP_PING_INTERVAL, DEFAULT_PING_INTERVAL, d -> this.pingInterval = d); + durationProperty(props, PROP_CLEANUP_INTERVAL, DEFAULT_REQUEST_CLEANUP_INTERVAL, d -> this.requestCleanupInterval = d); + intProperty(props, PROP_MAX_PINGS, DEFAULT_MAX_PINGS_OUT, i -> this.maxPingsOut = i); + booleanIfTrueProperty(props, PROP_USE_OLD_REQUEST_STYLE, alwaysTrue -> this.useOldRequestStyle = true); + + classnameProperty(props, PROP_ERROR_LISTENER, o -> this.errorListener = (ErrorListener) o); + classnameProperty(props, PROP_CONNECTION_CB, o -> this.connectionListener = (ConnectionListener) o); + + stringProperty(props, PROP_DATA_PORT_TYPE, s -> this.dataPortType = s); + stringProperty(props, PROP_INBOX_PREFIX, this::inboxPrefix); + intGtEqZeroProperty(props, PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, i -> this.maxMessagesInOutgoingQueue = i); + booleanIfTrueProperty(props, PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, alwaysTrue -> this.discardMessagesWhenOutgoingQueueFull = true); + + booleanIfTrueProperty(props, PROP_IGNORE_DISCOVERED_SERVERS, alwaysTrue -> this.ignoreDiscoveredServers = true); + classnameProperty(props, PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, o -> this.serverPool = (ServerPool) o); } // ---------------------------------------------------------------------------------------------------- @@ -878,7 +722,7 @@ static Object createInstanceOf(String className) { // ---------------------------------------------------------------------------------------------------- /** * Add a server to the list of known servers. - * + * * @param serverURL the URL for the server to add * @throws IllegalArgumentException if the url is not formatted correctly. * @return the Builder for chaining @@ -889,7 +733,7 @@ public Builder server(String serverURL) { /** * Add an array of servers to the list of known servers. - * + * * @param servers A list of server URIs * @throws IllegalArgumentException if any url is not formatted correctly. * @return the Builder for chaining @@ -991,7 +835,7 @@ public Builder clientSideLimitChecks(boolean checks) { } /** - * The client protocol is not clear about the encoding for subject names. For + * The client protocol is not clear about the encoding for subject names. For * performance reasons, the Java client defaults to ASCII. You can enable UTF8 * with this method. The server, written in go, treats byte to string as UTF8 by default * and should allow UTF8 subjects, but make sure to test any clients when using them. @@ -1006,7 +850,7 @@ public Builder supportUTF8Subjects() { /** * Set the connection's optional Name. - * + * * @param name the connections new name. * @return the Builder for chaining */ @@ -1017,7 +861,7 @@ public Builder connectionName(String name) { /** * Set the connection's inbox prefix. All inboxes will start with this string. - * + * * @param prefix prefix to use. * @return the Builder for chaining */ @@ -1070,7 +914,7 @@ public Builder traceConnection() { /** * Sets the options to use the default SSL Context, if it exists. - * + * * @throws NoSuchAlgorithmException If the default protocol is unavailable. * @throws IllegalArgumentException If there is no default SSL context. * @return the Builder for chaining @@ -1086,7 +930,7 @@ public Builder secure() throws NoSuchAlgorithmException, IllegalArgumentExceptio /** * Set the SSL context to one that accepts any server certificate and has no client certificates. - * + * * @throws NoSuchAlgorithmException If the tls protocol is unavailable. * @return the Builder for chaining */ @@ -1098,7 +942,7 @@ public Builder opentls() throws NoSuchAlgorithmException { /** * Set the SSL context, requires that the server supports TLS connections and * the URI specifies TLS. - * + * * @param ctx the SSL Context to use for TLS connections * @return the Builder for chaining */ @@ -1119,18 +963,18 @@ public Builder noReconnect() { /** * Set the maximum number of reconnect attempts. Use 0 to turn off * auto-reconnect. Use -1 to turn on infinite reconnects. - * + * *

The reconnect count is incremented on a per-server basis, so if the server list contains 5 servers - * but max reconnects is set to 3, only 3 of those servers will be tried. - * + * but max reconnects is set to 3, only 3 of those servers will be tried.

+ * *

This library has a slight difference from some NATS clients, if you set the maxReconnects to zero - * there will not be any reconnect attempts, regardless of the number of known servers. - * + * there will not be any reconnect attempts, regardless of the number of known servers.

+ * *

The reconnect state is entered when the connection is connected and loses * that connection. During the initial connection attempt, the client will cycle over * its server list one time, regardless of what maxReconnects is set to. The only exception - * to this is the experimental async connect method {@link Nats#connectAsynchronously(Options, boolean) connectAsynchronously}. - * + * to this is the experimental async connect method {@link Nats#connectAsynchronously(Options, boolean) connectAsynchronously}.

+ * * @param max the maximum reconnect attempts * @return the Builder for chaining */ @@ -1183,7 +1027,7 @@ public Builder reconnectJitterTls(Duration time) { * Set the maximum length of a control line sent by this connection. This value is also configured * in the server but the protocol doesn't currently forward that setting. Configure it here so that * the client can ensure that messages are valid before sending to the server. - * + * * @param bytes the max byte count * @return the Builder for chaining */ @@ -1195,7 +1039,7 @@ public Builder maxControlLine(int bytes) { /** * Set the timeout for connection attempts. Each server in the options is allowed this timeout * so if 3 servers are tried with a timeout of 5s the total time could be 15s. - * + * * @param time the time to wait * @return the Builder for chaining */ @@ -1214,7 +1058,7 @@ public Builder connectionTimeout(Duration time) { * force a disconnect/reconnect which can result in messages being held back or failed. In general, * the ping interval should be set in seconds but this value is not enforced as it would result in * an API change from the 2.0 release. - * + * * @param time the time between client to server pings * @return the Builder for chaining */ @@ -1226,10 +1070,10 @@ public Builder pingInterval(Duration time) { /** * Set the interval between cleaning passes on outstanding request futures that are cancelled or timeout * in the application code. - * + * *

The default value is probably reasonable, but this interval is useful in a very noisy network * situation where lots of requests are used. - * + * * @param time the cleaning interval * @return the Builder for chaining */ @@ -1240,7 +1084,7 @@ public Builder requestCleanupInterval(Duration time) { /** * Set the maximum number of pings the client can have in flight. - * + * * @param max the max pings * @return the Builder for chaining */ @@ -1266,7 +1110,7 @@ public Builder bufferSize(int size) { * A value of zero will disable the reconnect buffer, a value less than zero means unlimited. Caution * should be used for negative numbers as they can result in an unreliable network connection plus a * high message rate leading to an out of memory error. - * + * * @param size the size in bytes * @return the Builder for chaining */ @@ -1295,7 +1139,7 @@ public Builder userInfo(String userName, String password) { * Set the username and password for basic authentication. * If the user and password are set in the server URL, they will override these values. However, in a clustering situation, * these values can be used as a fallback. - * + * * @param userName a non-empty userName * @param password the password, in plain text * @return the Builder for chaining @@ -1309,7 +1153,7 @@ public Builder userInfo(char[] userName, char[] password) { /** * Set the token for token-based authentication. * If a token is provided in a server URI it overrides this value. - * + * * @param token The token * @return the Builder for chaining * @deprecated use the char[] version instead for better security @@ -1323,7 +1167,7 @@ public Builder token(String token) { /** * Set the token for token-based authentication. * If a token is provided in a server URI it overrides this value. - * + * * @param token The token * @return the Builder for chaining */ @@ -1333,7 +1177,7 @@ public Builder token(char[] token) { } /** - * Set the {@link AuthHandler AuthHandler} to sign the server nonce for authentication in + * Set the {@link AuthHandler AuthHandler} to sign the server nonce for authentication in * nonce-mode. * * @param handler The new AuthHandler for this connection. @@ -1358,7 +1202,7 @@ public Builder reconnectDelayHandler(ReconnectDelayHandler handler) { /** * Set the {@link ErrorListener ErrorListener} to receive asynchronous error events related to this * connection. - * + * * @param listener The new ErrorListener for this connection. * @return the Builder for chaining */ @@ -1370,7 +1214,7 @@ public Builder errorListener(ErrorListener listener) { /** * Set the {@link ConnectionListener ConnectionListener} to receive asynchronous notifications of disconnect * events. - * + * * @param listener The new ConnectionListener for this type of event. * @return the Builder for chaining */ @@ -1387,7 +1231,7 @@ public Builder connectionListener(ConnectionListener listener) { * since most threads from the executor are long-lived. If you customize, be sure to keep the shutdown * effect in mind, executors can block for their keepalive time. The default executor also marks threads * with priority normal and as non-daemon. - * + * * @param executor The ExecutorService to use for connections built with these options. * @return the Builder for chaining */ @@ -1435,7 +1279,7 @@ public Builder proxy(Proxy proxy) { /** * The class to use for this connections data port. This is an advanced setting * and primarily useful for testing. - * + * * @param dataPortClassName a valid and accessible class name * @return the Builder for chaining */ @@ -1476,7 +1320,6 @@ public Builder ignoreDiscoveredServers() { /** * Set the ServerPool implementation for connections to use instead of the default bahvior - * IMPORTANT! ServerPool IS CURRENTLY EXPERIMENTAL AND SUBJECT TO CHANGE. * @param serverPool the implementation * @return the Builder for chaining */ @@ -1487,7 +1330,7 @@ public Builder serverPool(ServerPool serverPool) { /** * Build an Options object from this Builder. - * + * *

If the Options builder was not provided with a server, a default one will be included * {@link Options#DEFAULT_URL}. If only a single server URI is included, the builder * will try a few things to make connecting easier: @@ -1499,7 +1342,7 @@ public Builder serverPool(ServerPool serverPool) { * that does not check the servers certificate for validity. This is not secure and only provided * for tests and development. * - * + * * @return the new options object * @throws IllegalStateException if there is a conflict in the options, like a token and a user/pass */ @@ -1510,7 +1353,11 @@ public Options build() throws IllegalStateException { if (this.username != null && this.token != null) { throw new IllegalStateException("Options can't have token and username"); } - + + if (inboxPrefix == null) { + inboxPrefix = DEFAULT_INBOX_PREFIX; + } + if (natsServerUris.size() == 0) { server(DEFAULT_URL); } @@ -1536,9 +1383,9 @@ else if (sslContext == null) { // see if we need to provide one if (this.executor == null) { String threadPrefix = (this.connectionName != null && this.connectionName.length() > 0) ? this.connectionName : DEFAULT_THREAD_NAME_PREFIX; this.executor = new ThreadPoolExecutor(0, Integer.MAX_VALUE, - 500L, TimeUnit.MILLISECONDS, - new SynchronousQueue<>(), - new DefaultThreadFactory(threadPrefix)); + 500L, TimeUnit.MILLISECONDS, + new SynchronousQueue<>(), + new DefaultThreadFactory(threadPrefix)); } return new Options(this); } @@ -1724,7 +1571,7 @@ public String getDataPortType() { * @return the data port described by these options */ public DataPort buildDataPort() { - return (DataPort) Options.Builder.createInstanceOf(dataPortType); + return (DataPort) Options.createInstanceOf(dataPortType); } /** @@ -1855,7 +1702,7 @@ public int getMaxControlLine() { } /** - * + * * @return true if there is an sslContext for this Options, otherwise false, see {@link Builder#secure() secure()} in the builder doc */ public boolean isTLSRequired() { @@ -2028,7 +1875,6 @@ public boolean isIgnoreDiscoveredServers() { /** * Get a provided ServerPool. If null, a default implementation is used. - * IMPORTANT! ServerPool IS CURRENTLY EXPERIMENTAL AND SUBJECT TO CHANGE. * @return the ServerPool implementation */ public ServerPool getServerPool() { @@ -2094,7 +1940,7 @@ public CharBuffer buildProtocolConnectOptionsString(String serverURI, boolean in String uriUser = null; String uriPass = null; String uriToken = null; - + // Values from URI override options try { URI uri = this.createURIForServer(serverURI); @@ -2138,19 +1984,22 @@ public CharBuffer buildProtocolConnectOptionsString(String serverURI, boolean in return connectString; } - private void appendOption(CharBuffer builder, String key, String value, boolean quotes, boolean comma) { + // ---------------------------------------------------------------------------------------------------- + // HELPER FUNCTIONS + // ---------------------------------------------------------------------------------------------------- + private static void appendOption(CharBuffer builder, String key, String value, boolean quotes, boolean comma) { _appendStart(builder, key, quotes, comma); builder.append(value); _appendOptionEnd(builder, quotes); } - private void appendOption(CharBuffer builder, String key, char[] value, boolean quotes, boolean comma) { + private static void appendOption(CharBuffer builder, String key, char[] value, boolean quotes, boolean comma) { _appendStart(builder, key, quotes, comma); builder.put(value); _appendOptionEnd(builder, quotes); } - private void _appendStart(CharBuffer builder, String key, boolean quotes, boolean comma) { + private static void _appendStart(CharBuffer builder, String key, boolean quotes, boolean comma) { if (comma) { builder.append(','); } @@ -2161,17 +2010,110 @@ private void _appendStart(CharBuffer builder, String key, boolean quotes, boolea _appendOptionEnd(builder, quotes); } - private void _appendOptionEnd(CharBuffer builder, boolean quotes) { + private static void _appendOptionEnd(CharBuffer builder, boolean quotes) { if (quotes) { builder.append('"'); } } - /** - * Set old request style. - * @param value true to use the old request style - */ - public void setOldRequestStyle(boolean value) { - useOldRequestStyle = value; + private static String getPropertyValue(Properties props, String key) { + String value = Validator.emptyAsNull(props.getProperty(key)); + if (value != null) { + return value; + } + if (key.startsWith(PFX)) { // if the key starts with the PFX, check the non PFX + return Validator.emptyAsNull(props.getProperty(key.substring(PFX_LEN))); + } + // otherwise check with the PFX + return Validator.emptyAsNull(props.getProperty(PFX + key)); + } + + private static void stringProperty(Properties props, String key, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value != null) { + consumer.accept(value); + } + } + + private static void booleanIfTrueProperty(Properties props, String key, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value != null) { + if (Boolean.parseBoolean(value)) { + consumer.accept(true); + } + } + } + + private static void charArrayProperty(Properties props, String key, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value != null) { + consumer.accept(value.toCharArray()); + } + } + + private static void intProperty(Properties props, String key, int defaultValue, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value == null) { + consumer.accept(defaultValue); + } + else { + consumer.accept(Integer.parseInt(value)); + } + } + + private static void intGtEqZeroProperty(Properties props, String key, int defaultValue, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value == null) { + consumer.accept(defaultValue); + } + else { + int i = Integer.parseInt(value); + if (i < 0) { + consumer.accept(defaultValue); + } + else { + consumer.accept(i); + } + } + } + + private static void longProperty(Properties props, String key, long defaultValue, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value == null) { + consumer.accept(defaultValue); + } + else { + consumer.accept(Long.parseLong(value)); + } + } + + private static void durationProperty(Properties props, String key, Duration defaultValue, java.util.function.Consumer consumer) { + String value = getPropertyValue(props, key); + if (value == null) { + consumer.accept(defaultValue); + } + else { + int ms = Integer.parseInt(value); + if (ms < 0) { + consumer.accept(defaultValue); + } + else { + consumer.accept(Duration.ofMillis(ms)); + } + } + } + + private static void classnameProperty(Properties props, String key, java.util.function.Consumer consumer) { + stringProperty(props, key, className -> consumer.accept(createInstanceOf(className))); + } + + private static Object createInstanceOf(String className) { + try { + Class clazz = Class.forName(className); + Constructor constructor = clazz.getConstructor(); + return constructor.newInstance(); + } catch (Exception e) { + throw new IllegalArgumentException(e); + } } } diff --git a/src/test/java/io/nats/client/OptionsTests.java b/src/test/java/io/nats/client/OptionsTests.java index 85991b429..07c73829a 100644 --- a/src/test/java/io/nats/client/OptionsTests.java +++ b/src/test/java/io/nats/client/OptionsTests.java @@ -691,15 +691,6 @@ public void testThrowOnBadServerURI() { () -> new Options.Builder().server("foo:/bar\\:blammer").build()); } - @Test - public void testThrowOnEmptyServersProp() { - assertThrows(IllegalArgumentException.class, () -> { - Properties props = new Properties(); - props.setProperty(Options.PROP_SERVERS, ""); - new Options.Builder(props).build(); - }); - } - @Test public void testThrowOnBadServersURI() { assertThrows(IllegalArgumentException.class, () -> { From 29a95b84fd28020b2bea9f9b0abe01c39e197220 Mon Sep 17 00:00:00 2001 From: scottf Date: Sun, 23 Jul 2023 11:05:43 -0400 Subject: [PATCH 2/8] Improving options to allow creation of TLS and JWT based connections via properties file --- src/main/java/io/nats/client/Options.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index 266618147..230416174 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -280,6 +280,11 @@ public class Options { * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#noHeaders() noHeaders}. */ public static final String PROP_NO_HEADERS = PFX + "noheaders"; + /** + * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#connectionName(String) + * connectionName}. + */ + public static final String PROP_CONNECTION_NAME = PFX + "name"; /** * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#noNoResponders() noNoResponders}. */ @@ -326,7 +331,6 @@ public class Options { * default SSL context. Set the default context before creating the options. */ public static final String PROP_SECURE = PFX + "secure"; - /** * Property used to configure a builder from a Properties object. * {@value}, see {@link Builder#sslContext(SSLContext) sslContext}. @@ -369,18 +373,18 @@ public class Options { * {@link Builder#serverPool(ServerPool) serverPool}. */ public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS = "servers_pool_implementation_class"; - /** - * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. - * @deprecated Client Side Limit checks are no longer performed. - */ - @Deprecated - public static final String PROP_CONNECTION_NAME = PFX + "name"; /** * This property is used to enable support for UTF8 subjects. See {@link Builder#supportUTF8Subjects() supportUTF8Subjcts()} * @deprecated only plain ascii subjects are supported */ @Deprecated public static final String PROP_UTF8_SUBJECTS = "allow.utf8.subjects"; + /** + * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. + * @deprecated Client Side Limit checks are no longer performed. + */ + @Deprecated + public static final String PROP_CLIENT_SIDE_LIMIT_CHECKS = PFX + "clientsidelimitchecks"; // ---------------------------------------------------------------------------------------------------- // PROTOCOL CONNECT OPTION CONSTANTS From 9b6b86a56b5754ae4d5cc3196226870447fded89 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 07:41:19 -0400 Subject: [PATCH 3/8] Adding test coverage --- src/main/java/io/nats/client/Options.java | 293 +++++++++++++----- .../java/io/nats/client/support/SSLUtils.java | 64 +++- .../java/io/nats/client/OptionsTests.java | 66 ++-- .../io/nats/client/utils/ResourceUtils.java | 17 +- .../options_coverage_with_prefix.properties | 11 + ...coverage_with_prefix_underscore.properties | 11 + ...options_coverage_without_prefix.properties | 11 + ...erage_without_prefix_underscore.properties | 11 + 8 files changed, 364 insertions(+), 120 deletions(-) create mode 100644 src/test/resources/options_coverage_with_prefix.properties create mode 100644 src/test/resources/options_coverage_with_prefix_underscore.properties create mode 100644 src/test/resources/options_coverage_without_prefix.properties create mode 100644 src/test/resources/options_coverage_without_prefix_underscore.properties diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index 230416174..806a87eb7 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -16,14 +16,19 @@ import io.nats.client.impl.DataPort; import io.nats.client.impl.ErrorListenerLoggerImpl; import io.nats.client.impl.SocketDataPort; -import io.nats.client.support.*; +import io.nats.client.support.HttpRequest; +import io.nats.client.support.NatsConstants; +import io.nats.client.support.NatsUri; +import io.nats.client.support.SSLUtils; import javax.net.ssl.SSLContext; +import java.io.File; import java.lang.reflect.Constructor; import java.net.Proxy; import java.net.URI; import java.net.URISyntaxException; import java.nio.CharBuffer; +import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.util.*; @@ -33,6 +38,9 @@ import static io.nats.client.support.Encoding.uriDecode; import static io.nats.client.support.NatsConstants.*; import static io.nats.client.support.NatsUri.DEFAULT_NATS_URI; +import static io.nats.client.support.SSLUtils.DEFAULT_TLS_ALGORITHM; +import static io.nats.client.support.Validator.emptyAsNull; +import static io.nats.client.support.Validator.emptyOrNullAs; /** * The Options class specifies the connection options for a new NATs connection, including the default options. @@ -46,7 +54,7 @@ public class Options { // NOTE TO DEVS!!! To add an option, you have to address: // ---------------------------------------------------------------------------------------------------- // CONSTANTS * optionally add a default value constant - // ENVIRONMENT * most of the time add an environment property, should always be in the form PFX + + // ENVIRONMENT PROPERTIES * most of the time add an environment property, should always be in the form PFX + // PROTOCOL CONNECT OPTION CONSTANTS * not related to options, but here because Options code uses them // CLASS VARIABLES * add a variable to the class // BUILDER VARIABLES * add a variable in builder @@ -160,7 +168,6 @@ public class Options { */ public static final int DEFAULT_BUFFER_SIZE = 64 * 1024; - /** * Default thread name prefix. Used by the default executor when creating threads. * This property is defined as {@value} @@ -200,7 +207,7 @@ public class Options { public static final boolean DEFAULT_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL = false; // ---------------------------------------------------------------------------------------------------- - // ENVIRONMENT + // ENVIRONMENT PROPERTIES // ---------------------------------------------------------------------------------------------------- static final String PFX = "io.nats.client."; static final int PFX_LEN = PFX.length(); @@ -368,23 +375,56 @@ public class Options { * Property used to set whether to ignore discovered servers when connecting */ public static final String PROP_IGNORE_DISCOVERED_SERVERS = "ignore_discovered_servers"; + /** + * Preferred property used to set whether to ignore discovered servers when connecting + */ + public static final String PROP_IGNORE_DISCOVERED_SERVERS_PREFERRED = "ignore.discovered.servers"; /** * Property used to set class name for ServerPool implementation * {@link Builder#serverPool(ServerPool) serverPool}. */ public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS = "servers_pool_implementation_class"; /** - * This property is used to enable support for UTF8 subjects. See {@link Builder#supportUTF8Subjects() supportUTF8Subjcts()} - * @deprecated only plain ascii subjects are supported + * Preferred property used to set class name for ServerPool implementation + * {@link Builder#serverPool(ServerPool) serverPool}. */ - @Deprecated - public static final String PROP_UTF8_SUBJECTS = "allow.utf8.subjects"; + public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS_PREFERRED = "servers.pool.implementation.class"; + /** + * Property used to set the path to a credentials file to be used in a FileAuthHandler + */ + public static final String PROP_CREDENTIAL_PATH = PFX + "credential.path"; + /** + * Property for the keystore path used to create an SSLContext + */ + public static final String PROP_KEYSTORE_PATH = PFX + "keystore.path"; + /** + * Property for the truststore path used to create an SSLContext + */ + public static final String PROP_TRUSTSTORE_PATH = PFX + "truststore.path"; + /** + * Property for the keystore password used to create an SSLContext + */ + public static final String PROP_KEYSTORE_PASSWORD = PFX + "keystore.password"; + /** + * Property for the truststore password used to create an SSLContext + */ + public static final String PROP_TRUSTSTORE_PASSWORD = PFX + "truststore.password"; + /** + * Property for the algorithm used to create an SSLContext + */ + public static final String PROP_TLS_ALGORITHM = PFX + "tls.algorithm"; /** * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. * @deprecated Client Side Limit checks are no longer performed. */ @Deprecated public static final String PROP_CLIENT_SIDE_LIMIT_CHECKS = PFX + "clientsidelimitchecks"; + /** + * This property is used to enable support for UTF8 subjects. See {@link Builder#supportUTF8Subjects() supportUTF8Subjects()} + * @deprecated only plain ascii subjects are supported + */ + @Deprecated + public static final String PROP_UTF8_SUBJECTS = "allow.utf8.subjects"; // ---------------------------------------------------------------------------------------------------- // PROTOCOL CONNECT OPTION CONSTANTS @@ -507,7 +547,6 @@ public class Options { private final boolean noEcho; private final boolean noHeaders; private final boolean noNoResponders; - private final boolean utf8Support; private final int maxMessagesInOutgoingQueue; private final boolean discardMessagesWhenOutgoingQueueFull; private final boolean ignoreDiscoveredServers; @@ -611,7 +650,6 @@ public static class Builder { private boolean noEcho = false; private boolean noHeaders = false; private boolean noNoResponders = false; - private boolean utf8Support = false; private String inboxPrefix = DEFAULT_INBOX_PREFIX; private int maxMessagesInOutgoingQueue = DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE; private boolean discardMessagesWhenOutgoingQueueFull = DEFAULT_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL; @@ -628,6 +666,15 @@ public static class Builder { private List> httpRequestInterceptors; private Proxy proxy; + private boolean useDefaultTls; + private boolean useTrustAllTls; + private String credentialPath; + private String keystorePath; + private String truststorePath; + private char[] keystorePassword; + private char[] truststorePassword; + private String tlsAlgorithm = DEFAULT_TLS_ALGORITHM; + /** * Constructs a new Builder with the default values. *

When {@link #build() build()} is called on a default builder it will add the {@link Options#DEFAULT_URL @@ -655,45 +702,35 @@ public Builder(Properties props) throws IllegalArgumentException { } stringProperty(props, PROP_URL, this::server); - charArrayProperty(props, PROP_USERNAME, ca -> this.username = ca); - charArrayProperty(props, PROP_PASSWORD, ca -> this.password = ca); - charArrayProperty(props, PROP_TOKEN, ca -> this.token = ca); - stringProperty(props, PROP_SERVERS, str -> { String[] servers = str.trim().split(",\\s*"); this.servers(servers); }); + charArrayProperty(props, PROP_USERNAME, ca -> this.username = ca); + charArrayProperty(props, PROP_PASSWORD, ca -> this.password = ca); + charArrayProperty(props, PROP_TOKEN, ca -> this.token = ca); + booleanIfTrueProperty(props, PROP_SECURE, alwaysTrue -> this.useDefaultTls = true); + booleanIfTrueProperty(props, PROP_OPENTLS, alwaysTrue -> this.useTrustAllTls = true); + + stringProperty(props, PROP_CREDENTIAL_PATH, s -> this.credentialPath = s); + stringProperty(props, PROP_KEYSTORE_PATH, s -> this.keystorePath = s); + stringProperty(props, PROP_TRUSTSTORE_PATH, s -> this.truststorePath = s); + charArrayProperty(props, PROP_KEYSTORE_PASSWORD, ca -> this.keystorePassword = ca); + charArrayProperty(props, PROP_TRUSTSTORE_PASSWORD, ca -> this.truststorePassword = ca); + stringProperty(props, PROP_TLS_ALGORITHM, s -> this.tlsAlgorithm = s); + + stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); + booleanIfTrueProperty(props, PROP_NORANDOMIZE, alwaysTrue -> this.noRandomize = true); booleanIfTrueProperty(props, PROP_NO_RESOLVE_HOSTNAMES, alwaysTrue -> this.noResolveHostnames = true); booleanIfTrueProperty(props, PROP_REPORT_NO_RESPONDERS, alwaysTrue -> this.reportNoResponders = true); - booleanIfTrueProperty(props, PROP_SECURE, alwaysTrue -> { - try { - this.sslContext = SSLContext.getDefault(); - } - catch (NoSuchAlgorithmException e) { - this.sslContext = null; - throw new IllegalArgumentException("Unable to retrieve default SSL context"); - } - }); - - booleanIfTrueProperty(props, PROP_OPENTLS, alwaysTrue -> { - try { - this.sslContext = SSLUtils.createOpenTLSContext(); - } - catch (Exception e) { - this.sslContext = null; - throw new IllegalArgumentException("Unable to create open SSL context"); - } - }); - stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); booleanIfTrueProperty(props, PROP_VERBOSE, alwaysTrue -> this.verbose = true); booleanIfTrueProperty(props, PROP_NO_ECHO, alwaysTrue -> this.noEcho = true); booleanIfTrueProperty(props, PROP_NO_HEADERS, alwaysTrue -> this.noHeaders = true); booleanIfTrueProperty(props, PROP_NO_NORESPONDERS, alwaysTrue -> this.noNoResponders = true); - booleanIfTrueProperty(props, PROP_UTF8_SUBJECTS, alwaysTrue -> this.utf8Support = true); booleanIfTrueProperty(props, PROP_PEDANTIC, alwaysTrue -> this.pedantic = true); intProperty(props, PROP_MAX_RECONNECT, DEFAULT_MAX_RECONNECT, i -> this.maxReconnect = i); @@ -848,7 +885,6 @@ public Builder clientSideLimitChecks(boolean checks) { */ @Deprecated public Builder supportUTF8Subjects() { - this.utf8Support = true; return this; } @@ -918,35 +954,27 @@ public Builder traceConnection() { /** * Sets the options to use the default SSL Context, if it exists. - * - * @throws NoSuchAlgorithmException If the default protocol is unavailable. - * @throws IllegalArgumentException If there is no default SSL context. + * @throws NoSuchAlgorithmException Not thrown, deferred to build() method, left in for backward compatibility * @return the Builder for chaining */ - public Builder secure() throws NoSuchAlgorithmException, IllegalArgumentException { - this.sslContext = SSLContext.getDefault(); - - if(this.sslContext == null) { - throw new IllegalArgumentException("No Default SSL Context"); - } + public Builder secure() throws NoSuchAlgorithmException { + useDefaultTls = true; return this; } /** - * Set the SSL context to one that accepts any server certificate and has no client certificates. - * - * @throws NoSuchAlgorithmException If the tls protocol is unavailable. + * Set the options to use an SSL context that accepts any server certificate and has no client certificates. + * @throws NoSuchAlgorithmException Not thrown, deferred to build() method, left in for backward compatibility * @return the Builder for chaining */ public Builder opentls() throws NoSuchAlgorithmException { - this.sslContext = SSLUtils.createOpenTLSContext(); + useTrustAllTls = true; return this; } /** * Set the SSL context, requires that the server supports TLS connections and * the URI specifies TLS. - * * @param ctx the SSL Context to use for TLS connections * @return the Builder for chaining */ @@ -955,6 +983,66 @@ public Builder sslContext(SSLContext ctx) { return this; } + /** + * + * @param credentialPath the path to the credentials file for creating an {@link AuthHandler AuthHandler} + * @return the Builder for chaining + */ + public Builder credentialPath(String credentialPath) { + this.credentialPath = emptyAsNull(credentialPath); + return this; + } + + /** + * + * @param keystorePath the path to the keystore file + * @return the Builder for chaining + */ + public Builder keystorePath(String keystorePath) { + this.keystorePath = emptyAsNull(keystorePath); + return this; + } + + /** + * + * @param truststorePath the path to the trust store file + * @return the Builder for chaining + */ + public Builder truststorePath(String truststorePath) { + this.truststorePath = emptyAsNull(truststorePath); + return this; + } + + /** + * + * @param keystorePassword the password for the keystore + * @return the Builder for chaining + */ + public Builder keystorePassword(char[] keystorePassword) { + this.keystorePassword = keystorePassword == null || keystorePassword.length == 0 ? null : keystorePassword; + return this; + } + + /** + * + * @param truststorePassword the password for the trust store + * @return the Builder for chaining + */ + public Builder truststorePassword(char[] truststorePassword) { + this.truststorePassword = truststorePassword == null || truststorePassword.length == 0 ? null : truststorePassword; + return this; + } + + /** + * + * @param tlsAlgorithm the tls algorithm. Default is {@value SSLUtils#DEFAULT_TLS_ALGORITHM} + * @return the Builder for chaining + */ + public Builder tlsAlgorithm(String tlsAlgorithm) { + this.tlsAlgorithm = emptyOrNullAs(tlsAlgorithm, DEFAULT_TLS_ALGORITHM); + return this; + } + /** * Equivalent to calling maxReconnects with 0, {@link #maxReconnects(int) maxReconnects}. * @return the Builder for chaining @@ -1362,28 +1450,72 @@ public Options build() throws IllegalStateException { inboxPrefix = DEFAULT_INBOX_PREFIX; } +// sslContext +// private boolean useDefaultTls; +// private boolean useTrustAllTls; +// private String keystorePath; +// private String truststorePath; +// private String keystorePassword; +// private String truststorePassword; + + boolean checkUrisForSecure = true; if (natsServerUris.size() == 0) { server(DEFAULT_URL); + checkUrisForSecure = false; + } + + if (keystorePath != null) { + try { + sslContext = SSLUtils.createSSLContext(keystorePath, keystorePassword, truststorePath, truststorePassword, tlsAlgorithm); + } + catch (Exception e) { + throw new IllegalStateException("Unable to create SSL context", e); + } } - else if (sslContext == null) { // see if we need to provide one - for (int i = 0; sslContext == null && i < natsServerUris.size(); i++) { - NatsUri natsUri = natsServerUris.get(i); - switch (natsUri.getScheme()) { - case TLS_PROTOCOL: - case SECURE_WEBSOCKET_PROTOCOL: - try { - this.sslContext = SSLContext.getDefault(); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("Unable to create default SSL context", e); - } - break; - case OPENTLS_PROTOCOL: - this.sslContext = SSLUtils.createOpenTLSContext(); - break; + + // if an sslContext has not been requested via keystore properties + if (sslContext == null) { + // if we haven't been told to use the default or the trust all context + // and the server isn't the default url, check to see if the server uris + // suggest we need an ssl context. + if (!useDefaultTls && !useTrustAllTls && checkUrisForSecure) { + for (int i = 0; sslContext == null && i < natsServerUris.size(); i++) { + NatsUri natsUri = natsServerUris.get(i); + switch (natsUri.getScheme()) { + case TLS_PROTOCOL: + case SECURE_WEBSOCKET_PROTOCOL: + useDefaultTls = true; + break; + case OPENTLS_PROTOCOL: + useTrustAllTls = true; + break; + } + } + } + + if (useDefaultTls) { + try { + this.sslContext = SSLContext.getDefault(); + } + catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("Unable to create default SSL context", e); + } + } + else if (useTrustAllTls) { + try { + this.sslContext = SSLUtils.createTrustAllTlsContext(); + } + catch (GeneralSecurityException e) { + throw new IllegalStateException("Unable to create SSL context", e); } } } + if (credentialPath != null) { + File file = new File(credentialPath).getAbsoluteFile(); + authHandler = Nats.credentials(file.toString()); + } + if (this.executor == null) { String threadPrefix = (this.connectionName != null && this.connectionName.length() > 0) ? this.connectionName : DEFAULT_THREAD_NAME_PREFIX; this.executor = new ThreadPoolExecutor(0, Integer.MAX_VALUE, @@ -1429,7 +1561,6 @@ public Builder(Options o) { this.noEcho = o.noEcho; this.noHeaders = o.noHeaders; this.noNoResponders = o.noNoResponders; - this.utf8Support = o.utf8Support; this.inboxPrefix = o.inboxPrefix; this.traceConnection = o.traceConnection; this.maxMessagesInOutgoingQueue = o.maxMessagesInOutgoingQueue; @@ -1488,7 +1619,6 @@ private Options(Builder b) { this.noEcho = b.noEcho; this.noHeaders = b.noHeaders; this.noNoResponders = b.noNoResponders; - this.utf8Support = b.utf8Support; this.inboxPrefix = b.inboxPrefix; this.traceConnection = b.traceConnection; this.maxMessagesInOutgoingQueue = b.maxMessagesInOutgoingQueue; @@ -1624,15 +1754,6 @@ public boolean isReportNoResponders() { return reportNoResponders; } - /** - * @deprecated Plans are to remove allowing utf8mode - * @return whether utf8 subjects are supported, see {@link Builder#supportUTF8Subjects() supportUTF8Subjects()} in the builder doc. - */ - @Deprecated - public boolean supportUTF8Subjects() { - return utf8Support; - } - /** * @return the connectionName, see {@link Builder#connectionName(String) connectionName()} in the builder doc */ @@ -1677,6 +1798,15 @@ public boolean clientSideLimitChecks() { return false; } + /** + * @deprecated Plans are to remove allowing utf8mode + * @return whether utf8 subjects are supported, see {@link Builder#supportUTF8Subjects() supportUTF8Subjects()} in the builder doc. + */ + @Deprecated + public boolean supportUTF8Subjects() { + return false; + } + /** * @return are we using pedantic protocol, see {@link Builder#pedantic() pedantic()} in the builder doc */ @@ -2021,15 +2151,20 @@ private static void _appendOptionEnd(CharBuffer builder, boolean quotes) { } private static String getPropertyValue(Properties props, String key) { - String value = Validator.emptyAsNull(props.getProperty(key)); + String value = emptyAsNull(props.getProperty(key)); if (value != null) { return value; } if (key.startsWith(PFX)) { // if the key starts with the PFX, check the non PFX - return Validator.emptyAsNull(props.getProperty(key.substring(PFX_LEN))); + return emptyAsNull(props.getProperty(key.substring(PFX_LEN))); } // otherwise check with the PFX - return Validator.emptyAsNull(props.getProperty(PFX + key)); + value = emptyAsNull(props.getProperty(PFX + key)); + if (value == null && key.contains("_")) { + // addressing where underscore was used in a key value instead of dot + return getPropertyValue(props, key.replace("_", ".")); + } + return value; } private static void stringProperty(Properties props, String key, java.util.function.Consumer consumer) { diff --git a/src/main/java/io/nats/client/support/SSLUtils.java b/src/main/java/io/nats/client/support/SSLUtils.java index 4d90327f6..651e3bb07 100644 --- a/src/main/java/io/nats/client/support/SSLUtils.java +++ b/src/main/java/io/nats/client/support/SSLUtils.java @@ -15,36 +15,72 @@ import io.nats.client.Options; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; +import javax.net.ssl.*; +import java.io.BufferedInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.GeneralSecurityException; +import java.security.KeyStore; import java.security.cert.X509Certificate; import static io.nats.client.support.RandomUtils.SRAND; public class SSLUtils { - private static TrustManager[] trustAllCerts = new TrustManager[] { new X509TrustManager() { + + public static final String DEFAULT_TLS_ALGORITHM = "SunX509"; + public static final String KEYSTORE_TYPE = "JKS"; + + private static final TrustManager[] TRUST_ALL_CERTS = new TrustManager[] { new X509TrustManager() { public java.security.cert.X509Certificate[] getAcceptedIssuers() { return null; } - public void checkClientTrusted(X509Certificate[] certs, String authType) { - } + public void checkClientTrusted(X509Certificate[] certs, String authType) {} - public void checkServerTrusted(X509Certificate[] certs, String authType) { - } + public void checkServerTrusted(X509Certificate[] certs, String authType) {} } }; public static SSLContext createOpenTLSContext() { - SSLContext context = null; - try { - context = SSLContext.getInstance(Options.DEFAULT_SSL_PROTOCOL); - context.init(null, trustAllCerts, SRAND); - } catch (Exception e) { - context = null; + return createTrustAllTlsContext(); } + catch (Exception e) { + return null; + } + } + public static SSLContext createTrustAllTlsContext() throws GeneralSecurityException { + SSLContext context = SSLContext.getInstance(Options.DEFAULT_SSL_PROTOCOL); + context.init(null, TRUST_ALL_CERTS, SRAND); return context; } + + public static KeyStore loadKeystore(String keystorePath, char[] keystorePwd) throws GeneralSecurityException, IOException { + final KeyStore store = KeyStore.getInstance(KEYSTORE_TYPE); + try (BufferedInputStream in = new BufferedInputStream(Files.newInputStream(Paths.get(keystorePath)))) { + store.load(in, keystorePwd); + } + return store; + } + + public static KeyManager[] createKeyManagers(String keystorePath, char[] keystorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { + KeyStore store = loadKeystore(keystorePath, keystorePwd); + KeyManagerFactory factory = KeyManagerFactory.getInstance(tlsAlgo); + factory.init(store, keystorePwd); + return factory.getKeyManagers(); + } + + public static TrustManager[] createTrustManagers(String truststorePath, char[] truststorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { + KeyStore store = loadKeystore(truststorePath, truststorePwd); + TrustManagerFactory factory = TrustManagerFactory.getInstance(tlsAlgo); + factory.init(store); + return factory.getTrustManagers(); + } + + public static SSLContext createSSLContext(String keystorePath, char[] keystorePwd, String truststorePath, char[] truststorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { + SSLContext ctx = SSLContext.getInstance(Options.DEFAULT_SSL_PROTOCOL); + ctx.init(createKeyManagers(keystorePath, keystorePwd, tlsAlgo), createTrustManagers(truststorePath, truststorePwd, tlsAlgo), SRAND); + return ctx; + } } \ No newline at end of file diff --git a/src/test/java/io/nats/client/OptionsTests.java b/src/test/java/io/nats/client/OptionsTests.java index 07c73829a..9419e8bcc 100644 --- a/src/test/java/io/nats/client/OptionsTests.java +++ b/src/test/java/io/nats/client/OptionsTests.java @@ -16,11 +16,11 @@ import io.nats.client.ConnectionListener.Events; import io.nats.client.impl.DataPort; import io.nats.client.impl.ErrorListenerLoggerImpl; -import io.nats.client.impl.NatsServerPool; import io.nats.client.impl.TestHandler; import io.nats.client.support.HttpRequest; import io.nats.client.support.NatsUri; import io.nats.client.utils.CloseOnUpgradeAttempt; +import io.nats.client.utils.ResourceUtils; import org.junit.jupiter.api.Test; import javax.net.ssl.SSLContext; @@ -68,7 +68,6 @@ private static void _testDefaultOptions(Options o) { assertFalse(o.isNoRandomize(), "default norandomize"); assertFalse(o.isOldRequestStyle(), "default oldstyle"); assertFalse(o.isNoEcho(), "default noEcho"); - assertFalse(o.supportUTF8Subjects(), "default UTF8 Support"); assertFalse(o.isNoHeaders(), "default header support"); assertFalse(o.isNoNoResponders(), "default no responders support"); assertEquals(Options.DEFAULT_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, o.isDiscardMessagesWhenOutgoingQueueFull(), @@ -102,13 +101,17 @@ private static void _testDefaultOptions(Options o) { public void testOldStyle() { Options o = new Options.Builder().build(); assertFalse(o.isOldRequestStyle(), "default oldstyle"); + //noinspection deprecation o.setOldRequestStyle(true); - assertTrue(o.isOldRequestStyle(), "default oldstyle"); + assertTrue(o.isOldRequestStyle(), "true oldstyle"); + //noinspection deprecation + o.setOldRequestStyle(false); + assertFalse(o.isOldRequestStyle(), "false oldstyle"); } @Test public void testChainedBooleanOptions() { - Options o = new Options.Builder().verbose().pedantic().noRandomize().supportUTF8Subjects() + Options o = new Options.Builder().verbose().pedantic().noRandomize() .noEcho().oldRequestStyle().noHeaders().noNoResponders() .discardMessagesWhenOutgoingQueueFull() .build(); @@ -123,7 +126,6 @@ private static void _testChainedBooleanOptions(Options o) { assertTrue(o.isNoRandomize(), "chained norandomize"); assertTrue(o.isOldRequestStyle(), "chained oldstyle"); assertTrue(o.isNoEcho(), "chained noecho"); - assertTrue(o.supportUTF8Subjects(), "chained utf8"); assertTrue(o.isNoHeaders(), "chained no headers"); assertTrue(o.isNoNoResponders(), "chained no noResponders"); assertTrue(o.isDiscardMessagesWhenOutgoingQueueFull(), "chained discard messages when outgoing queue full"); @@ -266,7 +268,6 @@ public void testPropertiesBooleanBuilder() { props.setProperty(Options.PROP_USE_OLD_REQUEST_STYLE, "true"); props.setProperty(Options.PROP_OPENTLS, "true"); props.setProperty(Options.PROP_NO_ECHO, "true"); - props.setProperty(Options.PROP_UTF8_SUBJECTS, "true"); props.setProperty(Options.PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, "true"); Options o = new Options.Builder(props).build(); @@ -281,7 +282,6 @@ private static void _testPropertiesBooleanBuilder(Options o) { assertTrue(o.isNoRandomize(), "property norandomize"); assertTrue(o.isOldRequestStyle(), "property oldstyle"); assertTrue(o.isNoEcho(), "property noecho"); - assertTrue(o.supportUTF8Subjects(), "property utf8"); assertTrue(o.isDiscardMessagesWhenOutgoingQueueFull(), "property discard messages when outgoing queue full"); assertNotNull(o.getSslContext(), "property opentls"); } @@ -329,28 +329,28 @@ private static void _testPropertiesSSLOptions(Options o) { assertNotNull(o.getSslContext(), "property context"); } + @SuppressWarnings("deprecation") @Test - public void testBuilderCoverageOptions() { + public void testDeprecated() { + // clientSideLimitChecks, supportUTF8Subjects are deprecated and always returns false Options o = new Options.Builder().build(); - assertFalse(o.clientSideLimitChecks()); // clientSideLimitChecks is deprecated and always returns false - assertNull(o.getServerPool()); // there is a default provider + assertFalse(o.clientSideLimitChecks()); + assertFalse(o.supportUTF8Subjects()); - o = new Options.Builder() - .clientSideLimitChecks(true).build(); - assertFalse(o.clientSideLimitChecks()); // clientSideLimitChecks is deprecated and always returns false + o = new Options.Builder().clientSideLimitChecks(true).supportUTF8Subjects().build(); + assertFalse(o.clientSideLimitChecks()); + assertFalse(o.supportUTF8Subjects()); - o = new Options.Builder() - .clientSideLimitChecks(false) - .serverPool(new NatsServerPool()) - .build(); + Properties props = new Properties(); + props.setProperty(Options.PROP_CLIENT_SIDE_LIMIT_CHECKS, "true"); + props.setProperty(Options.PROP_UTF8_SUBJECTS, "true"); + o = new Options.Builder(props).build(); assertFalse(o.clientSideLimitChecks()); - assertNotNull(o.getServerPool()); + assertFalse(o.supportUTF8Subjects()); } @Test public void testPropertiesCoverageOptions() throws Exception { - // don't use default for tests, issues with forcing algorithm exception in other tests break it - SSLContext.setDefault(TestSSLUtils.createTestSSLContext()); Properties props = new Properties(); props.setProperty(Options.PROP_SECURE, "false"); props.setProperty(Options.PROP_OPENTLS, "false"); @@ -358,7 +358,6 @@ public void testPropertiesCoverageOptions() throws Exception { props.setProperty(Options.PROP_NO_NORESPONDERS, "true"); props.setProperty(Options.PROP_RECONNECT_JITTER, "1000"); props.setProperty(Options.PROP_RECONNECT_JITTER_TLS, "2000"); - props.setProperty(Options.PROP_CLIENT_SIDE_LIMIT_CHECKS, "true"); // deprecated props.setProperty(Options.PROP_IGNORE_DISCOVERED_SERVERS, "true"); props.setProperty(Options.PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, "io.nats.client.utils.CoverageServerPool"); props.setProperty(Options.PROP_NO_RESOLVE_HOSTNAMES, "true"); @@ -366,13 +365,32 @@ public void testPropertiesCoverageOptions() throws Exception { Options o = new Options.Builder(props).build(); _testPropertiesCoverageOptions(o); _testPropertiesCoverageOptions(new Options.Builder(o).build()); + + props = new Properties(); + props.load(ResourceUtils.resourceAsInputStream("options_coverage_with_prefix.properties")); + o = new Options.Builder(props).build(); + _testPropertiesCoverageOptions(o); + + props = new Properties(); + props.load(ResourceUtils.resourceAsInputStream("options_coverage_with_prefix_underscore.properties")); + o = new Options.Builder(props).build(); + _testPropertiesCoverageOptions(o); + + props = new Properties(); + props.load(ResourceUtils.resourceAsInputStream("options_coverage_without_prefix.properties")); + o = new Options.Builder(props).build(); + _testPropertiesCoverageOptions(o); + + props = new Properties(); + props.load(ResourceUtils.resourceAsInputStream("options_coverage_without_prefix_underscore.properties")); + o = new Options.Builder(props).build(); + _testPropertiesCoverageOptions(o); } private static void _testPropertiesCoverageOptions(Options o) { assertNull(o.getSslContext(), "property context"); assertTrue(o.isNoHeaders()); assertTrue(o.isNoNoResponders()); - assertFalse(o.clientSideLimitChecks()); // clientSideLimitChecks is deprecated and always returns false assertTrue(o.isIgnoreDiscoveredServers()); assertNotNull(o.getServerPool()); assertTrue(o.isNoResolveHostnames()); @@ -694,9 +712,7 @@ public void testThrowOnBadServerURI() { @Test public void testThrowOnBadServersURI() { assertThrows(IllegalArgumentException.class, () -> { - String url1 = URL_PROTO_HOST_PORT_8080; - String url2 = "foo:/bar\\:blammer"; - String[] serverUrls = {url1, url2}; + String[] serverUrls = {URL_PROTO_HOST_PORT_8080, "foo:/bar\\:blammer"}; new Options.Builder().servers(serverUrls).build(); }); } diff --git a/src/test/java/io/nats/client/utils/ResourceUtils.java b/src/test/java/io/nats/client/utils/ResourceUtils.java index 038edfaf4..24805fd31 100644 --- a/src/test/java/io/nats/client/utils/ResourceUtils.java +++ b/src/test/java/io/nats/client/utils/ResourceUtils.java @@ -1,10 +1,12 @@ package io.nats.client.utils; import java.io.File; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.List; +@SuppressWarnings("DataFlowIssue") public abstract class ResourceUtils { public static List dataAsLines(String fileName) { return resourceAsLines("data/" + fileName); @@ -19,7 +21,8 @@ public static List resourceAsLines(String fileName) { ClassLoader classLoader = ResourceUtils.class.getClassLoader(); File file = new File(classLoader.getResource(fileName).getFile()); return Files.readAllLines(file.toPath()); - } catch (Exception e) { + } + catch (Exception e) { throw new RuntimeException(e); } @@ -30,7 +33,17 @@ public static String resourceAsString(String fileName) { ClassLoader classLoader = ResourceUtils.class.getClassLoader(); File file = new File(classLoader.getResource(fileName).getFile()); return new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); - } catch (Exception e) { + } + catch (Exception e) { + throw new RuntimeException(e); + } + } + + public static InputStream resourceAsInputStream(String fileName) { + try { + return ResourceUtils.class.getClassLoader().getResourceAsStream(fileName); + } + catch (Exception e) { throw new RuntimeException(e); } } diff --git a/src/test/resources/options_coverage_with_prefix.properties b/src/test/resources/options_coverage_with_prefix.properties new file mode 100644 index 000000000..cb79e2f7f --- /dev/null +++ b/src/test/resources/options_coverage_with_prefix.properties @@ -0,0 +1,11 @@ +# All keys have prefix "io.nats.client." +# All keys in dot format +io.nats.client.secure=false +io.nats.client.opentls=false +io.nats.client.noheaders=true +io.nats.client.nonoresponders=true +io.nats.client.reconnect.jitter=1000 +io.nats.client.reconnect.jitter.tls=2000 +io.nats.client.ignore.discovered.servers=true +io.nats.client.servers.pool.implementation.class=io.nats.client.utils.CoverageServerPool +io.nats.client.noResolveHostnames=true diff --git a/src/test/resources/options_coverage_with_prefix_underscore.properties b/src/test/resources/options_coverage_with_prefix_underscore.properties new file mode 100644 index 000000000..e36d9ad81 --- /dev/null +++ b/src/test/resources/options_coverage_with_prefix_underscore.properties @@ -0,0 +1,11 @@ +# All keys have prefix "io.nats.client." +# Some keys in underscore format +io.nats.client.secure=false +io.nats.client.opentls=false +io.nats.client.noheaders=true +io.nats.client.nonoresponders=true +io.nats.client.reconnect.jitter=1000 +io.nats.client.reconnect.jitter.tls=2000 +io.nats.client.ignore_discovered_servers=true +io.nats.client.servers_pool_implementation_class=io.nats.client.utils.CoverageServerPool +io.nats.client.noResolveHostnames=true diff --git a/src/test/resources/options_coverage_without_prefix.properties b/src/test/resources/options_coverage_without_prefix.properties new file mode 100644 index 000000000..fe04be239 --- /dev/null +++ b/src/test/resources/options_coverage_without_prefix.properties @@ -0,0 +1,11 @@ +# No keys have prefix "io.nats.client." +# All keys in dot format +secure=false +opentls=false +noheaders=true +nonoresponders=true +reconnect.jitter=1000 +reconnect.jitter.tls=2000 +ignore.discovered.servers=true +servers.pool.implementation.class=io.nats.client.utils.CoverageServerPool +noResolveHostnames=true diff --git a/src/test/resources/options_coverage_without_prefix_underscore.properties b/src/test/resources/options_coverage_without_prefix_underscore.properties new file mode 100644 index 000000000..961bb8a10 --- /dev/null +++ b/src/test/resources/options_coverage_without_prefix_underscore.properties @@ -0,0 +1,11 @@ +# No keys have prefix "io.nats.client." +# Some keys in underscore format +secure=false +opentls=false +noheaders=true +nonoresponders=true +reconnect.jitter=1000 +reconnect.jitter.tls=2000 +ignore_discovered_servers=true +servers_pool_implementation_class=io.nats.client.utils.CoverageServerPool +noResolveHostnames=true From cf5b31395b9f12982ea88e5bcaaf16871ed97fea Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 10:26:18 -0400 Subject: [PATCH 4/8] Adding test coverage for property driven ssl --- src/main/java/io/nats/client/Options.java | 20 +-- .../java/io/nats/client/support/SSLUtils.java | 15 +- .../java/io/nats/client/OptionsTests.java | 15 ++ .../java/io/nats/client/TestSSLUtils.java | 55 +++---- .../io/nats/client/impl/TLSConnectTests.java | 141 ++++++++++-------- 5 files changed, 132 insertions(+), 114 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index 806a87eb7..cc9be892f 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -396,19 +396,19 @@ public class Options { /** * Property for the keystore path used to create an SSLContext */ - public static final String PROP_KEYSTORE_PATH = PFX + "keystore.path"; + public static final String PROP_KEYSTORE_PATH = PFX + "keyStore"; /** * Property for the truststore path used to create an SSLContext */ - public static final String PROP_TRUSTSTORE_PATH = PFX + "truststore.path"; + public static final String PROP_TRUSTSTORE_PATH = PFX + "trustStore"; /** * Property for the keystore password used to create an SSLContext */ - public static final String PROP_KEYSTORE_PASSWORD = PFX + "keystore.password"; + public static final String PROP_KEYSTORE_PASSWORD = PFX + "keyStorePassword"; /** * Property for the truststore password used to create an SSLContext */ - public static final String PROP_TRUSTSTORE_PASSWORD = PFX + "truststore.password"; + public static final String PROP_TRUSTSTORE_PASSWORD = PFX + "trustStorePassword"; /** * Property for the algorithm used to create an SSLContext */ @@ -715,8 +715,8 @@ public Builder(Properties props) throws IllegalArgumentException { stringProperty(props, PROP_CREDENTIAL_PATH, s -> this.credentialPath = s); stringProperty(props, PROP_KEYSTORE_PATH, s -> this.keystorePath = s); - stringProperty(props, PROP_TRUSTSTORE_PATH, s -> this.truststorePath = s); charArrayProperty(props, PROP_KEYSTORE_PASSWORD, ca -> this.keystorePassword = ca); + stringProperty(props, PROP_TRUSTSTORE_PATH, s -> this.truststorePath = s); charArrayProperty(props, PROP_TRUSTSTORE_PASSWORD, ca -> this.truststorePassword = ca); stringProperty(props, PROP_TLS_ALGORITHM, s -> this.tlsAlgorithm = s); @@ -1450,14 +1450,6 @@ public Options build() throws IllegalStateException { inboxPrefix = DEFAULT_INBOX_PREFIX; } -// sslContext -// private boolean useDefaultTls; -// private boolean useTrustAllTls; -// private String keystorePath; -// private String truststorePath; -// private String keystorePassword; -// private String truststorePassword; - boolean checkUrisForSecure = true; if (natsServerUris.size() == 0) { server(DEFAULT_URL); @@ -1477,7 +1469,7 @@ public Options build() throws IllegalStateException { if (sslContext == null) { // if we haven't been told to use the default or the trust all context // and the server isn't the default url, check to see if the server uris - // suggest we need an ssl context. + // suggest we need the ssl context. if (!useDefaultTls && !useTrustAllTls && checkUrisForSecure) { for (int i = 0; sslContext == null && i < natsServerUris.size(); i++) { NatsUri natsUri = natsServerUris.get(i); diff --git a/src/main/java/io/nats/client/support/SSLUtils.java b/src/main/java/io/nats/client/support/SSLUtils.java index 651e3bb07..6a7257a9f 100644 --- a/src/main/java/io/nats/client/support/SSLUtils.java +++ b/src/main/java/io/nats/client/support/SSLUtils.java @@ -29,7 +29,7 @@ public class SSLUtils { public static final String DEFAULT_TLS_ALGORITHM = "SunX509"; - public static final String KEYSTORE_TYPE = "JKS"; + public static final String DEFAULT_KEYSTORE_TYPE = "JKS"; private static final TrustManager[] TRUST_ALL_CERTS = new TrustManager[] { new X509TrustManager() { public java.security.cert.X509Certificate[] getAcceptedIssuers() { @@ -57,13 +57,17 @@ public static SSLContext createTrustAllTlsContext() throws GeneralSecurityExcept } public static KeyStore loadKeystore(String keystorePath, char[] keystorePwd) throws GeneralSecurityException, IOException { - final KeyStore store = KeyStore.getInstance(KEYSTORE_TYPE); + final KeyStore store = KeyStore.getInstance(DEFAULT_KEYSTORE_TYPE); try (BufferedInputStream in = new BufferedInputStream(Files.newInputStream(Paths.get(keystorePath)))) { store.load(in, keystorePwd); } return store; } + public static KeyManager[] createKeyManagers(String keystorePath, char[] keystorePwd) throws GeneralSecurityException, IOException { + return createKeyManagers(keystorePath, keystorePwd, DEFAULT_TLS_ALGORITHM); + } + public static KeyManager[] createKeyManagers(String keystorePath, char[] keystorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { KeyStore store = loadKeystore(keystorePath, keystorePwd); KeyManagerFactory factory = KeyManagerFactory.getInstance(tlsAlgo); @@ -71,6 +75,10 @@ public static KeyManager[] createKeyManagers(String keystorePath, char[] keystor return factory.getKeyManagers(); } + public static TrustManager[] createTrustManagers(String truststorePath, char[] truststorePwd) throws GeneralSecurityException, IOException { + return createTrustManagers(truststorePath, truststorePwd, DEFAULT_TLS_ALGORITHM); + } + public static TrustManager[] createTrustManagers(String truststorePath, char[] truststorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { KeyStore store = loadKeystore(truststorePath, truststorePwd); TrustManagerFactory factory = TrustManagerFactory.getInstance(tlsAlgo); @@ -78,6 +86,9 @@ public static TrustManager[] createTrustManagers(String truststorePath, char[] t return factory.getTrustManagers(); } + public static SSLContext createSSLContext(String keystorePath, char[] keystorePwd, String truststorePath, char[] truststorePwd) throws GeneralSecurityException, IOException { + return createSSLContext(keystorePath, keystorePwd, truststorePath, truststorePwd, DEFAULT_TLS_ALGORITHM); + } public static SSLContext createSSLContext(String keystorePath, char[] keystorePwd, String truststorePath, char[] truststorePwd, String tlsAlgo) throws GeneralSecurityException, IOException { SSLContext ctx = SSLContext.getInstance(Options.DEFAULT_SSL_PROTOCOL); ctx.init(createKeyManagers(keystorePath, keystorePwd, tlsAlgo), createTrustManagers(truststorePath, truststorePwd, tlsAlgo), SRAND); diff --git a/src/test/java/io/nats/client/OptionsTests.java b/src/test/java/io/nats/client/OptionsTests.java index 9419e8bcc..694c061c6 100644 --- a/src/test/java/io/nats/client/OptionsTests.java +++ b/src/test/java/io/nats/client/OptionsTests.java @@ -366,6 +366,21 @@ public void testPropertiesCoverageOptions() throws Exception { _testPropertiesCoverageOptions(o); _testPropertiesCoverageOptions(new Options.Builder(o).build()); + props = new Properties(); + props.setProperty(Options.PROP_SECURE, "false"); + props.setProperty(Options.PROP_OPENTLS, "false"); + props.setProperty(Options.PROP_NO_HEADERS, "true"); + props.setProperty(Options.PROP_NO_NORESPONDERS, "true"); + props.setProperty(Options.PROP_RECONNECT_JITTER, "1000"); + props.setProperty(Options.PROP_RECONNECT_JITTER_TLS, "2000"); + props.setProperty(Options.PROP_IGNORE_DISCOVERED_SERVERS_PREFERRED, "true"); + props.setProperty(Options.PROP_SERVERS_POOL_IMPLEMENTATION_CLASS_PREFERRED, "io.nats.client.utils.CoverageServerPool"); + props.setProperty(Options.PROP_NO_RESOLVE_HOSTNAMES, "true"); + + o = new Options.Builder(props).build(); + _testPropertiesCoverageOptions(o); + _testPropertiesCoverageOptions(new Options.Builder(o).build()); + props = new Properties(); props.load(ResourceUtils.resourceAsInputStream("options_coverage_with_prefix.properties")); o = new Options.Builder(props).build(); diff --git a/src/test/java/io/nats/client/TestSSLUtils.java b/src/test/java/io/nats/client/TestSSLUtils.java index 16869a092..0b442981d 100644 --- a/src/test/java/io/nats/client/TestSSLUtils.java +++ b/src/test/java/io/nats/client/TestSSLUtils.java @@ -13,64 +13,51 @@ package io.nats.client; -import java.io.BufferedInputStream; -import java.io.FileInputStream; -import java.security.KeyStore; -import java.security.SecureRandom; +import io.nats.client.support.SSLUtils; import javax.net.ssl.KeyManager; -import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; +import java.security.KeyStore; +import java.security.SecureRandom; +import java.util.Properties; public class TestSSLUtils { public static String KEYSTORE_PATH = "src/test/resources/keystore.jks"; public static String TRUSTSTORE_PATH = "src/test/resources/truststore.jks"; - public static String STORE_PASSWORD = "password"; - public static String KEY_PASSWORD = "password"; - public static String ALGORITHM = "SunX509"; + public static String PASSWORD = "password"; + public static char[] PASSWORD_CHARS = PASSWORD.toCharArray(); public static KeyStore loadKeystore(String path) throws Exception { - KeyStore store = KeyStore.getInstance("JKS"); - BufferedInputStream in = new BufferedInputStream(new FileInputStream(path)); - - try { - store.load(in, STORE_PASSWORD.toCharArray()); - } finally { - if (in != null) { - in.close(); - } - } + return SSLUtils.loadKeystore(path, PASSWORD_CHARS); + } - return store; + public static Properties createTestSSLProperties() { + Properties props = new Properties(); + props.setProperty(Options.PROP_KEYSTORE_PATH, TestSSLUtils.KEYSTORE_PATH); + props.setProperty(Options.PROP_KEYSTORE_PASSWORD, TestSSLUtils.PASSWORD); + props.setProperty(Options.PROP_TRUSTSTORE_PATH, TestSSLUtils.KEYSTORE_PATH); + props.setProperty(Options.PROP_TRUSTSTORE_PASSWORD, TestSSLUtils.PASSWORD); + return props; } public static void setKeystoreSystemParameters() { - System.setProperty("javax.net.ssl.keyStore",KEYSTORE_PATH); - System.setProperty("javax.net.ssl.keyStorePassword",KEY_PASSWORD); + System.setProperty("javax.net.ssl.keyStore", KEYSTORE_PATH); + System.setProperty("javax.net.ssl.keyStorePassword", PASSWORD); System.setProperty("javax.net.ssl.trustStore",TRUSTSTORE_PATH); - System.setProperty("javax.net.ssl.trustStorePassword",STORE_PASSWORD); + System.setProperty("javax.net.ssl.trustStorePassword", PASSWORD); } public static KeyManager[] createTestKeyManagers() throws Exception { - KeyStore store = loadKeystore(KEYSTORE_PATH); - KeyManagerFactory factory = KeyManagerFactory.getInstance(ALGORITHM); - factory.init(store, KEY_PASSWORD.toCharArray()); - return factory.getKeyManagers(); + return SSLUtils.createKeyManagers(KEYSTORE_PATH, PASSWORD_CHARS); } public static TrustManager[] createTestTrustManagers() throws Exception { - KeyStore store = loadKeystore(TRUSTSTORE_PATH); - TrustManagerFactory factory = TrustManagerFactory.getInstance(ALGORITHM); - factory.init(store); - return factory.getTrustManagers(); + return SSLUtils.createTrustManagers(TRUSTSTORE_PATH, PASSWORD_CHARS); } public static SSLContext createTestSSLContext() throws Exception { - SSLContext ctx = SSLContext.getInstance(Options.DEFAULT_SSL_PROTOCOL); - ctx.init(createTestKeyManagers(), createTestTrustManagers(), new SecureRandom()); - return ctx; + return SSLUtils.createSSLContext(KEYSTORE_PATH, PASSWORD_CHARS, TRUSTSTORE_PATH, PASSWORD_CHARS); } public static SSLContext createEmptySSLContext() throws Exception { diff --git a/src/test/java/io/nats/client/impl/TLSConnectTests.java b/src/test/java/io/nats/client/impl/TLSConnectTests.java index 211b4ebf9..febd982df 100644 --- a/src/test/java/io/nats/client/impl/TLSConnectTests.java +++ b/src/test/java/io/nats/client/impl/TLSConnectTests.java @@ -21,6 +21,7 @@ import javax.net.ssl.SSLContext; import java.io.IOException; import java.time.Duration; +import java.util.Properties; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -36,22 +37,35 @@ private String convertToProtocol(String proto, NatsTestServer... servers) { if (x > 0) { sb.append(","); } - sb.append(proto).append("://localhost:").append(servers[0].getPort()); + sb.append(proto).append("://localhost:").append(servers[x].getPort()); } return sb.toString(); } + private static Options createTestOptionsManually(String servers) throws Exception { + return new Options.Builder() + .server(servers) + .maxReconnects(0) + .sslContext(TestSSLUtils.createTestSSLContext()) + .build(); + } + + private static Options createTestOptionsViaProperties(String servers) { + Options options; + Properties props = TestSSLUtils.createTestSSLProperties(); + props.setProperty(Options.PROP_SERVERS, servers); + props.setProperty(Options.PROP_MAX_RECONNECT, "0"); + options = new Options.Builder(props).build(); + return options; + } + @Test public void testSimpleTLSConnection() throws Exception { //System.setProperty("javax.net.debug", "all"); try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) { - SSLContext ctx = TestSSLUtils.createTestSSLContext(); - Options options = new Options.Builder() - .server(ts.getURI()) - .maxReconnects(0) - .sslContext(ctx) - .build(); - assertCanConnectAndPubSub(options); + String servers = ts.getURI(); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @@ -59,13 +73,9 @@ public void testSimpleTLSConnection() throws Exception { public void testSimpleUrlTLSConnection() throws Exception { //System.setProperty("javax.net.debug", "all"); try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) { - SSLContext ctx = TestSSLUtils.createTestSSLContext(); - Options options = new Options.Builder() - .server(convertToProtocol("tls", ts)) - .maxReconnects(0) - .sslContext(ctx) - .build(); - assertCanConnectAndPubSub(options); + String servers = convertToProtocol("tls", ts); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @@ -76,13 +86,8 @@ public void testMultipleUrlTLSConnectionSetContext() throws Exception { NatsTestServer server2 = new NatsTestServer("src/test/resources/tls.conf", false); ) { String servers = convertToProtocol("tls", server1, server2); - SSLContext ctx = TestSSLUtils.createTestSSLContext(); - Options options = new Options.Builder() - .server(servers) - .maxReconnects(0) - .sslContext(ctx) - .build(); - assertCanConnectAndPubSub(options); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @@ -90,73 +95,74 @@ public void testMultipleUrlTLSConnectionSetContext() throws Exception { public void testSimpleIPTLSConnection() throws Exception { //System.setProperty("javax.net.debug", "all"); try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) { - SSLContext ctx = TestSSLUtils.createTestSSLContext(); - Options options = new Options.Builder(). - server("127.0.0.1:" + ts.getPort()). - maxReconnects(0). - sslContext(ctx). - build(); - assertCanConnectAndPubSub(options); + String servers = "127.0.0.1:" + ts.getPort(); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @Test public void testVerifiedTLSConnection() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tlsverify.conf", false)) { - SSLContext ctx = TestSSLUtils.createTestSSLContext(); - Options options = new Options.Builder(). - server(ts.getURI()). - maxReconnects(0). - sslContext(ctx). - build(); - assertCanConnectAndPubSub(options); + String servers = ts.getURI(); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @Test public void testOpenTLSConnection() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) { - Options options = new Options.Builder(). - server(ts.getURI()). - maxReconnects(0). - opentls(). - build(); + String servers = ts.getURI(); + Options options = new Options.Builder() + .server(servers) + .maxReconnects(0) + .opentls() + .build(); assertCanConnectAndPubSub(options); + + Properties props = new Properties(); + props.setProperty(Options.PROP_SERVERS, servers); + props.setProperty(Options.PROP_MAX_RECONNECT, "0"); + props.setProperty(Options.PROP_OPENTLS, "true"); + assertCanConnectAndPubSub(new Options.Builder(props).build()); } } @Test public void testURISchemeTLSConnection() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tlsverify.conf", false)) { - Options options = new Options.Builder(). - server("tls://localhost:"+ts.getPort()). - sslContext(TestSSLUtils.createTestSSLContext()). // override the custom one - maxReconnects(0). - build(); - assertCanConnectAndPubSub(options); + String servers = "tls://localhost:"+ts.getPort(); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @Test public void testURISchemeIPTLSConnection() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tlsverify.conf", false)) { - Options options = new Options.Builder(). - server("tls://127.0.0.1:"+ts.getPort()). - sslContext(TestSSLUtils.createTestSSLContext()). // override the custom one - maxReconnects(0). - build(); - assertCanConnectAndPubSub(options); + String servers = "tls://127.0.0.1:"+ts.getPort(); + assertCanConnectAndPubSub(createTestOptionsManually(servers)); + assertCanConnectAndPubSub(createTestOptionsViaProperties(servers)); } } @Test public void testURISchemeOpenTLSConnection() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tls.conf", false)) { + String servers = convertToProtocol("opentls", ts); Options options = new Options.Builder() - .server(convertToProtocol("opentls", ts)) - .maxReconnects(0) - .build(); + .server(servers) + .maxReconnects(0) + .opentls() + .build(); assertCanConnectAndPubSub(options); + + Properties props = new Properties(); + props.setProperty(Options.PROP_SERVERS, servers); + props.setProperty(Options.PROP_MAX_RECONNECT, "0"); + props.setProperty(Options.PROP_OPENTLS, "true"); + assertCanConnectAndPubSub(new Options.Builder(props).build()); } } @@ -168,10 +174,17 @@ public void testMultipleUrlOpenTLSConnection() throws Exception { ) { String servers = convertToProtocol("opentls", server1, server2); Options options = new Options.Builder() - .server(servers) - .maxReconnects(0) - .build(); + .server(servers) + .maxReconnects(0) + .opentls() + .build(); assertCanConnectAndPubSub(options); + + Properties props = new Properties(); + props.setProperty(Options.PROP_SERVERS, servers); + props.setProperty(Options.PROP_MAX_RECONNECT, "0"); + props.setProperty(Options.PROP_OPENTLS, "true"); + assertCanConnectAndPubSub(new Options.Builder(props).build()); } } @@ -180,11 +193,11 @@ public void testTLSMessageFlow() throws Exception { try (NatsTestServer ts = new NatsTestServer("src/test/resources/tlsverify.conf", false)) { SSLContext ctx = TestSSLUtils.createTestSSLContext(); int msgCount = 100; - Options options = new Options.Builder(). - server(ts.getURI()). - maxReconnects(0). - sslContext(ctx). - build(); + Options options = new Options.Builder() + .server(ts.getURI()) + .maxReconnects(0) + .sslContext(ctx) + .build(); Connection nc = standardConnection(options); Dispatcher d = nc.createDispatcher((msg) -> { nc.publish(msg.getReplyTo(), new byte[16]); From df04fa34e1dfa45bce7c03bf1a6c333bff929448 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 11:19:32 -0400 Subject: [PATCH 5/8] check trust all (open) first, in case they provided both PROP_SECURE (secure) and PROP_OPENTLS (opentls) --- src/main/java/io/nats/client/Options.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index cc9be892f..72b2a9251 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -1485,20 +1485,22 @@ public Options build() throws IllegalStateException { } } - if (useDefaultTls) { + // check trust all (open) first, in case they provided both + // PROP_SECURE (secure) and PROP_OPENTLS (opentls) + if (useTrustAllTls) { try { - this.sslContext = SSLContext.getDefault(); + this.sslContext = SSLUtils.createTrustAllTlsContext(); } - catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("Unable to create default SSL context", e); + catch (GeneralSecurityException e) { + throw new IllegalStateException("Unable to create SSL context", e); } } - else if (useTrustAllTls) { + else if (useDefaultTls) { try { - this.sslContext = SSLUtils.createTrustAllTlsContext(); + this.sslContext = SSLContext.getDefault(); } - catch (GeneralSecurityException e) { - throw new IllegalStateException("Unable to create SSL context", e); + catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("Unable to create default SSL context", e); } } } From 8d0c08eb321a021b0452dfc40cab3e01a5a7d698 Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 13:39:53 -0400 Subject: [PATCH 6/8] properties builder setter method, better tests --- src/main/java/io/nats/client/Options.java | 61 +++++---- .../java/io/nats/client/OptionsTests.java | 116 +++++++++++++----- .../options_coverage_with_prefix.properties | 14 +-- ...coverage_with_prefix_underscore.properties | 14 +-- ...options_coverage_without_prefix.properties | 16 +-- ...erage_without_prefix_underscore.properties | 16 +-- 6 files changed, 149 insertions(+), 88 deletions(-) diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index 72b2a9251..d5336dcfc 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -697,10 +697,23 @@ public Builder() {} * @param props the {@link Properties} object */ public Builder(Properties props) throws IllegalArgumentException { + properties(props); + } + + // ---------------------------------------------------------------------------------------------------- + // BUILDER METHODS + // ---------------------------------------------------------------------------------------------------- + + /** + * Add settings defined in the properties object + * @param props the properties object + * @throws IllegalArgumentException if the properties object is null + * @return the Builder for chaining + */ + public Builder properties(Properties props) { if (props == null) { throw new IllegalArgumentException("Properties cannot be null"); } - stringProperty(props, PROP_URL, this::server); stringProperty(props, PROP_SERVERS, str -> { String[] servers = str.trim().split(",\\s*"); @@ -710,8 +723,8 @@ public Builder(Properties props) throws IllegalArgumentException { charArrayProperty(props, PROP_USERNAME, ca -> this.username = ca); charArrayProperty(props, PROP_PASSWORD, ca -> this.password = ca); charArrayProperty(props, PROP_TOKEN, ca -> this.token = ca); - booleanIfTrueProperty(props, PROP_SECURE, alwaysTrue -> this.useDefaultTls = true); - booleanIfTrueProperty(props, PROP_OPENTLS, alwaysTrue -> this.useTrustAllTls = true); + booleanProperty(props, PROP_SECURE, b -> this.useDefaultTls = b); + booleanProperty(props, PROP_OPENTLS, b -> this.useTrustAllTls = b); stringProperty(props, PROP_CREDENTIAL_PATH, s -> this.credentialPath = s); stringProperty(props, PROP_KEYSTORE_PATH, s -> this.keystorePath = s); @@ -722,16 +735,16 @@ public Builder(Properties props) throws IllegalArgumentException { stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); - booleanIfTrueProperty(props, PROP_NORANDOMIZE, alwaysTrue -> this.noRandomize = true); - booleanIfTrueProperty(props, PROP_NO_RESOLVE_HOSTNAMES, alwaysTrue -> this.noResolveHostnames = true); - booleanIfTrueProperty(props, PROP_REPORT_NO_RESPONDERS, alwaysTrue -> this.reportNoResponders = true); + booleanProperty(props, PROP_NORANDOMIZE, b -> this.noRandomize = b); + booleanProperty(props, PROP_NO_RESOLVE_HOSTNAMES, b -> this.noResolveHostnames = b); + booleanProperty(props, PROP_REPORT_NO_RESPONDERS, b -> this.reportNoResponders = b); stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); - booleanIfTrueProperty(props, PROP_VERBOSE, alwaysTrue -> this.verbose = true); - booleanIfTrueProperty(props, PROP_NO_ECHO, alwaysTrue -> this.noEcho = true); - booleanIfTrueProperty(props, PROP_NO_HEADERS, alwaysTrue -> this.noHeaders = true); - booleanIfTrueProperty(props, PROP_NO_NORESPONDERS, alwaysTrue -> this.noNoResponders = true); - booleanIfTrueProperty(props, PROP_PEDANTIC, alwaysTrue -> this.pedantic = true); + booleanProperty(props, PROP_VERBOSE, b -> this.verbose = b); + booleanProperty(props, PROP_NO_ECHO, b -> this.noEcho = b); + booleanProperty(props, PROP_NO_HEADERS, b -> this.noHeaders = b); + booleanProperty(props, PROP_NO_NORESPONDERS, b -> this.noNoResponders = b); + booleanProperty(props, PROP_PEDANTIC, b -> this.pedantic = b); intProperty(props, PROP_MAX_RECONNECT, DEFAULT_MAX_RECONNECT, i -> this.maxReconnect = i); durationProperty(props, PROP_RECONNECT_WAIT, DEFAULT_RECONNECT_WAIT, d -> this.reconnectWait = d); @@ -744,7 +757,7 @@ public Builder(Properties props) throws IllegalArgumentException { durationProperty(props, PROP_PING_INTERVAL, DEFAULT_PING_INTERVAL, d -> this.pingInterval = d); durationProperty(props, PROP_CLEANUP_INTERVAL, DEFAULT_REQUEST_CLEANUP_INTERVAL, d -> this.requestCleanupInterval = d); intProperty(props, PROP_MAX_PINGS, DEFAULT_MAX_PINGS_OUT, i -> this.maxPingsOut = i); - booleanIfTrueProperty(props, PROP_USE_OLD_REQUEST_STYLE, alwaysTrue -> this.useOldRequestStyle = true); + booleanProperty(props, PROP_USE_OLD_REQUEST_STYLE, b -> this.useOldRequestStyle = b); classnameProperty(props, PROP_ERROR_LISTENER, o -> this.errorListener = (ErrorListener) o); classnameProperty(props, PROP_CONNECTION_CB, o -> this.connectionListener = (ConnectionListener) o); @@ -752,15 +765,13 @@ public Builder(Properties props) throws IllegalArgumentException { stringProperty(props, PROP_DATA_PORT_TYPE, s -> this.dataPortType = s); stringProperty(props, PROP_INBOX_PREFIX, this::inboxPrefix); intGtEqZeroProperty(props, PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, i -> this.maxMessagesInOutgoingQueue = i); - booleanIfTrueProperty(props, PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, alwaysTrue -> this.discardMessagesWhenOutgoingQueueFull = true); + booleanProperty(props, PROP_DISCARD_MESSAGES_WHEN_OUTGOING_QUEUE_FULL, b -> this.discardMessagesWhenOutgoingQueueFull = b); - booleanIfTrueProperty(props, PROP_IGNORE_DISCOVERED_SERVERS, alwaysTrue -> this.ignoreDiscoveredServers = true); + booleanProperty(props, PROP_IGNORE_DISCOVERED_SERVERS, b -> this.ignoreDiscoveredServers = b); classnameProperty(props, PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, o -> this.serverPool = (ServerPool) o); + return this; } - // ---------------------------------------------------------------------------------------------------- - // BUILDER METHODS - // ---------------------------------------------------------------------------------------------------- /** * Add a server to the list of known servers. * @@ -1124,7 +1135,7 @@ public Builder reconnectJitterTls(Duration time) { * @return the Builder for chaining */ public Builder maxControlLine(int bytes) { - this.maxControlLine = bytes; + this.maxControlLine = bytes < 0 ? DEFAULT_MAX_CONTROL_LINE : bytes; return this; } @@ -1387,7 +1398,9 @@ public Builder dataPortType(String dataPortClassName) { * @return the Builder for chaining */ public Builder maxMessagesInOutgoingQueue(int maxMessagesInOutgoingQueue) { - this.maxMessagesInOutgoingQueue = maxMessagesInOutgoingQueue; + this.maxMessagesInOutgoingQueue = maxMessagesInOutgoingQueue < 0 + ? DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE + : maxMessagesInOutgoingQueue; return this; } @@ -2168,19 +2181,17 @@ private static void stringProperty(Properties props, String key, java.util.funct } } - private static void booleanIfTrueProperty(Properties props, String key, java.util.function.Consumer consumer) { + private static void charArrayProperty(Properties props, String key, java.util.function.Consumer consumer) { String value = getPropertyValue(props, key); if (value != null) { - if (Boolean.parseBoolean(value)) { - consumer.accept(true); - } + consumer.accept(value.toCharArray()); } } - private static void charArrayProperty(Properties props, String key, java.util.function.Consumer consumer) { + private static void booleanProperty(Properties props, String key, java.util.function.Consumer consumer) { String value = getPropertyValue(props, key); if (value != null) { - consumer.accept(value.toCharArray()); + consumer.accept(Boolean.parseBoolean(value)); } } diff --git a/src/test/java/io/nats/client/OptionsTests.java b/src/test/java/io/nats/client/OptionsTests.java index 694c061c6..4cbebdc01 100644 --- a/src/test/java/io/nats/client/OptionsTests.java +++ b/src/test/java/io/nats/client/OptionsTests.java @@ -20,6 +20,7 @@ import io.nats.client.support.HttpRequest; import io.nats.client.support.NatsUri; import io.nats.client.utils.CloseOnUpgradeAttempt; +import io.nats.client.utils.CoverageServerPool; import io.nats.client.utils.ResourceUtils; import org.junit.jupiter.api.Test; @@ -34,6 +35,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import static io.nats.client.Options.DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE; import static io.nats.client.support.NatsConstants.DEFAULT_PORT; import static org.junit.jupiter.api.Assertions.*; @@ -83,7 +85,7 @@ private static void _testDefaultOptions(Options o) { assertEquals(Options.DEFAULT_MAX_RECONNECT, o.getMaxReconnect(), "default max reconnect"); assertEquals(Options.DEFAULT_MAX_PINGS_OUT, o.getMaxPingsOut(), "default ping max"); assertEquals(Options.DEFAULT_RECONNECT_BUF_SIZE, o.getReconnectBufferSize(), "default reconnect buffer size"); - assertEquals(Options.DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, o.getMaxMessagesInOutgoingQueue(), + assertEquals(DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, o.getMaxMessagesInOutgoingQueue(), "default max messages in outgoing queue"); assertEquals(Options.DEFAULT_RECONNECT_WAIT, o.getReconnectWait(), "default reconnect wait"); @@ -350,64 +352,112 @@ public void testDeprecated() { } @Test - public void testPropertiesCoverageOptions() throws Exception { + public void testProperties() throws Exception { Properties props = new Properties(); - props.setProperty(Options.PROP_SECURE, "false"); - props.setProperty(Options.PROP_OPENTLS, "false"); - props.setProperty(Options.PROP_NO_HEADERS, "true"); - props.setProperty(Options.PROP_NO_NORESPONDERS, "true"); - props.setProperty(Options.PROP_RECONNECT_JITTER, "1000"); - props.setProperty(Options.PROP_RECONNECT_JITTER_TLS, "2000"); - props.setProperty(Options.PROP_IGNORE_DISCOVERED_SERVERS, "true"); - props.setProperty(Options.PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, "io.nats.client.utils.CoverageServerPool"); - props.setProperty(Options.PROP_NO_RESOLVE_HOSTNAMES, "true"); - Options o = new Options.Builder(props).build(); - _testPropertiesCoverageOptions(o); - _testPropertiesCoverageOptions(new Options.Builder(o).build()); + // stringProperty + props.setProperty(Options.PROP_CONNECTION_NAME, "name"); - props = new Properties(); - props.setProperty(Options.PROP_SECURE, "false"); - props.setProperty(Options.PROP_OPENTLS, "false"); - props.setProperty(Options.PROP_NO_HEADERS, "true"); - props.setProperty(Options.PROP_NO_NORESPONDERS, "true"); - props.setProperty(Options.PROP_RECONNECT_JITTER, "1000"); - props.setProperty(Options.PROP_RECONNECT_JITTER_TLS, "2000"); - props.setProperty(Options.PROP_IGNORE_DISCOVERED_SERVERS_PREFERRED, "true"); - props.setProperty(Options.PROP_SERVERS_POOL_IMPLEMENTATION_CLASS_PREFERRED, "io.nats.client.utils.CoverageServerPool"); - props.setProperty(Options.PROP_NO_RESOLVE_HOSTNAMES, "true"); + // stringProperty builds an auth handler + props.setProperty(Options.PROP_CREDENTIAL_PATH, "src/test/resources/jwt_nkey/test.creds"); - o = new Options.Builder(props).build(); - _testPropertiesCoverageOptions(o); - _testPropertiesCoverageOptions(new Options.Builder(o).build()); + // charArrayProperty + props.setProperty(Options.PROP_USERNAME, "user"); + + // intProperty + props.setProperty(Options.PROP_MAX_RECONNECT, "10"); + + // intGtEqZeroProperty + props.setProperty(Options.PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, "11"); + + // longProperty + props.setProperty(Options.PROP_RECONNECT_BUF_SIZE, "2999999999"); + + // durationProperty + props.setProperty(Options.PROP_PING_INTERVAL, "1000"); + + // classnameProperty + props.setProperty(Options.PROP_SERVERS_POOL_IMPLEMENTATION_CLASS, "io.nats.client.utils.CoverageServerPool"); + + Options o = new Options.Builder(props).build(); + _testProperties(o); props = new Properties(); props.load(ResourceUtils.resourceAsInputStream("options_coverage_with_prefix.properties")); o = new Options.Builder(props).build(); - _testPropertiesCoverageOptions(o); + _testProperties(o); props = new Properties(); props.load(ResourceUtils.resourceAsInputStream("options_coverage_with_prefix_underscore.properties")); o = new Options.Builder(props).build(); - _testPropertiesCoverageOptions(o); + _testProperties(o); props = new Properties(); props.load(ResourceUtils.resourceAsInputStream("options_coverage_without_prefix.properties")); o = new Options.Builder(props).build(); - _testPropertiesCoverageOptions(o); + _testProperties(o); props = new Properties(); props.load(ResourceUtils.resourceAsInputStream("options_coverage_without_prefix_underscore.properties")); o = new Options.Builder(props).build(); + _testProperties(o); + + // intGtEqZeroProperty not gt zero gives default + props.setProperty(Options.PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, "-1"); + o = new Options.Builder(props).build(); + assertEquals(DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, o.getMaxMessagesInOutgoingQueue()); + + // last one wins + props.setProperty(Options.PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, "500"); + o = new Options.Builder(props) + .maxMessagesInOutgoingQueue(1000) + .build(); + assertEquals(1000, o.getMaxMessagesInOutgoingQueue()); + + o = new Options.Builder() + .maxMessagesInOutgoingQueue(1000) + .properties(props) + .build(); + assertEquals(500, o.getMaxMessagesInOutgoingQueue()); + } + + private static void _testProperties(Options o) { + assertEquals("name", o.getConnectionName()); + assertNotNull(o.getUsernameChars()); + assertEquals("user", new String(o.getUsernameChars())); + assertEquals(10, o.getMaxReconnect()); + assertEquals(11, o.getMaxMessagesInOutgoingQueue()); + assertEquals(2999999999L, o.getReconnectBufferSize()); + assertEquals(1000, o.getPingInterval().toMillis()); + assertNotNull(o.getAuthHandler()); + assertNotNull(o.getServerPool()); + assertTrue(o.getServerPool() instanceof CoverageServerPool); + } + + @Test + public void testPropertiesCoverageOptions() throws Exception { + Properties props = new Properties(); + props.setProperty(Options.PROP_SECURE, "false"); + props.setProperty(Options.PROP_OPENTLS, "false"); + props.setProperty(Options.PROP_NO_HEADERS, "true"); + props.setProperty(Options.PROP_NO_NORESPONDERS, "true"); + props.setProperty(Options.PROP_RECONNECT_JITTER, "1000"); + props.setProperty(Options.PROP_RECONNECT_JITTER_TLS, "2000"); + props.setProperty(Options.PROP_CLIENT_SIDE_LIMIT_CHECKS, "true"); // deprecated + props.setProperty(Options.PROP_IGNORE_DISCOVERED_SERVERS, "true"); + props.setProperty(Options.PROP_NO_RESOLVE_HOSTNAMES, "true"); + + Options o = new Options.Builder(props).build(); _testPropertiesCoverageOptions(o); + _testPropertiesCoverageOptions(new Options.Builder(o).build()); } private static void _testPropertiesCoverageOptions(Options o) { - assertNull(o.getSslContext(), "property context"); + assertNull(o.getSslContext()); assertTrue(o.isNoHeaders()); assertTrue(o.isNoNoResponders()); + assertFalse(o.clientSideLimitChecks()); // clientSideLimitChecks is deprecated and always returns false assertTrue(o.isIgnoreDiscoveredServers()); - assertNotNull(o.getServerPool()); assertTrue(o.isNoResolveHostnames()); } @@ -458,7 +508,7 @@ private static void _testDefaultPropertyIntOptions(Options o) { assertEquals(Options.DEFAULT_PING_INTERVAL, o.getPingInterval(), "default ping interval"); assertEquals(Options.DEFAULT_REQUEST_CLEANUP_INTERVAL, o.getRequestCleanupInterval(), "default cleanup interval"); - assertEquals(Options.DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, o.getMaxMessagesInOutgoingQueue(), + assertEquals(DEFAULT_MAX_MESSAGES_IN_OUTGOING_QUEUE, o.getMaxMessagesInOutgoingQueue(), "default max messages in outgoing queue"); } diff --git a/src/test/resources/options_coverage_with_prefix.properties b/src/test/resources/options_coverage_with_prefix.properties index cb79e2f7f..73ae238b7 100644 --- a/src/test/resources/options_coverage_with_prefix.properties +++ b/src/test/resources/options_coverage_with_prefix.properties @@ -1,11 +1,11 @@ # All keys have prefix "io.nats.client." # All keys in dot format -io.nats.client.secure=false -io.nats.client.opentls=false -io.nats.client.noheaders=true -io.nats.client.nonoresponders=true -io.nats.client.reconnect.jitter=1000 -io.nats.client.reconnect.jitter.tls=2000 +io.nats.client.name=name +io.nats.client.credential.path=src/test/resources/jwt_nkey/test.creds +io.nats.client.username=user +io.nats.client.reconnect.max=10 +io.nats.client.outgoingqueue.maxmessages=11 +io.nats.client.reconnect.buffer.size=2999999999 +io.nats.client.pinginterval=1000 io.nats.client.ignore.discovered.servers=true io.nats.client.servers.pool.implementation.class=io.nats.client.utils.CoverageServerPool -io.nats.client.noResolveHostnames=true diff --git a/src/test/resources/options_coverage_with_prefix_underscore.properties b/src/test/resources/options_coverage_with_prefix_underscore.properties index e36d9ad81..8895653a3 100644 --- a/src/test/resources/options_coverage_with_prefix_underscore.properties +++ b/src/test/resources/options_coverage_with_prefix_underscore.properties @@ -1,11 +1,11 @@ # All keys have prefix "io.nats.client." # Some keys in underscore format -io.nats.client.secure=false -io.nats.client.opentls=false -io.nats.client.noheaders=true -io.nats.client.nonoresponders=true -io.nats.client.reconnect.jitter=1000 -io.nats.client.reconnect.jitter.tls=2000 +io.nats.client.name=name +io.nats.client.credential.path=src/test/resources/jwt_nkey/test.creds +io.nats.client.username=user +io.nats.client.reconnect.max=10 +io.nats.client.outgoingqueue.maxmessages=11 +io.nats.client.reconnect.buffer.size=2999999999 +io.nats.client.pinginterval=1000 io.nats.client.ignore_discovered_servers=true io.nats.client.servers_pool_implementation_class=io.nats.client.utils.CoverageServerPool -io.nats.client.noResolveHostnames=true diff --git a/src/test/resources/options_coverage_without_prefix.properties b/src/test/resources/options_coverage_without_prefix.properties index fe04be239..27b22fcff 100644 --- a/src/test/resources/options_coverage_without_prefix.properties +++ b/src/test/resources/options_coverage_without_prefix.properties @@ -1,11 +1,11 @@ -# No keys have prefix "io.nats.client." +# No keys have prefix "" # All keys in dot format -secure=false -opentls=false -noheaders=true -nonoresponders=true -reconnect.jitter=1000 -reconnect.jitter.tls=2000 +name=name +credential.path=src/test/resources/jwt_nkey/test.creds +username=user +reconnect.max=10 +outgoingqueue.maxmessages=11 +reconnect.buffer.size=2999999999 +pinginterval=1000 ignore.discovered.servers=true servers.pool.implementation.class=io.nats.client.utils.CoverageServerPool -noResolveHostnames=true diff --git a/src/test/resources/options_coverage_without_prefix_underscore.properties b/src/test/resources/options_coverage_without_prefix_underscore.properties index 961bb8a10..4f8a7f7d3 100644 --- a/src/test/resources/options_coverage_without_prefix_underscore.properties +++ b/src/test/resources/options_coverage_without_prefix_underscore.properties @@ -1,11 +1,11 @@ -# No keys have prefix "io.nats.client." +# No keys have prefix "" # Some keys in underscore format -secure=false -opentls=false -noheaders=true -nonoresponders=true -reconnect.jitter=1000 -reconnect.jitter.tls=2000 +name=name +credential.path=src/test/resources/jwt_nkey/test.creds +username=user +reconnect.max=10 +outgoingqueue.maxmessages=11 +reconnect.buffer.size=2999999999 +pinginterval=1000 ignore_discovered_servers=true servers_pool_implementation_class=io.nats.client.utils.CoverageServerPool -noResolveHostnames=true From 86299cf471a580fa598f53d5dbae34cb3bb7592d Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 14:51:57 -0400 Subject: [PATCH 7/8] updated README.md --- README.md | 90 ++++++++++- src/main/java/io/nats/client/Options.java | 144 +++++++++--------- src/test/java/io/nats/client/AuthTests.java | 9 +- .../java/io/nats/client/TestSSLUtils.java | 4 +- 4 files changed, 168 insertions(+), 79 deletions(-) diff --git a/README.md b/README.md index ff717a978..90893abc5 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ The Services Framework further streamlines their development by providing observ Check out the [ServiceExample](src/examples/java/io/nats/examples/service/ServiceExample.java) -## Versions Specific Notes +### Versions Specific Notes This is version 2.x of the java-nats library. This version is a ground up rewrite of the original library. Part of the goal of this re-write was to address the excessive use of threads, we created a Dispatcher construct to allow applications to control thread creation more intentionally. This version also removes all non-JDK runtime dependencies. @@ -50,7 +50,16 @@ Version 2.5.0 adds some back pressure to publish calls to alleviate issues when Previous versions are still available in the repo. -### Version 2.16.12 Max Payload Check +#### Version 2.16.14 Options properties improvements + +In this release, support was added to +* support properties keys with or without the prefix 'io.nats.client.' +* allow creation of connections requiring an AuthHandler for JWT to specify the credentials file in a properties files, instead of needing to provide an instance of AuthHandler in code. +* allow creation of connections requiring an SSL context to specify key and trust store information in a properties files so an SSLContext can be created automatically instead of needing to provide an instance of an SSLContext in code. + +For details on the other features, see the "Options" sections + +#### Version 2.16.12 Max Payload Check As of version 2.16.12, there is no longer client side checking 1. that a message payload is less than the server configuration (Core and JetStream publishes) @@ -61,7 +70,7 @@ Please see unit test for examples of this behavior. and `testMaxPayloadJs` in [JetStreamPubTests.cs](src/test/java/io/nats/client/impl/JetStreamPubTests.java) -### Version 2.16.0 Consumer Create +#### Version 2.16.0 Consumer Create This release by default will use a new JetStream consumer create API when interacting with nats-server version 2.9.0 or higher. This changes the subjects used by the client to create consumers, which might in some cases require changes in access and import/export configuration. @@ -79,6 +88,77 @@ ObjectStore os = connection.objectStore("bucket", ObjectStoreOptions.builder(jso ObjectStoreManagement osm = connection.objectStoreManagement(ObjectStoreOptions.builder(jso).build()); ``` + +### Options + +#### Properties with or without prefix... + +The `io.nats.client.` prefix is not required in the properties file anymore. These are now equivalent: +```properties +io.nats.client.servers=nats://localhost:4222 +``` +```properties +servers=nats://localhost:4222 +``` + +#### AuthHandler / JWT +In previous versions the user would have to manually create the AuthHandler and set it in the options +```java +AuthHandler ah = Nats.credentials("path/to/my.creds"); +Options options = new Options.Builder() + .authHandler(ah) + .build(); +``` + +The developer can now set the file path directly and an AuthHandler will be created: +```java +Options options = new Options.Builder() + .credentialPath("path/to/my.creds") + .build(); +``` +The developer can also set the credential path in a properties file: +```properties +io.nats.client.credential.path=path/to/my.creds +``` + +#### Options - SSLContext + +The Options builder has several options which affect creation or use of an `SSLContext` +```java +public Builder sslContext(SSLContext ctx) + +// Generic SSL Creation +public Builder secure() +public Builder opentls() + +// Specific SSL Creation Properties +public Builder keystore(String keystore) +public Builder keystorePassword(char[] keystorePassword) +public Builder truststore(String truststore) +public Builder truststorePassword(char[] truststorePassword) +public Builder tlsAlgorithm(String tlsAlgorithm) +``` + +There are equivalent properties for these builder methods (except sslContext): +```properties +# Generic SSL Creation +io.nats.client.secure=true +io.nats.client.opentls=true + +# Specific SSL Creation Properties +io.nats.client.keyStore=path/to/keystore.jks +io.nats.client.keyStorePassword=kspassword +io.nats.client.trustStore=path/to/truststore.jks +io.nats.client.trustStorePassword=tspassword +io.nats.client.tls.algorithm=SunX509 +``` + +When options are built, the ssl context will be accepted or created in the following order. +1. If it's directly provided via the builder `sslContext(SSLContext ctx)` method. +2. If `keyStore` is provided, one will be created using all "Specific SSL Creation Properties". The default tls algorithm, if not supplied, is `SunX509` +3. If `opentls` is true or any of the bootstrap servers has `opentls` as their scheme, a generic SSLContext will be created that **trusts all certs**. +4. If `secure` is true or any of the bootstrap servers has `tls` or `wss`, the `javax.net.ssl.SSLContext.getDefault()` will be used. + ### SSL/TLS Performance After recent tests we realized that TLS performance is lower than we would like. After researching the problem and possible solutions we came to a few conclusions: @@ -105,9 +185,9 @@ For more information, see the Oracle Java documentation page on [Client-Driven O Also, there is a detailed [OCSP Example](https://github.com/nats-io/java-nats-examples/tree/main/ocsp) that shows how to create SSL contexts enabling OCSP stapling. -### UTF-8 Subjects +### Subject Validation -The client protocol spec doesn't explicitly state the encoding on subjects. Some clients use ASCII and some use UTF-8 which matches ASCII for a-Z and 0-9. Until 2.1.2 the 2.0+ version of the Java client used ASCII for performance reasons. As of 2.1.2 you can choose to support UTF-8 subjects via the Options. Keep in mind that there is a small performance penalty for UTF-8 encoding and decoding in benchmarks, but depending on your application this cost may be negligible. Also, keep in mind that not all clients support UTF-8 and test accordingly. +The current version of this client supports subjects with ASCII printable characters and wildcards where appropriate. ### NKey-based Challenge Response Authentication diff --git a/src/main/java/io/nats/client/Options.java b/src/main/java/io/nats/client/Options.java index d5336dcfc..5a3f7725d 100644 --- a/src/main/java/io/nats/client/Options.java +++ b/src/main/java/io/nats/client/Options.java @@ -389,22 +389,18 @@ public class Options { * {@link Builder#serverPool(ServerPool) serverPool}. */ public static final String PROP_SERVERS_POOL_IMPLEMENTATION_CLASS_PREFERRED = "servers.pool.implementation.class"; - /** - * Property used to set the path to a credentials file to be used in a FileAuthHandler - */ - public static final String PROP_CREDENTIAL_PATH = PFX + "credential.path"; /** * Property for the keystore path used to create an SSLContext */ - public static final String PROP_KEYSTORE_PATH = PFX + "keyStore"; - /** - * Property for the truststore path used to create an SSLContext - */ - public static final String PROP_TRUSTSTORE_PATH = PFX + "trustStore"; + public static final String PROP_KEYSTORE = PFX + "keyStore"; /** * Property for the keystore password used to create an SSLContext */ public static final String PROP_KEYSTORE_PASSWORD = PFX + "keyStorePassword"; + /** + * Property for the truststore path used to create an SSLContext + */ + public static final String PROP_TRUSTSTORE = PFX + "trustStore"; /** * Property for the truststore password used to create an SSLContext */ @@ -413,6 +409,10 @@ public class Options { * Property for the algorithm used to create an SSLContext */ public static final String PROP_TLS_ALGORITHM = PFX + "tls.algorithm"; + /** + * Property used to set the path to a credentials file to be used in a FileAuthHandler + */ + public static final String PROP_CREDENTIAL_PATH = PFX + "credential.path"; /** * Property used to configure a builder from a Properties object. {@value}, see {@link Builder#clientSideLimitChecks() clientSideLimitChecks}. * @deprecated Client Side Limit checks are no longer performed. @@ -668,12 +668,12 @@ public static class Builder { private boolean useDefaultTls; private boolean useTrustAllTls; - private String credentialPath; - private String keystorePath; - private String truststorePath; + private String keystore; private char[] keystorePassword; + private String truststore; private char[] truststorePassword; private String tlsAlgorithm = DEFAULT_TLS_ALGORITHM; + private String credentialPath; /** * Constructs a new Builder with the default values. @@ -726,13 +726,14 @@ public Builder properties(Properties props) { booleanProperty(props, PROP_SECURE, b -> this.useDefaultTls = b); booleanProperty(props, PROP_OPENTLS, b -> this.useTrustAllTls = b); - stringProperty(props, PROP_CREDENTIAL_PATH, s -> this.credentialPath = s); - stringProperty(props, PROP_KEYSTORE_PATH, s -> this.keystorePath = s); + stringProperty(props, PROP_KEYSTORE, s -> this.keystore = s); charArrayProperty(props, PROP_KEYSTORE_PASSWORD, ca -> this.keystorePassword = ca); - stringProperty(props, PROP_TRUSTSTORE_PATH, s -> this.truststorePath = s); + stringProperty(props, PROP_TRUSTSTORE, s -> this.truststore = s); charArrayProperty(props, PROP_TRUSTSTORE_PASSWORD, ca -> this.truststorePassword = ca); stringProperty(props, PROP_TLS_ALGORITHM, s -> this.tlsAlgorithm = s); + stringProperty(props, PROP_CREDENTIAL_PATH, s -> this.credentialPath = s); + stringProperty(props, PROP_CONNECTION_NAME, s -> this.connectionName = s); booleanProperty(props, PROP_NORANDOMIZE, b -> this.noRandomize = b); @@ -996,61 +997,61 @@ public Builder sslContext(SSLContext ctx) { /** * - * @param credentialPath the path to the credentials file for creating an {@link AuthHandler AuthHandler} + * @param keystore the path to the keystore file * @return the Builder for chaining */ - public Builder credentialPath(String credentialPath) { - this.credentialPath = emptyAsNull(credentialPath); + public Builder keystorePath(String keystore) { + this.keystore = emptyAsNull(keystore); return this; } /** * - * @param keystorePath the path to the keystore file + * @param keystorePassword the password for the keystore * @return the Builder for chaining */ - public Builder keystorePath(String keystorePath) { - this.keystorePath = emptyAsNull(keystorePath); + public Builder keystorePassword(char[] keystorePassword) { + this.keystorePassword = keystorePassword == null || keystorePassword.length == 0 ? null : keystorePassword; return this; } /** * - * @param truststorePath the path to the trust store file + * @param truststore the path to the trust store file * @return the Builder for chaining */ - public Builder truststorePath(String truststorePath) { - this.truststorePath = emptyAsNull(truststorePath); + public Builder truststorePath(String truststore) { + this.truststore = emptyAsNull(truststore); return this; } /** * - * @param keystorePassword the password for the keystore + * @param truststorePassword the password for the trust store * @return the Builder for chaining */ - public Builder keystorePassword(char[] keystorePassword) { - this.keystorePassword = keystorePassword == null || keystorePassword.length == 0 ? null : keystorePassword; + public Builder truststorePassword(char[] truststorePassword) { + this.truststorePassword = truststorePassword == null || truststorePassword.length == 0 ? null : truststorePassword; return this; } /** * - * @param truststorePassword the password for the trust store + * @param tlsAlgorithm the tls algorithm. Default is {@value SSLUtils#DEFAULT_TLS_ALGORITHM} * @return the Builder for chaining */ - public Builder truststorePassword(char[] truststorePassword) { - this.truststorePassword = truststorePassword == null || truststorePassword.length == 0 ? null : truststorePassword; + public Builder tlsAlgorithm(String tlsAlgorithm) { + this.tlsAlgorithm = emptyOrNullAs(tlsAlgorithm, DEFAULT_TLS_ALGORITHM); return this; } /** * - * @param tlsAlgorithm the tls algorithm. Default is {@value SSLUtils#DEFAULT_TLS_ALGORITHM} + * @param credentialPath the path to the credentials file for creating an {@link AuthHandler AuthHandler} * @return the Builder for chaining */ - public Builder tlsAlgorithm(String tlsAlgorithm) { - this.tlsAlgorithm = emptyOrNullAs(tlsAlgorithm, DEFAULT_TLS_ALGORITHM); + public Builder credentialPath(String credentialPath) { + this.credentialPath = emptyAsNull(credentialPath); return this; } @@ -1469,51 +1470,52 @@ public Options build() throws IllegalStateException { checkUrisForSecure = false; } - if (keystorePath != null) { - try { - sslContext = SSLUtils.createSSLContext(keystorePath, keystorePassword, truststorePath, truststorePassword, tlsAlgorithm); - } - catch (Exception e) { - throw new IllegalStateException("Unable to create SSL context", e); - } - } - - // if an sslContext has not been requested via keystore properties if (sslContext == null) { - // if we haven't been told to use the default or the trust all context - // and the server isn't the default url, check to see if the server uris - // suggest we need the ssl context. - if (!useDefaultTls && !useTrustAllTls && checkUrisForSecure) { - for (int i = 0; sslContext == null && i < natsServerUris.size(); i++) { - NatsUri natsUri = natsServerUris.get(i); - switch (natsUri.getScheme()) { - case TLS_PROTOCOL: - case SECURE_WEBSOCKET_PROTOCOL: - useDefaultTls = true; - break; - case OPENTLS_PROTOCOL: - useTrustAllTls = true; - break; - } - } - } - - // check trust all (open) first, in case they provided both - // PROP_SECURE (secure) and PROP_OPENTLS (opentls) - if (useTrustAllTls) { + // ssl context can be directly provided, but if it's not + if (keystore != null) { try { - this.sslContext = SSLUtils.createTrustAllTlsContext(); + sslContext = SSLUtils.createSSLContext(keystore, keystorePassword, truststore, truststorePassword, tlsAlgorithm); } - catch (GeneralSecurityException e) { + catch (Exception e) { throw new IllegalStateException("Unable to create SSL context", e); } } - else if (useDefaultTls) { - try { - this.sslContext = SSLContext.getDefault(); + else { // the sslContext has not been requested via keystore properties + // if we haven't been told to use the default or the trust all context + // and the server isn't the default url, check to see if the server uris + // suggest we need the ssl context. + if (!useDefaultTls && !useTrustAllTls && checkUrisForSecure) { + for (int i = 0; sslContext == null && i < natsServerUris.size(); i++) { + NatsUri natsUri = natsServerUris.get(i); + switch (natsUri.getScheme()) { + case TLS_PROTOCOL: + case SECURE_WEBSOCKET_PROTOCOL: + useDefaultTls = true; + break; + case OPENTLS_PROTOCOL: + useTrustAllTls = true; + break; + } + } } - catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("Unable to create default SSL context", e); + + // check trust all (open) first, in case they provided both + // PROP_SECURE (secure) and PROP_OPENTLS (opentls) + if (useTrustAllTls) { + try { + this.sslContext = SSLUtils.createTrustAllTlsContext(); + } + catch (GeneralSecurityException e) { + throw new IllegalStateException("Unable to create SSL context", e); + } + } + else if (useDefaultTls) { + try { + this.sslContext = SSLContext.getDefault(); + } + catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("Unable to create default SSL context", e); + } } } } diff --git a/src/test/java/io/nats/client/AuthTests.java b/src/test/java/io/nats/client/AuthTests.java index 1d2936c94..1d18680f2 100644 --- a/src/test/java/io/nats/client/AuthTests.java +++ b/src/test/java/io/nats/client/AuthTests.java @@ -424,9 +424,16 @@ public void testStaticNKeyAuth() throws Exception { @Test public void testJWTAuthWithCredsFile() throws Exception { + // manual auth handler or credential path try (NatsTestServer ts = new NatsTestServer("src/test/resources/operator.conf", false)) { Options options = new Options.Builder().server(ts.getURI()).maxReconnects(0) - .authHandler(Nats.credentials("src/test/resources/jwt_nkey/user.creds")).build(); + .authHandler(Nats.credentials("src/test/resources/jwt_nkey/user.creds")) + .build(); + assertCanConnect(options); + + options = new Options.Builder().server(ts.getURI()).maxReconnects(0) + .credentialPath("src/test/resources/jwt_nkey/user.creds") + .build(); assertCanConnect(options); } diff --git a/src/test/java/io/nats/client/TestSSLUtils.java b/src/test/java/io/nats/client/TestSSLUtils.java index 0b442981d..ac9a32695 100644 --- a/src/test/java/io/nats/client/TestSSLUtils.java +++ b/src/test/java/io/nats/client/TestSSLUtils.java @@ -34,9 +34,9 @@ public static KeyStore loadKeystore(String path) throws Exception { public static Properties createTestSSLProperties() { Properties props = new Properties(); - props.setProperty(Options.PROP_KEYSTORE_PATH, TestSSLUtils.KEYSTORE_PATH); + props.setProperty(Options.PROP_KEYSTORE, TestSSLUtils.KEYSTORE_PATH); props.setProperty(Options.PROP_KEYSTORE_PASSWORD, TestSSLUtils.PASSWORD); - props.setProperty(Options.PROP_TRUSTSTORE_PATH, TestSSLUtils.KEYSTORE_PATH); + props.setProperty(Options.PROP_TRUSTSTORE, TestSSLUtils.TRUSTSTORE_PATH); props.setProperty(Options.PROP_TRUSTSTORE_PASSWORD, TestSSLUtils.PASSWORD); return props; } From b85d3abd6b372d267abf7cc78fb9138f56607c2b Mon Sep 17 00:00:00 2001 From: scottf Date: Mon, 24 Jul 2023 14:57:38 -0400 Subject: [PATCH 8/8] updated README.md --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/README.md b/README.md index 90893abc5..5ac68fe03 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,32 @@ io.nats.client.servers=nats://localhost:4222 servers=nats://localhost:4222 ``` +#### Last one wins +The Options builder allows you to use both properties and code. When it comes to the builder, the last one called wins. +This applies to each individual property. + +```java +props.setProperty(Options.PROP_MAX_MESSAGES_IN_OUTGOING_QUEUE, "7000"); + +o = new Options.Builder() + .properties(props) + .maxMessagesInOutgoingQueue(6000) + .build(); +assertEquals(6000, o.getMaxMessagesInOutgoingQueue()); + +o = new Options.Builder() + .maxMessagesInOutgoingQueue(6000) + .properties(props) + .build(); +assertEquals(7000, o.getMaxMessagesInOutgoingQueue()); + +o = new Options.Builder() + .maxMessagesInOutgoingQueue(6000) + .maxMessagesInOutgoingQueue(8000) + .build(); + assertEquals(8000, o.getMaxMessagesInOutgoingQueue()); +``` + #### AuthHandler / JWT In previous versions the user would have to manually create the AuthHandler and set it in the options ```java