Skip to content

Commit

Permalink
[#1425] Fix URL parsing error
Browse files Browse the repository at this point in the history 
Relaxed host value character constraints and added test to verify
  • Loading branch information
@blafond @DavideD
blafond authored and DavideD committed Dec 5, 2022
1 parent dc13463 commit 095a3bc
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 88 deletions.
107 changes: 84 additions & 23 deletions ...ore/src/main/java/org/hibernate/reactive/pool/impl/DefaultSqlClientPoolConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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( '?' );
}
}
Expand All @@ -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
Expand All @@ -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":
Expand Down
63 changes: 0 additions & 63 deletions hibernate-reactive-core/src/test/java/org/hibernate/reactive/DefaultPortTest.java
View file

This file was deleted.

68 changes: 66 additions & 2 deletions ...e-reactive-core/src/test/java/org/hibernate/reactive/configuration/JdbcUrlParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
package org.hibernate.reactive.configuration;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.Map;

import org.hibernate.HibernateError;
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;
Expand Down Expand Up @@ -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<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 );
}

@Test
public void testDefaultPortIsSet() throws URISyntaxException {
Map<String, String> 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<String, String> 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<String, String> parameters) {
private void assertOptions(String url, String expectedDbName, String expectedHost, Map<String, String> parameters) {
URI uri = DefaultSqlClientPool.parse( url );
SqlConnectOptions options = new DefaultSqlClientPoolConfiguration().connectOptions( uri );

Expand All @@ -125,10 +175,24 @@ 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
assertThat( options.getProperties() ).as( "URL: " + url ).containsExactlyInAnyOrderEntriesOf( parameters );
}

/**
* 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 );
}

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 );
}
}

0 comments on commit 095a3bc

Please sign in to comment.