Skip to content

Commit 624756e

Browse files
committed
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<void> { // 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<void> { $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<void> { $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
1 parent 80100e4 commit 624756e

1 file changed

Lines changed: 13 additions & 3 deletions

File tree

hphp/runtime/ext/async_mysql/ext_async_mysql.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,9 +326,11 @@ static String HHLibSQLQuery__toString__FOR_DEBUGGING_ONLY(
326326
val(this_->propRvalAtOffset(s_query_format_idx).tv()).pstr;
327327
const auto args = val(this_->propRvalAtOffset(s_query_args_idx).tv()).parr;
328328
const auto query = amquery_from_queryf(format, args);
329-
auto mysql = Native::data<AsyncMysqlConnection>(conn)
330-
->m_conn
331-
->mysql_for_testing_only();
329+
330+
auto* data = Native::data<AsyncMysqlConnection>(conn);
331+
data->verifyValidConnection();
332+
333+
auto mysql = data->m_conn->mysql_for_testing_only();
332334
const auto str = query.render(mysql);
333335
return String(str.data(), str.length(), CopyString);
334336
}
@@ -1286,6 +1288,8 @@ static void HHVM_METHOD(AsyncMysqlConnection, close) {
12861288

12871289
static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) {
12881290
auto* data = Native::data<AsyncMysqlConnection>(this_);
1291+
data->verifyValidConnection();
1292+
12891293
const auto* context = data->m_conn->getConnectionContext();
12901294
if (context && context->sslCertCn.hasValue()) {
12911295
return context->sslCertCn.value();
@@ -1296,6 +1300,8 @@ static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) {
12961300

12971301
static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) {
12981302
auto* data = Native::data<AsyncMysqlConnection>(this_);
1303+
data->verifyValidConnection();
1304+
12991305
auto ret = req::make<c_Vector>();
13001306
const auto* context = data->m_conn->getConnectionContext();
13011307
if (context && context->sslCertSan.hasValue()) {
@@ -1308,6 +1314,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) {
13081314

13091315
static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) {
13101316
auto* data = Native::data<AsyncMysqlConnection>(this_);
1317+
data->verifyValidConnection();
1318+
13111319
auto ret = req::make<c_Vector>();
13121320
const auto* context = data->m_conn->getConnectionContext();
13131321
if (context && context->sslCertIdentities.hasValue()) {
@@ -1320,6 +1328,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) {
13201328

13211329
static bool HHVM_METHOD(AsyncMysqlConnection, isSslCertValidationEnforced) {
13221330
auto* data = Native::data<AsyncMysqlConnection>(this_);
1331+
data->verifyValidConnection();
1332+
13231333
const auto* context = data->m_conn->getConnectionContext();
13241334
return context && context->isServerCertValidated;
13251335
}

0 commit comments

Comments
 (0)