HTTP/2 - fix handling of GOAWAY frame

- Fix the case when we erroneously handled stream ID == 0 in a GOAWAY frame as
an invalid stream ID.
- _q_receivedReply: convert do{}while() loop into to while(){} to prevent
it from handling any frames after GOAWAY frame received and all active frame
finished.
- sendRequest - if we received GOAWAY, also clear spdyRequests in the connection
channel, otherwise it keeps re-trying to send requests!
- Http network connection channel never resets a protocolHandler in _q_encrypted/
_q_connected, which is BAD for HTTP/2, since HTTP/2 has unique per-connection
compression context and must be reset - now we recreate the protocol handler in
_q_encrypted or _q_connected (https/http).
- Update autotest.

Task-number: QTBUG-57600
Change-Id: Ib864ce52287bab23334ff43a83ba4b0b7cb52c60
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

1 files changed, 18 insertions, 9 deletions

diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index 3fa0c18dc0..f7383d1d3a 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -208,7 +208,7 @@ void QHttp2ProtocolHandler::_q_receiveReply()
     Q_ASSERT(m_socket);
     Q_ASSERT(m_channel);
 
-    do {
+    while (!goingAway || activeStreams.size()) {
         const auto result = frameReader.read(*m_socket);
         switch (result) {
         case FrameStatus::incompleteFrame:
@@ -264,13 +264,19 @@ void QHttp2ProtocolHandler::_q_receiveReply()
             // 5.1 - ignore unknown frames.
             break;
         }
-    } while (!goingAway || activeStreams.size());
+    }
 }
 
 bool QHttp2ProtocolHandler::sendRequest()
 {
-    if (goingAway)
+    if (goingAway) {
+        // Stop further calls to this method: we have received GOAWAY
+        // so we cannot create new streams.
+        m_channel->emitFinishedWithError(QNetworkReply::ProtocolUnknownError,
+                                         "GOAWAY received, cannot start a request");
+        m_channel->spdyRequestsToSend.clear();
         return false;
+    }
 
     if (!prefaceSent && !sendClientPreface())
         return false;
@@ -756,14 +762,10 @@ void QHttp2ProtocolHandler::handleGOAWAY()
         // "The last stream identifier can be set to 0 if no
         // streams were processed."
         lastStreamID = 1;
-    }
-
-    if (!(lastStreamID & 0x1)) {
+    } else if (!(lastStreamID & 0x1)) {
         // 5.1.1 - we (client) use only odd numbers as stream identifiers.
         return connectionError(PROTOCOL_ERROR, "GOAWAY with invalid last stream ID");
-    }
-
-    if (lastStreamID >= nextID) {
+    } else if (lastStreamID >= nextID) {
         // "A server that is attempting to gracefully shut down a connection SHOULD
         // send an initial GOAWAY frame with the last stream identifier set to 2^31-1
         // and a NO_ERROR code."
@@ -776,6 +778,13 @@ void QHttp2ProtocolHandler::handleGOAWAY()
 
     goingAway = true;
 
+    // For the requests (and streams) we did not start yet, we have to report an
+    // error.
+    m_channel->emitFinishedWithError(QNetworkReply::ProtocolUnknownError,
+                                     "GOAWAY received, cannot start a request");
+    // Also, prevent further calls to sendRequest:
+    m_channel->spdyRequestsToSend.clear();
+
     QNetworkReply::NetworkError error = QNetworkReply::NoError;
     QString message;
     qt_error(errorCode, error, message);