From 23369fad935ae2a61e1b2e8daa83f3ceab21f00f Mon Sep 17 00:00:00 2001 From: Samikshya Chand Date: Wed, 6 Nov 2024 21:38:56 +0530 Subject: [PATCH 1/4] Fix vulnerabilities --- .../com/databricks/sdk/core/ConfigLoader.java | 39 ++++++++++--------- pom.xml | 4 +- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java index 933fa50ac..4504d0890 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java @@ -1,16 +1,17 @@ package com.databricks.sdk.core; import com.databricks.sdk.core.utils.Environment; -import java.io.File; import java.io.FileNotFoundException; +import java.io.FileReader; import java.io.IOException; import java.lang.reflect.Field; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.Paths; import java.util.*; -import org.ini4j.Ini; -import org.ini4j.Profile; +import org.apache.commons.configuration2.INIConfiguration; +import org.apache.commons.configuration2.SubnodeConfiguration; +import org.apache.commons.configuration2.ex.ConfigurationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,14 +60,14 @@ static void loadFromEnvironmentVariables(DatabricksConfig cfg) throws IllegalAcc } } catch (DatabricksException e) { String msg = - String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); + String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); throw new DatabricksException(msg, e); } } static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { if (isNullOrEmpty(cfg.getProfile()) - && (isAnyAuthConfigured(cfg) + && (isAnyAuthConfigured(cfg) || !isNullOrEmpty(cfg.getHost()) || !isNullOrEmpty(cfg.getAzureWorkspaceResourceId()))) { return; @@ -86,7 +87,7 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { configFile = configFile.replaceFirst("^~", userHome); } - Ini ini = parseDatabricksCfg(configFile, isDefaultConfig); + INIConfiguration ini = parseDatabricksCfg(configFile, isDefaultConfig); if (ini == null) return; String profile = cfg.getProfile(); boolean hasExplicitProfile = !isNullOrEmpty(profile); @@ -94,7 +95,7 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { profile = "DEFAULT"; } - Profile.Section section = ini.get(profile); + SubnodeConfiguration section = ini.getSection(profile); if (section == null && !hasExplicitProfile) { LOG.info("{} has no {} profile configured", configFile, profile); return; @@ -106,7 +107,7 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { } for (ConfigAttributeAccessor accessor : accessors) { - String value = section.get(accessor.getName()); + String value = section.getString(accessor.getName()); if (!isNullOrEmpty(accessor.getValueFromConfig(cfg))) { continue; } @@ -114,18 +115,18 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { } } - private static Ini parseDatabricksCfg(String configFile, boolean isDefaultConfig) { - Ini ini = new Ini(); - try { - ini.load(new File(configFile)); + private static INIConfiguration parseDatabricksCfg(String configFile, boolean isDefaultConfig) { + INIConfiguration iniConfig = new INIConfiguration(); + try (FileReader reader = new FileReader(configFile)) { + iniConfig.read(reader); } catch (FileNotFoundException e) { if (isDefaultConfig) { return null; } - } catch (IOException e) { + } catch (IOException | ConfigurationException e) { throw new DatabricksException("Cannot load " + configFile, e); } - return ini; + return iniConfig; } public static void fixHostIfNeeded(DatabricksConfig cfg) { @@ -166,21 +167,21 @@ static void validate(DatabricksConfig cfg) throws DatabricksException { if (authSet.size() <= 1) return; String names = String.join(" and ", authSet); throw new DatabricksException( - String.format("validate: more than one authorization method configured: %s", names)); + String.format("validate: more than one authorization method configured: %s", names)); } catch (IllegalAccessException e) { throw new DatabricksException("Cannot create default config", e); } } public static DatabricksException makeNicerError( - String message, Exception e, DatabricksConfig cfg) { + String message, Exception e, DatabricksConfig cfg) { return makeNicerError(message, e, 200, cfg); } public static DatabricksException makeNicerError( - String message, Exception e, Integer statusCode, DatabricksConfig cfg) { + String message, Exception e, Integer statusCode, DatabricksConfig cfg) { boolean isHttpUnauthorizedOrForbidden = - true; // TODO - pass status code with exception, default this to false + true; // TODO - pass status code with exception, default this to false if (statusCode == 401 || statusCode == 402) isHttpUnauthorizedOrForbidden = true; String debugString = ""; if (cfg.getEnv() != null) { @@ -264,4 +265,4 @@ public static boolean isAnyAuthConfigured(DatabricksConfig cfg) throws IllegalAc } return false; } -} +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index 789662edd..c0b84a720 100644 --- a/pom.xml +++ b/pom.xml @@ -1,6 +1,6 @@ + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 com.databricks databricks-sdk-parent @@ -291,4 +291,4 @@ - + \ No newline at end of file From 247090022ac0cd347c9f411a61ca758bcd2b174f Mon Sep 17 00:00:00 2001 From: Samikshya Chand Date: Wed, 6 Nov 2024 21:55:08 +0530 Subject: [PATCH 2/4] update pom --- databricks-sdk-java/pom.xml | 10 +++++----- pom.xml | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/databricks-sdk-java/pom.xml b/databricks-sdk-java/pom.xml index cba1f7855..d50659b75 100644 --- a/databricks-sdk-java/pom.xml +++ b/databricks-sdk-java/pom.xml @@ -1,6 +1,6 @@ + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 com.databricks @@ -49,9 +49,9 @@ provided - org.ini4j - ini4j - 0.5.4 + org.apache.commons + commons-configuration2 + 2.11.0 compile @@ -67,7 +67,7 @@ commons-io commons-io - 2.13.0 + 2.14.0 org.junit.jupiter diff --git a/pom.xml b/pom.xml index c0b84a720..789662edd 100644 --- a/pom.xml +++ b/pom.xml @@ -1,6 +1,6 @@ + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 com.databricks databricks-sdk-parent @@ -291,4 +291,4 @@ - \ No newline at end of file + From 1bfd4a3056cf1e14aeaaa7e0596e02c7eef190b3 Mon Sep 17 00:00:00 2001 From: Samikshya Chand Date: Thu, 7 Nov 2024 00:12:19 +0530 Subject: [PATCH 3/4] fmt --- databricks-sdk-java/pom.xml | 2 +- .../com/databricks/sdk/core/ConfigLoader.java | 22 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/databricks-sdk-java/pom.xml b/databricks-sdk-java/pom.xml index d50659b75..a49f22054 100644 --- a/databricks-sdk-java/pom.xml +++ b/databricks-sdk-java/pom.xml @@ -1,6 +1,6 @@ + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 com.databricks diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java index 4504d0890..00daa27a3 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java @@ -59,8 +59,7 @@ static void loadFromEnvironmentVariables(DatabricksConfig cfg) throws IllegalAcc accessor.setValueOnConfig(cfg, env); } } catch (DatabricksException e) { - String msg = - String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); + String msg = String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); throw new DatabricksException(msg, e); } } @@ -89,6 +88,7 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { INIConfiguration ini = parseDatabricksCfg(configFile, isDefaultConfig); if (ini == null) return; + String profile = cfg.getProfile(); boolean hasExplicitProfile = !isNullOrEmpty(profile); if (!hasExplicitProfile) { @@ -166,22 +166,18 @@ static void validate(DatabricksConfig cfg) throws DatabricksException { } if (authSet.size() <= 1) return; String names = String.join(" and ", authSet); - throw new DatabricksException( - String.format("validate: more than one authorization method configured: %s", names)); + throw new DatabricksException(String.format("validate: more than one authorization method configured: %s", names)); } catch (IllegalAccessException e) { throw new DatabricksException("Cannot create default config", e); } } - public static DatabricksException makeNicerError( - String message, Exception e, DatabricksConfig cfg) { + public static DatabricksException makeNicerError(String message, Exception e, DatabricksConfig cfg) { return makeNicerError(message, e, 200, cfg); } - public static DatabricksException makeNicerError( - String message, Exception e, Integer statusCode, DatabricksConfig cfg) { - boolean isHttpUnauthorizedOrForbidden = - true; // TODO - pass status code with exception, default this to false + public static DatabricksException makeNicerError(String message, Exception e, Integer statusCode, DatabricksConfig cfg) { + boolean isHttpUnauthorizedOrForbidden = true; // TODO - pass status code with exception, default this to false if (statusCode == 401 || statusCode == 402) isHttpUnauthorizedOrForbidden = true; String debugString = ""; if (cfg.getEnv() != null) { @@ -231,12 +227,12 @@ public static String debugString(DatabricksConfig cfg) { if (!attrsUsed.isEmpty()) { buf.add(String.format("Config: %s", String.join(", ", attrsUsed))); } else { - buf.add(String.format("Config: ")); + buf.add("Config: "); } if (!envsUsed.isEmpty()) { buf.add(String.format("Env: %s", String.join(", ", envsUsed))); } else { - buf.add(String.format("Env: ")); + buf.add("Env: "); } return String.join(". ", buf); } catch (IllegalAccessException e) { @@ -265,4 +261,4 @@ public static boolean isAnyAuthConfigured(DatabricksConfig cfg) throws IllegalAc } return false; } -} \ No newline at end of file +} From 6d23597b91bbaa157507791ae471b15a541d20eb Mon Sep 17 00:00:00 2001 From: Samikshya Chand Date: Thu, 7 Nov 2024 14:55:41 +0530 Subject: [PATCH 4/4] fix tests --- .../com/databricks/sdk/core/ConfigLoader.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java index 00daa27a3..47779d0ee 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/ConfigLoader.java @@ -41,7 +41,7 @@ public static DatabricksConfig resolve(DatabricksConfig cfg) throws DatabricksEx } } - static void loadFromEnvironmentVariables(DatabricksConfig cfg) throws IllegalAccessException { + static void loadFromEnvironmentVariables(DatabricksConfig cfg) { if (cfg.getEnv() == null) { return; } @@ -58,15 +58,16 @@ static void loadFromEnvironmentVariables(DatabricksConfig cfg) throws IllegalAcc } accessor.setValueOnConfig(cfg, env); } - } catch (DatabricksException e) { - String msg = String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); + } catch (DatabricksException | IllegalAccessException e) { + String msg = + String.format("%s auth: %s", cfg.getCredentialsProvider().authType(), e.getMessage()); throw new DatabricksException(msg, e); } } static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { if (isNullOrEmpty(cfg.getProfile()) - && (isAnyAuthConfigured(cfg) + && (isAnyAuthConfigured(cfg) || !isNullOrEmpty(cfg.getHost()) || !isNullOrEmpty(cfg.getAzureWorkspaceResourceId()))) { return; @@ -94,14 +95,13 @@ static void loadFromConfig(DatabricksConfig cfg) throws IllegalAccessException { if (!hasExplicitProfile) { profile = "DEFAULT"; } - SubnodeConfiguration section = ini.getSection(profile); - if (section == null && !hasExplicitProfile) { + boolean sectionNotPresent = section == null || section.isEmpty(); + if (sectionNotPresent && !hasExplicitProfile) { LOG.info("{} has no {} profile configured", configFile, profile); return; } - - if (section == null) { + if (sectionNotPresent) { String msg = String.format("resolve: %s has no %s profile configured", configFile, profile); throw new DatabricksException(msg); } @@ -166,18 +166,22 @@ static void validate(DatabricksConfig cfg) throws DatabricksException { } if (authSet.size() <= 1) return; String names = String.join(" and ", authSet); - throw new DatabricksException(String.format("validate: more than one authorization method configured: %s", names)); + throw new DatabricksException( + String.format("validate: more than one authorization method configured: %s", names)); } catch (IllegalAccessException e) { throw new DatabricksException("Cannot create default config", e); } } - public static DatabricksException makeNicerError(String message, Exception e, DatabricksConfig cfg) { + public static DatabricksException makeNicerError( + String message, Exception e, DatabricksConfig cfg) { return makeNicerError(message, e, 200, cfg); } - public static DatabricksException makeNicerError(String message, Exception e, Integer statusCode, DatabricksConfig cfg) { - boolean isHttpUnauthorizedOrForbidden = true; // TODO - pass status code with exception, default this to false + public static DatabricksException makeNicerError( + String message, Exception e, Integer statusCode, DatabricksConfig cfg) { + boolean isHttpUnauthorizedOrForbidden = + true; // TODO - pass status code with exception, default this to false if (statusCode == 401 || statusCode == 402) isHttpUnauthorizedOrForbidden = true; String debugString = ""; if (cfg.getEnv() != null) {