From 095a3bceeaaf8183b5fdccbae65c32e1eff34b13 Mon Sep 17 00:00:00 2001 From: blafond Date: Fri, 2 Dec 2022 11:05:33 -0600 Subject: [PATCH] [#1425] Fix URL parsing error Relaxed host value character constraints and added test to verify --- .../DefaultSqlClientPoolConfiguration.java | 107 ++++++++++++++---- .../hibernate/reactive/DefaultPortTest.java | 63 ----------- .../configuration/JdbcUrlParserTest.java | 68 ++++++++++- 3 files changed, 150 insertions(+), 88 deletions(-) delete mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java index 2e114d4bc..78781674c 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java @@ -106,27 +106,8 @@ public SqlConnectOptions connectOptions(URI uri) { database = database.substring( 0, database.indexOf( ':' ) ); } - String host = scheme.equals( "oracle" ) - ? oracleHost( uri ) - : uri.getHost(); - - int port = scheme.equals( "oracle" ) - ? oraclePort( uri ) - : uri.getPort(); - - int index = uri.toString().indexOf( ';' ); - if ( scheme.equals( "sqlserver" ) && index > 0 ) { - // SQL Server separates parameters in the url with a semicolon (';') - // and the URI class doesn't get the right value for host and port when the url - // contains parameters - URI uriWithoutParams = URI.create( uri.toString().substring( 0, index ) ); - host = uriWithoutParams.getHost(); - port = uriWithoutParams.getPort(); - } - - if ( port == -1 ) { - port = defaultPort( scheme ); - } + String host = findHost( uri, scheme ); + int port = findPort( uri, scheme ); //see if the credentials were specified via properties String username = user; @@ -274,7 +255,7 @@ private String oracleHost(URI uri) { int end = host.indexOf( ':' ); if ( end == -1 ) { end = host.indexOf( '/' ); - if ( end == -1) { + if ( end == -1 ) { end = host.indexOf( '?' ); } } @@ -296,7 +277,7 @@ private String oraclePath(URI uri) { string = string.substring( i + 1 ); // Check the start of the path int start = string.indexOf( '/' ); - if ( start == -1) { + if ( start == -1 ) { return ""; } int end = string.indexOf( '?' ) == -1 @@ -305,6 +286,86 @@ private String oraclePath(URI uri) { return string.substring( start, end ); } + // Example sqlserver://localhost:1433;database=name;user=somename + private String sqlServerHost(URI uri) { + String s = uri.toString(); + String remainder = s.substring( s.indexOf( "://" ) + 3 ); + char endOfHostPortChar = ';'; + String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) ); + int endOfHost = hostPortString.indexOf( ':' ); + if ( endOfHost == -1 ) { + return hostPortString; + } + return hostPortString.substring( 0, endOfHost ); + } + + private int sqlServerPort(URI uri) { + String s = uri.toString(); + String remainder = s.substring( s.indexOf( "://" ) + 3 ); + char endOfHostPortChar = ';'; + String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) ); + int startOfPort = hostPortString.indexOf( ':' ); + if ( startOfPort == -1 ) { + return -1; + } + return Integer.valueOf( hostPortString.substring( startOfPort + 1 ) ); + } + + private String extractHost(URI uri) { + String s = uri.toString(); + String remainder = s.substring( s.indexOf( "://" ) + 3 ); + char endOfHostPortChar = '/'; + String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) ); + int endOfHost = hostPortString.indexOf( ':' ); + if ( endOfHost == -1 ) { + return hostPortString; + } + return hostPortString.substring( 0, endOfHost ); + } + + private int extractPort(URI uri) { + String s = uri.toString(); + String remainder = s.substring( s.indexOf( "://" ) + 3 ); + char endOfHostPortChar = '/'; + String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) ); + int startOfPort = hostPortString.indexOf( ':' ); + if ( startOfPort == -1 ) { + return -1; + } + return Integer.valueOf( hostPortString.substring( startOfPort + 1 ) ); + } + + private String findHost(URI uri, String scheme) { + if ( uri.getHost() != null ) { + return uri.getHost(); + } + if ( "oracle".equals( scheme ) ) { + return oracleHost( uri ); + } + if ( "sqlserver".equals( scheme ) ) { + // SqlServer host + return sqlServerHost( uri ); + } + return extractHost( uri ); + } + + private int findPort(URI uri, String scheme) { + int port = -1; + if ( "oracle".equals( scheme ) ) { + port = oraclePort( uri ); + } + else if ( "sqlserver".equals( scheme ) ) { + port = sqlServerPort( uri ); + } + else { + port = extractPort( uri ); + } + if ( port == -1 ) { + return defaultPort( scheme ); + } + return port; + } + private int defaultPort(String scheme) { switch ( scheme ) { case "postgresql": diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java deleted file mode 100644 index 13d54b4d6..000000000 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java +++ /dev/null @@ -1,63 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive; - -import java.net.URI; -import java.net.URISyntaxException; -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; - -import org.hibernate.reactive.containers.DatabaseConfiguration; -import org.hibernate.reactive.pool.impl.DefaultSqlClientPoolConfiguration; -import org.hibernate.reactive.provider.Settings; - -import org.junit.Assert; -import org.junit.Test; - -import io.vertx.sqlclient.SqlConnectOptions; -import org.assertj.core.api.Assertions; - -import static org.hibernate.reactive.containers.DatabaseConfiguration.dbType; - -/** - * Test the default port is set correctly when using {@link DefaultSqlClientPoolConfiguration} - */ -public class DefaultPortTest { - - @Test - public void testDefaultPortIsSet() throws URISyntaxException { - DefaultSqlClientPoolConfiguration configuration = new DefaultSqlClientPoolConfiguration(); - configuration.configure( requiredProperties() ); - SqlConnectOptions sqlConnectOptions = configuration.connectOptions( new URI( scheme() + "://localhost/database" ) ); - Assertions.assertThat( sqlConnectOptions.getPort() ) - .as( "Default port not defined for " + dbType() ) - .isEqualTo( dbType().getDefaultPort() ); - } - - @Test - public void testUnrecognizedSchemeException() throws URISyntaxException { - Assert.assertThrows( IllegalArgumentException.class, () -> { - URI uri = new URI( "bogusScheme://localhost/database" ); - new DefaultSqlClientPoolConfiguration().connectOptions( uri ); - } ); - } - - private static String scheme() { - if ( dbType() == DatabaseConfiguration.DBType.MARIA ) { - return "mariadb"; - } - return dbType().name().toLowerCase( Locale.ROOT ); - } - - // Only set the required properties - // We are not connecting to a db so it doesn't matter - private Map requiredProperties() { - Map map = new HashMap(); - map.put( Settings.USER, "BogusUser" ); - return map; - } -} diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java index 51b679062..b2d8482a1 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java @@ -6,6 +6,7 @@ package org.hibernate.reactive.configuration; import java.net.URI; +import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; @@ -13,6 +14,7 @@ import org.hibernate.reactive.pool.impl.DefaultSqlClientPool; import org.hibernate.reactive.pool.impl.DefaultSqlClientPoolConfiguration; +import org.junit.Assert; import org.junit.Test; import io.vertx.sqlclient.SqlConnectOptions; @@ -108,11 +110,59 @@ public void testDatabaseAsProperty() { assertOptions( url, "helloDatabase", params ); } + // URI regex does not include the '_' underscore character, so URI parsing will set the `host` and `userInfo` + // to NULL. This test verifies that the processing captures the actual host value and includes it in the + // connect options. Example: postgresql://local_host:5432/my_schema + @Test + public void testInvalidHostSucceeds() { + Map params = new HashMap<>(); + params.put( "user", "hello" ); + + String url = createJdbcUrl( "local_host", dbType().getDefaultPort(), "my_db", params ); + assertOptions( url, "my_db", "local_host", params ); + } + + @Test + public void testInvalidHostWithoutPort() { + Map params = new HashMap<>(); + params.put( "user", "hello" ); + + // Port -1 so it won't be added to the url + String url = createJdbcUrl( "local_host", -1, "my_db", params ); + assertOptions( url, "my_db", "local_host", params ); + } + + @Test + public void testDefaultPortIsSet() throws URISyntaxException { + Map params = new HashMap<>(); + params.put( "user", "hello" ); + + String url = createJdbcUrl( "localhost", -1, "my_db", params ); + assertPort( url, dbType().getDefaultPort() ); + } + + @Test + public void testCustomPortIsSet() throws URISyntaxException { + Map params = new HashMap<>(); + params.put( "user", "hello" ); + + String url = createJdbcUrl( "localhost", 19191, "my_db", params ); + assertPort( url, 19191 ); + } + + @Test + public void testUnrecognizedSchemeException() throws URISyntaxException { + Assert.assertThrows( IllegalArgumentException.class, () -> { + URI uri = new URI( "bogusScheme://localhost/database" ); + new DefaultSqlClientPoolConfiguration().connectOptions( uri ); + } ); + } + /** * Create the default {@link SqlConnectOptions} with the given extra properties * and assert that's correct. */ - private void assertOptions(String url, String expectedDbName, Map parameters) { + private void assertOptions(String url, String expectedDbName, String expectedHost, Map parameters) { URI uri = DefaultSqlClientPool.parse( url ); SqlConnectOptions options = new DefaultSqlClientPoolConfiguration().connectOptions( uri ); @@ -125,10 +175,24 @@ private void assertOptions(String url, String expectedDbName, Map parameters) { + assertOptions( url, expectedDbName, "localhost", parameters ); + } + + private void assertPort(String url, int expectedPort) { + URI uri = DefaultSqlClientPool.parse( url ); + SqlConnectOptions options = new DefaultSqlClientPoolConfiguration().connectOptions( uri ); + assertThat( options.getPort() ).as( "URL: " + url ).isEqualTo( expectedPort ); + } }