QNAM: Fix upload corruptions when server closes connection

This patch fixes several upload corruptions if the server closes the connection
while/before we send data into it. They happen inside multiple places in the HTTP
layer and are explained in the comments.
Corruptions are:
* The upload byte device has an in-flight signal with pending upload data, if
it gets reset (because server closes the connection) then the re-send of the
request was sometimes taking this stale in-flight pending upload data.
* Because some signals were DirectConnection and some were QueuedConnection, there
was a chance that a direct signal overtakes a queued signal. The state machine
then sent data down the socket which was buffered there (and sent later) although
it did not match the current state of the state machine when it was actually sent.
* A socket was seen as being able to have requests sent even though it was not
encrypted yet. This relates to the previous corruption where data is stored inside
the socket's buffer and then sent later.

The included auto test produces all fixed corruptions, I detected no regressions
via the other tests.
This code also adds a bit of sanity checking to protect from possible further
problems.

[ChangeLog][QtNetwork] Fix HTTP(s) upload corruption when server closes connection

Change-Id: I54c883925ec897050941498f139c4b523030432e
Reviewed-by: Peter Hartmann <peter-qt@hartmann.tk>

1 files changed, 28 insertions, 7 deletions

diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 9f63280bf8..49c6793b1f 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -106,15 +106,19 @@ void QHttpNetworkConnectionChannel::init()
     socket->setProxy(QNetworkProxy::NoProxy);
 #endif
 
+    // We want all signals (except the interactive ones) be connected as QueuedConnection
+    // because else we're falling into cases where we recurse back into the socket code
+    // and mess up the state. Always going to the event loop (and expecting that when reading/writing)
+    // is safer.
     QObject::connect(socket, SIGNAL(bytesWritten(qint64)),
                      this, SLOT(_q_bytesWritten(qint64)),
-                     Qt::DirectConnection);
+                     Qt::QueuedConnection);
     QObject::connect(socket, SIGNAL(connected()),
                      this, SLOT(_q_connected()),
-                     Qt::DirectConnection);
+                     Qt::QueuedConnection);
     QObject::connect(socket, SIGNAL(readyRead()),
                      this, SLOT(_q_readyRead()),
-                     Qt::DirectConnection);
+                     Qt::QueuedConnection);
 
     // The disconnected() and error() signals may already come
     // while calling connectToHost().
@@ -143,13 +147,13 @@ void QHttpNetworkConnectionChannel::init()
         // won't be a sslSocket if encrypt is false
         QObject::connect(sslSocket, SIGNAL(encrypted()),
                          this, SLOT(_q_encrypted()),
-                         Qt::DirectConnection);
+                         Qt::QueuedConnection);
         QObject::connect(sslSocket, SIGNAL(sslErrors(QList<QSslError>)),
                          this, SLOT(_q_sslErrors(QList<QSslError>)),
                          Qt::DirectConnection);
         QObject::connect(sslSocket, SIGNAL(encryptedBytesWritten(qint64)),
                          this, SLOT(_q_encryptedBytesWritten(qint64)),
-                         Qt::DirectConnection);
+                         Qt::QueuedConnection);
 
         if (ignoreAllSslErrors)
             sslSocket->ignoreSslErrors();
@@ -186,8 +190,11 @@ void QHttpNetworkConnectionChannel::close()
     // pendingEncrypt must only be true in between connected and encrypted states
     pendingEncrypt = false;
 
-    if (socket)
+    if (socket) {
+        // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while
+        // there is no socket yet.
         socket->close();
+    }
 }
 
 
@@ -353,6 +360,14 @@ bool QHttpNetworkConnectionChannel::ensureConnection()
         }
         return false;
     }
+
+    // This code path for ConnectedState
+    if (pendingEncrypt) {
+        // Let's only be really connected when we have received the encrypted() signal. Else the state machine seems to mess up
+        // and corrupt the things sent to the server.
+        return false;
+    }
+
     return true;
 }
 
@@ -659,6 +674,12 @@ bool QHttpNetworkConnectionChannel::isSocketReading() const
 void QHttpNetworkConnectionChannel::_q_bytesWritten(qint64 bytes)
 {
     Q_UNUSED(bytes);
+    if (ssl) {
+        // In the SSL case we want to send data from encryptedBytesWritten signal since that one
+        // is the one going down to the actual network, not only into some SSL buffer.
+        return;
+    }
+
     // bytes have been written to the socket. write even more of them :)
     if (isSocketWriting())
         sendRequest();
@@ -734,7 +755,7 @@ void QHttpNetworkConnectionChannel::_q_connected()
 
     // ### FIXME: if the server closes the connection unexpectedly, we shouldn't send the same broken request again!
     //channels[i].reconnectAttempts = 2;
-    if (pendingEncrypt) {
+    if (ssl || pendingEncrypt) { // FIXME: Didn't work properly with pendingEncrypt only, we should refactor this into an EncrypingState
 #ifndef QT_NO_SSL
         if (connection->sslContext().isNull()) {
             // this socket is making the 1st handshake for this connection,