diff options
author | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2016-12-13 16:26:06 +0100 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2017-01-11 10:39:43 +0000 |
commit | 1636f3bc9294cb3f3612bcc02bd3be305fb3f707 (patch) | |
tree | 20592ba3ab05cbd30f9582edbb7c6c09958ddc01 /src/network | |
parent | 6f504a1cdd1c076365e7916f0851c5d35a8f7ad1 (diff) |
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>
Diffstat (limited to 'src/network')
-rw-r--r-- | src/network/access/qhttp2protocolhandler.cpp | 27 | ||||
-rw-r--r-- | src/network/access/qhttpnetworkconnectionchannel.cpp | 11 |
2 files changed, 26 insertions, 12 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); diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index 205490b830..646ede0022 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -180,9 +180,7 @@ void QHttpNetworkConnectionChannel::init() sslSocket->setSslConfiguration(sslConfiguration); } else { #endif // !QT_NO_SSL - if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2) - protocolHandler.reset(new QHttp2ProtocolHandler(this)); - else + if (connection->connectionType() != QHttpNetworkConnection::ConnectionTypeHTTP2) protocolHandler.reset(new QHttpProtocolHandler(this)); #ifndef QT_NO_SSL } @@ -839,6 +837,9 @@ void QHttpNetworkConnectionChannel::_q_connected() } else { state = QHttpNetworkConnectionChannel::IdleState; if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2) { + // We have to reset QHttp2ProtocolHandler's state machine, it's a new + // connection and the handler's state is unique per connection. + protocolHandler.reset(new QHttp2ProtocolHandler(this)); if (spdyRequestsToSend.count() > 0) { // wait for data from the server first (e.g. initial window, max concurrent requests) QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection); @@ -1092,6 +1093,10 @@ void QHttpNetworkConnectionChannel::_q_encrypted() emitFinishedWithError(QNetworkReply::SslHandshakeFailedError, "detected unknown Next Protocol Negotiation protocol"); } + } else if (connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2) { + // We have to reset QHttp2ProtocolHandler's state machine, it's a new + // connection and the handler's state is unique per connection. + protocolHandler.reset(new QHttp2ProtocolHandler(this)); } if (!socket) |