SpongeAPI: Expected configuration format for SqlService is ambiguous

Sponge’s SqlService provides managed JDBC DataSources to clients. A DataSource is requested by providing the configuration as String. The format of this String is defined in the API specification as follows:

jdbc:<engine>://[<username>[:<password>]@]<host>/<schema>

This is problematic: schema may be omitted because not every supported DBMS requires one (they are optional for H2, and not supported by SQLite). In these cases host may still include slashes as the the host is a path to file on the system. As a result, Sponge is unable to know whether a database is given or not.

Example

For example, look at this:

jdbc:h2:username:password@./path/to/dabase/file

The user expects Sponge to connect to a H2 database file under ./path/to/dabase/file that does not use any schema. Instead, Sponge should (from the API specification) connect to a database file under ./path/to/dabase/ with the schema file.

Even worse for SQLite:

jdbc:sqlite:./path/to/dabase/file

Sponge should try to parse file as a schema even if SQLite does not support schemas.

Fix

The simple fix would be to change the separator between host and schema. Instead of / a character (or multiple characters) should be used, that cannot be used within an URL or path - e.g. //. Obviously, this would break existing configurations but this is inevitable.

About this issue

Original URL
State: open
Created 7 years ago
Comments: 16 (6 by maintainers)

Most upvoted comments

I did not catch what the problem is. Sponge do not parse all the url and do nothing with any url parts beyond login:password pair. I made some tests on urls, that was provided by @TheE and there is no problems:

    @Test
    public void testSchemaMissH2() throws SQLException {
        final String jdbcUrl = "jdbc:h2:username:password@./path/to/dabase/file";
        final SqlServiceImpl.ConnectionInfo subject = SqlServiceImpl.ConnectionInfo.fromUrl(null, jdbcUrl);

        assertEquals("username", subject.getUser());
        assertEquals("password", subject.getPassword());
        assertEquals("jdbc:h2:./path/to/dabase/file", subject.getAuthlessUrl());
        assertEquals("org.h2.Driver", subject.getDriverClassName());
    }

    @Test
    public void testSchemaMissSqlite() throws SQLException {
        final String jdbcUrl = "jdbc:sqlite:./path/to/dabase/file";
        final SqlServiceImpl.ConnectionInfo subject = SqlServiceImpl.ConnectionInfo.fromUrl(null, jdbcUrl);

        assertNull(subject.getUser());
        assertNull(subject.getPassword());
        assertEquals("jdbc:sqlite:./path/to/dabase/file", subject.getAuthlessUrl());
        assertEquals("org.sqlite.JDBC", subject.getDriverClassName());
    }

The problem is with poor documentation, not with the code. More examples and clarifications in the javadoc of SqlService#getDataSource methods may be sufficient to resolve this issue.

While I agree, the problem is that HikariCP, the library Sponge uses for connection pooling, requires username and password to be given separately from the JDBC url (see here). Without any specification, it would be impossible to extract these information from the JDBC url as different databases require different formats.

This is not true. HikariCP do not mandate how credentials must be set. They may be passed via setUsername/setPassword methods, but setting them in the jdbc url works equally fine.

onyn on Dec 6, 2018

The issue here is that sponge expects the URLs to be formatted in a specific way, where it really should just allow any valid JDBC url.

While I agree, the problem is that HikariCP, the library Sponge uses for connection pooling, requires username and password to be given separately from the JDBC url (see here). Without any specification, it would be impossible to extract these information from the JDBC url as different databases require different formats.

It might however be technically possible to change SqlService to something like the following (although handling of aliases would be awkward):

getDataSource(String jdbcUrl, @Nullable String username, @Nullable password);
getDataSource(String jdbcUrl, @Nullable Properties connectionProperties);

Changing the syntax here would definetely do more harm then good, since for now it is at least consistent with the majority of JDBC drivers. Instead, any further requirements should be lifted.

While this format is indeed supported by major DBMS like MySQL or PostgreSQL, it neither supports SQLite nor H2 (it does not support schema less databases)

From my own experience as developer and server admin I am inclined to say that most users are probably using these flat-file databases. (Even if they are not, I doubt that any noticeable number of users opts for something different than MySQL or MariaDB).

TheE on Jan 3, 2018