Browse Source

Added "validate.properties" property to JDBC's list of allowed properties. (#39050)

This defaults to "true" (current behavior) and will throw an exception
if there is a property that cannot be recognized. If "false", it will
ignore anything unrecognizable.
Andrei Stefan 6 years ago
parent
commit
38fbf9792b

+ 10 - 4
docs/reference/sql/endpoints/jdbc.asciidoc

@@ -58,9 +58,9 @@ jdbc:es://<1>[[http|https]://]*<2>[host[:port]]*<3>/[prefix]*<4>[?[option=value]
 <2> type of HTTP connection to make - `http` (default) or `https`. Optional.
 <3> host (`localhost` by default) and port (`9200` by default). Optional.
 <4> prefix (empty by default). Typically used when hosting {es} under a certain path. Optional.
-<5> Parameters for the JDBC driver. Empty by default. Optional.
+<5> Properties for the JDBC driver. Empty by default. Optional.
 
-The driver recognized the following parameters:
+The driver recognized the following properties:
 
 [[jdbc-cfg]]
 [float]
@@ -122,6 +122,12 @@ Query timeout (in seconds). That is the maximum amount of time waiting for a que
 
 `proxy.socks`:: SOCKS proxy host name
 
+[float]
+==== Additional
+
+`validate.properties` (default true):: If disabled, it will ignore any misspellings or unrecognizable properties. When enabled, an exception
+will be thrown if the provided property cannot be recognized.
+
 
 To put all of it together, the following URL:
 
@@ -161,10 +167,10 @@ HTTP traffic. By default 9200.
 instance is fine for unsecured Elasticsearch.
 
 Which one to use? Typically client applications that provide most
-configuration parameters in the URL rely on the `DriverManager`-style
+configuration properties in the URL rely on the `DriverManager`-style
 while `DataSource` is preferred when being _passed_ around since it can be
 configured in one place and the consumer only has to call `getConnection`
-without having to worry about any other parameters.
+without having to worry about any other properties.
 
 To connect to a secured Elasticsearch server the `Properties`
 should look like:

+ 66 - 2
x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java

@@ -18,7 +18,11 @@ import java.util.Properties;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.CONNECT_TIMEOUT;
+import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.NETWORK_TIMEOUT;
+import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PAGE_SIZE;
 import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PAGE_TIMEOUT;
+import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PROPERTIES_VALIDATION;
+import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.QUERY_TIMEOUT;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
@@ -69,7 +73,7 @@ public class JdbcConfigurationTests extends ESTestCase {
 
     public void testTypeInParam() throws Exception {
         Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://a:1/foo/bar/tar?debug=true&debug.out=jdbc.out"));
-        assertEquals("Unknown parameter [debug.out] ; did you mean [debug.output]", e.getMessage());
+        assertEquals("Unknown parameter [debug.out]; did you mean [debug.output]", e.getMessage());
     }
 
     public void testDebugOutWithSuffix() throws Exception {
@@ -113,6 +117,66 @@ public class JdbcConfigurationTests extends ESTestCase {
         Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://https://test?ssl=false"));
         assertEquals("Cannot enable SSL: HTTPS protocol being used in the URL and SSL disabled in properties", e.getMessage());
     }
+    
+    public void testValidatePropertiesDefault() {
+        Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?pagee.size=12"));
+        assertEquals("Unknown parameter [pagee.size]; did you mean [page.size]", e.getMessage());
+        
+        e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?foo=bar"));
+        assertEquals("Unknown parameter [foo]; did you mean [ssl]", e.getMessage());
+    }
+    
+    public void testValidateProperties() {
+        Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?pagee.size=12&validate.properties=true"));
+        assertEquals("Unknown parameter [pagee.size]; did you mean [page.size]", e.getMessage());
+        
+        e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?&validate.properties=true&something=some_value"));
+        assertEquals("Unknown parameter [something]; did you mean []", e.getMessage());
+        
+        Properties properties  = new Properties();
+        properties.setProperty(PROPERTIES_VALIDATION, "true");
+        e = expectThrows(JdbcSQLException.class, () -> JdbcConfiguration.create("jdbc:es://test:9200?something=some_value", properties, 0));
+        assertEquals("Unknown parameter [something]; did you mean []", e.getMessage());
+    }
+    
+    public void testNoPropertiesValidation() throws SQLException {
+        JdbcConfiguration ci = ci("jdbc:es://test:9200?pagee.size=12&validate.properties=false");
+        assertEquals(false, ci.validateProperties());
+        
+        // URL properties test
+        long queryTimeout = randomNonNegativeLong();
+        long connectTimeout = randomNonNegativeLong();
+        long networkTimeout = randomNonNegativeLong();
+        long pageTimeout = randomNonNegativeLong();
+        int pageSize = randomIntBetween(0, Integer.MAX_VALUE);
+        
+        ci = ci("jdbc:es://test:9200?validate.properties=false&something=some_value&query.timeout=" + queryTimeout + "&connect.timeout="
+                + connectTimeout + "&network.timeout=" + networkTimeout + "&page.timeout=" + pageTimeout + "&page.size=" + pageSize);
+        assertEquals(false, ci.validateProperties());
+        assertEquals(queryTimeout, ci.queryTimeout());
+        assertEquals(connectTimeout, ci.connectTimeout());
+        assertEquals(networkTimeout, ci.networkTimeout());
+        assertEquals(pageTimeout, ci.pageTimeout());
+        assertEquals(pageSize, ci.pageSize());
+        
+        // Properties test
+        Properties properties  = new Properties();
+        properties.setProperty(PROPERTIES_VALIDATION, "false");
+        properties.put(QUERY_TIMEOUT, Long.toString(queryTimeout));
+        properties.put(PAGE_TIMEOUT, Long.toString(pageTimeout));
+        properties.put(CONNECT_TIMEOUT, Long.toString(connectTimeout));
+        properties.put(NETWORK_TIMEOUT, Long.toString(networkTimeout));
+        properties.put(PAGE_SIZE, Integer.toString(pageSize));
+        
+        // also putting validate.properties in URL to be overriden by the properties value
+        ci = JdbcConfiguration.create("jdbc:es://test:9200?validate.properties=true&something=some_value", properties, 0);
+        assertEquals(false, ci.validateProperties());
+        assertEquals(queryTimeout, ci.queryTimeout());
+        assertEquals(connectTimeout, ci.connectTimeout());
+        assertEquals(networkTimeout, ci.networkTimeout());
+        assertEquals(pageTimeout, ci.pageTimeout());
+        assertEquals(pageSize, ci.pageSize());
+    }
 
     public void testTimoutOverride() throws Exception {
         Properties properties  = new Properties();
@@ -284,6 +348,6 @@ public class JdbcConfigurationTests extends ESTestCase {
     private void assertJdbcSqlException(String wrongSetting, String correctSetting, String url, Properties props) {
         JdbcSQLException ex = expectThrows(JdbcSQLException.class, 
                 () -> JdbcConfiguration.create(url, props, 0));
-        assertEquals("Unknown parameter [" + wrongSetting + "] ; did you mean [" + correctSetting + "]", ex.getMessage());
+        assertEquals("Unknown parameter [" + wrongSetting + "]; did you mean [" + correctSetting + "]", ex.getMessage());
     }
 }

+ 22 - 6
x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java

@@ -28,6 +28,10 @@ import static java.util.Collections.emptyList;
  * to move away from the loose Strings...
  */
 public class ConnectionConfiguration {
+    
+    // Validation
+    public static final String PROPERTIES_VALIDATION = "validate.properties";
+    public static final String PROPERTIES_VALIDATION_DEFAULT = "true";
 
     // Timeouts
 
@@ -59,12 +63,15 @@ public class ConnectionConfiguration {
     public static final String AUTH_PASS = "password";
 
     protected static final Set<String> OPTION_NAMES = new LinkedHashSet<>(
-            Arrays.asList(CONNECT_TIMEOUT, NETWORK_TIMEOUT, QUERY_TIMEOUT, PAGE_TIMEOUT, PAGE_SIZE, AUTH_USER, AUTH_PASS));
+            Arrays.asList(PROPERTIES_VALIDATION, CONNECT_TIMEOUT, NETWORK_TIMEOUT, QUERY_TIMEOUT, PAGE_TIMEOUT, PAGE_SIZE,
+                    AUTH_USER, AUTH_PASS));
 
     static {
         OPTION_NAMES.addAll(SslConfig.OPTION_NAMES);
         OPTION_NAMES.addAll(ProxyConfig.OPTION_NAMES);
     }
+    
+    private final boolean validateProperties;
 
     // Base URI for all request
     private final URI baseURI;
@@ -87,7 +94,11 @@ public class ConnectionConfiguration {
         this.connectionString = connectionString;
         Properties settings = props != null ? props : new Properties();
 
-        checkPropertyNames(settings, optionNames());
+        validateProperties = parseValue(PROPERTIES_VALIDATION, settings.getProperty(PROPERTIES_VALIDATION, PROPERTIES_VALIDATION_DEFAULT),
+                Boolean::parseBoolean);
+        if (validateProperties) {
+            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);
@@ -106,9 +117,10 @@ public class ConnectionConfiguration {
         this.baseURI = normalizeSchema(baseURI, connectionString, sslConfig.isEnabled());
     }
 
-    public ConnectionConfiguration(URI baseURI, String connectionString, long connectTimeout, long networkTimeout, long queryTimeout,
-                                   long pageTimeout, int pageSize, String user, String pass, SslConfig sslConfig,
-                                   ProxyConfig proxyConfig) throws ClientException {
+    public ConnectionConfiguration(URI baseURI, String connectionString, boolean validateProperties, long connectTimeout,
+                                   long networkTimeout, long queryTimeout, long pageTimeout, int pageSize, String user, String pass,
+                                   SslConfig sslConfig, ProxyConfig proxyConfig) throws ClientException {
+        this.validateProperties = validateProperties;
         this.connectionString = connectionString;
         this.connectTimeout = connectTimeout;
         this.networkTimeout = networkTimeout;
@@ -161,7 +173,7 @@ public class ConnectionConfiguration {
         if (knownOptions.contains(propertyName)) {
             return null;
         }
-        return "Unknown parameter [" + propertyName + "] ; did you mean " + StringUtils.findSimilar(propertyName, knownOptions);
+        return "Unknown parameter [" + propertyName + "]; did you mean " + StringUtils.findSimilar(propertyName, knownOptions);
     }
 
     protected <T> T parseValue(String key, String value, Function<String, T> parser) {
@@ -175,6 +187,10 @@ public class ConnectionConfiguration {
     protected boolean isSSLEnabled() {
         return sslConfig.isEnabled();
     }
+    
+    public boolean validateProperties() {
+        return validateProperties;
+    }
 
     public SslConfig sslConfig() {
         return sslConfig;

+ 1 - 1
x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java

@@ -106,7 +106,7 @@ public class HttpClient {
     }
 
     private boolean head(String path, long timeoutInMs) throws SQLException {
-        ConnectionConfiguration pingCfg = new ConnectionConfiguration(cfg.baseUri(), cfg.connectionString(),
+        ConnectionConfiguration pingCfg = new ConnectionConfiguration(cfg.baseUri(), cfg.connectionString(), cfg.validateProperties(),
             cfg.connectTimeout(), timeoutInMs, cfg.queryTimeout(), cfg.pageTimeout(), cfg.pageSize(),
             cfg.authUser(), cfg.authPass(), cfg.sslConfig(), cfg.proxyConfig());
         try {