mysql port syntax in config

[ghost]

nextcloud-server：16.0.1

If the mysql server port not default 3306, then your config.php dbport not have effect.
error log show us : refuse to connect mysql server

and the solution, is make the dbhost ip:port

like

    'dbport' => '192.168.1.3:3358',

[BernieO]

There is no configuration value like dbport in the admin manual and the admin manual clearly states that the port has to be added in dbhost:

https://docs.nextcloud.com/server/16/admin_manual/configuration_server/config_sample_php_parameters.html#default-parameters

To specify a port use hostname:####

This is desired and documented behaviour and not a bug in my opinion.

EDIT: this was already the case in the stable9 branch of the documentation:
https://github.com/nextcloud/documentation/blob/stable9/admin_manual/configuration_server/config_sample_php_parameters.rst

[kesselb]

Thank you for triage @BernieO 🎉

Feel free to repopen if dbport is documented somewhere @netroby.

[ghost]

@BernieO @kesselb

https://github.com/nextcloud/server/search?q=dbport&unscoped_q=dbport

The nextcloud install wizard will create configure file, contains the dbport config section.
So it not a document problem your code should not contains dbport configure section,
It was not right.

[kesselb]

Fair enough ;)

@nextcloud/server-triage Remove dbport or use the value?

[BernieO]

good find @netroby

[MorrisJobke]

@nextcloud/server-triage Remove dbport or use the value?

I guess I don't get it fully. The port is written back to the config file, but this should not cause problems. Also the port is used and only if not filled then it uses the part from the host config:

server/lib/private/Setup/AbstractDatabase.php

Lines 120 to 135 in 0eebff1

	if (!empty($this->dbPort)) {
	if (ctype_digit($this->dbPort)) {
	$connectionParams['port'] = $this->dbPort;
	} else {
	$connectionParams['unix_socket'] = $this->dbPort;
	}
	} else if (strpos($this->dbHost, ':')) {
	// Host variable may carry a port or socket.
	list($host, $portOrSocket) = explode(':', $this->dbHost, 2);
	if (ctype_digit($portOrSocket)) {
	$connectionParams['port'] = $portOrSocket;
	} else {
	$connectionParams['unix_socket'] = $portOrSocket;
	}
	$connectionParams['host'] = $host;
	}

[BernieO]

Is this code exclusively used when installing Nextcloud?

[kesselb]

@BernieO i think so ;)

server/lib/private/DB/ConnectionFactory.php

Line 188 in 0ee191b

public function createConnectionParams() {

Usually the connection factory is used to create the db connection. dbport is not used there. Lets fix this mess 🤣

[MorrisJobke]

Is this code exclusively used when installing Nextcloud?

Not really - it's used on every DB connection init call - so on every request. I was wrong. It is used during setup only, because that is the AbstractDatabase class inside the setup namespace and not the normal DB code. Sorry for the confusion.

[DasSkelett]

Somewhat related:
As stated above, the code always assumes the port to be specified in dbhost, and ignores dbport.
Thus it's a real mess to specify an IPv6 address for the db. You can't write it in square brackets like everywhere else in the config.
This results in a very messy look, and possibly buggy behaviour, because the port cannot be unambiguously separated from the address.

Examples:
Fictional DB IP: [2001:DB8::1:2]
Fictional DB port: 5432

This doesn't work, but should (assuming specifying the port in dbhost is allowed, which is currently):

'dbhost' => '[2001:DB8::1:2]:5432',
'dbport' => '',

This doesn't work, but should:

'dbhost' => '[2001:DB8::1:2]',
'dbport' => '5432',

This doesn't work, and maybe should:

'dbhost' => '2001:DB8::1:2',
'dbport' => '5432',

This works (in my case), but is very susceptible to errors. It assumes the part after the last : is always the port, happens in splitHostFromPortAndSocket():

server/lib/private/DB/ConnectionFactory.php

Lines 237 to 254 in 0ee191b

	protected function splitHostFromPortAndSocket($host): array {
	$params = [
	'host' => $host,
	];
	$matches = [];
	if (preg_match('/^(.*):([^\]:]+)$/', $host, $matches)) {
	// Host variable carries a port or socket.
	$params['host'] = $matches[1];
	if (is_numeric($matches[2])) {
	$params['port'] = (int) $matches[2];
	} else {
	$params['unix_socket'] = $matches[2];
	}
	}
	return $params;
	}

Which is fatal if it isn't specified to make use of the default values:

'dbhost' => '2001:DB8::1:2:5432',
'dbport' => '',

The DB ConnectionFactory code ignores IPv6 totally, and it only works by pure chance in some cases.

From a first look it doesn't seem too hard to fix the code, however a proper fix might break backwards compatibility for the config.php, especially for IPv6 addresses.

If you motivate me a little, I can try to do a PR. But prepare for some review work, it would be my first for Nextcloud.

[kesselb]

That seems to be okay to me. I think we should remove dbport and keep it with dbhost.