Refactor JDBC URL parsing to support different Oracle URLs (#4476)

Detects SID `:` or service name `/` in the supplied TC JDBC URL.
In addition, the affected regex patterns have been refactored to use named groups.


Co-authored-by: Justin Wyer <[email protected]>

Author: rnorth

modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java

@@ -7,9 +7,9 @@
 import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collector;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
@@ -97,33 +97,33 @@ private void parseUrl() {
                 throw new IllegalArgumentException("JDBC URL matches jdbc:tc: prefix but the database or tag name could not be identified");
             }
         }
-        databaseType = urlMatcher.group(1);
+        databaseType = urlMatcher.group("databaseType");
 
-        imageTag = Optional.ofNullable(urlMatcher.group(3));
+        imageTag = Optional.ofNullable(urlMatcher.group("imageTag"));
 
         //String like hostname:port/database name, which may vary based on target database.
         //Clients can further parse it as needed.
-        dbHostString = urlMatcher.group(4);
+        dbHostString = urlMatcher.group("dbHostString");
 
         //In case it matches to the default pattern
         Matcher dbInstanceMatcher = Patterns.DB_INSTANCE_MATCHING_PATTERN.matcher(dbHostString);
         if (dbInstanceMatcher.matches()) {
-            databaseHost = Optional.of(dbInstanceMatcher.group(1));
-            databasePort = Optional.ofNullable(dbInstanceMatcher.group(3)).map(value -> Integer.valueOf(value));
-            databaseName = Optional.of(dbInstanceMatcher.group(4));
+            databaseHost = Optional.of(dbInstanceMatcher.group("databaseHost"));
+            databasePort = Optional.ofNullable(dbInstanceMatcher.group("databasePort")).map(Integer::valueOf);
+            databaseName = Optional.of(dbInstanceMatcher.group("databaseName"));
         }
 
         queryParameters = Collections.unmodifiableMap(
             parseQueryParameters(
-                Optional.ofNullable(urlMatcher.group(5)).orElse("")));
+                Optional.ofNullable(urlMatcher.group("queryParameters")).orElse("")));
 
         String query = queryParameters
             .entrySet()
             .stream()
             .map(e -> e.getKey() + "=" + e.getValue())
             .collect(Collectors.joining("&"));
 
-        if (query == null || query.trim().length() == 0) {
+        if (query.trim().length() == 0) {
             queryString = Optional.empty();
         } else {
             queryString = Optional.of("?" + query);
@@ -149,7 +149,7 @@ private void parseUrl() {
     }
 
     private Map<String, String> parseTmpfsOptions(Map<String, String> containerParameters) {
-        if(!containerParameters.containsKey("TC_TMPFS")){
+        if (!containerParameters.containsKey("TC_TMPFS")) {
             return Collections.emptyMap();
         }
 
@@ -209,26 +209,59 @@ public Map<String, String> getTmpfsOptions() {
      * @author manikmagar
      */
     public interface Patterns {
-        Pattern URL_MATCHING_PATTERN = Pattern.compile("jdbc:tc:([a-z0-9]+)(:([^:]+))?://([^\\?]+)(\\?.*)?");
-
-        Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile("jdbc:tc:([a-z]+)(:([^(thin:)]+))?:thin:@([^\\?]+)(\\?.*)?");
+        Pattern URL_MATCHING_PATTERN = Pattern.compile(
+            "jdbc:tc:" +
+                "(?<databaseType>[a-z0-9]+)" +
+                "(:(?<imageTag>[^:]+))?" +
+                "://" +
+                "(?<dbHostString>[^?]+)" +
+                "(?<queryParameters>\\?.*)?"
+        );
+
+        Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile(
+            "jdbc:tc:" +
+                "(?<databaseType>[a-z]+)" +
+                "(:(?<imageTag>(?!thin).+))?:thin:(//)?" +
+                "(" +
+                "(?<username>[^:" +
+                "?^/]+)/(?<password>[^?^/]+)" +
+                ")?" +
+                "@" +
+                "(?<dbHostString>[^?]+)" +
+                "(?<queryParameters>\\?.*)?"
+        );
 
         //Matches to part of string - hostname:port/databasename
-        Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile("([^:]+)(:([0-9]+))?/([^\\\\?]+)");
-
-        Pattern DAEMON_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_DAEMON=([^\\?&]+).*");
-        Pattern INITSCRIPT_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITSCRIPT=([^\\?&]+).*");
-        Pattern INITFUNCTION_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITFUNCTION=" +
+        Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile(
+            "(?<databaseHost>[^:]+)" +
+                "(:(?<databasePort>[0-9]+))?" +
+                "(" +
+                "(?<sidOrServiceName>[:/])" +
+                "|" +
+                ";databaseName=" +
+                ")" +
+                "(?<databaseName>[^\\\\?]+)"
+        );
+
+        Pattern DAEMON_MATCHING_PATTERN = Pattern.compile(".*([?&]?)TC_DAEMON=([^?&]+).*");
+
+        /**
+         * @deprecated for removal
+         */
+        @Deprecated
+        Pattern INITSCRIPT_MATCHING_PATTERN = Pattern.compile(".*([?&]?)TC_INITSCRIPT=([^?&]+).*");
+
+        Pattern INITFUNCTION_MATCHING_PATTERN = Pattern.compile(".*([?&]?)TC_INITFUNCTION=" +
             "((\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*\\.)*\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" +
             "::" +
             "(\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" +
             ".*");
 
         String TC_PARAM_NAME_PATTERN = "(TC_[A-Z_]+)";
 
-        Pattern TC_PARAM_MATCHING_PATTERN = Pattern.compile(TC_PARAM_NAME_PATTERN + "=([^\\?&]+)");
+        Pattern TC_PARAM_MATCHING_PATTERN = Pattern.compile(TC_PARAM_NAME_PATTERN + "=([^?&]+)");
 
-        Pattern QUERY_PARAM_MATCHING_PATTERN = Pattern.compile("([^\\?&=]+)=([^\\?&]*)");
+        Pattern QUERY_PARAM_MATCHING_PATTERN = Pattern.compile("([^?&=]+)=([^?&]*)");
 
     }
 

modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java

@@ -37,12 +37,17 @@ public static Iterable<Object[]> data() {
                 {"jdbc:tc:mysql://hostname/test", "mysql", Optional.empty(), "hostname/test", "test"},
                 {"jdbc:tc:postgresql:1.2.3://hostname/test", "postgresql", Optional.of("1.2.3"), "hostname/test", "test"},
                 {"jdbc:tc:postgresql://hostname/test", "postgresql", Optional.empty(), "hostname/test", "test"},
-                {"jdbc:tc:sqlserver:1.2.3://localhost;instance=SQLEXPRESS:1433;databaseName=test", "sqlserver", Optional.of("1.2.3"), "localhost;instance=SQLEXPRESS:1433;databaseName=test", ""},
-                {"jdbc:tc:sqlserver://localhost;instance=SQLEXPRESS:1433;databaseName=test", "sqlserver", Optional.empty(), "localhost;instance=SQLEXPRESS:1433;databaseName=test", ""},
+                {"jdbc:tc:sqlserver:1.2.3://localhost;instance=SQLEXPRESS:1433;databaseName=test", "sqlserver", Optional.of("1.2.3"), "localhost;instance=SQLEXPRESS:1433;databaseName=test", "test"},
+                {"jdbc:tc:sqlserver://localhost;instance=SQLEXPRESS:1433;databaseName=test", "sqlserver", Optional.empty(), "localhost;instance=SQLEXPRESS:1433;databaseName=test", "test"},
                 {"jdbc:tc:mariadb:1.2.3://localhost:3306/test", "mariadb", Optional.of("1.2.3"), "localhost:3306/test", "test"},
                 {"jdbc:tc:mariadb://localhost:3306/test", "mariadb", Optional.empty(), "localhost:3306/test", "test"},
+                {"jdbc:tc:oracle:1.2.3:thin://@localhost:1521/test", "oracle", Optional.of("1.2.3"), "localhost:1521/test", "test"},
                 {"jdbc:tc:oracle:1.2.3:thin:@localhost:1521/test", "oracle", Optional.of("1.2.3"), "localhost:1521/test", "test"},
-                {"jdbc:tc:oracle:thin:@localhost:1521/test", "oracle", Optional.empty(), "localhost:1521/test", "test"}
+                {"jdbc:tc:oracle:thin:@localhost:1521/test", "oracle", Optional.empty(), "localhost:1521/test", "test"},
+                {"jdbc:tc:oracle:1.2.3:thin:@localhost:1521:test", "oracle", Optional.of("1.2.3"), "localhost:1521:test", "test"},
+                {"jdbc:tc:oracle:1.2.3:thin://@localhost:1521:test", "oracle", Optional.of("1.2.3"), "localhost:1521:test", "test"},
+                {"jdbc:tc:oracle:1.2.3-anything:thin://@localhost:1521:test", "oracle", Optional.of("1.2.3-anything"), "localhost:1521:test", "test"},
+                {"jdbc:tc:oracle:thin:@localhost:1521:test", "oracle", Optional.empty(), "localhost:1521:test", "test"}
             });
     }