diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java index 8ed90bc8e4e..db385f1826e 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java @@ -14,8 +14,11 @@ import java.net.URL; import java.sql.DriverPropertyInfo; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; import java.util.Properties; +import java.util.Set; import java.util.TimeZone; // @@ -41,41 +44,57 @@ public class JdbcConfiguration extends ConnectionConfiguration { static final String DEBUG_OUTPUT_DEFAULT = "err"; static final String TIME_ZONE = "timezone"; - // follow the JDBC spec and use the JVM default... // to avoid inconsistency, the default is picked up once at startup and reused across connections // to cater to the principle of least surprise // really, the way to move forward is to specify a calendar or the timezone manually static final String TIME_ZONE_DEFAULT = TimeZone.getDefault().getID(); - private static final List KNOWN_OPTIONS = Arrays.asList(DEBUG, DEBUG_OUTPUT, TIME_ZONE); + // options that don't change at runtime + private static final Set OPTION_NAMES = new LinkedHashSet<>(Arrays.asList(TIME_ZONE, DEBUG, DEBUG_OUTPUT)); - private HostAndPort hostAndPort; - private String originalUrl; - private String urlFile = "/"; + // immutable properties + private final HostAndPort hostAndPort; + private final String originalUrl; + private final String urlFile; + private final boolean debug; + private final String debugOut; - private boolean debug = false; - private String debugOut = DEBUG_OUTPUT_DEFAULT; + // mutable ones private TimeZone timeZone; - public JdbcConfiguration(String u, Properties props) throws JdbcSQLException { - super(props); - originalUrl = u; - parseUrl(u); + public static JdbcConfiguration create(String u, Properties props) throws JdbcSQLException { + Object[] result = parseUrl(u); - Properties set = settings(); - debug = Boolean.parseBoolean(set.getProperty(DEBUG, DEBUG_DEFAULT)); - debugOut = settings().getProperty(DEBUG_OUTPUT, DEBUG_OUTPUT_DEFAULT); - timeZone = TimeZone.getTimeZone(settings().getProperty(TIME_ZONE, TIME_ZONE_DEFAULT)); + String urlFile = (String) result[0]; + HostAndPort hostAndPort = (HostAndPort) result[1]; + Properties urlProps = (Properties) result[2]; + + // override properties set in the URL with the ones specified programmatically + if (props != null) { + urlProps.putAll(props); + } + + try { + return new JdbcConfiguration(u, urlFile, hostAndPort, urlProps); + } catch (JdbcSQLException e) { + throw e; + } catch (Exception ex) { + throw new JdbcSQLException(ex, ex.getMessage()); + } } - private void parseUrl(String u) throws JdbcSQLException { + private static Object[] parseUrl(String u) throws JdbcSQLException { String url = u; String format = "jdbc:es://[host[:port]]*/[prefix]*[?[option=value]&]*"; if (!canAccept(u)) { throw new JdbcSQLException("Expected [" + URL_PREFIX + "] url, received [" + u +"]"); } + String urlFile = "/"; + HostAndPort destination; + Properties props = new Properties(); + try { if (u.endsWith("/")) { u = u.substring(0, u.length() - 1); @@ -141,14 +160,15 @@ public class JdbcConfiguration extends ConnectionConfiguration { String host = hostAndPort.substring(0, index); String port = hostAndPort.substring(index + 1); - this.hostAndPort = new HostAndPort(host, Integer.parseInt(port)); + destination = new HostAndPort(host, Integer.parseInt(port)); } else { - this.hostAndPort = new HostAndPort(hostAndPort); + destination = new HostAndPort(hostAndPort); } // // parse params // + if (params != null) { // parse properties List prms = StringUtils.tokenize(params, "&"); @@ -157,13 +177,8 @@ public class JdbcConfiguration extends ConnectionConfiguration { if (args.size() != 2) { throw new JdbcSQLException("Invalid parameter [" + param + "], format needs to be key=value"); } - String pName = args.get(0); - if (!KNOWN_OPTIONS.contains(pName)) { - throw new JdbcSQLException("Unknown parameter [" + pName + "] ; did you mean " + - StringUtils.findSimiliar(pName, KNOWN_OPTIONS)); - } - - settings().setProperty(args.get(0), args.get(1)); + // further validation happens in the constructor (since extra properties might be specified either way) + props.setProperty(args.get(0).trim(), args.get(1).trim()); } } } catch (JdbcSQLException e) { @@ -172,10 +187,31 @@ public class JdbcConfiguration extends ConnectionConfiguration { // Add the url to unexpected exceptions throw new IllegalArgumentException("Failed to parse acceptable jdbc url [" + u + "]", e); } + + return new Object[] { urlFile, destination, props }; + } + + // constructor is private to force the use of a factory in order to catch and convert any validation exception + // and also do input processing as oppose to handling this from the constructor (which is tricky or impossible) + private JdbcConfiguration(String u, String urlFile, HostAndPort hostAndPort, Properties props) throws JdbcSQLException { + super(props); + + this.originalUrl = u; + this.urlFile = urlFile; + this.hostAndPort = hostAndPort; + + this.debug = parseValue(DEBUG, props.getProperty(DEBUG, DEBUG_DEFAULT), Boolean::parseBoolean); + this.debugOut = props.getProperty(DEBUG_OUTPUT, DEBUG_OUTPUT_DEFAULT); + + this.timeZone = parseValue(TIME_ZONE, props.getProperty(TIME_ZONE, TIME_ZONE_DEFAULT), TimeZone::getTimeZone); + } + + @Override + protected Collection extraOptions() { + return OPTION_NAMES; } public URL asUrl() throws JdbcSQLException { - // TODO: need to assemble all the various params here try { return new URL(isSSLEnabled() ? "https" : "http", hostAndPort.ip, port(), urlFile); } catch (MalformedURLException ex) { @@ -199,8 +235,8 @@ public class JdbcConfiguration extends ConnectionConfiguration { return timeZone; } - public void timeZone(TimeZone tz) { - timeZone = tz; + public void timeZone(TimeZone timeZone) { + this.timeZone = timeZone; } public static boolean canAccept(String url) { @@ -209,7 +245,7 @@ public class JdbcConfiguration extends ConnectionConfiguration { public DriverPropertyInfo[] driverPropertyInfo() { List info = new ArrayList<>(); - for (String option : KNOWN_OPTIONS) { + for (String option : OPTION_NAMES) { String value = null; DriverPropertyInfo prop = new DriverPropertyInfo(option, value); info.add(prop); diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java index 91f2e661f44..d39a500f3e7 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java @@ -7,12 +7,10 @@ package org.elasticsearch.xpack.sql.jdbc.jdbc; import org.elasticsearch.xpack.sql.jdbc.debug.Debug; import org.elasticsearch.xpack.sql.jdbc.net.client.JdbcHttpClient; -import org.elasticsearch.xpack.sql.net.client.util.StringUtils; import java.sql.Array; import java.sql.Blob; import java.sql.CallableStatement; -import java.sql.ClientInfoStatus; import java.sql.Clob; import java.sql.Connection; import java.sql.DatabaseMetaData; @@ -33,8 +31,6 @@ import java.util.TimeZone; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; -import static java.util.Collections.singletonMap; - /** * Implementation of {@link Connection} for Elasticsearch. */ @@ -47,7 +43,6 @@ public class JdbcConnection implements Connection, JdbcWrapper { private boolean closed = false; private String catalog; private String schema; - private Properties clientInfo = new Properties(); public JdbcConnection(JdbcConfiguration connectionInfo) throws SQLException { cfg = connectionInfo; @@ -331,42 +326,36 @@ public class JdbcConnection implements Connection, JdbcWrapper { private void checkOpenClientInfo() throws SQLClientInfoException { if (isClosed()) { - throw new SQLClientInfoException(); + throw new SQLClientInfoException("Connection closed", null); } } @Override public void setClientInfo(String name, String value) throws SQLClientInfoException { checkOpenClientInfo(); - if (!StringUtils.hasText(name)) { - throw new SQLClientInfoException(singletonMap(name, ClientInfoStatus.REASON_VALUE_INVALID)); - } - if (value != null) { - clientInfo.put(name, value); - } - else { - clientInfo.remove(name); - } + // no-op + throw new SQLClientInfoException("Unsupported operation", null); } @Override public void setClientInfo(Properties properties) throws SQLClientInfoException { checkOpenClientInfo(); - clientInfo.putAll(properties); + // no-op + throw new SQLClientInfoException("Unsupported operation", null); } @Override public String getClientInfo(String name) throws SQLException { checkOpenClientInfo(); - return clientInfo.getProperty(name); + // we don't support client info - the docs indicate we should return null if properties are not supported + return null; } @Override public Properties getClientInfo() throws SQLException { checkOpenClientInfo(); - Properties clone = new Properties(); - clone.putAll(clientInfo); - return clone; + // similar to getClientInfo - return an empty object instead of an exception + return new Properties(); } @Override diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java index 6cdea586039..5d963010184 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java @@ -1124,7 +1124,7 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper { @Override public ResultSet getClientInfoProperties() throws SQLException { - throw new UnsupportedOperationException("Client info not implemented yet"); + throw new SQLException("Client info not implemented yet"); } @Override diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDriver.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDriver.java index a1373faf3dd..e7a21590805 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDriver.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDriver.java @@ -81,7 +81,7 @@ public class JdbcDriver implements java.sql.Driver, Closeable { } private static JdbcConfiguration initCfg(String url, Properties props) throws JdbcSQLException { - JdbcConfiguration ci = new JdbcConfiguration(url, props); + JdbcConfiguration ci = JdbcConfiguration.create(url, props); // if there's a timeout set on the DriverManager, make sure to use it if (DriverManager.getLoginTimeout() > 0) { @@ -100,7 +100,7 @@ public class JdbcDriver implements java.sql.Driver, Closeable { if (!acceptsURL(url)) { return new DriverPropertyInfo[0]; } - return new JdbcConfiguration(url, info).driverPropertyInfo(); + return JdbcConfiguration.create(url, info).driverPropertyInfo(); } @Override diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbcx/JdbcDataSource.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbcx/JdbcDataSource.java index 27d0f404212..cc08066d3ca 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbcx/JdbcDataSource.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbcx/JdbcDataSource.java @@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import javax.sql.DataSource; + public class JdbcDataSource implements DataSource, Wrapper, Closeable { private String url; @@ -93,7 +94,7 @@ public class JdbcDataSource implements DataSource, Wrapper, Closeable { } private Connection doGetConnection(Properties p) throws SQLException { - JdbcConfiguration cfg = new JdbcConfiguration(url, p); + JdbcConfiguration cfg = JdbcConfiguration.create(url, p); if (loginTimeout > 0) { cfg.connectTimeout(TimeUnit.SECONDS.toMillis(loginTimeout)); } diff --git a/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java b/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java index d17b29ae92b..5abee679d2c 100644 --- a/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java +++ b/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java @@ -15,7 +15,7 @@ import static org.hamcrest.Matchers.is; public class JdbcConfigurationTests extends ESTestCase { private JdbcConfiguration ci(String url) throws SQLException { - return new JdbcConfiguration(url, null); + return JdbcConfiguration.create(url, null); } public void testJustThePrefix() throws Exception { diff --git a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ConnectionConfiguration.java b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ConnectionConfiguration.java index 1912ea9b18c..bc4d0912af3 100644 --- a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ConnectionConfiguration.java +++ b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ConnectionConfiguration.java @@ -5,9 +5,28 @@ */ package org.elasticsearch.xpack.sql.net.client; -import java.util.Properties; -import java.util.concurrent.TimeUnit; +import org.elasticsearch.xpack.sql.net.client.util.StringUtils; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Enumeration; +import java.util.LinkedHashSet; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; + +import static java.util.Collections.emptyList; + +/** + * Common configuration class used for client. + * Uses a Properties object to be created (as clients would use strings to configure it). + * While this is convenient, it makes validation tricky (of both the names and values) and thus + * it's available only during construction. + * Some values might be updated later on in a typed fashion (dedicated method) in order + * to move away from the loose Strings... + */ public class ConnectionConfiguration { public static class HostAndPort { @@ -55,9 +74,15 @@ public class ConnectionConfiguration { public static final String AUTH_USER = "user"; public static final String AUTH_PASS = "pass"; - // Proxy + protected static final Set OPTION_NAMES = new LinkedHashSet<>( + Arrays.asList(CONNECT_TIMEOUT, NETWORK_TIMEOUT, QUERY_TIMEOUT, PAGE_TIMEOUT, PAGE_SIZE)); - private final Properties settings; + static { + OPTION_NAMES.addAll(SslConfig.OPTION_NAMES); + OPTION_NAMES.addAll(ProxyConfig.OPTION_NAMES); + } + + // Proxy private long connectTimeout; private long networkTimeout; @@ -68,19 +93,20 @@ public class ConnectionConfiguration { private final String user, pass; - private final SslConfig sslConfig; private final ProxyConfig proxyConfig; - public ConnectionConfiguration(Properties props) { - settings = props != null ? new Properties(props) : new Properties(); + public ConnectionConfiguration(Properties props) throws ClientException { + Properties settings = props != null ? props : new Properties(); - connectTimeout = Long.parseLong(settings.getProperty(CONNECT_TIMEOUT, CONNECT_TIMEOUT_DEFAULT)); - networkTimeout = Long.parseLong(settings.getProperty(NETWORK_TIMEOUT, NETWORK_TIMEOUT_DEFAULT)); - queryTimeout = Long.parseLong(settings.getProperty(QUERY_TIMEOUT, QUERY_TIMEOUT_DEFAULT)); + checkPropertyNames(settings, optionNames()); + + connectTimeout = parseValue(CONNECT_TIMEOUT, settings.getProperty(CONNECT_TIMEOUT, CONNECT_TIMEOUT_DEFAULT), Long::parseLong); + networkTimeout = parseValue(NETWORK_TIMEOUT, settings.getProperty(NETWORK_TIMEOUT, NETWORK_TIMEOUT_DEFAULT), Long::parseLong); + queryTimeout = parseValue(QUERY_TIMEOUT, settings.getProperty(QUERY_TIMEOUT, QUERY_TIMEOUT_DEFAULT), Long::parseLong); // page - pageTimeout = Long.parseLong(settings.getProperty(PAGE_TIMEOUT, PAGE_TIMEOUT_DEFAULT)); - pageSize = Integer.parseInt(settings.getProperty(PAGE_SIZE, PAGE_SIZE_DEFAULT)); + pageTimeout = parseValue(PAGE_TIMEOUT, settings.getProperty(PAGE_TIMEOUT, PAGE_TIMEOUT_DEFAULT), Long::parseLong); + pageSize = parseValue(PAGE_SIZE, settings.getProperty(PAGE_SIZE, PAGE_SIZE_DEFAULT), Integer::parseInt); // auth user = settings.getProperty(AUTH_USER); @@ -90,6 +116,42 @@ public class ConnectionConfiguration { proxyConfig = new ProxyConfig(settings); } + private Collection optionNames() { + Collection options = new ArrayList<>(OPTION_NAMES); + options.addAll(extraOptions()); + return options; + } + + protected Collection extraOptions() { + return emptyList(); + } + + private static void checkPropertyNames(Properties settings, Collection knownNames) throws ClientException { + // validate specified properties to pick up typos and such + Enumeration pNames = settings.propertyNames(); + while (pNames.hasMoreElements()) { + String message = isKnownProperty(pNames.nextElement().toString(), knownNames); + if (message != null) { + throw new ClientException(message); + } + } + } + + private static String isKnownProperty(String propertyName, Collection knownOptions) { + if (knownOptions.contains(propertyName)) { + return null; + } + return "Unknown parameter [" + propertyName + "] ; did you mean " + StringUtils.findSimiliar(propertyName, knownOptions); + } + + protected T parseValue(String key, String value, Function parser) { + try { + return parser.apply(value); + } catch (Exception ex) { + throw new ClientException("Cannot parse property [" + key + "] with value [" + value + "]; " + ex.getMessage()); + } + } + protected boolean isSSLEnabled() { return sslConfig.isEnabled(); } @@ -102,10 +164,6 @@ public class ConnectionConfiguration { return proxyConfig; } - protected Properties settings() { - return settings; - } - public void connectTimeout(long millis) { connectTimeout = millis; } @@ -139,7 +197,6 @@ public class ConnectionConfiguration { } // auth - public String authUser() { return user; } diff --git a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ProxyConfig.java b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ProxyConfig.java index 762520013df..6b9bc7bdb79 100644 --- a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ProxyConfig.java +++ b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ProxyConfig.java @@ -11,7 +11,10 @@ import java.net.InetSocketAddress; import java.net.Proxy; import java.net.URI; import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.Properties; +import java.util.Set; class ProxyConfig { @@ -20,6 +23,8 @@ class ProxyConfig { private static final String SOCKS_PROXY = "proxy.socks"; private static final String SOCKS_PROXY_DEFAULT = StringUtils.EMPTY; + static final Set OPTION_NAMES = new LinkedHashSet<>(Arrays.asList(HTTP_PROXY, SOCKS_PROXY)); + private final Proxy proxy; ProxyConfig(Properties settings) { @@ -32,12 +37,7 @@ class ProxyConfig { address = host(settings.getProperty(SOCKS_PROXY, SOCKS_PROXY_DEFAULT), 1080); type = Proxy.Type.SOCKS; } - if (address != null) { - proxy = createProxy(type, address); - } - else { - proxy = null; - } + proxy = address != null ? createProxy(type, address) : null; } @SuppressForbidden(reason = "create the actual proxy") diff --git a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/SslConfig.java b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/SslConfig.java index a135734f9e8..6c745f0a25b 100644 --- a/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/SslConfig.java +++ b/sql/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/SslConfig.java @@ -15,8 +15,11 @@ import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.security.GeneralSecurityException; import java.security.KeyStore; +import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.Objects; import java.util.Properties; +import java.util.Set; import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; @@ -51,6 +54,10 @@ class SslConfig { private static final String SSL_TRUSTSTORE_TYPE = "ssl.truststore.type"; private static final String SSL_TRUSTSTORE_TYPE_DEFAULT = "JKS"; + static final Set OPTION_NAMES = new LinkedHashSet<>(Arrays.asList(SSL, SSL_PROTOCOL, + SSL_KEYSTORE_LOCATION, SSL_KEYSTORE_PASS, SSL_KEYSTORE_TYPE, + SSL_TRUSTSTORE_LOCATION, SSL_TRUSTSTORE_PASS, SSL_TRUSTSTORE_TYPE)); + private final boolean enabled; private final String protocol, keystoreLocation, keystorePass, keystoreType; private final String truststoreLocation, truststorePass, truststoreType;