Fix bug #67563: mysqli compiled with mysqlnd does not take ipv6 adress as parameter #19750
Conversation
…s as parameter In the past, when libmysqlclient could be used, it accepted ipv6 addresses as hostname without enclosing it first in brackets. However, in mysqlnd this never worked. In the past this caused a discrepancy between the two implementations. Nowadays, mysqli only works with mysqlnd so we don't even have to cater to libmysqlclient. However, a plain ipv6 address should still work as a hostname. Also for people migrating to newer PHP versions it's nice if this keeps working. The solution is to check if we're dealing with an ipv6 address not yet enclosed in brackets. In that case we add the brackets automatically.
nielsdos
requested review from
SakiTakamachi,
bukka and
kamil-tekiela
as code owners
This should now also work properly in PDO-MYQSL, whereas previously it did not work. |
|
The change looks good - nice and simple! About the test, CI should support ::1 and think it's even resolving localhost to ::1 so why can't you add a test connection to ::1 (with maybe some grant update if needed)? |
ext/mysqli/tests/bug67563.phpt
Outdated
|
|
||
| $host = '::1'; | ||
|
|
||
| if (!$link = my_mysqli_connect($host, 'pamtest', 'pamtest', $db, $port, $socket)) { |
There was a problem hiding this comment.
| if (!$link = my_mysqli_connect($host, 'pamtest', 'pamtest', $db, $port, $socket)) { | |
| if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { |
Should it not be like this?
There was a problem hiding this comment.
Yeah I copied this from somewhere, still have to make it work
There was a problem hiding this comment.
I don't know why but I am getting a memory leak when running this patch:
004+ [001] Cannot connect to the server using host=::1, user=pamtest, passwd=pamtest dbname=test, port=3306, socket=
005+ done![Tue Sep 30 12:06:46 2025] Script: '/mnt/f/projects/php-src/ext/mysqli/tests/bug67563.php'
006+ /mnt/f/projects/php-src/Zend/zend_smart_str.c(172) : Freeing 0x00007f2516494300 (224 bytes), script=/mnt/f/projects/php-src/ext/mysqli/tests/bug67563.php
007+ === Total 1 memory leaks detected ===
There was a problem hiding this comment.
Shouldn't be introduced by this patch. I suppose it's a pre-existing bug where on failure something isn't freed. I can take a look after a while. I'll let CI run first.
There was a problem hiding this comment.
I can't trigger that leak on my system
|
CI finally green after fighting CI a bit |
|
Is there a way to modify the test so that if the MySQL server is not configured for IPv6, it doesn't fail? |
I suppose it could test [::1] as a host in the skipif |
3 participants
In the past, when libmysqlclient could be used, it accepted ipv6 addresses as hostname without enclosing it first in brackets. However, in mysqlnd this never worked. In the past this caused a discrepancy between the two implementations.
Nowadays, mysqli only works with mysqlnd so we don't even have to cater to libmysqlclient. However, a plain ipv6 address should still work as a hostname. Also for people migrating to newer PHP versions it's nice if this keeps working.
The solution is to check if we're dealing with an ipv6 address not yet enclosed in brackets. In that case we add the brackets automatically.
Testing this automatically seems not straight-forward. The connection string is not logged. Using the fake server requires modifications to specify a host and can't be easily reused in the same phpt file. This was tested by hand.