From bb4386b678f72ff9e28d4f2b0b928546bd1cbe8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Wed, 25 Jun 2025 02:48:08 +0200 Subject: [PATCH] Consistently check connection validity in AsyncMysqlConnection The Squangle connection pointer wrapped by AsyncMysqlConnection may be nullptr if the connection was closed or is currently busy waiting for the result of an async query. Most code paths already call either verifyValidConnection() to raise an appropriate Hack exception or explicitly check for and handle a null backing connection, but Query::toString__FOR_DEBUGGING_ONLY() and the SSL-related getters from D33663743 do not, which can lead to segfaults. Slightly simplified reproducer from #8678: ```hack use namespace HH\Lib\SQL; <<__EntryPoint>> async function main_async(): Awaitable { // connection parameters as needed $async_conn = await AsyncMysqlClient::connect('127.0.0.1', 3306, 'foo', 'root', 'pass'); $async_conn->close(); var_dump($async_conn->getSslCertCn()); } async function func_async(AsyncMysqlConnection $asyncMysql, SQL\Query $query): Awaitable { $query->toString__FOR_DEBUGGING_ONLY($asyncMysql); var_dump($asyncMysql->getSslCertCn()); await $asyncMysql->queryf('SELECT %s', 'something'); } ``` and for `(get|is)Ssl.*`: ```hack use namespace HH\Lib\SQL; <<__EntryPoint>> async function main_async(): Awaitable { $async_conn = await AsyncMysqlClient::connect('127.0.0.1', 3306, 'foo', 'root', 'wikia123456'); $async_conn->close(); var_dump($async_conn->getSslCertCn()); } ``` Call verifyValidConnection() in all these cases, as raising an appropriate exception (e.g. closed/busy connection) seems appropriate here. Fixes #8678 --- hphp/runtime/ext/async_mysql/ext_async_mysql.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp b/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp index cb481d53a6bb8..a9b09382ab3b3 100644 --- a/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp +++ b/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp @@ -326,9 +326,11 @@ static String HHLibSQLQuery__toString__FOR_DEBUGGING_ONLY( val(this_->propRvalAtOffset(s_query_format_idx).tv()).pstr; const auto args = val(this_->propRvalAtOffset(s_query_args_idx).tv()).parr; const auto query = amquery_from_queryf(format, args); - auto mysql = Native::data(conn) - ->m_conn - ->mysql_for_testing_only(); + + auto* data = Native::data(conn); + data->verifyValidConnection(); + + auto mysql = data->m_conn->mysql_for_testing_only(); const auto str = query.render(mysql); return String(str.data(), str.length(), CopyString); } @@ -1244,6 +1246,8 @@ static void HHVM_METHOD(AsyncMysqlConnection, close) { static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) { auto* data = Native::data(this_); + data->verifyValidConnection(); + const auto* context = data->m_conn->getConnectionContext(); if (context && context->sslCertCn.hasValue()) { return context->sslCertCn.value(); @@ -1254,6 +1258,8 @@ static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) { static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) { auto* data = Native::data(this_); + data->verifyValidConnection(); + auto ret = req::make(); const auto* context = data->m_conn->getConnectionContext(); if (context && context->sslCertSan.hasValue()) { @@ -1266,6 +1272,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) { static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) { auto* data = Native::data(this_); + data->verifyValidConnection(); + auto ret = req::make(); const auto* context = data->m_conn->getConnectionContext(); if (context && context->sslCertIdentities.hasValue()) { @@ -1278,6 +1286,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) { static bool HHVM_METHOD(AsyncMysqlConnection, isSslCertValidationEnforced) { auto* data = Native::data(this_); + data->verifyValidConnection(); + const auto* context = data->m_conn->getConnectionContext(); return context && context->isServerCertValidated; }