From d59aac58b3e7da7ad01a194fe9840d89725ea229 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 18 Jul 2019 15:25:59 +0200 Subject: Report errors from stream read and write operations The php_stream_read() and php_stream_write() functions now return an ssize_t value, with negative results indicating failure. Functions like fread() and fwrite() will return false in that case. As a special case, EWOULDBLOCK and EAGAIN on non-blocking streams should not be regarded as error conditions, and be reported as successful zero-length reads/writes instead. The handling of EINTR remains unclear and is internally inconsistent (e.g. some code-paths will automatically retry on EINTR, while some won't). I'm landing this now to make sure the stream wrapper ops API changes make it into 7.4 -- however, if the user-facing changes turn out to be problematic we have the option of clamping negative returns to zero in php_stream_read() and php_stream_write() to restore the old behavior in a relatively non-intrusive manner. --- ext/mysqlnd/mysqlnd_net.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'ext/mysqlnd/mysqlnd_net.c') diff --git a/ext/mysqlnd/mysqlnd_net.c b/ext/mysqlnd/mysqlnd_net.c index 9f684288e6..f886b6e005 100644 --- a/ext/mysqlnd/mysqlnd_net.c +++ b/ext/mysqlnd/mysqlnd_net.c @@ -88,7 +88,7 @@ MYSQLND_METHOD(mysqlnd_net, network_read_ex)(MYSQLND_NET * const net, zend_uchar enum_func_status return_value = PASS; php_stream * net_stream = net->data->m.get_stream(net); size_t old_chunk_size = net_stream->chunk_size; - size_t to_read = count, ret; + size_t to_read = count; zend_uchar * p = buffer; DBG_ENTER("mysqlnd_net::network_read_ex"); @@ -96,7 +96,8 @@ MYSQLND_METHOD(mysqlnd_net, network_read_ex)(MYSQLND_NET * const net, zend_uchar net_stream->chunk_size = MIN(to_read, net->data->options.net_read_buffer_size); while (to_read) { - if (!(ret = php_stream_read(net_stream, (char *) p, to_read))) { + ssize_t ret = php_stream_read(net_stream, (char *) p, to_read); + if (ret <= 0) { DBG_ERR_FMT("Error while reading header from socket"); return_value = FAIL; break; @@ -112,11 +113,11 @@ MYSQLND_METHOD(mysqlnd_net, network_read_ex)(MYSQLND_NET * const net, zend_uchar /* {{{ mysqlnd_net::network_write_ex */ -static size_t +static ssize_t MYSQLND_METHOD(mysqlnd_net, network_write_ex)(MYSQLND_NET * const net, const zend_uchar * const buffer, const size_t count, MYSQLND_STATS * const stats, MYSQLND_ERROR_INFO * const error_info) { - size_t ret; + ssize_t ret; DBG_ENTER("mysqlnd_net::network_write_ex"); DBG_INF_FMT("sending %u bytes", count); ret = php_stream_write(net->data->m.get_stream(net), (char *)buffer, count); @@ -364,11 +365,12 @@ MYSQLND_METHOD(mysqlnd_net, send_ex)(MYSQLND_NET * const net, zend_uchar * const { zend_uchar safe_buf[((MYSQLND_HEADER_SIZE) + (sizeof(zend_uchar)) - 1) / (sizeof(zend_uchar))]; zend_uchar * safe_storage = safe_buf; - size_t bytes_sent, packets_sent = 1; + size_t packets_sent = 1; size_t left = count; zend_uchar * p = (zend_uchar *) buffer; zend_uchar * compress_buf = NULL; size_t to_be_sent; + ssize_t bytes_sent; DBG_ENTER("mysqlnd_net::send_ex"); DBG_INF_FMT("count=" MYSQLND_SZ_T_SPEC " compression=%u", count, net->data->compressed); @@ -458,7 +460,7 @@ MYSQLND_METHOD(mysqlnd_net, send_ex)(MYSQLND_NET * const net, zend_uchar * const indeed it then loop once more, then to_be_sent will become 0, left will stay 0. Empty packet will be sent and this loop will end. */ - } while (bytes_sent && (left > 0 || to_be_sent == MYSQLND_MAX_PACKET_SIZE)); + } while (bytes_sent > 0 && (left > 0 || to_be_sent == MYSQLND_MAX_PACKET_SIZE)); DBG_INF_FMT("packet_size="MYSQLND_SZ_T_SPEC" packet_no=%u", left, net->packet_no); @@ -472,7 +474,7 @@ MYSQLND_METHOD(mysqlnd_net, send_ex)(MYSQLND_NET * const net, zend_uchar * const } /* Even for zero size payload we have to send a packet */ - if (!bytes_sent) { + if (bytes_sent <= 0) { DBG_ERR_FMT("Can't %u send bytes", count); SET_CLIENT_ERROR(*error_info, CR_SERVER_GONE_ERROR, UNKNOWN_SQLSTATE, mysqlnd_server_gone); } @@ -866,10 +868,14 @@ MYSQLND_METHOD(mysqlnd_net, consume_uneaten_data)(MYSQLND_NET * const net, enum if (PHP_STREAM_OPTION_RETURN_ERR != was_blocked) { /* Do a read of 1 byte */ - int bytes_consumed; + ssize_t bytes_consumed; do { - skipped_bytes += (bytes_consumed = php_stream_read(net_stream, tmp_buf, sizeof(tmp_buf))); + bytes_consumed = php_stream_read(net_stream, tmp_buf, sizeof(tmp_buf)); + if (bytes_consumed <= 0) { + break; + } + skipped_bytes += bytes_consumed; } while (bytes_consumed == sizeof(tmp_buf)); if (was_blocked) { @@ -877,9 +883,9 @@ MYSQLND_METHOD(mysqlnd_net, consume_uneaten_data)(MYSQLND_NET * const net, enum } if (bytes_consumed) { - DBG_ERR_FMT("Skipped %u bytes. Last command %s hasn't consumed all the output from the server", + DBG_ERR_FMT("Skipped %zu bytes. Last command %s hasn't consumed all the output from the server", bytes_consumed, mysqlnd_command_to_text[net->last_command]); - php_error_docref(NULL, E_WARNING, "Skipped %u bytes. Last command %s hasn't " + php_error_docref(NULL, E_WARNING, "Skipped %zu bytes. Last command %s hasn't " "consumed all the output from the server", bytes_consumed, mysqlnd_command_to_text[net->last_command]); } -- cgit v1.2.1