From 0df5d079290b4c3b13e58e9397fabdc1dfdba96b Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@theqtcompany.com>
Date: Fri, 25 Sep 2015 13:23:46 +0200
Subject: Don't let closed http sockets pass as valid connections

A QAbstractSocket can be close()'d at any time, independently of its
current connection state. being closed means that we cannot use it to
read or write data, but internally it might still have some data to
send or receive, for example to an http server. We can even get a
connected() signal after close()'ing the socket.

We need to catch this condition and mark any pending data not yet
written to the socket for resending.

Task-number: QTBUG-48326
Change-Id: I6f61c35f2c567f2a138f8cfe9ade7fd1ec039be6
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>
---
 .../access/qhttpnetworkconnectionchannel.cpp       |  7 ++-
 .../tst_qhttpnetworkconnection.cpp                 | 54 ++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
index 293909c914..b4eda3477e 100644
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
@@ -272,7 +272,12 @@ bool QHttpNetworkConnectionChannel::ensureConnection()
     QAbstractSocket::SocketState socketState = socket->state();
 
     // resend this request after we receive the disconnected signal
-    if (socketState == QAbstractSocket::ClosingState) {
+    // If !socket->isOpen() then we have already called close() on the socket, but there was still a
+    // pending connectToHost() for which we hadn't seen a connected() signal, yet. The connected()
+    // has now arrived (as indicated by socketState != ClosingState), but we cannot send anything on
+    // such a socket anymore.
+    if (socketState == QAbstractSocket::ClosingState ||
+            (socketState != QAbstractSocket::UnconnectedState && !socket->isOpen())) {
         if (reply)
             resendCurrent = true;
         return false;
diff --git a/tests/auto/network/access/qhttpnetworkconnection/tst_qhttpnetworkconnection.cpp b/tests/auto/network/access/qhttpnetworkconnection/tst_qhttpnetworkconnection.cpp
index 5d072af6d5..0d188a8fec 100644
--- a/tests/auto/network/access/qhttpnetworkconnection/tst_qhttpnetworkconnection.cpp
+++ b/tests/auto/network/access/qhttpnetworkconnection/tst_qhttpnetworkconnection.cpp
@@ -36,6 +36,7 @@
 #include "private/qhttpnetworkconnection_p.h"
 #include "private/qnoncontiguousbytedevice_p.h"
 #include <QAuthenticator>
+#include <QTcpServer>
 
 #include "../../../network-settings.h"
 
@@ -106,6 +107,8 @@ private Q_SLOTS:
 
     void getAndThenDeleteObject();
     void getAndThenDeleteObject_data();
+
+    void overlappingCloseAndWrite();
 };
 
 tst_QHttpNetworkConnection::tst_QHttpNetworkConnection()
@@ -1112,6 +1115,57 @@ void tst_QHttpNetworkConnection::getAndThenDeleteObject()
     }
 }
 
+class TestTcpServer : public QTcpServer
+{
+    Q_OBJECT
+public:
+    TestTcpServer() : errorCodeReports(0)
+    {
+        connect(this, &QTcpServer::newConnection, this, &TestTcpServer::onNewConnection);
+        QVERIFY(listen(QHostAddress::LocalHost));
+    }
+
+    int errorCodeReports;
+
+public slots:
+    void onNewConnection()
+    {
+        QTcpSocket *socket = nextPendingConnection();
+        if (!socket)
+            return;
+        // close socket instantly!
+        connect(socket, &QTcpSocket::readyRead, socket, &QTcpSocket::close);
+    }
+
+    void onReply(QNetworkReply::NetworkError code)
+    {
+        QCOMPARE(code, QNetworkReply::RemoteHostClosedError);
+        ++errorCodeReports;
+    }
+};
+
+void tst_QHttpNetworkConnection::overlappingCloseAndWrite()
+{
+    // server accepts connections, but closes the socket instantly
+    TestTcpServer server;
+    QNetworkAccessManager accessManager;
+
+    // ten requests are scheduled. All should result in an RemoteHostClosed...
+    QUrl url;
+    url.setScheme(QStringLiteral("http"));
+    url.setHost(server.serverAddress().toString());
+    url.setPort(server.serverPort());
+    for (int i = 0; i < 10; ++i) {
+        QNetworkRequest request(url);
+        QNetworkReply *reply = accessManager.get(request);
+        // Not using Qt5 connection syntax here because of overly baroque syntax to discern between
+        // different error() methods.
+        QObject::connect(reply, SIGNAL(error(QNetworkReply::NetworkError)),
+                         &server, SLOT(onReply(QNetworkReply::NetworkError)));
+    }
+
+    QTRY_COMPARE(server.errorCodeReports, 10);
+}
 
 
 QTEST_MAIN(tst_QHttpNetworkConnection)
-- 
cgit v1.2.3