uw Wed, 16 Sep 2009 17:03:44 +0000 Revision: http://svn.php.net/viewvc?view=revision&revision=288379
Log: Fix (by Andrey) and test for bug #49442 . Don't use efree() for memory allocated with malloc()... If a connection gets created by mysqli_init(), mysqlnd makes it 'persistent'. 'Persistent' means that mysqlnd uses malloc(). mysqlnd does use malloc() instead of ealloc() because it is unknown if the connection will become a true persistent connection in the sense of ext/mysqli. It is unknown if the user wants a persistent connection or not until the user calls mysqli_real_connect(). To avoid tricky conversions mysqlnd uses malloc(), which sets a private persistent flag in the mysqlnd structures. A precondition for the crash to happen was that the private persistent flag is set. The flag is also set when creating a real persistent connection (in the sense of ext/mysqli) and so the bug can happen with mysql_init()/mysqli_real_connect() and mysql_connect('p:<host>', ...). Therefore we test both cases. Note the (tricky?) difference between the implementation detail'mysqlnd private ! persistent flag = use malloc()' and persistent connections from a user perspective. Although mysqlnd will always set its private persistent flag and use malloc() for connections created with mysqli_init() it is still up to the user to decide in mysqli_real_connect() if the connection shall become a (true) persistent connection or not. Bug: http://bugs.php.net/49442 (Assigned) Some queries crash PHP with mysqli_real_connect() Changed paths: A php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt U php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h U php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h U php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c A php/php-src/trunk/ext/mysqli/tests/bug49442.phpt U php/php-src/trunk/ext/mysqlnd/mysqlnd.h U php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h U php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c
Added: php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt (rev 0) +++ php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt 2009-09-16 17:03:44 UTC (rev 288379) @@ -0,0 +1,119 @@ +--TEST-- +Bug #49422 (mysqlnd: mysqli_real_connect() and LOAD DATA INFILE crash) +--SKIPIF-- +<?php +require_once('skipif.inc'); +require_once('skipifconnectfailure.inc'); +?> +--INI-- +mysqli.allow_local_infile=1 +mysqli.allow_persistent=1 +mysqli.max_persistent=1 +--FILE-- +<?php + include ("connect.inc"); + + $link = mysqli_init(); + if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) { + printf("[001] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error()); + } + + if (!mysqli_query($link, 'DROP TABLE IF EXISTS test')) { + printf("[002] Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + if (!mysqli_query($link, 'CREATE TABLE test(id INT, label CHAR(1), PRIMARY KEY(id)) ENGINE=' . $engine)) { + printf("[003] Failed to create test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + include("local_infile_tools.inc"); + $file = create_standard_csv(4); + + if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s' + INTO TABLE test + FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\'' + LINES TERMINATED BY '\n'", + mysqli_real_escape_string($link, $file)))) { + printf("[005] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id")) + printf("[006] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + + $rows = array(); + while ($row = mysqli_fetch_assoc($res)) { + var_dump($row); + $rows[] = $row; + } + + mysqli_free_result($res); + + mysqli_query($link, "DELETE FROM test"); + mysqli_close($link); + + if ($IS_MYSQLND) { + /* + mysqlnd makes a connection created through mysql_init()/mysqli_real_connect() always a 'persistent' one. + At this point 'persistent' is not to be confused with what a user calls a 'persistent' - in this case + 'persistent' means that mysqlnd uses malloc() instead of emalloc(). nothing else. ext/mysqli will + not consider it as a 'persistent' connection in a user sense, ext/mysqli will not appy max_persistent etc. + Its only about malloc() vs. emalloc(). + + However, the bug is about malloc() and efree(). You can make make mysqlnd use malloc() by either using + pconnect or mysql_init() - so we should test pconnect as well.. + */ + $host = 'p:' . $host; + if (!$link = mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("[007] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error()); + } + + /* bug happened during query processing */ + if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s' + INTO TABLE test + FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\'' + LINES TERMINATED BY '\n'", + mysqli_real_escape_string($link, $file)))) { + printf("[008] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + /* we survived? that's good enough... */ + + if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id")) + printf("[009] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + + $i = 0; + while ($row = mysqli_fetch_assoc($res)) { + if (($row['id'] != $rows[$i]['id']) || ($row['label'] != $rows[$i]['label'])) { + printf("[010] Wrong values, check manually!\n"); + } + $i++; + } + mysqli_close($link); + } + + print "done!"; +?> +--CLEAN-- +<?php + require_once("clean_table.inc"); +?> +--EXPECTF-- +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "97" + [%u|b%"label"]=> + %unicode|string%(1) "x" +} +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "98" + [%u|b%"label"]=> + %unicode|string%(1) "y" +} +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "99" + [%u|b%"label"]=> + %unicode|string%(1) "z" +} +done! \ No newline at end of file Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h 2009-09-16 17:03:44 UTC (rev 288379) @@ -26,7 +26,7 @@ #define MYSQLND_VERSION_ID 50005 /* This forces inlining of some accessor functions */ -#define MYSQLND_USE_OPTIMISATIONS 1 +#define MYSQLND_USE_OPTIMISATIONS 0 #define MYSQLND_STRING_TO_INT_CONVERSION /* Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h 2009-09-16 17:03:44 UTC (rev 288379) @@ -104,10 +104,12 @@ if ((buf)) { \ pefree((buf), (persistent)); \ } \ - (buf) = (message); \ + if ((message)) { \ + (buf) = pestrndup((message), (len), (persistent)); \ + } else { \ + buf = NULL; \ + } \ (buf_len) = (len); \ - /* Transfer ownership*/ \ - (message) = NULL; \ } #define SET_EMPTY_MESSAGE(buf, buf_len, persistent) \ Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c 2009-09-16 17:03:44 UTC (rev 288379) @@ -813,7 +813,7 @@ /* There is a message */ if (packet->header.size > p - buf && (i = php_mysqlnd_net_field_length(&p))) { - packet->message = pestrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)), conn->persistent); + packet->message = estrndup((char *)p, MIN(i, sizeof(buf) - (p - buf))); packet->message_len = i; } else { packet->message = NULL; @@ -1032,7 +1032,7 @@ Thus, the name is size - 1. And we add 1 for a trailing \0. */ len = packet->header.size - 1; - packet->info_or_local_file = mnd_pemalloc(len + 1, conn->persistent); + packet->info_or_local_file = mnd_emalloc(len + 1); memcpy(packet->info_or_local_file, p, len); packet->info_or_local_file[len] = '\0'; packet->info_or_local_file_len = len; @@ -1867,7 +1867,7 @@ PACKET_READ_HEADER_AND_BODY(packet, conn, buf, sizeof(buf), "statistics", PROT_STATS_PACKET); - packet->message = mnd_pemalloc(packet->header.size + 1, conn->persistent); + packet->message = mnd_emalloc(packet->header.size + 1); memcpy(packet->message, buf, packet->header.size); packet->message[packet->header.size] = '\0'; packet->message_len = packet->header.size; Added: php/php-src/trunk/ext/mysqli/tests/bug49442.phpt =================================================================== --- php/php-src/trunk/ext/mysqli/tests/bug49442.phpt (rev 0) +++ php/php-src/trunk/ext/mysqli/tests/bug49442.phpt 2009-09-16 17:03:44 UTC (rev 288379) @@ -0,0 +1,119 @@ +--TEST-- +Bug #49422 (mysqlnd: mysqli_real_connect() and LOAD DATA INFILE crash) +--SKIPIF-- +<?php +require_once('skipif.inc'); +require_once('skipifconnectfailure.inc'); +?> +--INI-- +mysqli.allow_local_infile=1 +mysqli.allow_persistent=1 +mysqli.max_persistent=1 +--FILE-- +<?php + include ("connect.inc"); + + $link = mysqli_init(); + if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) { + printf("[001] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error()); + } + + if (!mysqli_query($link, 'DROP TABLE IF EXISTS test')) { + printf("[002] Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + if (!mysqli_query($link, 'CREATE TABLE test(id INT, label CHAR(1), PRIMARY KEY(id)) ENGINE=' . $engine)) { + printf("[003] Failed to create test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + include("local_infile_tools.inc"); + $file = create_standard_csv(4); + + if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s' + INTO TABLE test + FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\'' + LINES TERMINATED BY '\n'", + mysqli_real_escape_string($link, $file)))) { + printf("[005] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id")) + printf("[006] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + + $rows = array(); + while ($row = mysqli_fetch_assoc($res)) { + var_dump($row); + $rows[] = $row; + } + + mysqli_free_result($res); + + mysqli_query($link, "DELETE FROM test"); + mysqli_close($link); + + if ($IS_MYSQLND) { + /* + mysqlnd makes a connection created through mysql_init()/mysqli_real_connect() always a 'persistent' one. + At this point 'persistent' is not to be confused with what a user calls a 'persistent' - in this case + 'persistent' means that mysqlnd uses malloc() instead of emalloc(). nothing else. ext/mysqli will + not consider it as a 'persistent' connection in a user sense, ext/mysqli will not appy max_persistent etc. + Its only about malloc() vs. emalloc(). + + However, the bug is about malloc() and efree(). You can make make mysqlnd use malloc() by either using + pconnect or mysql_init() - so we should test pconnect as well.. + */ + $host = 'p:' . $host; + if (!$link = mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("[007] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error()); + } + + /* bug happened during query processing */ + if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s' + INTO TABLE test + FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\'' + LINES TERMINATED BY '\n'", + mysqli_real_escape_string($link, $file)))) { + printf("[008] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + } + + /* we survived? that's good enough... */ + + if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id")) + printf("[009] [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + + $i = 0; + while ($row = mysqli_fetch_assoc($res)) { + if (($row['id'] != $rows[$i]['id']) || ($row['label'] != $rows[$i]['label'])) { + printf("[010] Wrong values, check manually!\n"); + } + $i++; + } + mysqli_close($link); + } + + print "done!"; +?> +--CLEAN-- +<?php + require_once("clean_table.inc"); +?> +--EXPECTF-- +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "97" + [%u|b%"label"]=> + %unicode|string%(1) "x" +} +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "98" + [%u|b%"label"]=> + %unicode|string%(1) "y" +} +array(2) { + [%u|b%"id"]=> + %unicode|string%(2) "99" + [%u|b%"label"]=> + %unicode|string%(1) "z" +} +done! \ No newline at end of file Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd.h =================================================================== --- php/php-src/trunk/ext/mysqlnd/mysqlnd.h 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/trunk/ext/mysqlnd/mysqlnd.h 2009-09-16 17:03:44 UTC (rev 288379) @@ -26,7 +26,7 @@ #define MYSQLND_VERSION_ID 50005 /* This forces inlining of some accessor functions */ -#define MYSQLND_USE_OPTIMISATIONS 1 +#define MYSQLND_USE_OPTIMISATIONS 0 #define MYSQLND_STRING_TO_INT_CONVERSION /* Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h =================================================================== --- php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h 2009-09-16 17:03:44 UTC (rev 288379) @@ -104,10 +104,12 @@ if ((buf)) { \ pefree((buf), (persistent)); \ } \ - (buf) = (message); \ + if ((message)) { \ + (buf) = pestrndup((message), (len), (persistent)); \ + } else { \ + buf = NULL; \ + } \ (buf_len) = (len); \ - /* Transfer ownership*/ \ - (message) = NULL; \ } #define SET_EMPTY_MESSAGE(buf, buf_len, persistent) \ Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c =================================================================== --- php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c 2009-09-16 15:00:54 UTC (rev 288378) +++ php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c 2009-09-16 17:03:44 UTC (rev 288379) @@ -813,7 +813,7 @@ /* There is a message */ if (packet->header.size > p - buf && (i = php_mysqlnd_net_field_length(&p))) { - packet->message = pestrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)), conn->persistent); + packet->message = estrndup((char *)p, MIN(i, sizeof(buf) - (p - buf))); packet->message_len = i; } else { packet->message = NULL; @@ -1032,7 +1032,7 @@ Thus, the name is size - 1. And we add 1 for a trailing \0. */ len = packet->header.size - 1; - packet->info_or_local_file = mnd_pemalloc(len + 1, conn->persistent); + packet->info_or_local_file = mnd_emalloc(len + 1); memcpy(packet->info_or_local_file, p, len); packet->info_or_local_file[len] = '\0'; packet->info_or_local_file_len = len; @@ -1867,7 +1867,7 @@ PACKET_READ_HEADER_AND_BODY(packet, conn, buf, sizeof(buf), "statistics", PROT_STATS_PACKET); - packet->message = mnd_pemalloc(packet->header.size + 1, conn->persistent); + packet->message = mnd_emalloc(packet->header.size + 1); memcpy(packet->message, buf, packet->header.size); packet->message[packet->header.size] = '\0'; packet->message_len = packet->header.size;
-- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php