Skip to content

Harden JDBC connection-string config parsing against null/wrong-type values #3210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
terryozaki-wq wants to merge 1 commit into
base: dev
Choose a base branch
from
+652 −269
Open

Harden JDBC connection-string config parsing against null/wrong-type values #3210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 111 additions & 6 deletions src/prerna/util/sql/AbstractSqlQueryUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,118 @@ public abstract class AbstractSqlQueryUtil {

/**
* Build the connection string after the connection details have been set
*
*
* @return
*/
public abstract String buildConnectionString();

/////////////////////////////////////////////////////////////////////////////////////

/*
* Type-safe accessors for reading connection configuration out of a
* Map<String, Object> (FE/JSON input) or a CaseInsensitiveProperties (SMSS
* input). Both inputs are untyped at the API boundary - a key can be absent,
* null, or holding a value of the wrong type - so setConnectionDetailsfromMap
* / setConnectionDetailsFromSMSS implementations should read connection
* config through these instead of casting configMap.get(KEY)/prop.get(KEY)
* directly, which turns a missing or mistyped key into a silent null or a
* ClassCastException far from the actual bad input.
*/

/**
* Get an optional String value, returning {@code defaultValue} if the key is
* absent. Still fails fast, naming the key, if it is present but not actually
* a String.
*
* @param configMap
* @param key
* @param defaultValue
* @return
*/
protected String getOptionalStringValue(Map<String, Object> configMap, String key, String defaultValue) {
return optionalString(key, configMap.get(key), defaultValue);
}

/**
* @see #getOptionalStringValue(Map, String, String)
*/
protected String getOptionalStringValue(CaseInsensitiveProperties prop, String key, String defaultValue) {
return optionalString(key, prop.get(key), defaultValue);
}

/**
* Get an optional boolean value, returning {@code defaultValue} if the key is
* absent. Accepts an actual Boolean, or a String parsed via
* {@link Boolean#parseBoolean(String)}; still fails fast, naming the key, for
* any other type instead of silently coercing it - e.g. via
* {@code value + ""} string concatenation - to false.
*
* @param configMap
* @param key
* @param defaultValue
* @return
*/
protected boolean getOptionalBooleanValue(Map<String, Object> configMap, String key, boolean defaultValue) {
return optionalBoolean(key, configMap.get(key), defaultValue);
}

/**
* @see #getOptionalBooleanValue(Map, String, boolean)
*/
protected boolean getOptionalBooleanValue(CaseInsensitiveProperties prop, String key, boolean defaultValue) {
return optionalBoolean(key, prop.get(key), defaultValue);
}

/**
* Assert that a (conditionally) required configuration value was actually
* resolved - e.g. a hostname that is only required when no full connection
* URL was separately provided. Unlike the config-map accessors above, this
* validates accumulated object state rather than a single input argument, so
* it fails with {@link IllegalStateException} rather than
* {@link IllegalArgumentException}.
*
* @param key the configuration key this value came from, used only for the
* error message
* @param value the resolved value to check
* @return value, unchanged, if it is non-null and non-empty
*/
protected String requireNonBlank(String key, String value) {
if (value == null || value.isEmpty()) {
throw new IllegalStateException("Must provide a value for required configuration key '" + key + "'");
}
return value;
}

private String optionalString(String key, Object value, String defaultValue) {
if (value == null) {
return defaultValue;
}
if (!(value instanceof String)) {
throw new IllegalArgumentException(
"Configuration key '" + key + "' must be a String, but found " + value.getClass().getName());
}
return (String) value;
}

private boolean optionalBoolean(String key, Object value, boolean defaultValue) {
if (value == null) {
return defaultValue;
}
if (value instanceof Boolean) {
return (Boolean) value;
}
if (value instanceof String) {
return Boolean.parseBoolean((String) value);
}
throw new IllegalArgumentException("Configuration key '" + key + "' must be a Boolean or String, but found "
+ value.getClass().getName());
}

/////////////////////////////////////////////////////////////////////////////////////

/**
* Method to get a connection to an existing RDBMS engine
*
*
* @param driverEnum
* @param connectionUrl
* @param connectionDetails
Expand All @@ -218,13 +322,13 @@ public abstract class AbstractSqlQueryUtil {
public static Connection makeConnection(AbstractSqlQueryUtil util, String connectionUrl,
Map<String, Object> connectionDetails) throws SQLException {
return AbstractSqlQueryUtil.makeConnection(util.getDbType(), connectionUrl,
(String) connectionDetails.get(util.getConnectionUserKey()),
(String) connectionDetails.get(util.getConnectionPasswordKey()));
util.getOptionalStringValue(connectionDetails, util.getConnectionUserKey(), null),
util.getOptionalStringValue(connectionDetails, util.getConnectionPasswordKey(), null));
}

/**
* Method to get a connection to an existing RDBMS engine
*
*
* @param driverEnum
* @param connectionUrl
* @param connectionDetails
Expand All @@ -234,7 +338,8 @@ public static Connection makeConnection(AbstractSqlQueryUtil util, String connec
public static Connection makeConnection(AbstractSqlQueryUtil util, String connectionUrl,
CaseInsensitiveProperties prop) throws SQLException {
return AbstractSqlQueryUtil.makeConnection(util.getDbType(), connectionUrl,
(String) prop.get(util.getConnectionUserKey()), (String) prop.get(util.getConnectionPasswordKey()));
util.getOptionalStringValue(prop, util.getConnectionUserKey(), null),
util.getOptionalStringValue(prop, util.getConnectionPasswordKey(), null));
}

/**
Expand Down
37 changes: 16 additions & 21 deletions src/prerna/util/sql/AnsiSqlQueryUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1913,13 +1913,13 @@ public String setConnectionDetailsfromMap(Map<String, Object> configMap) throws
throw new RuntimeException("Configuration map is null or empty");
}

this.connectionUrl = (String) configMap.get(AbstractSqlQueryUtil.CONNECTION_URL);
this.hostname = (String) configMap.get(AbstractSqlQueryUtil.HOSTNAME);
this.port = (String) configMap.get(AbstractSqlQueryUtil.PORT);
this.schema = (String) configMap.get(AbstractSqlQueryUtil.SCHEMA);
this.additionalProps = (String) configMap.get(AbstractSqlQueryUtil.ADDITIONAL);
this.username = (String) configMap.get(AbstractSqlQueryUtil.USERNAME);
this.password = (String) configMap.get(AbstractSqlQueryUtil.PASSWORD);
this.connectionUrl = getOptionalStringValue(configMap, AbstractSqlQueryUtil.CONNECTION_URL, null);
this.hostname = getOptionalStringValue(configMap, AbstractSqlQueryUtil.HOSTNAME, null);
this.port = getOptionalStringValue(configMap, AbstractSqlQueryUtil.PORT, null);
this.schema = getOptionalStringValue(configMap, AbstractSqlQueryUtil.SCHEMA, null);
this.additionalProps = getOptionalStringValue(configMap, AbstractSqlQueryUtil.ADDITIONAL, null);
this.username = getOptionalStringValue(configMap, AbstractSqlQueryUtil.USERNAME, null);
this.password = getOptionalStringValue(configMap, AbstractSqlQueryUtil.PASSWORD, null);

return buildConnectionString();
}
Expand All @@ -1933,13 +1933,13 @@ public String setConnectionDetailsFromSMSS(CaseInsensitiveProperties prop) throw
throw new RuntimeException("Properties object is null or empty");
}

this.connectionUrl = (String) prop.get(AbstractSqlQueryUtil.CONNECTION_URL);
this.hostname = (String) prop.get(AbstractSqlQueryUtil.HOSTNAME);
this.port = (String) prop.get(AbstractSqlQueryUtil.PORT);
this.schema = (String) prop.get(AbstractSqlQueryUtil.SCHEMA);
this.additionalProps = (String) prop.get(AbstractSqlQueryUtil.ADDITIONAL);
this.username = (String) prop.get(AbstractSqlQueryUtil.USERNAME);
this.password = (String) prop.get(AbstractSqlQueryUtil.PASSWORD);
this.connectionUrl = getOptionalStringValue(prop, AbstractSqlQueryUtil.CONNECTION_URL, null);
this.hostname = getOptionalStringValue(prop, AbstractSqlQueryUtil.HOSTNAME, null);
this.port = getOptionalStringValue(prop, AbstractSqlQueryUtil.PORT, null);
this.schema = getOptionalStringValue(prop, AbstractSqlQueryUtil.SCHEMA, null);
this.additionalProps = getOptionalStringValue(prop, AbstractSqlQueryUtil.ADDITIONAL, null);
this.username = getOptionalStringValue(prop, AbstractSqlQueryUtil.USERNAME, null);
this.password = getOptionalStringValue(prop, AbstractSqlQueryUtil.PASSWORD, null);

return buildConnectionString();
}
Expand All @@ -1953,13 +1953,8 @@ public String buildConnectionString() {
return this.connectionUrl;
}

if (this.hostname == null || this.hostname.isEmpty()) {
throw new RuntimeException("Must pass in a hostname");
}

if (this.schema == null || this.schema.isEmpty()) {
throw new RuntimeException("Must pass in schema name");
}
requireNonBlank(AbstractSqlQueryUtil.HOSTNAME, this.hostname);
requireNonBlank(AbstractSqlQueryUtil.SCHEMA, this.schema);

String port = getPort();
if (port != null && !port.isEmpty()) {
Expand Down
28 changes: 14 additions & 14 deletions src/prerna/util/sql/AthenaQueryUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ public String setConnectionDetailsfromMap(Map<String, Object> configMap) throws
throw new RuntimeException("Configuration map is null or empty");
}

this.connectionUrl = (String) configMap.get(AbstractSqlQueryUtil.CONNECTION_URL);
this.region = (String) configMap.get(AbstractSqlQueryUtil.REGION);
this.accessKey = (String) configMap.get(AbstractSqlQueryUtil.ACCESS_KEY);
this.secretKey = (String) configMap.get(AbstractSqlQueryUtil.SECRET_KEY);
this.output = (String) configMap.get(AbstractSqlQueryUtil.OUTPUT);
this.schema = (String) configMap.get(AbstractSqlQueryUtil.SCHEMA);
this.connectionUrl = getOptionalStringValue(configMap, AbstractSqlQueryUtil.CONNECTION_URL, null);
this.region = getOptionalStringValue(configMap, AbstractSqlQueryUtil.REGION, null);
this.accessKey = getOptionalStringValue(configMap, AbstractSqlQueryUtil.ACCESS_KEY, null);
this.secretKey = getOptionalStringValue(configMap, AbstractSqlQueryUtil.SECRET_KEY, null);
this.output = getOptionalStringValue(configMap, AbstractSqlQueryUtil.OUTPUT, null);
this.schema = getOptionalStringValue(configMap, AbstractSqlQueryUtil.SCHEMA, null);
if (this.schema == null || this.schema.isEmpty()) {
this.schema = "default";
}
this.additionalProps = (String) configMap.get(AbstractSqlQueryUtil.ADDITIONAL);
this.additionalProps = getOptionalStringValue(configMap, AbstractSqlQueryUtil.ADDITIONAL, null);
return buildConnectionString();
}

Expand All @@ -73,16 +73,16 @@ public String setConnectionDetailsFromSMSS(CaseInsensitiveProperties prop) throw
throw new RuntimeException("Properties object is null or empty");
}

this.connectionUrl = (String) prop.get(AbstractSqlQueryUtil.CONNECTION_URL);
this.region = (String) prop.get(AbstractSqlQueryUtil.REGION);
this.accessKey = (String) prop.get(AbstractSqlQueryUtil.ACCESS_KEY);
this.secretKey = (String) prop.get(AbstractSqlQueryUtil.SECRET_KEY);
this.output = (String) prop.get(AbstractSqlQueryUtil.OUTPUT);
this.schema = (String) prop.get(AbstractSqlQueryUtil.SCHEMA);
this.connectionUrl = getOptionalStringValue(prop, AbstractSqlQueryUtil.CONNECTION_URL, null);
this.region = getOptionalStringValue(prop, AbstractSqlQueryUtil.REGION, null);
this.accessKey = getOptionalStringValue(prop, AbstractSqlQueryUtil.ACCESS_KEY, null);
this.secretKey = getOptionalStringValue(prop, AbstractSqlQueryUtil.SECRET_KEY, null);
this.output = getOptionalStringValue(prop, AbstractSqlQueryUtil.OUTPUT, null);
this.schema = getOptionalStringValue(prop, AbstractSqlQueryUtil.SCHEMA, null);
if (this.schema == null || this.schema.isEmpty()) {
this.schema = "default";
}
this.additionalProps = (String) prop.get(AbstractSqlQueryUtil.ADDITIONAL);
this.additionalProps = getOptionalStringValue(prop, AbstractSqlQueryUtil.ADDITIONAL, null);
return buildConnectionString();
}

Expand Down
Loading
Loading