Wrong support for URI

[florent-lb]

Hi Hibernate !

I worked with Hibernate Reactive to introduce it in my client's spring boot environment. But the OPS team in my client use an api to generate URL DB. Theses url contains _ (underscore). Using _ will never be a problem with "standard" hibernate, I use these url today without any issue but you choose to use URI instead of URL and URI doesn't support underscore.

The problem is here : DefaultSqlClientPoolConfiguration

Example value (anything with _) : jdbc:postgresql://my_db:5432.com/my_schema

URI doesn't throw any error but the value Host & port stay null and authority is fill instead. but DefaultSqlClientPoolConfiguration (L 111) :

link

String host = scheme.equals("oracle") ? oracleHost(uri) : uri.getHost();

link
int port = scheme.equals("oracle") ? oraclePort(uri) : uri.getPort();

Try to read only getHost() and getPort()

This problem doesn't occurs with URL, but URL doesn't support all scheme like URI.
But URI must support _ (https://www.rfc-editor.org/rfc/rfc3986#section-2.3), I'll try to replace URI with %5F but still broken.

Replace by ipv4 adresse == no problem

Problem follow here in OpenJDK : https://bugs.openjdk.org/browse/JDK-8291591

But today is a regression comparing to Hibernate "standard". And Without dns name I can't go to production with Hibernate Reactive today :'(

For more in formation :

Hibernate version :
org.hibernate.reactive:hibernate-reactive-core:1.1.9.Final

[DavideD]

Thanks, it seems like a bug and we will have to revisit that method.

As a workaround, you can register your own SqlClientPoolConfiguration implementation and return a valid SqlConnectOptions. Basically, you have to override DefaultSqlClientPoolConfiguration#connectOptions(URI uri).

We describe how to do it for the VertxInstace in the documentation, but the process is the same for SqlClientPoolConfiguration.

[DavideD]

@blafond, would you have time to look into this?

[florent-lb]

Thank for the link sharing I look that asap.

[blafond]

Looking at it

[DavideD]

Thanks

[blafond]

@florent-lb Could you add a little stack trace to this issue containing the path to DefaultSqlClientPoolConfiguration?

[blafond]

I've written a simple test that calls new DefaultSqlClientPoolConfiguration().connectOptions( "jdbc:postgresql://my_db:5432.com/my_schema" ); and I get the same error and removing the jdbc: segment the test works.

In all of our current tests that require a connection a DefaultSqlCientPool is used to convert a URL into a URI via the static parse( String url) method.

In our reactive world the URI's scheme is expected to be jdbc: and we currently remove that from the configuration properties.

[DavideD]

We've decided some time ago to accept JDBC as configuration even if underneath we are not using JDBC so that people could use the same URL they would use for ORM. Also, I think we expected the Vert.x SQL client to reuse the same syntax (this is not actually the case). In the end we've also made the jdbc part optional considering that jdbc is not actually used (it would be nice in the future to allow the use of the vert.x syntax, but that's for another issue).
Anyway, I don't think that's the issue, but if it's the case, then it shouldn't be too hard to solve.

It think the best approach for this issue is to add a test in JdbcUrlParserTest that uses my_host as localhost (maybe other values as well). And test that the resulting SqlConnectOptions object contains the correct info.

Then we have to figure out if there's a different way to parse the url string that doesn't exclude values in DefaultSqlClientPoolConfiguration#connectOptions. I think we can get the whole string url using Uri.toString(), so we might not need to change all parameters but I'm not sure.

@blafond Does this help?

[DavideD]

I've added a quick test JdbcUrlParserTest and it fails:

	@Test
	public void testIssue1425() {
		Map<String, String> params = new HashMap<>();
		params.put( "user", "hello" );

		String url = createJdbcUrl( "local_host", dbType().getDefaultPort(), "my_db", params );
		assertOptions( url, "my_db", params );
	}

This is the url for PostgreSQL: jdbc:postgresql://local_host:5432/my_db?user=hello

[gavinking]

We've decided some time ago to accept JDBC as configuration even if underneath we are not using JDBC so that people could use the same URL they would use for ORM.

I still think this is a good idea. Hibernate users already know this syntax, and it lets them easily copy/paste config between the two things.

[DavideD]

It seems to work fine and I'm not against it.
But I would like to also support the vert.x syntax using the vertx: prefix.

It would be less work for us and It would also be a nice fallback for cases like this.

[DavideD]

One thing that we are doing right now is trying to parse any possible jdbc url, it would probably be easier to identify the dialect first and then parse the url accordingly.