From dc162a4eb72fa9faad2d982350e5362ed7cdd83b Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Fri, 18 Oct 2024 10:30:16 -0400 Subject: [PATCH] fix(agent): don't skip arguments when calling `mysqli::real_connect` (#976) Co-authored-by: Konstantin Kovshenin Co-authored-by: Amber Sistla --- agent/php_mysqli.c | 61 ++++--- docker-compose.yaml | 9 ++ .../mysqli/test_explain_connect_socket.php | 148 +++++++++++++++++ .../mysqli/test_explain_construct_socket.php | 150 ++++++++++++++++++ 4 files changed, 342 insertions(+), 26 deletions(-) create mode 100644 tests/integration/mysqli/test_explain_connect_socket.php create mode 100644 tests/integration/mysqli/test_explain_construct_socket.php diff --git a/agent/php_mysqli.c b/agent/php_mysqli.c index b17fd2491..24a321b02 100644 --- a/agent/php_mysqli.c +++ b/agent/php_mysqli.c @@ -398,42 +398,51 @@ static nr_status_t nr_php_mysqli_link_real_connect( zval* link, const nr_mysqli_metadata_link_t* metadata TSRMLS_DC) { zend_ulong argc = 0; + zend_ulong arg_required = 0; zval* argv[7] = {0}; zend_ulong i; zval* retval = NULL; -#define ADD_IF_INT_SET(args, argc, value) \ - if (value) { \ - args[argc] = nr_php_zval_alloc(); \ - ZVAL_LONG(args[argc], value); \ - argc++; \ - } - -#define ADD_IF_STR_SET(args, argc, value) \ - if (value) { \ - args[argc] = nr_php_zval_alloc(); \ - nr_php_zval_str(args[argc], value); \ - argc++; \ - } - - ADD_IF_STR_SET(argv, argc, +#define ADD_IF_INT_SET(null_ok, args, argc, value) \ + if (value) { \ + args[argc] = nr_php_zval_alloc(); \ + ZVAL_LONG(args[argc], value); \ + argc++; \ + } else if (true == null_ok) { \ + args[argc] = nr_php_zval_alloc(); \ + ZVAL_NULL(args[argc]); \ + argc++; \ + } + +#define ADD_IF_STR_SET(null_ok, args, argc, value) \ + if (value) { \ + args[argc] = nr_php_zval_alloc(); \ + nr_php_zval_str(args[argc], value); \ + argc++; \ + } else if (true == null_ok) { \ + args[argc] = nr_php_zval_alloc(); \ + ZVAL_NULL(args[argc]); \ + argc++; \ + } + + ADD_IF_STR_SET(false, argv, argc, nr_php_mysqli_strip_persistent_prefix(metadata->host)); - ADD_IF_STR_SET(argv, argc, metadata->user); - ADD_IF_STR_SET(argv, argc, metadata->password); + ADD_IF_STR_SET(false, argv, argc, metadata->user); + ADD_IF_STR_SET(false, argv, argc, metadata->password); /* * We can only add the remaining metadata fields if we already have three * arguments (host, user and password) above, lest we accidentally set the - * wrong positional argument to something it doesn't mean. + * wrong positional argument to something it doesn't mean. Note, prior + * to 7.4 not all args are nullable. */ + arg_required = argc; if (argc == 3) { - ADD_IF_STR_SET(argv, argc, metadata->database); - ADD_IF_INT_SET(argv, argc, metadata->port); - ADD_IF_STR_SET(argv, argc, metadata->socket); - ADD_IF_INT_SET(argv, argc, metadata->flags); - } - - retval = nr_php_call_user_func(link, "real_connect", argc, argv TSRMLS_CC); + ADD_IF_STR_SET(true, argv, argc, metadata->database); + ADD_IF_INT_SET(true, argv, argc, metadata->port); + ADD_IF_STR_SET(true, argv, argc, metadata->socket); + ADD_IF_INT_SET(false, argv, argc, metadata->flags); + } retval = nr_php_call_user_func(link, "real_connect", argc, argv TSRMLS_CC); for (i = 0; i < argc; i++) { nr_php_zval_free(&argv[i]); @@ -450,7 +459,7 @@ static nr_status_t nr_php_mysqli_link_real_connect( * If we didn't specify the database in the connection parameters, we need to * call mysqli::select_db here. */ - if (metadata->database && (argc < 4)) { + if (metadata->database && (arg_required < 3)) { zval* database = nr_php_zval_alloc(); nr_php_zval_str(database, metadata->database); diff --git a/docker-compose.yaml b/docker-compose.yaml index 4d57753ff..2ea1d11d3 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -24,6 +24,8 @@ services: retries: 3 start_period: 20s container_name: mysqldb + volumes: + - var-run-mysqld:/var/run/mysqld redisdb: image: redis restart: always @@ -56,6 +58,7 @@ services: MYSQL_USER: admin MYSQL_PASSWD: admin MYSQL_HOST: mysqldb + MYSQL_SOCKET: /var/run/mysqld/mysqld.sock PG_HOST: postgres PG_PORT: 5432 @@ -67,6 +70,7 @@ services: volumes: - ${AGENT_CODE:-$PWD}:/usr/local/src/newrelic-php-agent + - var-run-mysqld:/var/run/mysqld entrypoint: tail command: -f /dev/null container_name: nr-php @@ -83,6 +87,7 @@ services: MYSQL_USER: admin MYSQL_PASSWD: admin MYSQL_HOST: mysqldb + MYSQL_SOCKET: /var/run/mysqld/mysqld.sock PG_HOST: postgres PG_PORT: 5432 @@ -97,8 +102,12 @@ services: NEWRELIC_LICENSE_KEY: ${NEW_RELIC_LICENSE_KEY} volumes: - ${PWD}:/usr/src/myapp + - var-run-mysqld:/var/run/mysqld working_dir: /usr/src/myapp stdin_open: true tty: true container_name: agent-devenv profiles: ["dev"] + +volumes: + var-run-mysqld: diff --git a/tests/integration/mysqli/test_explain_connect_socket.php b/tests/integration/mysqli/test_explain_connect_socket.php new file mode 100644 index 000000000..0c56e3bc9 --- /dev/null +++ b/tests/integration/mysqli/test_explain_connect_socket.php @@ -0,0 +1,148 @@ +", + "?? SQL ID", + "SELECT TABLE_NAME FROM information_schema.tables WHERE table_name=?", + "Datastore/statement/MySQL/tables/select", + 1, + "?? total time", + "?? min time", + "?? max time", + { + "explain_plan": [ + [ + "id", + "select_type", + "table", + "type", + "possible_keys", + "key", + "key_len", + "ref", + "rows", + "Extra" + ], + [ + [ + 1, + "SIMPLE", + "tables", + "ALL", + null, + "TABLE_NAME", + null, + null, + null, + "Using where; Skip_open_table; Scanned 1 database" + ] + ] + ], + "backtrace": [ + " in mysqli_stmt_execute called at __FILE__ (??)", + " in test_prepare called at __FILE__ (??)" + ] + } + ] + ] +] +*/ + +/*EXPECT_TRACED_ERRORS +null +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/config.php'); + +function test_prepare($link) +{ + $query = "SELECT TABLE_NAME FROM information_schema.tables WHERE table_name='STATISTICS'"; + + $stmt = mysqli_prepare($link, $query); + if (FALSE === $stmt) { + echo mysqli_error($link) . "\n"; + return; + } + + if (FALSE === mysqli_stmt_execute($stmt)) { + echo mysqli_stmt_error($stmt) . "\n"; + return; + } + + if (FALSE === mysqli_stmt_bind_result($stmt, $value)) { + echo mysqli_stmt_error($stmt) . "\n"; + return; + } + + while (mysqli_stmt_fetch($stmt)) { + echo $value . "\n"; + } + + mysqli_stmt_close($stmt); +} + +$link = mysqli_connect('localhost', $MYSQL_USER, $MYSQL_PASSWD, $MYSQL_DB, null, $MYSQL_SOCKET); +if (mysqli_connect_errno()) { + echo mysqli_connect_error() . "\n"; + exit(1); +} + +test_prepare($link); +mysqli_close($link); diff --git a/tests/integration/mysqli/test_explain_construct_socket.php b/tests/integration/mysqli/test_explain_construct_socket.php new file mode 100644 index 000000000..20c6e1481 --- /dev/null +++ b/tests/integration/mysqli/test_explain_construct_socket.php @@ -0,0 +1,150 @@ +", + "?? SQL ID", + "SELECT TABLE_NAME FROM information_schema.tables WHERE table_name=?", + "Datastore/statement/MySQL/tables/select", + 1, + "?? total time", + "?? min time", + "?? max time", + { + "explain_plan": [ + [ + "id", + "select_type", + "table", + "type", + "possible_keys", + "key", + "key_len", + "ref", + "rows", + "Extra" + ], + [ + [ + 1, + "SIMPLE", + "tables", + "ALL", + null, + "TABLE_NAME", + null, + null, + null, + "Using where; Skip_open_table; Scanned 1 database" + ] + ] + ], + "backtrace": [ + " in mysqli_stmt_execute called at __FILE__ (??)", + " in test_prepare called at __FILE__ (??)" + ] + } + ] + ] +] +*/ + +/*EXPECT_TRACED_ERRORS +null +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/config.php'); + +function test_prepare($link) +{ + + $query = "SELECT TABLE_NAME FROM information_schema.tables WHERE table_name='STATISTICS'"; + + $stmt = mysqli_prepare($link, $query); + if (FALSE === $stmt) { + echo mysqli_error($link) . "\n"; + return; + } + + if (FALSE === mysqli_stmt_execute($stmt)) { + echo mysqli_stmt_error($stmt) . "\n"; + return; + } + + if (FALSE === mysqli_stmt_bind_result($stmt, $value)) { + echo mysqli_stmt_error($stmt) . "\n"; + return; + } + + while (mysqli_stmt_fetch($stmt)) { + echo $value . "\n"; + } + + mysqli_stmt_close($stmt); +} + +$link = new mysqli('localhost', $MYSQL_USER, $MYSQL_PASSWD, $MYSQL_DB, null, $MYSQL_SOCKET); +if (mysqli_connect_errno()) { + echo mysqli_connect_error() . "\n"; + exit(1); +} + +test_prepare($link); +mysqli_close($link);