[hibernate#1425] Fix URL parsing error

Relaxed host value character constraints and added test to verify

hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java

@@ -108,21 +108,11 @@ public SqlConnectOptions connectOptions(URI uri) {
 
 		String host = scheme.equals( "oracle" )
 				? oracleHost( uri )
-				: uri.getHost();
+				: findHost( uri, scheme );
 
 		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();
-		}
+				: findPort( uri, scheme );
 
 		if ( port == -1 ) {
 			port = defaultPort( scheme );
@@ -274,7 +264,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 +286,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 +295,45 @@ private String oraclePath(URI uri) {
 		return string.substring( start, end );
 	}
 
+	/**
+	 * if the host contains invalid characters {@link java.net.URI} parseAuthority(int start, int n) will end up
+	 * throwing a {@link java.net.URISyntaxException} which results in setting the host to NULL.
+	 * This method will reparse if host is NULL and set the host value
+	 */
+	private String findHost(URI uri, String scheme) {
+		if ( uri.getHost() == null ) {
+			String s = uri.toString();
+			String remainder = s.substring( s.indexOf( "://" ) + 3 );
+			// If no port defined than check for '/' or ';'
+			char endOfHostPortChar = ( scheme.equals( "sqlserver" )) ? ';' : '/';
+			String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) );
+			int endOfHost = hostPortString.indexOf( ':' );
+			if ( endOfHost == -1 ) {
+				return hostPortString;
+			}
+			return hostPortString.substring( 0, endOfHost );
+		}
+
+		return uri.getHost();
+	}
+
+	private int findPort(URI uri, String scheme) {
+		if ( uri.getPort() == -1 ) {
+			String s = uri.toString();
+			String remainder = s.substring( s.indexOf( "://" ) + 3 );
+			// If no port defined than check for '/' or ';'
+			char endOfHostPortChar = ( scheme.equals( "sqlserver" )) ? ';' : '/';
+			String hostPortString = remainder.substring( 0, remainder.indexOf( endOfHostPortChar ) );
+			int startOfPort = hostPortString.indexOf( ':' );
+			if ( startOfPort == -1 ) {
+				return -1;
+			}
+			return Integer.valueOf( hostPortString.substring( startOfPort+1 ) );
+		}
+
+		return uri.getPort();
+	}
+
 	private int defaultPort(String scheme) {
 		switch ( scheme ) {
 			case "postgresql":

hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java

@@ -32,7 +32,11 @@ public class DefaultPortTest {
 	public void testDefaultPortIsSet() throws URISyntaxException {
 		DefaultSqlClientPoolConfiguration configuration = new DefaultSqlClientPoolConfiguration();
 		configuration.configure( requiredProperties() );
-		SqlConnectOptions sqlConnectOptions = configuration.connectOptions( new URI( scheme() + "://localhost/database" ) );
+		// all supported DB URLs except for sqlserver use the '/' delimiter separating host/port and user/db info
+		// sqlserver URL uses ';' as the delimeter so added a check for sqlserver to insure connectOptions() call succeeds
+		// with valid URL syntax
+		String partialValidUrl = scheme().equalsIgnoreCase( "sqlserver" ) ? "://localhost;database=my_db" : "://localhost/database=my_db";
+		SqlConnectOptions sqlConnectOptions = configuration.connectOptions( new URI( scheme() + partialValidUrl ) );
 		Assertions.assertThat( sqlConnectOptions.getPort() )
 				.as( "Default port not defined for " + dbType() )
 				.isEqualTo( dbType().getDefaultPort() );

hibernate-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java

@@ -108,11 +108,41 @@ 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<String, String> 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<String, String> 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 );
+	}
+
 	/**
 	 * Create the default {@link SqlConnectOptions} with the given extra properties
 	 * and assert that's correct.
 	 */
 	private void assertOptions(String url, String expectedDbName, Map<String, String> parameters) {
+		assertOptions( url, expectedDbName, "localhost", parameters );
+	}
+
+	/**
+	 * Create the default {@link SqlConnectOptions} with the given extra properties
+	 * and assert that's correct.
+	 */
+	private void assertOptions(String url, String expectedDbName, String expectedHost, Map<String, String> parameters) {
 		URI uri = DefaultSqlClientPool.parse( url );
 		SqlConnectOptions options = new DefaultSqlClientPoolConfiguration().connectOptions( uri );
 
@@ -125,7 +155,7 @@ private void assertOptions(String url, String expectedDbName, Map<String, String
 		assertThat( options.getUser() ).as( "URL: " + url ).isEqualTo( username );
 		assertThat( options.getPassword() ).as( "URL: " + url ).isEqualTo( password );
 		assertThat( options.getDatabase() ).as( "URL: " + url ).isEqualTo( expectedDbName );
-		assertThat( options.getHost() ).as( "URL: " + url ).isEqualTo( "localhost" );
+		assertThat( options.getHost() ).as( "URL: " + url ).isEqualTo( expectedHost );
 		assertThat( options.getPort() ).as( "URL: " + url ).isEqualTo( dbType().getDefaultPort() );
 
 		// Check extra properties