Newsgroups: php.cvs,php.internals
Path: news.php.net
Xref: news.php.net php.cvs:128074 php.internals:123110
Feedback-ID: i6f4c46c2:Fastmail
Message-ID: <2906b71c-811e-40ff-a58b-a306a5ee4f96@beccati.com>
Date: Thu, 11 Apr 2024 15:07:16 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: [PHP-DEV] Re: [PHP-CVS] [php-src] master: ext/mysqlnd: support ER_CLIENT_INTERACTION_TIMEOUT
To: Appla <bhg@live.it>, Kamil Tekiela <tekiela246@gmail.com>
References: <VTX0phIGs38ynrqbR70NdxI75YAECL9tSK4Imhm6o@main.php.net>
Content-Language: it
Cc: php-cvs@lists.php.net, "internals@lists.php.net" <internals@lists.php.net>
In-Reply-To: <VTX0phIGs38ynrqbR70NdxI75YAECL9tSK4Imhm6o@main.php.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
From: php@beccati.com (Matteo Beccati)

Hi,

I've left a comment on github, but perhaps it is best I also post here:

The commit is breaking several tests on doctrine/dbal. Spedifically 2 
tests on the 3.8.x branch and 5 tests on the 4.0.x:

https://github.com/mbeccati/php-latest-builds/runs/23707742797

This could mean it is not working properly or it is not fully safe for 
8.2 / 8.3.


Cheers

Il 10/04/2024 13:38, Appla via Kamil Tekiela ha scritto:
> Author: Appla (Appla)
> Committer: Kamil Tekiela (kamil-tekiela)
> Date: 2024-04-10T13:33:04+02:00
> 
> Commit: https://github.com/php/php-src/commit/5035b8509016c4cf2cda883b4d3f245054a71626
> Raw diff: https://github.com/php/php-src/commit/5035b8509016c4cf2cda883b4d3f245054a71626.diff
> 
> ext/mysqlnd: support ER_CLIENT_INTERACTION_TIMEOUT
> 
> Closes GH-13618.
> 
> Changed paths:
>    A  ext/mysqli/tests/bug81335.phpt
>    M  NEWS
>    M  ext/mysqlnd/mysqlnd_enum_n_def.h
>    M  ext/mysqlnd/mysqlnd_result.c
>    M  ext/mysqlnd/mysqlnd_wireprotocol.c
> 
> 
> Diff:
> 
> diff --git a/NEWS b/NEWS
> index bfd2617878d5f..e96e9bbc4fe06 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,6 +16,8 @@ PHP                                                                        NEWS
>   
>   - MySQLnd:
>     . Fix shift out of bounds on 32-bit non-fast-path platforms. (nielsdos)
> +  . Fixed bug #81335 (PHP Warning, Packets out of order after connection
> +    timeout). (Appla)
>   
>   - OpenSSL:
>     . Fixed bug GH-10495 (feof on OpenSSL stream hangs indefinitely).
> diff --git a/ext/mysqli/tests/bug81335.phpt b/ext/mysqli/tests/bug81335.phpt
> new file mode 100644
> index 0000000000000..26f6bcd7e2d14
> --- /dev/null
> +++ b/ext/mysqli/tests/bug81335.phpt
> @@ -0,0 +1,34 @@
> +--TEST--
> +Bug #81335: Packets out of order after connection timeout
> +--EXTENSIONS--
> +mysqli
> +--SKIPIF--
> +<?php
> +if (PHP_OS === 'WINNT') die('skip on windows');
> +if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
> +
> +require_once 'connect.inc';
> +if (!$link = @my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) die("skip cannot connect");
> +if (mysqli_get_server_version($link) < 80024 || str_contains(mysqli_get_server_info($link), 'MariaDB')) {
> +    die("skip: Due to many MySQL Server differences, the test requires >= 8.0.24");
> +}
> +?>
> +--FILE--
> +<?php
> +
> +require_once 'connect.inc';
> +mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
> +$mysqli = new my_mysqli($host, $user, $passwd, $db, $port, $socket);
> +$mysqli->query('SET WAIT_TIMEOUT=1');
> +usleep(1000000 * 1.1);
> +try {
> +    $mysqli->query('SELECT 1 + 1');
> +} catch(mysqli_sql_exception $e) {
> +    echo $e->getMessage();
> +    echo "\n";
> +    echo $e->getCode();
> +}
> +?>
> +--EXPECTF--
> +The client was disconnected by the server because of inactivity. See wait_timeout and interactive_timeout for configuring this behavior.
> +4031
> diff --git a/ext/mysqlnd/mysqlnd_enum_n_def.h b/ext/mysqlnd/mysqlnd_enum_n_def.h
> index 110e62c71bab0..00b46e8c7d22d 100644
> --- a/ext/mysqlnd/mysqlnd_enum_n_def.h
> +++ b/ext/mysqlnd/mysqlnd_enum_n_def.h
> @@ -130,6 +130,7 @@
>   #define CR_INVALID_PARAMETER_NO	2034
>   #define CR_INVALID_BUFFER_USE	2035
>   #define CR_LOAD_DATA_LOCAL_INFILE_REJECTED 2068
> +#define CR_CLIENT_INTERACTION_TIMEOUT 4031
>   
>   #define MYSQLND_EE_FILENOTFOUND	 7890
>   
> diff --git a/ext/mysqlnd/mysqlnd_result.c b/ext/mysqlnd/mysqlnd_result.c
> index 0331518d7d3d6..cf5013ed42d51 100644
> --- a/ext/mysqlnd/mysqlnd_result.c
> +++ b/ext/mysqlnd/mysqlnd_result.c
> @@ -185,7 +185,7 @@ mysqlnd_query_read_result_set_header(MYSQLND_CONN_DATA * conn, MYSQLND_STMT * s)
>   		UPSERT_STATUS_SET_AFFECTED_ROWS_TO_ERROR(conn->upsert_status);
>   
>   		if (FAIL == (ret = PACKET_READ(conn, &rset_header))) {
> -			if (conn->error_info->error_no != CR_SERVER_GONE_ERROR) {
> +			if (conn->error_info->error_no != CR_SERVER_GONE_ERROR && conn->error_info->error_no != CR_CLIENT_INTERACTION_TIMEOUT) {
>   				php_error_docref(NULL, E_WARNING, "Error reading result set's header");
>   			}
>   			break;
> diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.c b/ext/mysqlnd/mysqlnd_wireprotocol.c
> index fed191c74fa52..16ca9aaf29d42 100644
> --- a/ext/mysqlnd/mysqlnd_wireprotocol.c
> +++ b/ext/mysqlnd/mysqlnd_wireprotocol.c
> @@ -267,6 +267,19 @@ mysqlnd_read_header(MYSQLND_PFC * pfc, MYSQLND_VIO * vio, MYSQLND_PACKET_HEADER
>   		pfc->data->packet_no++;
>   		DBG_RETURN(PASS);
>   	}
> +	// @see https://dev.mysql.com/worklog/task/?id=12999
> +	if (header->size > 0) {
> +		zend_uchar *buf = mnd_emalloc(header->size);
> +		if ((PASS == pfc->data->m.receive(pfc, vio, buf, header->size, conn_stats, error_info)) && buf[0] == ERROR_MARKER) {
> +			php_mysqlnd_read_error_from_line(buf + 1, header->size - 1,
> +			                                 error_info->error, sizeof(error_info->error),
> +			                                 &error_info->error_no, error_info->sqlstate
> +			);
> +			mnd_efree(buf);
> +			DBG_RETURN(FAIL);
> +		}
> +		mnd_efree(buf);
> +	}
>   
>   	DBG_ERR_FMT("Logical link: packets out of order. Expected %u received %u. Packet size=%zu",
>   				pfc->data->packet_no, header->packet_no, header->size);
> @@ -294,7 +307,9 @@ mysqlnd_read_packet_header_and_body(MYSQLND_PACKET_HEADER * packet_header,
>   	DBG_INF_FMT("buf=%p size=%zu", buf, buf_size);
>   	if (FAIL == mysqlnd_read_header(pfc, vio, packet_header, stats, error_info)) {
>   		SET_CONNECTION_STATE(connection_state, CONN_QUIT_SENT);
> -		SET_CLIENT_ERROR(error_info, CR_SERVER_GONE_ERROR, UNKNOWN_SQLSTATE, mysqlnd_server_gone);
> +		if (error_info->error_no == 0) {
> +			SET_CLIENT_ERROR(error_info, CR_SERVER_GONE_ERROR, UNKNOWN_SQLSTATE, mysqlnd_server_gone);
> +		}
>   		DBG_ERR_FMT("Can't read %s's header", packet_type_as_text);
>   		DBG_RETURN(FAIL);
>   	}

-- 
Matteo Beccati

Development & Consulting - http://www.beccati.com/