HTTP/2 - reset uploadByteDevice if necessary

1. If a request was redirected or some error was encountered, we
   try to reset the uploading byte-device.
2. Disconnecting from the byte-device is not enough, since we have a
   queued connection, _q_uploadDataReadyRead() gets called even if
   byte-device was deleted and thus sender() can return null -
   we have to check this condition.
3. Update auto-test with a case where our server immediately
   replies with a redirect status code.

Task-number: QTBUG-67469
Task-number: QTBUG-66913
Change-Id: I9b364cf3dee1717940ddbe50cba37c3398cc9c95
Reviewed-by: Edward Welbourne <[email protected]>

src/network/access/qhttp2protocolhandler.cpp

@@ -212,6 +212,9 @@ QHttp2ProtocolHandler::QHttp2ProtocolHandler(QHttpNetworkConnectionChannel *chan
 
 void QHttp2ProtocolHandler::_q_uploadDataReadyRead()
 {
+    if (!sender()) // QueuedConnection, firing after sender (byte device) was deleted.
+        return;
+
     auto data = qobject_cast<QNonContiguousByteDevice *>(sender());
     Q_ASSERT(data);
     const qint32 streamID = data->property("HTTP2StreamID").toInt();
@@ -1110,6 +1113,17 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader
     if (QHttpNetworkReply::isHttpRedirect(statusCode) && redirectUrl.isValid())
         httpReply->setRedirectUrl(redirectUrl);
 
+    if (QHttpNetworkReply::isHttpRedirect(statusCode)
+        || statusCode == 401 || statusCode == 407) {
+        // These are the status codes that can trigger uploadByteDevice->reset()
+        // in QHttpNetworkConnectionChannel::handleStatus. Alas, we have no
+        // single request/reply, we multiplex several requests and thus we never
+        // simply call 'handleStatus'. If we have byte-device - we try to reset
+        // it here, we don't (and can't) handle any error during reset operation.
+        if (stream.data())
+            stream.data()->reset();
+    }
+
     if (connectionType == Qt::DirectConnection)
         emit httpReply->headerChanged();
     else

tests/auto/network/access/http2/http2srv.cpp

@@ -123,6 +123,12 @@ void Http2Server::emulateGOAWAY(int timeout)
     goawayTimeout = timeout;
 }
 
+void Http2Server::redirectOpenStream(quint16 port)
+{
+    redirectWhileReading = true;
+    targetPort = port;
+}
+
 void Http2Server::startServer()
 {
 #ifdef QT_NO_SSL
@@ -775,7 +781,19 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody)
     if (emptyBody)
         writer.addFlag(FrameFlag::END_STREAM);
 
-    HttpHeader header = {{":status", "200"}};
+    HttpHeader header;
+    if (redirectWhileReading) {
+        qDebug("server received HEADERS frame (followed by DATA frames), redirecting ...");
+        Q_ASSERT(targetPort);
+        header.push_back({":status", "308"});
+        const QString url("%1://localhost:%2/");
+        header.push_back({"location", url.arg(clearTextHTTP2 ? QStringLiteral("http") : QStringLiteral("https"),
+                                              QString::number(targetPort)).toLatin1()});
+
+    } else {
+        header.push_back({":status", "200"});
+    }
+
     if (!emptyBody) {
         header.push_back(HPack::HeaderField("content-length",
                          QString("%1").arg(responseBody.size()).toLatin1()));
@@ -871,7 +889,13 @@ void Http2Server::processRequest()
     activeRequests[streamID] = decoder.decodedHeader();
     if (headersFrame.flags().testFlag(FrameFlag::END_STREAM))
         emit receivedRequest(streamID);
-    // else - we're waiting for incoming DATA frames ...
+
+    if (redirectWhileReading) {
+        sendResponse(streamID, true);
+        // Don't try to read any DATA frames ...
+        socket->disconnect();
+    } // else - we're waiting for incoming DATA frames ...
+
     continuedRequest.clear();
 }
 

tests/auto/network/access/http2/http2srv.h

@@ -74,6 +74,7 @@ class Http2Server : public QTcpServer
     void enablePushPromise(bool enabled, const QByteArray &path = QByteArray());
     void setResponseBody(const QByteArray &body);
     void emulateGOAWAY(int timeout);
+    void redirectOpenStream(quint16 targetPort);
 
     // Invokables, since we can call them from the main thread,
     // but server (can) work on its own thread.
@@ -186,6 +187,10 @@ private slots:
     // We need it for PUSH_PROMISE, with the correct port number appended,
     // when replying to essentially 1.1 request.
     QByteArray authority;
+    // Redirect, with status code 308, as soon as we've seen headers, while client
+    // may still be sending DATA frames.  See tst_Http2::earlyResponse().
+    bool redirectWhileReading = false;
+    quint16 targetPort = 0;
 protected slots:
     void ignoreErrorSlot();
 };

tests/auto/network/access/http2/tst_http2.cpp

@@ -74,6 +74,7 @@ private slots:
     void pushPromise();
     void goaway_data();
     void goaway();
+    void earlyResponse();
 
 protected slots:
     // Slots to listen to our in-process server:
@@ -439,6 +440,47 @@ void tst_Http2::goaway()
     QVERIFY(!serverGotSettingsACK);
 }
 
+void tst_Http2::earlyResponse()
+{
+    // In this test we'd like to verify client side can handle HEADERS frame while
+    // its stream is in 'open' state. To achieve this, we send a POST request
+    // with some payload, so that the client is first sending HEADERS and then
+    // DATA frames without END_STREAM flag set yet (thus the stream is in Stream::open
+    // state). Upon receiving the client's HEADERS frame our server ('redirector')
+    // immediately (without trying to read any DATA frames) responds with status
+    // code 308. The client should properly handle this.
+
+    clearHTTP2State();
+
+    serverPort = 0;
+    nRequests = 1;
+
+    ServerPtr targetServer(newServer(defaultServerSettings));
+
+    QMetaObject::invokeMethod(targetServer.data(), "startServer", Qt::QueuedConnection);
+    runEventLoop();
+
+    QVERIFY(serverPort != 0);
+
+    const quint16 targetPort = serverPort;
+    serverPort = 0;
+
+    ServerPtr redirector(newServer(defaultServerSettings));
+    redirector->redirectOpenStream(targetPort);
+
+    QMetaObject::invokeMethod(redirector.data(), "startServer", Qt::QueuedConnection);
+    runEventLoop();
+
+    QVERIFY(serverPort);
+    sendRequest(1, QNetworkRequest::NormalPriority, {10000000, Qt::Uninitialized});
+
+    runEventLoop();
+
+    QVERIFY(nRequests == 0);
+    QVERIFY(prefaceOK);
+    QVERIFY(serverGotSettingsACK);
+}
+
 void tst_Http2::serverStarted(quint16 port)
 {
     serverPort = port;
@@ -500,6 +542,7 @@ void tst_Http2::sendRequest(int streamNumber,
 
     QNetworkRequest request(url);
     request.setAttribute(QNetworkRequest::HTTP2AllowedAttribute, QVariant(true));
+    request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, QVariant(true));
     request.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("text/plain"));
     request.setPriority(priority);
 
@@ -592,6 +635,10 @@ void tst_Http2::replyFinished()
         const QVariant spdyUsed(reply->attribute(QNetworkRequest::SpdyWasUsedAttribute));
         QVERIFY(spdyUsed.isValid());
         QVERIFY(!spdyUsed.toBool());
+        const QVariant code(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute));
+        QVERIFY(code.isValid());
+        QVERIFY(code.canConvert<int>());
+        QCOMPARE(code.value<int>(), 200);
     }
 
     --nRequests;