From e22f7e4a13452bf71f472b18523cbd6da508805f Mon Sep 17 00:00:00 2001 From: jvictorhuguenin Date: Fri, 29 Jul 2022 23:25:22 -0300 Subject: [PATCH 1/3] Implement case insensitive for the property keys --- .../arrow/driver/jdbc/ArrowFlightJdbcDriver.java | 10 +++++++++- .../jdbc/utils/ArrowFlightConnectionConfigImpl.java | 4 ++-- .../org/apache/arrow/driver/jdbc/utils/UrlParser.java | 10 ++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java index 216c37fb5d32d..4acbbf4a9f7df 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java @@ -27,6 +27,7 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.sql.SQLException; +import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -247,8 +248,15 @@ private Map getUrlsArgs(String url) final String extraParams = uri.getRawQuery(); // optional params final Map keyValuePairs = UrlParser.parse(extraParams, "&"); - resultMap.putAll(keyValuePairs); + final Map lowerCaseKeyValuePairs = lowerCaseKeys(keyValuePairs); + resultMap.putAll(lowerCaseKeyValuePairs); return resultMap; } + + private Map lowerCaseKeys(final Map keyValues) { + Map resultMap = new HashMap<>(); + keyValues.forEach((k, v) -> resultMap.put(k.toLowerCase(), v)); + return resultMap; + } } diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java index 57fd816d9dce3..b8d95dc759d82 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java @@ -202,9 +202,9 @@ public enum ArrowFlightConnectionProperty implements ConnectionProperty { public Object get(final Properties properties) { Preconditions.checkNotNull(properties, "Properties cannot be null."); Preconditions.checkState( - properties.containsKey(camelName) || !required, + properties.containsKey(camelName.toLowerCase()) || !required, format("Required property not provided: <%s>.", this)); - return properties.getOrDefault(camelName, defaultValue); + return properties.getOrDefault(camelName.toLowerCase(), defaultValue); } /** diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java index fbef721793b02..fec3bf916bd56 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java @@ -36,14 +36,12 @@ public static Map parse(String url, String separator) { for (String keyValue : keyValues) { int separatorKey = keyValue.indexOf("="); // Find the first equal sign to split key and value. - String key = keyValue.substring(0, separatorKey); - String value = ""; - if (!keyValue.endsWith("=")) { // Avoid crashes for empty values. - value = keyValue.substring(separatorKey + 1); + if (separatorKey != -1) { // Avoid crashes when not finding an equal sign in the property value. + String key = keyValue.substring(0, separatorKey); + String value = keyValue.substring(separatorKey + 1); + resultMap.put(key, value); } - resultMap.put(key, value); } - return resultMap; } } From b2af3a0748bb700f8f8ae5f5dc32d57f309531f6 Mon Sep 17 00:00:00 2001 From: jvictorhuguenin Date: Tue, 2 Aug 2022 14:37:08 -0300 Subject: [PATCH 2/3] Fix property lowercasing only for the connection string --- .../arrow/driver/jdbc/ArrowFlightJdbcDriver.java | 14 ++++++-------- .../utils/ArrowFlightConnectionConfigImpl.java | 11 +++++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java index 4acbbf4a9f7df..18faf3990206c 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java @@ -27,7 +27,6 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.sql.SQLException; -import java.util.HashMap; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -81,7 +80,7 @@ public ArrowFlightConnection connect(final String url, final Properties info) this, factory, url, - properties, + lowerCasePropertyKeys(properties), new RootAllocator(Long.MAX_VALUE)); } catch (final FlightRuntimeException e) { throw new SQLException("Failed to connect.", e); @@ -248,15 +247,14 @@ private Map getUrlsArgs(String url) final String extraParams = uri.getRawQuery(); // optional params final Map keyValuePairs = UrlParser.parse(extraParams, "&"); - final Map lowerCaseKeyValuePairs = lowerCaseKeys(keyValuePairs); - resultMap.putAll(lowerCaseKeyValuePairs); + resultMap.putAll(keyValuePairs); return resultMap; } - private Map lowerCaseKeys(final Map keyValues) { - Map resultMap = new HashMap<>(); - keyValues.forEach((k, v) -> resultMap.put(k.toLowerCase(), v)); - return resultMap; + private Properties lowerCasePropertyKeys(final Properties properties) { + final Properties resultProperty = new Properties(); + properties.forEach((k, v) -> resultProperty.put(k.toString().toLowerCase(), v)); + return resultProperty; } } diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java index b8d95dc759d82..b8d4e7240a122 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java @@ -201,10 +201,13 @@ public enum ArrowFlightConnectionProperty implements ConnectionProperty { */ public Object get(final Properties properties) { Preconditions.checkNotNull(properties, "Properties cannot be null."); - Preconditions.checkState( - properties.containsKey(camelName.toLowerCase()) || !required, - format("Required property not provided: <%s>.", this)); - return properties.getOrDefault(camelName.toLowerCase(), defaultValue); + Object value = properties.get(camelName.toLowerCase()); + if (required) { + Preconditions.checkNotNull(value, format("Required property not provided: <%s>.", this)); + return value; + } else { + return value != null ? value : defaultValue; + } } /** From 6eb3cfa4b5cee71cf63cc5a58d4e87cb53689ab7 Mon Sep 17 00:00:00 2001 From: jvictorhuguenin Date: Wed, 3 Aug 2022 14:11:24 -0300 Subject: [PATCH 3/3] Add unit tests and fix possible crash on the parser --- .../arrow/driver/jdbc/utils/UrlParser.java | 16 +++++---- .../jdbc/ArrowFlightJdbcDriverTest.java | 34 ++++++++++++++++++- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java index fec3bf916bd56..787a22312609f 100644 --- a/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java +++ b/java/flight/flight-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/utils/UrlParser.java @@ -32,14 +32,16 @@ public class UrlParser { */ public static Map parse(String url, String separator) { Map resultMap = new HashMap<>(); - String[] keyValues = url.split(separator); + if (url != null) { + String[] keyValues = url.split(separator); - for (String keyValue : keyValues) { - int separatorKey = keyValue.indexOf("="); // Find the first equal sign to split key and value. - if (separatorKey != -1) { // Avoid crashes when not finding an equal sign in the property value. - String key = keyValue.substring(0, separatorKey); - String value = keyValue.substring(separatorKey + 1); - resultMap.put(key, value); + for (String keyValue : keyValues) { + int separatorKey = keyValue.indexOf("="); // Find the first equal sign to split key and value. + if (separatorKey != -1) { // Avoid crashes when not finding an equal sign in the property value. + String key = keyValue.substring(0, separatorKey); + String value = keyValue.substring(separatorKey + 1); + resultMap.put(key, value); + } } } return resultMap; diff --git a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java b/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java index d39ec61f3099e..6aa0062f706e1 100644 --- a/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java +++ b/java/flight/flight-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java @@ -28,6 +28,7 @@ import java.util.Collection; import java.util.Map; +import java.util.Properties; import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication; import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty; import org.apache.arrow.driver.jdbc.utils.MockFlightSqlProducer; @@ -116,7 +117,38 @@ public void testShouldConnectWhenProvidedWithValidUrl() throws Exception { dataSource.getConfig().getHost() + ":" + dataSource.getConfig().getPort() + "?" + "useEncryption=false", - dataSource.getProperties(dataSource.getConfig().getUser(), dataSource.getConfig().getPassword()))) { + dataSource.getProperties(dataSource.getConfig().getUser(), dataSource.getConfig().getPassword()))) { + assert connection.isValid(300); + } + } + + @Test + public void testConnectWithInsensitiveCasePropertyKeys() throws Exception { + // Get the Arrow Flight JDBC driver by providing a URL with insensitive case property keys. + final Driver driver = new ArrowFlightJdbcDriver(); + + try (Connection connection = + driver.connect("jdbc:arrow-flight://" + + dataSource.getConfig().getHost() + ":" + + dataSource.getConfig().getPort() + "?" + + "UseEncryptiOn=false", + dataSource.getProperties(dataSource.getConfig().getUser(), dataSource.getConfig().getPassword()))) { + assert connection.isValid(300); + } + } + + @Test + public void testConnectWithInsensitiveCasePropertyKeys2() throws Exception { + // Get the Arrow Flight JDBC driver by providing a property object with insensitive case keys. + final Driver driver = new ArrowFlightJdbcDriver(); + Properties properties = + dataSource.getProperties(dataSource.getConfig().getUser(), dataSource.getConfig().getPassword()); + properties.put("UseEncryptiOn", "false"); + + try (Connection connection = + driver.connect("jdbc:arrow-flight://" + + dataSource.getConfig().getHost() + ":" + + dataSource.getConfig().getPort(), properties)) { assert connection.isValid(300); } }