Copilot commented on code in PR #7317:
URL: https://github.com/apache/kyuubi/pull/7317#discussion_r2800375787
##########
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java:
##########
@@ -565,13 +565,16 @@ static int getVersionPart(String fullVersion, int
position) {
}
public static String parsePropertyFromUrl(final String url, final String
key) {
- String[] tokens = url.split(";");
- for (String token : tokens) {
- if (token.trim().startsWith(key.trim() + "=")) {
- return token.trim().substring((key.trim() + "=").length());
- }
+ String clientPropertiesSection = url.split("[?#]")[0];
Review Comment:
The method does not validate the input URL before processing. Consider
adding null checks and URL format validation to prevent potential
NullPointerException or ArrayIndexOutOfBoundsException when processing
malformed URLs.
```suggestion
if (url == null || key == null || key.isEmpty()) {
return null;
}
String[] urlParts = url.split("[?#]", 2);
if (urlParts.length == 0 || urlParts[0].isEmpty()) {
return null;
}
String clientPropertiesSection = urlParts[0];
```
##########
kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java:
##########
@@ -210,4 +210,43 @@ public void testSplitSqlStatement() {
splitSql.get(0));
assertEquals("", splitSql.get(1));
}
+
+ @Test
+ public void testParsePropertyFromUrlConsistentWithExtractURLComponents()
+ throws JdbcUriParseException {
+ // Test case for JWT auth with query string
+ String url =
+
"jdbc:hive2://host:10012/access_views;transportMode=http;httpPath=cliservice;"
+ + "ssl=true;auth=JWT?kyuubi.session.cluster=clusterA";
+
+ // Parse using parsePropertyFromUrl
+ String authFromParseProperty = Utils.parsePropertyFromUrl(url, "auth");
+
+ // Parse using extractURLComponents
+ JdbcConnectionParams params = Utils.extractURLComponents(url, new
Properties());
+ String authFromExtract = params.getSessionVars().get("auth");
+
+ // Both methods should return "JWT", not
"JWT?kyuubi.session.cluster=clusterA"
+ assertEquals("JWT", authFromParseProperty);
+ assertEquals("JWT", authFromExtract);
+ assertEquals(
+ "parsePropertyFromUrl and extractURLComponents should return the same
auth value",
+ authFromExtract,
+ authFromParseProperty);
+
+ // Verify query string parameters are correctly parsed
+ assertEquals("clusterA",
params.getHiveConfs().get("kyuubi.session.cluster"));
+ }
+
+ @Test
+ public void testParsePropertyFromUrlWithoutQueryString() {
Review Comment:
Consider adding test coverage for edge cases such as: null URLs, empty URLs,
URLs with missing semicolons, URLs with only a query string (no properties),
and URLs with special characters in property values to ensure robust handling
of all scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
