From 556b2ee7737b1dfdbc5223e9a1230b5df6843a01 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 14 Dec 2016 15:31:58 +0100 Subject: Network (HTTPS): prevent recursion among ->close() methods We observed a stack-trace in which, while handling an error, QHttpNetworkConnectionChannel::close()'s call to its socket->close() triggered (when the socket was a QSslSocket) a flush() which asked its backend to transmit() which tripped over the original error, which duly triggered endless recursion. Transiently clear the socket member, during its ->close(), to prevent this; do the same in abort(), to preserve its structural correspondence to close(). Restructure both so that any recursive call's setting of state is overwritten by the top-level call's, while this still uses the prior socket state (not the state after close() or abort() and any recursion) to determine final state. Task-number: QTBUG-56476 Change-Id: If69e97f7a77a729bf2338ed14214c65aa95f8b05 Reviewed-by: Timur Pocheptsov --- .../access/qhttpnetworkconnectionchannel.cpp | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index 646ede0022..d8ce28a7b0 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -44,6 +44,7 @@ #include #include +#include #ifndef QT_NO_HTTP @@ -196,41 +197,42 @@ void QHttpNetworkConnectionChannel::init() void QHttpNetworkConnectionChannel::close() { - if (!socket) - state = QHttpNetworkConnectionChannel::IdleState; - else if (socket->state() == QAbstractSocket::UnconnectedState) - state = QHttpNetworkConnectionChannel::IdleState; - else - state = QHttpNetworkConnectionChannel::ClosingState; - + // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while + // there is no socket yet; we may also clear it, below or in abort, to avoid recursion. + const bool idle = !socket || socket->state() == QAbstractSocket::UnconnectedState; + const ChannelState final = idle ? IdleState : ClosingState; // pendingEncrypt must only be true in between connected and encrypted states pendingEncrypt = false; if (socket) { - // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while - // there is no socket yet. - socket->close(); + QAbstractSocket *const detached = socket; + QScopedValueRollback rollback(socket, nullptr); + state = final; // Needed during close(), which may over-write it. + detached->close(); // Error states can cause recursion back to this method. } -} + // Set state *after* close(), to override any recursive call's idle setting: + state = final; +} void QHttpNetworkConnectionChannel::abort() { - if (!socket) - state = QHttpNetworkConnectionChannel::IdleState; - else if (socket->state() == QAbstractSocket::UnconnectedState) - state = QHttpNetworkConnectionChannel::IdleState; - else - state = QHttpNetworkConnectionChannel::ClosingState; - + // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while + // there is no socket yet; we may also clear it, below or in close, to avoid recursion. + const bool idle = !socket || socket->state() == QAbstractSocket::UnconnectedState; + const ChannelState final = idle ? IdleState : ClosingState; // pendingEncrypt must only be true in between connected and encrypted states pendingEncrypt = false; if (socket) { - // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while - // there is no socket yet. - socket->abort(); + QAbstractSocket *const detached = socket; + QScopedValueRollback rollback(socket, nullptr); + state = final; + detached->abort(); } + + // Set state *after* abort(), to override any recursive call's idle setting: + state = final; } -- cgit v1.2.3