Skip to content

Commit 8e3b901

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 facebook#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 facebook#8678
1 parent 82e4304 commit 8e3b901

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

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
}
@@ -1244,6 +1246,8 @@ static void HHVM_METHOD(AsyncMysqlConnection, close) {
12441246

12451247
static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) {
12461248
auto* data = Native::data<AsyncMysqlConnection>(this_);
1249+
data->verifyValidConnection();
1250+
12471251
const auto* context = data->m_conn->getConnectionContext();
12481252
if (context && context->sslCertCn.hasValue()) {
12491253
return context->sslCertCn.value();
@@ -1254,6 +1258,8 @@ static String HHVM_METHOD(AsyncMysqlConnection, getSslCertCn) {
12541258

12551259
static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) {
12561260
auto* data = Native::data<AsyncMysqlConnection>(this_);
1261+
data->verifyValidConnection();
1262+
12571263
auto ret = req::make<c_Vector>();
12581264
const auto* context = data->m_conn->getConnectionContext();
12591265
if (context && context->sslCertSan.hasValue()) {
@@ -1266,6 +1272,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertSan) {
12661272

12671273
static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) {
12681274
auto* data = Native::data<AsyncMysqlConnection>(this_);
1275+
data->verifyValidConnection();
1276+
12691277
auto ret = req::make<c_Vector>();
12701278
const auto* context = data->m_conn->getConnectionContext();
12711279
if (context && context->sslCertIdentities.hasValue()) {
@@ -1278,6 +1286,8 @@ static Object HHVM_METHOD(AsyncMysqlConnection, getSslCertExtensions) {
12781286

12791287
static bool HHVM_METHOD(AsyncMysqlConnection, isSslCertValidationEnforced) {
12801288
auto* data = Native::data<AsyncMysqlConnection>(this_);
1289+
data->verifyValidConnection();
1290+
12811291
const auto* context = data->m_conn->getConnectionContext();
12821292
return context && context->isServerCertValidated;
12831293
}

0 commit comments

Comments
 (0)