diff options
author | Edward Welbourne <edward.welbourne@qt.io> | 2016-12-14 15:31:58 +0100 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2017-01-17 05:12:57 +0000 |
commit | 556b2ee7737b1dfdbc5223e9a1230b5df6843a01 (patch) | |
tree | ef3121d29ee99f9ac94a19fed77bcf19810466ac /src/network | |
parent | 7634225ad270994933f1b8d896bfbbf3eb3f6132 (diff) |
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 <timur.pocheptsov@qt.io>
Diffstat (limited to 'src/network')
-rw-r--r-- | src/network/access/qhttpnetworkconnectionchannel.cpp | 44 |
1 files changed, 23 insertions, 21 deletions
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 <qpair.h> #include <qdebug.h> +#include <QScopedValueRollback> #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<QAbstractSocket *> 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<QAbstractSocket *> rollback(socket, nullptr); + state = final; + detached->abort(); } + + // Set state *after* abort(), to override any recursive call's idle setting: + state = final; } |