[5.3] Feature Request – Support non-standard database ports

muhme commented 1 month ago

Last edited on 5 September 2024 to link PRs and crossed out new field variant

Is your feature request related to a problem? Please describe.

To be able to enter non-standard port numbers for all supported database servers in the Web Installer and using it in Cypress System Tests.

This is currently possible for MySQL and MariaDB, but not for PostgreSQL. See screenshots: mysql mariadb error

Describe the solution you'd like

Be able to enter host:port ~~or have new additional field for the port number~~ in the Web Installer for all supported databases and databases types::

MySQL with MySQL (PDO) and MySQLi
MariaDB with MySQL (PDO) and MySQLi
PostgreSQL with PostgreSQL (PDO)

And adopting the System Tests to be able to handle non-standard database port numbers.
There is an old attempt to implement this with https://github.com/joomla/joomla-cms/pull/4165

Issues Implementing this Feature Request

Some small implementations are missing in joomla-cypress
- 👉 https://github.com/joomla-projects/joomla-cypress/pull/33
- 👉 https://github.com/joomla-projects/joomla-cypress/pull/36
- After merge it is also needed to create new joomla-cypress release and to use the new release in all active Joomla branches
Some small implementation is missing in the System Tests 👉 https://github.com/joomla/joomla-cms/pull/43968
~~There is need for design and implementation in PHP Web Installer.~~
And to document the feature.
There is a need to extend the Joomla Framework Database driver 👉 https://github.com/joomla-framework/database/pull/310
~~There is a need to extend administrator | Global configuration | Server to show the port number 👉 https://github.com/joomla/joomla-cms/pull/43926~~
There is a need to test IPv6 👉 I can handle this.

richard67 commented 1 month ago

When reading my old PRs which fixed the database host check for port numbers in the host name it seems to me that it once was working for PostgreSQL: #29567 , #29568 .

muhme commented 1 month ago

When reading my old PRs which fixed the database host check for port numbers in the host name it seems to me that it once was working for PostgreSQL: #29567 , #29568 .

Thank you for checking and adding the PRs 👍 And it looks to me as if the test instructions demand that PostgreSQL should be tested, but it was not tested: "no PostgreSQL".

richard67 commented 1 month ago

And it looks to me as if the test instructions demand that PostgreSQL should be tested, but it was not tested: "no PostgreSQL".

I would assume that I had tested it myself on PostgreSQL, but I don’t remember it now.

richard67 commented 1 month ago

@muhme What happens if you use the "MySQL (PDO)" driver with MySQL or MariaDB and not the "MySQLi"?

And when you have tested PostgreSQL (PDO), was the database server listing at that port? If not: Could you try again with the PostgreSQL database server being configured to work on the custom port used for the test?

When you enter database connection data and then continue, the connection is tested by the installation. It might be that this works slightly different with the PDO and the MySQLi drivers, so that it fails with that error message when the connection to that port doesn't work.

Could you check and report back if that is the case?

muhme commented 1 month ago

What happens if you use the "MySQL (PDO)" driver with MySQL or MariaDB and not the "MySQLi"?

Successful tested host.docker.internal:7011 is also working for "MySQL (PDO)" with MySQL and host.docker.internal:7012 for MariaDB. Added as requirement to the solution, good point.

was the database server listing at that port?

Yes, tested database host:port is reachable from web server container. If not, it takes a long time until the timeout. I assume, failing here is the name resolution as host:port together are used as hostname.

PostgreSQL database server being configured to work on the custom port used for the test?

No, inside the docker container PostgreSQL is running on port 5432. Or do you mean using the standard port :5432, yes this was tried and is also not working. Or do you mean reaching the database from web server, yes from web server telnet jbt_pg 5432 can establish a connection.

I guess host:port is used as hostname and not working for PostgreSQL driver. I saw samples like pg_connect("host=sheep port=5432 dbname=mary user=lamb password=foo"); so port may have to be given in different way.

richard67 commented 1 month ago

No, inside the docker container PostgreSQL is running on port 5432.

@muhme What I still don't understand: For the others you have used host.docker.internal:xyz, with xyz being the custom port. For PostgreSQL your screenshot shows localhost. So is the webserver running in the same container as the database server or in a different container? A localhost in the installation form will resolve to the webserver.

muhme commented 1 month ago

I tried a lot and picked up the screenshot with localhost as localhost is always reachable. Currently double checked with container hostname and with mapped port:

could not translate host name "jbt_pg:5432" to address: Name or service not known
could not translate host name "host.docker.internal:7013" to address: Name or service not known

Both fail and the two analogue configurations work for MySQL and MariaDB. But anyway, thank you for your questions. 👍

richard67 commented 1 month ago

When using localhost the PDO driver may end up in using a unix socket, so 127.0.0.1 should be used to avoid that. See e.g. https://stackoverflow.com/questions/21046672/pdo-not-working-with-port .

Then I found out that the MySQLi driver extracts the port number from the host string here https://github.com/joomla-framework/database/blob/3.x-dev/src/Mysqli/MysqliDriver.php#L201-L252 because the MySQLi client requires the port to be specified with a separate parameter and not in the host name.

That piece of code by the way also gives some hints in comments e.g. on how to specify in IPv6 address with a port (e.g. [fe80:102::2%eth1]:3306).

The MySQL (PDO) and the PostgreSQL (PDO) drivers seem not to do that. The PDO driver base class determines the data source string here: https://github.com/joomla-framework/database/blob/3.x-dev/src/Pdo/PdoDriver.php#L128-L321

I have to dig deeper to see why it works with MySQL (PDO) but not with PostgreSQL (PDO).

alikon commented 3 weeks ago

a draft pr #43926 about

There is a need to extend administrator | Global configuration | Server to show the port number

richard67 commented 3 weeks ago

a draft pr #43926 about

There is a need to extend administrator | Global configuration | Server to show the port number

Well that how to do it with a new field in global configuration, which is of course the clean way.

On the other hand, we do not really want to add new fields if we can avoid it.

The alternative to the PR mentioned above would be to add similar code to the other database drivers as we have it in the MySQLi driver in the connect method to extract the port from the host name when it has a port at the end, separated from the rest of the host name by a colon.

I know that might be more ugly than a new configuration field, as all database drivers already support it to get the port with a separate parameter. But it would make all 3 drivers behave in the same way like the MySQLi driver already does.

muhme commented 2 weeks ago

it would make all 3 drivers behave in the same way like the MySQLi driver already does

Note, as it was tested today for the joomla-cypress PR: Currently all four drivers mysqli, mysql, mariadbi and mariadb work in Web Installer with host:port, only pgsql does not

muhme commented 1 week ago

@alikon and me had a video call today to align and to see what are the next steps:

We are in favour of a solution for non-standard database ports with an additional field in the web installer and in the backend administration screens.
We assume that it will be a 5.3 enhancement.
Strictly speaking, it is about the non-standard database port field. It is therefore an optional field that is initially empty. The default port is not entered in the form, as otherwise it may have to be changed if the user changes the database type.
Additional open TODOs (until the updated list in the initial description) are
- Impement in driver/postgresql.sql
- Display in System Information | Configuration File
Heiko will test https://github.com/joomla/joomla-cms/pull/43926 together with https://github.com/joomla-projects/joomla-cypress/pull/34

HLeithner commented 1 week ago

I'm not a fan of a new field if you get the value as everyone else in a couple of lines of code: https://3v4l.org/oFFaE or a bit cleaned up https://3v4l.org/C2ADn

<?php

$values = [
    'ip' => '127.0.0.1',
    'hostname' => 'localhost',
    'ipport' => '127.0.0.1:3307',
    'hostname' => 'localhost:3308',
    'ipv6' => '[2001:0DB8::1234]',
    'ipv6port' => '[2001:0DB8::1234]:3309',
    ];

foreach($values as $k=>$hostport) {
$host = $hostport;
$port = 'not found';

// has port or ipv6
if (strpos($hostport, ':')) {
    // ipv6
    if (str_starts_with($hostport, '[')) {
        $portposition = strpos($hostport,']:');
        if ($portposition) {

          $port = substr($hostport, $portposition+2);
          $host = substr($hostport, 0, $portposition+1);
        }
    } else {
        [$host, $port] = explode(':', $hostport);
    }
}

echo $k .': Hostname: ' . $host . ' Port: ' . $port ."\n";

}

don't add options if we don't really need it, the database stuff as already many options, especially if it comes to encryption

richard67 commented 1 week ago

As far as I remember, the database drivers (framework) should already support to pass the port number as a separate option. The MySQL drivers contain code to extract the port number from the host name if it is given with the host name (appended with colon) and the separate option is not given. The PostgreSQL driver does not contain that code.

See my comment above with links to the relevant parts of the code: https://github.com/joomla/joomla-cms/issues/43902#issuecomment-2282809586

I think we should extend the PostgreSQL driver in the framework so that it behaves the same as the other drivers and you can specify the port either appended to the host name or given with the separate option.

In this way we would not need a new field in the installation and the backend (but could add it later if we want), but when you have a 3rd party extension with code to connect to another database, they could use the separate option to tell the driver which port to use.

HLeithner commented 1 week ago

sorry didn't read you comment only the post by @muhme so yes copy the code from mysql to pgsql or parts of it to the connect function is better in my opinion.

richard67 commented 1 week ago

@HLeithner Your code should not use the same array key 'hostname' for 2 cases. I would suggest to use 'hostnameport' for the 2nd one, so you have:

$values = [
    'ip' => '127.0.0.1',
    'hostname' => 'localhost',
    'ipport' => '127.0.0.1:3307',
    'hostnameport' => 'localhost:3308',
    'ipv6' => '[2001:0DB8::1234]',
    'ipv6port' => '[2001:0DB8::1234]:3309',
    ];

But anyway I think this code (or other code which we already have and which does the same thing) should not be in the CMS, it should be in the database driver.

richard67 commented 1 week ago

@muhme @alikon I have created a PR in the database framework. Please test https://github.com/joomla-framework/database/pull/310 . But @alikon please don't close this issue here until we have a new version with that PR in the framework and a CMS PR to update to that new version.

richard67 commented 1 week ago

P.S.: @alikon 's PR #43926 should still work with the changes from my framework PR.

muhme commented 1 week ago

Since @HLeithner and @richard67 have argued against a new "Non-Standard Database Port" field, and @richard67 has already taken action thankfully with https://github.com/joomla-framework/database/pull/310, @alikon and me are okay with implementing and documenting the non-standard database port as host:port without having a new field in the Web Installer and backend forms.

joomla / joomla-cms