From 51eb24b2660479ed8d06e23003bc0d2b84ac8f5a Mon Sep 17 00:00:00 2001 From: Justin Wyer Date: Thu, 19 Aug 2021 14:37:17 +0200 Subject: [PATCH 1/5] - Detect SID `:` or service name `/` in the supplied TC jdbc URL and honor this in the underlying JDBC URL. - Switched affected regex patterns to use named groups - Add tests for both SID and service name connections --- .../testcontainers/jdbc/ConnectionUrl.java | 34 +++++++++++++------ .../jdbc/ConnectionUrlDriversTests.java | 10 ++++-- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java index 4f339b43fe5..3524ed4da3e 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java @@ -97,25 +97,25 @@ 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(value -> Integer.valueOf(value)); + 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() @@ -209,12 +209,24 @@ public Map 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:" + + "(?[a-z0-9]+)" + + "(:(?[^:]+))?" + + "://" + + "(?[^\\?]+)" + + "(?\\?.*)?"); + + Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile("jdbc:tc:" + + "(?[a-z]+)" + + "(:(?[^(:thin:)]+))?:" + + "thin:(//)?" + + "((?[^\\?^/]+)/(?[^\\?^/]+))?" + + "@" + + "(?[^\\?]+)" + + "(?\\?.*)?"); //Matches to part of string - hostname:port/databasename - Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile("([^:]+)(:([0-9]+))?/([^\\\\?]+)"); + Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile("(?[^:]+)(:(?[0-9]+))?((?[:/])|;databaseName=)(?[^\\\\?]+)"); Pattern DAEMON_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_DAEMON=([^\\?&]+).*"); Pattern INITSCRIPT_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITSCRIPT=([^\\?&]+).*"); diff --git a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java index 920fab04016..ac53d819726 100644 --- a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java +++ b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java @@ -37,12 +37,16 @@ public static Iterable 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:thin:@localhost:1521:test", "oracle", Optional.empty(), "localhost:1521:test", "test"} }); } From b971a1e99a07d189aae59ad09d886da136d3d994 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 22 Sep 2021 16:58:21 +0100 Subject: [PATCH 2/5] Further tidying --- .../testcontainers/jdbc/ConnectionUrl.java | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java index 3524ed4da3e..f9a8f559765 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java @@ -10,6 +10,7 @@ import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; + import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -149,7 +150,7 @@ private void parseUrl() { } private Map parseTmpfsOptions(Map containerParameters) { - if(!containerParameters.containsKey("TC_TMPFS")){ + if (!containerParameters.containsKey("TC_TMPFS")) { return Collections.emptyMap(); } @@ -209,24 +210,37 @@ public Map 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:" + + "(?[a-z0-9]+)" + + "(:(?[^:]+))?" + + "://" + + "(?[^\\?]+)" + + "(?\\?.*)?" + ); + + Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile( + "jdbc:tc:" + + "(?[a-z]+)" + + "(:(?[^(:thin:)]+))?:" + + "thin:(//)?" + + "((?[^\\?^/]+)/(?[^\\?^/]+))?" + + "@" + + "(?[^\\?]+)" + + "(?\\?.*)?" + ); //Matches to part of string - hostname:port/databasename - Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile("(?[^:]+)(:(?[0-9]+))?((?[:/])|;databaseName=)(?[^\\\\?]+)"); + Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile( + "(?[^:]+)" + + "(:(?[0-9]+))?" + + "(" + + "(?[:/])" + + "|" + + ";databaseName=" + + ")" + + "(?[^\\\\?]+)" + ); Pattern DAEMON_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_DAEMON=([^\\?&]+).*"); Pattern INITSCRIPT_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITSCRIPT=([^\\?&]+).*"); From 3fbd7fa2d7c4209d0f16da47ddbdd48aa41edaf4 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 22 Sep 2021 17:00:13 +0100 Subject: [PATCH 3/5] Further tidying --- .../testcontainers/jdbc/ConnectionUrl.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java index f9a8f559765..43f96a0d638 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java @@ -7,7 +7,6 @@ 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; @@ -110,7 +109,7 @@ private void parseUrl() { Matcher dbInstanceMatcher = Patterns.DB_INSTANCE_MATCHING_PATTERN.matcher(dbHostString); if (dbInstanceMatcher.matches()) { databaseHost = Optional.of(dbInstanceMatcher.group("databaseHost")); - databasePort = Optional.ofNullable(dbInstanceMatcher.group("databasePort")).map(value -> Integer.valueOf(value)); + databasePort = Optional.ofNullable(dbInstanceMatcher.group("databasePort")).map(Integer::valueOf); databaseName = Optional.of(dbInstanceMatcher.group("databaseName")); } @@ -124,7 +123,7 @@ private void parseUrl() { .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); @@ -215,7 +214,7 @@ public interface Patterns { "(?[a-z0-9]+)" + "(:(?[^:]+))?" + "://" + - "(?[^\\?]+)" + + "(?[^?]+)" + "(?\\?.*)?" ); @@ -224,9 +223,9 @@ public interface Patterns { "(?[a-z]+)" + "(:(?[^(:thin:)]+))?:" + "thin:(//)?" + - "((?[^\\?^/]+)/(?[^\\?^/]+))?" + + "((?[^?^/]+)/(?[^?^/]+))?" + "@" + - "(?[^\\?]+)" + + "(?[^?]+)" + "(?\\?.*)?" ); @@ -242,9 +241,8 @@ public interface Patterns { "(?[^\\\\?]+)" ); - 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 DAEMON_MATCHING_PATTERN = Pattern.compile(".*([?&]?)TC_DAEMON=([^?&]+).*"); + Pattern INITFUNCTION_MATCHING_PATTERN = Pattern.compile(".*([?&]?)TC_INITFUNCTION=" + "((\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*\\.)*\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" + "::" + "(\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" + @@ -252,9 +250,9 @@ public interface Patterns { 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("([^?&=]+)=([^?&]*)"); } From d3bc44c5479f2e4228aa3e6bbdfb89a007db6604 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 22 Sep 2021 17:18:22 +0100 Subject: [PATCH 4/5] Fix image tag extraction Tags containing any of the chars [t,h,i,n] would break --- .../main/java/org/testcontainers/jdbc/ConnectionUrl.java | 8 +++++--- .../testcontainers/jdbc/ConnectionUrlDriversTests.java | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java index 43f96a0d638..4b7d869eb2a 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java @@ -221,9 +221,11 @@ public interface Patterns { Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile( "jdbc:tc:" + "(?[a-z]+)" + - "(:(?[^(:thin:)]+))?:" + - "thin:(//)?" + - "((?[^?^/]+)/(?[^?^/]+))?" + + "(:(?(?!thin).+))?:thin:(//)?" + + "(" + + "(?[^:" + + "?^/]+)/(?[^?^/]+)" + + ")?" + "@" + "(?[^?]+)" + "(?\\?.*)?" diff --git a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java index ac53d819726..12ca5c28c3d 100644 --- a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java +++ b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ConnectionUrlDriversTests.java @@ -46,6 +46,7 @@ public static Iterable data() { {"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"} }); } From 97f627a4c8aaa94dcd01d139cc84e9a69249a025 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 22 Sep 2021 17:43:25 +0100 Subject: [PATCH 5/5] Reinstate and deprecate unused constant --- .../main/java/org/testcontainers/jdbc/ConnectionUrl.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java index 4b7d869eb2a..3354ed45c39 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ConnectionUrl.java @@ -244,6 +244,13 @@ public interface Patterns { ); 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}*)" + "::" +