Fix GH-7932: MariaDB version prefix not always stripped

[cmb69]

For consistency with mysqlnd_conn_data#get_server_version, we strip
the MariaDB version prefix also in mysqlnd_command#handshake.

---

The definition of MARIA_DB_VERSION_HACK_PREFIX should better go into a header (maybe mysqlnd_portability.h), but since all mysqlnd headers are installed, we better delay that for master to avoid compatibility issues.

@kamil-tekiela, what do you think? The alternative would be to remove the stripping in mysqlnd_conn_data#get_server_version, but that would bring back https://bugs.php.net/78179.

[kamil-tekiela]

I understand the confusion, however it works exactly how it is supposed to now (probably).

server_info should return a "string" exactly as provided by the server. We can't strip any information because we are not in any position to do so. We haven't been instructed by MariaDB to strip it and from what I saw in their own code, they don't strip it either.

server_version returns the server version as an "int". This information is retrieved from server_info by parsing that string. It is expected to be in the format (\d+)\.(\d+)\.(\d+).* The formula is 1000*$1 + 100*$2 + 10*$3.
Because MariaDB is prefixing the server_info string with false version information, PHP has a hack for this and ignores it. I am not sure if PHP should really be doing that, but it seems to make more sense for PHP. It also matches the expectation of end-users, even though we don't fully adhere to the spec. This means that in case of MariaDB 10+ connection, we change the format to be 5.5.5-(\d+)\.(\d+)\.(\d+).*.

If we want to fix this, we should remove the hack that we currently have in PHP. Let MariaDB 10 claim to be version 5.5.5 if they want. This will probably result in bug reports from users claiming that our hacky logic makes more sense. So as a compromise, I would leave it as it is now.

The string returned by server_info doesn't have to match select version(). It's not the same!

[cmb69]

We haven't been instructed by MariaDB to strip it and from what I saw in their own code, they don't strip it either.

C:\Program Files\MariaDB 10.5\bin>mysql -u root -p
Enter password: ****
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 1294
Server version: 10.5.13-MariaDB mariadb.org binary distribution

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> select version();
+-----------------+
| version()       |
+-----------------+
| 10.5.13-MariaDB |
+-----------------+
1 row in set (0.000 sec)

But under the hood, this server claims to be 5.5.5-10.5.13-MariaDB.

In my opinion, that version mess (it's not only about the compatibility prefix) will bite many clients in the future, but somehow we have to deal with this.

I'm okay with leaving everything as is, but the inconsistency could be confusing for users. Maybe we should introduce an INI setting for this, but that could cause even more confusion. At the very least we should properly document how mysqlnd handles this.

[kamil-tekiela]

As long as we provide the server_info verbatim, users can do whatever they want in their code. They can ignore the prefix. Users can also fetch the version information from SELECT VERSION(). It's not something that we should deal with on the language level. We can add a note in the manual like this user note

[cmb69]

Okay, then let's make that a doc issue. Thank you!

[mvorisek]

We haven't been instructed by MariaDB to strip it and from what I saw in their own code, they don't strip it either.

@kamil-tekiela links like https://jira.mariadb.org/browse/MDEV-9565?page=com.googlecode.jira-suite-utilities%3Atransitions-summary-tabpanel https://stackoverflow.com/questions/56601304/what-does-the-first-part-of-the-mariadb-version-string-mean clearly show this is a pure * transport * hack and it should never reach the end user.

Is this enought proof to reopen or can you please connect wit someone from MariaDB to reconfirm? The 5.5.5 prefix is causing issues as a lot of clients parse the version from the version string beginning, which is currently wrong because of the transport version hack.

[vuvova]

You've mentioned me in a previous (now edited-away) version of the above comment, so I'm here to reconfirm.

Yes, it is a pure transport hack. MySQL replication used to decide whether the server is using a new or old replication protocol by looking at the first byte of the version. 3 meant old, 4 or above meant new, below 3 was an error, MySQL first ever version was 3.x. Thus MySQL wasn't able to replicate from MariaDB 10.0, so we've added that hack.

Any conforming client is expected to strip this prefix away.

[jabcreations]

I just discovered that my server broke with the following command about an hour and change ago:

$version = mysqli_get_server_info($db);

I've seen at least one other thread about this. I can see the automatic update from 8.0.15 to 8.0.16 via yum history info 120. Live Linux server and the MySQL version has now been stripped.

This code had been working fine for the past several years minimally. I'm not seeing any direct commitment to change the string output though obviously someone somewhere did. I've been relying on it to avoid another pointless SQL query to the server to ensure that various versions of the software (e.g. PHP, MariaDB) are installed for day-to-day maintenance as well as when I have to do server migrations manually as my host only totally sucks when it comes to migrations and I need to ensure that the various versions of software are proper. Could someone please check and make sure someone didn't go crying to dad in another thread because mom wouldn't do so-and-so? I don't need things breaking because someone else doesn't know how to work with string functions.

[kamil-tekiela]

@jabcreations Every bug fix can potentially break someone else's workflow. Mandatory XKCD reference.
Let's approach this professionally. What exactly is your use case and what exactly broke in your opinion. How were you using mysqli_get_server_info? We have put considerable thought into this bug fix, but we would like to hear your opinion and use case.

[cmb69]

@jabcreations, you're likely referring to commit 5fc0db9. That fix has been done to work around the MariaDB versioning issues.

[jabcreations]

This was very old code though I test things such as changing the numbers around to ensure that it would trigger if things changed. This was working until about 5AM earlier this morning when the server updated from 8.0.15 to 8.0.16:

 $mysql_ver = mysqli_get_server_info($db);
 if ($mysql_ver>=5.5) {$xml .= 'good';}
 else {$xml .= 'emergency';}

I have a log viewer that I use with MariaDB and this command apparently does not create an SQL query so it has a slight performance benefit over using SELECT VERSION();.

I don't mind horribly using something else (ideally, not adding a database query albeit only for myself as the Administrator) though we're programmers dealing with strings all of the time so I'd like to avoid having to have the concerns of questioning if I'll have to start rewriting code years later because of a literal minor update like this.

[mvorisek]

please post here the actual/expected result of mysqli_get_server_info($db) and SELECT VERSION()

[kamil-tekiela]

Comparing a string to a float seems to be a very naive and potentially invalid behaviour. Are you sure this piece of code is correct? Maybe it should be using mysqli_get_server_version instead which returns an int?

You mentioned something about using string functions. Can you demonstrate how you are using this function with any of the string functions?

[jabcreations]

The code worked fine then and as I said I had tested it against static copies with different versions.

5.5.5-10.5.4-MariaDB-log

mvorisek I recommend looking in to PHP functions such as explode, substr and substr_count.

[kamil-tekiela]

@jabcreations Why do you expect the version with the prefix to be returned by this function? I am still unclear on what you think the regression is or why you think the prefix should be there. The actual MariaDB version is 10.5.4. Anyway, if you want to know the version number, you are using the wrong function.

To clarify why your code is currently broken.
"5.5.5-10.5.4-MariaDB-log" <=> "5.5" = 1
"10.5.4-MariaDB-log" <=> "5.5" = -1
This is because 1 is less than 5 in ASCII.

[cmb69]

Apparently, that code worked by sheer luck as of PHP 8.0.0, where that comparison behavior has changed. You're likely better off using version_compare(): https://3v4l.org/ZNQkq

[popcorner]

I'm not sure why the mysqlnd should do additional procedures for mariadb, and cause incompatible results.
When I write a program using mysqli, I will assume that this is a real mysql, which have version 5 and version 8. There is no version 10 in mysql.
If mysqli returns a version number of mariadb, it will be a disaster for a program which don't want to support mariadb. previously the program may work as an undocumented feature, but now the program won't work anymore.

There are lots of databases trying to use mysql-like protocol, like percona and tidb,and lots of cloud databases like amazon aurora, TDSQL-C and more.
if they all have their own behavior, can php support all of them? Why should php give mariadb special treatment? mysqlnd is intended for connecting mysql, not something else.

[kamil-tekiela]

I'm not sure why the mysqlnd should do additional procedures for mariadb, and cause incompatible results.

In the context of this bug, only the prefix in the version string is stripped out. The version number has already been parsed ignoring the prefix. The prefix was added as a technical workaround by MariaDB and if you follow the conversation, to make it compatible with MySQL, the prefix must be stripped

mysqli returns a version number of mariadb, it will be a disaster for a program which don't want to support mariadb.

That has always been the case. Mysqli returns the information that is provided by the server. MariaDB returns information identifying itself in a MySQL compatible way (if we ignore that MySQL has a single-digit version and MariaDB double-digit).

Why should php give mariadb special treatment? mysqlnd is intended for connecting mysql, not something else.

The mysqli extension is meant to be used to connect to MySQL-like databases. While it's true that MySQL and MariaDB are diverging more and more, they are still compatible enough to not require any special treatment. This small change that was done to fix this bug, isn't breaking compatibility. It's only stripping away technical information that should not be exposed to PHP code anyway. If you really need to have that prefix, please go ahead and prepend 5.5.5- to the version string.

As an aside, I would really be interested to know what code relied on this prefix that seems to have stopped working now. It seems very odd to me that it would be used rather than the mysqli_get_server_version() value.

[laozhoubuluo]

I'm not sure why the mysqlnd should do additional procedures for mariadb, and cause incompatible results.

In the context of this bug, only the prefix in the version string is stripped out. The version number has already been parsed ignoring the prefix. The prefix was added as a technical workaround by MariaDB and if you follow the conversation, to make it compatible with MySQL, the prefix must be stripped

mysqli returns a version number of mariadb, it will be a disaster for a program which don't want to support mariadb.

That has always been the case. Mysqli returns the information that is provided by the server. MariaDB returns information identifying itself in a MySQL compatible way (if we ignore that MySQL has a single-digit version and MariaDB double-digit).

Why should php give mariadb special treatment? mysqlnd is intended for connecting mysql, not something else.

The mysqli extension is meant to be used to connect to MySQL-like databases. While it's true that MySQL and MariaDB are diverging more and more, they are still compatible enough to not require any special treatment. This small change that was done to fix this bug, isn't breaking compatibility. It's only stripping away technical information that should not be exposed to PHP code anyway. If you really need to have that prefix, please go ahead and prepend 5.5.5- to the version string.

As an aside, I would really be interested to know what code relied on this prefix that seems to have stopped working now. It seems very odd to me that it would be used rather than the mysqli_get_server_version() value.

It needs to be clear that the MariaDB side announces to the application side that its database has a certain compatibility with MySQL 5.5.5 through MariaDB/server@c50ee6c#diff-5b45fa673c88c06a9651c7906364f592, and the application side can process its related processes according to MySQL 5.5.5.

The version number actively reported by MariaDB should be respected. If they only want to use this version number during replication, MariaDB or the application should actively distinguish between replication requests and ordinary requests, instead of directly hacking them.

Stepping back 10,000 steps, the current modification method actually removes this compatibility layer and causes BC on the application side. Such modification should at least be done in the major version like PHP 8.2, otherwise it will cause incompatibility between the old minor version and the new minor version.

My current version of the software is not GA, so I can complete compatibility through troubleshooting and modification. I also do not deny that the method of directly comparing the size or intercepting the first position of the version number is inappropriate, but the PHP ecosystem determines that most of the software is not used by developers, and it is very difficult for users to troubleshoot and diagnose such problems. so upstream should consider the issue of forward compatibility when making changes in the minor version number. If the software is already GA and claims that the current version supports PHP 8.0, then this modification will directly break the downstream commitment to end users, which will cause application developers to lose trust in users, and may cause application developers to violate licenses The protocol was even sued, hoping that the upstream would consider the downstream instead of modifying it arbitrarily when making incompatible changes to the minor version.

[popcorner]

@kamil-tekiela Thanks for your reply!
The prefix can be used for claiming itself as a MySQL-like version number, so previously developers often use this to determine which kind of SQL should be used in their programs. But when the rules changed, the version of MariaDB is higher than all the MySQL releases, but MariaDB doesn't have these features in MySQL actually (especially in MySQL 8), which causes unexpected breaks of these programs.

As an aside, I would really be interested to know what code relied on this prefix that seems to have stopped working now. It seems very odd to me that it would be used rather than the mysqli_get_server_version() value.

For example, this is a demo working properly in PHP 8.1.2 and 10.1.37-MariaDB:

error_reporting(E_ALL);
// connect the mysql server
$conn = new mysqli();
$conn->real_connect('127.0.0.1','testdb','testdb','testdb', null, null, MYSQLI_CLIENT_COMPRESS);
$conn->set_charset('utf8');
$conn->query("SET sql_mode=''");
$conn->query("SET character_set_client=binary");

// trying to create table
// mysql version 5.7 and later can use larger index key prefixes
var_dump($conn->server_info);
$keylimit = version_compare($conn->server_info, '5.6', '>') ? '' : '(180)';
$sql = <<<EOT
CREATE TABLE `mail` (
	`id` int(11) NOT NULL AUTO_INCREMENT,
	`mail` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL,
	PRIMARY KEY (`id`),
	UNIQUE KEY `mail` (`mail`$keylimit)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
EOT;
if(!$conn->query($sql)) {
	var_dump($conn->error);
} else {
	echo 'create table success';
}

varchar(255) is commonly used and will work in older databases. But when they try to add full UTF-8 support (like emoji), they need to change the whold database to utf8mb4, then the 767 bytes max key length is not enough anymore so such workarounds may be used to solve this problem. And it do works for many years.

But when the users update the PHP from 8.1.2 to 8.1.3, it will not work anymore (10.1.37-MariaDB):

Fatal error: Uncaught mysqli_sql_exception: Specified key was too long; max key length is 767 bytes in sql.php:22
Stack trace:
0 sql.php(22): mysqli->query('CREATE TABLE `m...')
1 {main} thrown in sql.php on line 22

mysqli_get_server_version is not suitable for this situation because you can't use this without writing external rules for version 10.

And MySQL will continue to publish new versions in the future and maybe MySQL 10 will be released someday. What can we do at that time?

[kamil-tekiela]

Ok, I understand, but in that case, you are talking about an entirely different problem. The dummy prefix wasn't meant to be used like you did in the code above. It was only meant to tell the replication software that it's not talking to MySQL 1 database. All MariaDB 10 versions have that prefix that should be used only in replication and stripped anywhere else. If you want to write code that can be interoperable on both databases and have feature detection, you need to build some rules. As I said, both databases are diverging. The features and even SQL syntax differs between them. You might only rarely encounter this but as shown in the example, it is a real problem. It won't help you if we retain the MariaDB replication prefix as that wasn't actually telling you anything useful about the capabilities of the server. You would have to check for MariaDB in the info string and then compare the version. This means, you would have to build rules and create some feature detection. There might already be PHP libraries available to do this. The problem you are describing isn't to do with PHP, mysqli or mysqlnd. It's just the fact that these two databases have small differences between them. If you would like MariaDB to change the info string, then you would have to file a bug with them. Else, you can write some code in PHP yourself and try to guess which features your database supports based on the currently given version number.

PHP-SRC can't accommodate your request as the bug has been fixed correctly. The prefix is gone now and the true info string is returned by mysqli. You can easily patch your current code just by adding && !str_contains($conn->server_info, 'MariaDB'). Although, I would question whether such usage is just a lucky coincidence.