From 512934f7e70592ed06a790fcb46dde1e435b488e Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 10 Oct 2016 15:29:26 +0200 Subject: [PATCH] HTTP/2 - fix the handling of PUSH_PROMISE HTTP/2 allows a server to pre-emptively send (or "push") responses (along with corresponding "promised" requests) to a client in association with a previous client-initiated request. This can be useful when the server knows the client will need to have those responses available in order to fully process the response to the original request. Server push is semantically equivalent to a server responding to a request; however, in this case, that request is also sent by the server, as a PUSH_PROMISE frame. The PUSH_PROMISE frame includes a header block that contains a complete set of request header fields that the server attributes to the request. After sending the PUSH_PROMISE frame, the server can begin delivering the pushed response as a response on a server-initiated stream that uses the promised stream identifier. This patch: - fixes the HPACK decompression of PUSH_PROMISE frames; - allows a user to enable PUSH_PROMISE; - processes and caches pushed data for promised streams; - updates auto-test - emulates a simple PUSH_PROMISE scenario. Change-Id: Ic4850863a5e3895320baac3871a723fc091b4aca Reviewed-by: Edward Welbourne --- src/network/access/http2/http2frames.cpp | 33 ++ src/network/access/http2/http2frames_p.h | 35 +- src/network/access/http2/http2protocol_p.h | 4 + src/network/access/http2/http2streams.cpp | 11 +- src/network/access/http2/http2streams_p.h | 25 +- src/network/access/qhttp2protocolhandler.cpp | 372 ++++++++++++++---- src/network/access/qhttp2protocolhandler_p.h | 19 +- tests/auto/network/access/http2/http2srv.cpp | 63 ++- tests/auto/network/access/http2/http2srv.h | 4 + tests/auto/network/access/http2/tst_http2.cpp | 148 +++++-- 10 files changed, 586 insertions(+), 128 deletions(-) diff --git a/src/network/access/http2/http2frames.cpp b/src/network/access/http2/http2frames.cpp index 978bee09b1..5a684c2f41 100644 --- a/src/network/access/http2/http2frames.cpp +++ b/src/network/access/http2/http2frames.cpp @@ -244,6 +244,24 @@ quint32 Frame::dataSize() const return size; } +quint32 Frame::hpackBlockSize() const +{ + Q_ASSERT(validatePayload() == FrameStatus::goodFrame); + + const auto frameType = type(); + Q_ASSERT(frameType == FrameType::HEADERS || + frameType == FrameType::PUSH_PROMISE || + frameType == FrameType::CONTINUATION); + + quint32 size = dataSize(); + if (frameType == FrameType::PUSH_PROMISE) { + Q_ASSERT(size >= 4); + size -= 4; + } + + return size; +} + const uchar *Frame::dataBegin() const { Q_ASSERT(validatePayload() == FrameStatus::goodFrame); @@ -260,6 +278,21 @@ const uchar *Frame::dataBegin() const return src; } +const uchar *Frame::hpackBlockBegin() const +{ + Q_ASSERT(validatePayload() == FrameStatus::goodFrame); + + const auto frameType = type(); + Q_ASSERT(frameType == FrameType::HEADERS || + frameType == FrameType::PUSH_PROMISE || + frameType == FrameType::CONTINUATION); + + const uchar *begin = dataBegin(); + if (frameType == FrameType::PUSH_PROMISE) + begin += 4; // That's a promised stream, skip it. + return begin; +} + FrameStatus FrameReader::read(QAbstractSocket &socket) { if (offset < frameHeaderSize) { diff --git a/src/network/access/http2/http2frames_p.h b/src/network/access/http2/http2frames_p.h index 84ba9c3662..e5f6d46c67 100644 --- a/src/network/access/http2/http2frames_p.h +++ b/src/network/access/http2/http2frames_p.h @@ -71,27 +71,29 @@ namespace Http2 struct Q_AUTOTEST_EXPORT Frame { Frame(); - // Reading these values without first forming a valid frame - // (either reading it from a socket or building it) will result - // in undefined behavior: + // Reading these values without first forming a valid frame (either reading + // it from a socket or building it) will result in undefined behavior: FrameType type() const; quint32 streamID() const; FrameFlags flags() const; quint32 payloadSize() const; uchar padding() const; - // In HTTP/2 a stream's priority is specified by its weight - // and a stream (id) it depends on: + // In HTTP/2 a stream's priority is specified by its weight and a stream + // (id) it depends on: bool priority(quint32 *streamID = nullptr, uchar *weight = nullptr) const; FrameStatus validateHeader() const; FrameStatus validatePayload() const; - // Number of payload bytes without padding and/or priority + // Number of payload bytes without padding and/or priority. quint32 dataSize() const; - // Beginning of payload without priority/padding - // bytes. + // HEADERS data size for HEADERS, PUSH_PROMISE and CONTINUATION streams: + quint32 hpackBlockSize() const; + // Beginning of payload without priority/padding bytes. const uchar *dataBegin() const; + // HEADERS data beginning for HEADERS, PUSH_PROMISE and CONTINUATION streams: + const uchar *hpackBlockBegin() const; std::vector buffer; }; @@ -134,8 +136,7 @@ public: void setFlags(FrameFlags flags); void addFlag(FrameFlag flag); - // All append functions also update frame's payload - // length. + // All append functions also update frame's payload length. template void append(ValueType val) { @@ -161,16 +162,14 @@ public: // Write as a single frame: bool write(QAbstractSocket &socket) const; - // Two types of frames we are sending are affected by - // frame size limits: HEADERS and DATA. HEADERS' payload - // (hpacked HTTP headers, following a frame header) - // is always in our 'buffer', we send the initial HEADERS + // Two types of frames we are sending are affected by frame size limits: + // HEADERS and DATA. HEADERS' payload (hpacked HTTP headers, following a + // frame header) is always in our 'buffer', we send the initial HEADERS // frame first and then CONTINUTATION frame(s) if needed: bool writeHEADERS(QAbstractSocket &socket, quint32 sizeLimit); - // With DATA frames the actual payload is never in our 'buffer', - // it's a 'readPointer' from QNonContiguousData. We split - // this payload as needed into DATA frames with correct - // payload size fitting into frame size limit: + // With DATA frames the actual payload is never in our 'buffer', it's a + // 'readPointer' from QNonContiguousData. We split this payload as needed + // into DATA frames with correct payload size fitting into frame size limit: bool writeDATA(QAbstractSocket &socket, quint32 sizeLimit, const uchar *src, quint32 size); private: diff --git a/src/network/access/http2/http2protocol_p.h b/src/network/access/http2/http2protocol_p.h index 5c46949e23..5d730404bb 100644 --- a/src/network/access/http2/http2protocol_p.h +++ b/src/network/access/http2/http2protocol_p.h @@ -127,6 +127,10 @@ enum Http2PredefinedParameters maxConcurrentStreams = 100 // HTTP/2, 6.5.2 }; +// It's int, it has internal linkage, it's ok to have it in headers - +// no ODR violation is possible. +const quint32 lastValidStreamID((quint32(1) << 31) - 1); // HTTP/2, 5.1.1 + extern const Q_AUTOTEST_EXPORT char Http2clientPreface[clientPrefaceLength]; enum class FrameStatus diff --git a/src/network/access/http2/http2streams.cpp b/src/network/access/http2/http2streams.cpp index 660100f5e4..fa39c1d57b 100644 --- a/src/network/access/http2/http2streams.cpp +++ b/src/network/access/http2/http2streams.cpp @@ -61,6 +61,15 @@ Stream::Stream(const HttpMessagePair &message, quint32 id, qint32 sendSize, qint { } +Stream::Stream(const QString &cacheKey, quint32 id, qint32 recvSize) + : streamID(id), + // sendWindow is 0, this stream only receives data + recvWindow(recvSize), + state(remoteReserved), + key(cacheKey) +{ +} + QHttpNetworkReply *Stream::reply() const { return httpPair.second; @@ -99,6 +108,6 @@ QNonContiguousByteDevice *Stream::data() const return httpPair.first.uploadByteDevice(); } -} +} // namespace Http2 QT_END_NAMESPACE diff --git a/src/network/access/http2/http2streams_p.h b/src/network/access/http2/http2streams_p.h index 8a825a5457..8465486ae8 100644 --- a/src/network/access/http2/http2streams_p.h +++ b/src/network/access/http2/http2streams_p.h @@ -51,10 +51,16 @@ // We mean it. // +#include "http2frames_p.h" +#include "hpack_p.h" + #include #include #include +#include + +#include QT_BEGIN_NAMESPACE @@ -70,12 +76,16 @@ struct Q_AUTOTEST_EXPORT Stream open, halfClosedLocal, halfClosedRemote, + remoteReserved, closed }; Stream(); + // That's a ctor for a client-initiated stream: Stream(const HttpMessagePair &message, quint32 streamID, qint32 sendSize, qint32 recvSize); + // That's a reserved stream, created by PUSH_PROMISE from a server: + Stream(const QString &key, quint32 streamID, qint32 recvSize); QHttpNetworkReply *reply() const; const QHttpNetworkRequest &request() const; @@ -92,9 +102,22 @@ struct Q_AUTOTEST_EXPORT Stream qint32 recvWindow = 65535; StreamState state = idle; + QString key; // for PUSH_PROMISE }; -} +struct PushPromise +{ + quint32 reservedID = 0; + // PUSH_PROMISE has its own HEADERS, + // usually similar to what request has: + HPack::HttpHeader pushHeader; + // Response has its own (normal) HEADERS: + HPack::HttpHeader responseHeader; + // DATA frames on a promised stream: + std::vector dataFrames; +}; + +} // namespace Http2 QT_END_NAMESPACE diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp index 68a00c6837..3fa0c18dc0 100644 --- a/src/network/access/qhttp2protocolhandler.cpp +++ b/src/network/access/qhttp2protocolhandler.cpp @@ -108,6 +108,41 @@ HPack::HttpHeader build_headers(const QHttpNetworkRequest &request, quint32 maxH return header; } +std::vector assemble_hpack_block(const std::vector &frames) +{ + std::vector hpackBlock; + + quint32 total = 0; + for (const auto &frame : frames) + total += frame.hpackBlockSize(); + + if (!total) + return hpackBlock; + + hpackBlock.resize(total); + auto dst = hpackBlock.begin(); + for (const auto &frame : frames) { + if (const auto hpackBlockSize = frame.hpackBlockSize()) { + const uchar *src = frame.hpackBlockBegin(); + std::copy(src, src + hpackBlockSize, dst); + dst += hpackBlockSize; + } + } + + return hpackBlock; +} + +QUrl urlkey_from_request(const QHttpNetworkRequest &request) +{ + QUrl url; + + url.setScheme(request.url().scheme()); + url.setAuthority(request.url().authority(QUrl::FullyEncoded | QUrl::RemoveUserInfo)); + url.setPath(QLatin1String(request.uri(false))); + + return url; +} + bool sum_will_overflow(qint32 windowSize, qint32 delta) { if (windowSize > 0) @@ -131,6 +166,9 @@ QHttp2ProtocolHandler::QHttp2ProtocolHandler(QHttpNetworkConnectionChannel *chan encoder(HPack::FieldLookupTable::DefaultSize, true) { continuedFrames.reserve(20); + bool ok = false; + const int env = qEnvironmentVariableIntValue("QT_HTTP2_ENABLE_PUSH_PROMISE", &ok); + pushPromiseEnabled = ok && env; } void QHttp2ProtocolHandler::_q_uploadDataReadyRead() @@ -241,10 +279,25 @@ bool QHttp2ProtocolHandler::sendRequest() if (!requests.size()) return true; + m_channel->state = QHttpNetworkConnectionChannel::WritingState; + // Check what was promised/pushed, maybe we do not have to send a request + // and have a response already? + + for (auto it = requests.begin(), endIt = requests.end(); it != endIt;) { + const auto key = urlkey_from_request(it->first).toString(); + if (!promisedData.contains(key)) { + ++it; + continue; + } + // Woo-hoo, we do not have to ask, the answer is ready for us: + HttpMessagePair message = *it; + it = requests.erase(it); + initReplyFromPushPromise(message, key); + } + const auto streamsToUse = std::min(maxConcurrentStreams - activeStreams.size(), requests.size()); auto it = requests.begin(); - m_channel->state = QHttpNetworkConnectionChannel::WritingState; for (quint32 i = 0; i < streamsToUse; ++i) { const qint32 newStreamID = createNewStream(*it); if (!newStreamID) { @@ -293,11 +346,11 @@ bool QHttp2ProtocolHandler::sendClientPreface() // 6.5 SETTINGS frameWriter.start(FrameType::SETTINGS, FrameFlag::EMPTY, Http2::connectionStreamID); - // MAX frame size (16 kb), disable PUSH + // MAX frame size (16 kb), enable/disable PUSH frameWriter.append(Settings::MAX_FRAME_SIZE_ID); frameWriter.append(quint32(Http2::maxFrameSize)); frameWriter.append(Settings::ENABLE_PUSH_ID); - frameWriter.append(quint32(0)); + frameWriter.append(quint32(pushPromiseEnabled)); if (!frameWriter.write(*m_socket)) return false; @@ -621,7 +674,7 @@ void QHttp2ProtocolHandler::handlePUSH_PROMISE() // 6.6 PUSH_PROMISE. Q_ASSERT(inboundFrame.type() == FrameType::PUSH_PROMISE); - if (prefaceSent && !waitingForSettingsACK) { + if (!pushPromiseEnabled && prefaceSent && !waitingForSettingsACK) { // This means, server ACKed our 'NO PUSH', // but sent us PUSH_PROMISE anyway. return connectionError(PROTOCOL_ERROR, "unexpected PUSH_PROMISE frame"); @@ -639,15 +692,19 @@ void QHttp2ProtocolHandler::handlePUSH_PROMISE() } const auto reservedID = qFromBigEndian(inboundFrame.dataBegin()); - if (!reservedID || (reservedID & 0x1)) { + if ((reservedID & 1) || reservedID <= lastPromisedID || + reservedID > Http2::lastValidStreamID) { return connectionError(PROTOCOL_ERROR, "PUSH_PROMISE with invalid promised stream ID"); } - // "ignoring a PUSH_PROMISE frame causes the stream - // state to become indeterminate" - let's RST_STREAM it then ... - sendRST_STREAM(reservedID, REFUSE_STREAM); - markAsReset(reservedID); + lastPromisedID = reservedID; + + if (!pushPromiseEnabled) { + // "ignoring a PUSH_PROMISE frame causes the stream state to become + // indeterminate" - let's send RST_STREAM frame with REFUSE_STREAM code. + resetPromisedStream(inboundFrame, Http2::REFUSE_STREAM); + } const bool endHeaders = inboundFrame.flags().testFlag(FrameFlag::END_HEADERS); continuedFrames.clear(); @@ -710,7 +767,7 @@ void QHttp2ProtocolHandler::handleGOAWAY() // "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." - if (lastStreamID != (quint32(1) << 31) - 1 || errorCode != HTTP2_NO_ERROR) + if (lastStreamID != Http2::lastValidStreamID || errorCode != HTTP2_NO_ERROR) return connectionError(PROTOCOL_ERROR, "GOAWAY invalid stream/error code"); lastStreamID = 1; } else { @@ -795,16 +852,24 @@ void QHttp2ProtocolHandler::handleCONTINUATION() void QHttp2ProtocolHandler::handleContinuedHEADERS() { + // 'Continued' HEADERS can be: the initial HEADERS/PUSH_PROMISE frame + // with/without END_HEADERS flag set plus, if no END_HEADERS flag, + // a sequence of one or more CONTINUATION frames. Q_ASSERT(continuedFrames.size()); + const auto firstFrameType = continuedFrames[0].type(); + Q_ASSERT(firstFrameType == FrameType::HEADERS || + firstFrameType == FrameType::PUSH_PROMISE); const auto streamID = continuedFrames[0].streamID(); - if (continuedFrames[0].type() == FrameType::HEADERS) { + if (firstFrameType == FrameType::HEADERS) { if (activeStreams.contains(streamID)) { Stream &stream = activeStreams[streamID]; - if (stream.state != Stream::halfClosedLocal) { - // If we're receiving headers, they're a response to a request we sent; - // and we closed our end when we finished sending that. + if (stream.state != Stream::halfClosedLocal + && stream.state != Stream::remoteReserved) { + // We can receive HEADERS on streams initiated by our requests + // (these streams are in halfClosedLocal state) or remote-reserved + // streams from a server's PUSH_PROMISE. finishStreamWithError(stream, QNetworkReply::ProtocolInvalidOperationError, QLatin1String("HEADERS on invalid stream")); sendRST_STREAM(streamID, CANCEL); @@ -815,42 +880,49 @@ void QHttp2ProtocolHandler::handleContinuedHEADERS() } else if (!streamWasReset(streamID)) { return connectionError(PROTOCOL_ERROR, "HEADERS on invalid stream"); } + // Else: we cannot just ignore our peer's HEADERS frames - they change + // HPACK context - even though the stream was reset; apparently the peer + // has yet to see the reset. } - quint32 total = 0; - for (const auto &frame : continuedFrames) - total += frame.dataSize(); + std::vector hpackBlock(assemble_hpack_block(continuedFrames)); + if (!hpackBlock.size()) { + // It could be a PRIORITY sent in HEADERS - already handled by this + // point in handleHEADERS. If it was PUSH_PROMISE (HTTP/2 8.2.1): + // "The header fields in PUSH_PROMISE and any subsequent CONTINUATION + // frames MUST be a valid and complete set of request header fields + // (Section 8.1.2.3) ... If a client receives a PUSH_PROMISE that does + // not include a complete and valid set of header fields or the :method + // pseudo-header field identifies a method that is not safe, it MUST + // respond with a stream error (Section 5.4.2) of type PROTOCOL_ERROR." + if (firstFrameType == FrameType::PUSH_PROMISE) + resetPromisedStream(continuedFrames[0], Http2::PROTOCOL_ERROR); - if (!total) { - // It could be a PRIORITY sent in HEADERS - handled by this point. return; } - std::vector hpackBlock(total); - auto dst = hpackBlock.begin(); - for (const auto &frame : continuedFrames) { - if (!frame.dataSize()) - continue; - const uchar *src = frame.dataBegin(); - std::copy(src, src + frame.dataSize(), dst); - dst += frame.dataSize(); - } - - HPack::BitIStream inputStream{&hpackBlock[0], - &hpackBlock[0] + hpackBlock.size()}; - + HPack::BitIStream inputStream{&hpackBlock[0], &hpackBlock[0] + hpackBlock.size()}; if (!decoder.decodeHeaderFields(inputStream)) return connectionError(COMPRESSION_ERROR, "HPACK decompression failed"); - if (continuedFrames[0].type() == FrameType::HEADERS) { + switch (firstFrameType) { + case FrameType::HEADERS: if (activeStreams.contains(streamID)) { Stream &stream = activeStreams[streamID]; updateStream(stream, decoder.decodedHeader()); + // No DATA frames. if (continuedFrames[0].flags() & FrameFlag::END_STREAM) { finishStream(stream); deleteActiveStream(stream.streamID); } } + break; + case FrameType::PUSH_PROMISE: + if (!tryReserveStream(continuedFrames[0], decoder.decodedHeader())) + resetPromisedStream(continuedFrames[0], Http2::PROTOCOL_ERROR); + break; + default: + break; } } @@ -923,10 +995,26 @@ bool QHttp2ProtocolHandler::acceptSetting(Http2::Settings identifier, quint32 ne return true; } -void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader &headers) +void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader &headers, + Qt::ConnectionType connectionType) { const auto httpReply = stream.reply(); - Q_ASSERT(httpReply); + Q_ASSERT(httpReply || stream.state == Stream::remoteReserved); + + if (!httpReply) { + // It's a PUSH_PROMISEd HEADERS, no actual request/reply + // exists yet, we have to cache this data for a future + // (potential) request. + + // TODO: the part with assignment is not especially cool + // or beautiful, good that at least QByteArray is implicitly + // sharing data. To be refactored (std::move). + Q_ASSERT(promisedData.contains(stream.key)); + PushPromise &promise = promisedData[stream.key]; + promise.responseHeader = headers; + return; + } + const auto httpReplyPrivate = httpReply->d_func(); for (const auto &pair : headers) { const auto &name = pair.name; @@ -951,18 +1039,30 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader } } - emit httpReply->headerChanged(); + if (connectionType == Qt::DirectConnection) + emit httpReply->headerChanged(); + else + QMetaObject::invokeMethod(httpReply, "headerChanged", connectionType); } -void QHttp2ProtocolHandler::updateStream(Stream &stream, const Frame &frame) +void QHttp2ProtocolHandler::updateStream(Stream &stream, const Frame &frame, + Qt::ConnectionType connectionType) { Q_ASSERT(frame.type() == FrameType::DATA); + auto httpReply = stream.reply(); + Q_ASSERT(httpReply || stream.state == Stream::remoteReserved); + + if (!httpReply) { + Q_ASSERT(promisedData.contains(stream.key)); + PushPromise &promise = promisedData[stream.key]; + // TODO: refactor this to use std::move. + promise.dataFrames.push_back(frame); + return; + } if (const auto length = frame.dataSize()) { const char *data = reinterpret_cast(frame.dataBegin()); auto &httpRequest = stream.request(); - auto httpReply = stream.reply(); - Q_ASSERT(httpReply); auto replyPrivate = httpReply->d_func(); replyPrivate->compressedData.append(data, length); @@ -978,24 +1078,38 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const Frame &frame) } if (replyPrivate->shouldEmitSignals()) { - emit httpReply->readyRead(); - emit httpReply->dataReadProgress(replyPrivate->totalProgress, replyPrivate->bodyLength); + if (connectionType == Qt::DirectConnection) { + emit httpReply->readyRead(); + emit httpReply->dataReadProgress(replyPrivate->totalProgress, + replyPrivate->bodyLength); + } else { + QMetaObject::invokeMethod(httpReply, "readyRead", connectionType); + QMetaObject::invokeMethod(httpReply, "dataReadProgress", connectionType, + Q_ARG(qint64, replyPrivate->totalProgress), + Q_ARG(qint64, replyPrivate->bodyLength)); + } } } } -void QHttp2ProtocolHandler::finishStream(Stream &stream) +void QHttp2ProtocolHandler::finishStream(Stream &stream, Qt::ConnectionType connectionType) { + Q_ASSERT(stream.state == Stream::remoteReserved || stream.reply()); + stream.state = Stream::closed; auto httpReply = stream.reply(); - Q_ASSERT(httpReply); - httpReply->disconnect(this); - if (stream.data()) - stream.data()->disconnect(this); + if (httpReply) { + httpReply->disconnect(this); + if (stream.data()) + stream.data()->disconnect(this); + + if (connectionType == Qt::DirectConnection) + emit httpReply->finished(); + else + QMetaObject::invokeMethod(httpReply, "finished", connectionType); + } qCDebug(QT_HTTP2) << "stream" << stream.streamID << "closed"; - - emit httpReply->finished(); } void QHttp2ProtocolHandler::finishStreamWithError(Stream &stream, quint32 errorCode) @@ -1009,18 +1123,20 @@ void QHttp2ProtocolHandler::finishStreamWithError(Stream &stream, quint32 errorC void QHttp2ProtocolHandler::finishStreamWithError(Stream &stream, QNetworkReply::NetworkError error, const QString &message) { + Q_ASSERT(stream.state == Stream::remoteReserved || stream.reply()); + stream.state = Stream::closed; - auto httpReply = stream.reply(); - Q_ASSERT(httpReply); - httpReply->disconnect(this); - if (stream.data()) - stream.data()->disconnect(this); + if (auto httpReply = stream.reply()) { + httpReply->disconnect(this); + if (stream.data()) + stream.data()->disconnect(this); + + // TODO: error message must be translated!!! (tr) + emit httpReply->finishedWithError(error, message); + } qCWarning(QT_HTTP2) << "stream" << stream.streamID << "finished with error:" << message; - - // TODO: error message must be translated!!! (tr) - emit httpReply->finishedWithError(error, message); } quint32 QHttp2ProtocolHandler::createNewStream(const HttpMessagePair &message) @@ -1066,26 +1182,24 @@ void QHttp2ProtocolHandler::addToSuspended(Stream &stream) void QHttp2ProtocolHandler::markAsReset(quint32 streamID) { - // For now, we trace only client's streams (created by us, - // odd integer numbers). - if (streamID & 0x1) { - qCDebug(QT_HTTP2) << "stream" << streamID << "was reset"; - // This part is quite tricky: I have to clear this set - // so that it does not become tOOO big. - if (recycledStreams.size() > maxRecycledStreams) { - // At least, I'm erasing the oldest first ... - recycledStreams.erase(recycledStreams.begin(), - recycledStreams.begin() + - recycledStreams.size() / 2); - } + Q_ASSERT(streamID); - const auto it = std::lower_bound(recycledStreams.begin(), recycledStreams.end(), - streamID); - if (it != recycledStreams.end() && *it == streamID) - return; - - recycledStreams.insert(it, streamID); + qCDebug(QT_HTTP2) << "stream" << streamID << "was reset"; + // This part is quite tricky: I have to clear this set + // so that it does not become tOOO big. + if (recycledStreams.size() > maxRecycledStreams) { + // At least, I'm erasing the oldest first ... + recycledStreams.erase(recycledStreams.begin(), + recycledStreams.begin() + + recycledStreams.size() / 2); } + + const auto it = std::lower_bound(recycledStreams.begin(), recycledStreams.end(), + streamID); + if (it != recycledStreams.end() && *it == streamID) + return; + + recycledStreams.insert(it, streamID); } quint32 QHttp2ProtocolHandler::popStreamToResume() @@ -1175,7 +1289,7 @@ quint32 QHttp2ProtocolHandler::allocateStreamID() { // With protocol upgrade streamID == 1 will become // invalid. The logic must be updated. - if (nextID > quint32(std::numeric_limits::max())) + if (nextID > Http2::lastValidStreamID) return 0; const quint32 streamID = nextID; @@ -1184,6 +1298,114 @@ quint32 QHttp2ProtocolHandler::allocateStreamID() return streamID; } +bool QHttp2ProtocolHandler::tryReserveStream(const Http2::Frame &pushPromiseFrame, + const HPack::HttpHeader &requestHeader) +{ + Q_ASSERT(pushPromiseFrame.type() == FrameType::PUSH_PROMISE); + + QMap pseudoHeaders; + for (const auto &field : requestHeader) { + if (field.name == ":scheme" || field.name == ":path" + || field.name == ":authority" || field.name == ":method") { + if (field.value.isEmpty() || pseudoHeaders.contains(field.name)) + return false; + pseudoHeaders[field.name] = field.value; + } + } + + if (pseudoHeaders.size() != 4) { + // All four required, HTTP/2 8.1.2.3. + return false; + } + + const auto method = pseudoHeaders[":method"].toLower(); + if (method != "get" && method != "head") + return false; + + QUrl url; + url.setScheme(QLatin1String(pseudoHeaders[":scheme"])); + url.setAuthority(QLatin1String(pseudoHeaders[":authority"])); + url.setPath(QLatin1String(pseudoHeaders[":path"])); + + if (!url.isValid()) + return false; + + Q_ASSERT(activeStreams.contains(pushPromiseFrame.streamID())); + const Stream &associatedStream = activeStreams[pushPromiseFrame.streamID()]; + + const auto associatedUrl = urlkey_from_request(associatedStream.request()); + if (url.adjusted(QUrl::RemovePath) != associatedUrl.adjusted(QUrl::RemovePath)) + return false; + + const auto urlKey = url.toString(); + if (promisedData.contains(urlKey)) // duplicate push promise + return false; + + const auto reservedID = qFromBigEndian(pushPromiseFrame.dataBegin()); + // By this time all sanity checks on reservedID were done already + // in handlePUSH_PROMISE. We do not repeat them, only those below: + Q_ASSERT(!activeStreams.contains(reservedID)); + Q_ASSERT(!streamWasReset(reservedID)); + + auto &promise = promisedData[urlKey]; + promise.reservedID = reservedID; + promise.pushHeader = requestHeader; + + activeStreams.insert(reservedID, Stream(urlKey, reservedID, streamInitialRecvWindowSize)); + return true; +} + +void QHttp2ProtocolHandler::resetPromisedStream(const Frame &pushPromiseFrame, + Http2::Http2Error reason) +{ + Q_ASSERT(pushPromiseFrame.type() == FrameType::PUSH_PROMISE); + const auto reservedID = qFromBigEndian(pushPromiseFrame.dataBegin()); + sendRST_STREAM(reservedID, reason); + markAsReset(reservedID); +} + +void QHttp2ProtocolHandler::initReplyFromPushPromise(const HttpMessagePair &message, + const QString &cacheKey) +{ + Q_ASSERT(promisedData.contains(cacheKey)); + auto promise = promisedData.take(cacheKey); + + qCDebug(QT_HTTP2) << "found cached/promised response on stream" << promise.reservedID; + + bool replyFinished = false; + Stream *promisedStream = nullptr; + if (activeStreams.contains(promise.reservedID)) { + promisedStream = &activeStreams[promise.reservedID]; + // Ok, we have an active (not closed yet) stream waiting for more frames, + // let's pretend we requested it: + promisedStream->httpPair = message; + } else { + // Let's pretent we're sending a request now: + Stream closedStream(message, promise.reservedID, + streamInitialSendWindowSize, + streamInitialRecvWindowSize); + closedStream.state = Stream::halfClosedLocal; + activeStreams.insert(promise.reservedID, closedStream); + promisedStream = &activeStreams[promise.reservedID]; + replyFinished = true; + } + + Q_ASSERT(promisedStream); + + if (!promise.responseHeader.empty()) + updateStream(*promisedStream, promise.responseHeader, Qt::QueuedConnection); + + for (const auto &frame : promise.dataFrames) + updateStream(*promisedStream, frame, Qt::QueuedConnection); + + if (replyFinished) { + // Good, we already have received ALL the frames of that PUSH_PROMISE, + // nothing more to do. + finishStream(*promisedStream, Qt::QueuedConnection); + deleteActiveStream(promisedStream->streamID); + } +} + void QHttp2ProtocolHandler::connectionError(Http2::Http2Error errorCode, const char *message) { diff --git a/src/network/access/qhttp2protocolhandler_p.h b/src/network/access/qhttp2protocolhandler_p.h index 92c6851078..df0cf6a288 100644 --- a/src/network/access/qhttp2protocolhandler_p.h +++ b/src/network/access/qhttp2protocolhandler_p.h @@ -63,6 +63,7 @@ #include "http2/hpacktable_p.h" #include "http2/hpack_p.h" +#include #include #include #include @@ -123,9 +124,11 @@ private: bool acceptSetting(Http2::Settings identifier, quint32 newValue); - void updateStream(Stream &stream, const HPack::HttpHeader &headers); - void updateStream(Stream &stream, const Http2::Frame &dataFrame); - void finishStream(Stream &stream); + void updateStream(Stream &stream, const HPack::HttpHeader &headers, + Qt::ConnectionType connectionType = Qt::DirectConnection); + void updateStream(Stream &stream, const Http2::Frame &dataFrame, + Qt::ConnectionType connectionType = Qt::DirectConnection); + void finishStream(Stream &stream, Qt::ConnectionType connectionType = Qt::DirectConnection); // Error code send by a peer (GOAWAY/RST_STREAM): void finishStreamWithError(Stream &stream, quint32 errorCode); // Locally encountered error: @@ -194,7 +197,15 @@ private: quint32 allocateStreamID(); bool validPeerStreamID() const; bool goingAway = false; - + bool pushPromiseEnabled = false; + quint32 lastPromisedID = Http2::connectionStreamID; + QHash promisedData; + bool tryReserveStream(const Http2::Frame &pushPromiseFrame, + const HPack::HttpHeader &requestHeader); + void resetPromisedStream(const Http2::Frame &pushPromiseFrame, + Http2::Http2Error reason); + void initReplyFromPushPromise(const HttpMessagePair &message, + const QString &cacheKey); // Errors: void connectionError(Http2::Http2Error errorCode, const char *message); diff --git a/tests/auto/network/access/http2/http2srv.cpp b/tests/auto/network/access/http2/http2srv.cpp index f919937fc3..9d68b5c798 100644 --- a/tests/auto/network/access/http2/http2srv.cpp +++ b/tests/auto/network/access/http2/http2srv.cpp @@ -63,6 +63,16 @@ inline bool is_valid_client_stream(quint32 streamID) return (streamID & 0x1) && streamID <= std::numeric_limits::max(); } +void fill_push_header(const HttpHeader &originalRequest, HttpHeader &promisedRequest) +{ + for (const auto &field : originalRequest) { + if (field.name == QByteArray(":authority") || + field.name == QByteArray(":scheme")) { + promisedRequest.push_back(field); + } + } +} + } Http2Server::Http2Server(bool h2c, const Http2Settings &ss, const Http2Settings &cs) @@ -96,6 +106,12 @@ Http2Server::~Http2Server() { } +void Http2Server::enablePushPromise(bool pushEnabled, const QByteArray &path) +{ + pushPromiseEnabled = pushEnabled; + pushPath = path; +} + void Http2Server::setResponseBody(const QByteArray &body) { responseBody = body; @@ -112,7 +128,6 @@ void Http2Server::startServer() emit serverStarted(serverPort()); } - void Http2Server::sendServerSettings() { Q_ASSERT(socket); @@ -206,7 +221,7 @@ void Http2Server::incomingConnection(qintptr socketDescriptor) if (clearTextHTTP2) { socket.reset(new QTcpSocket); const bool set = socket->setSocketDescriptor(socketDescriptor); - Q_UNUSED(set) Q_ASSERT(set); + Q_ASSERT(set); // Stop listening: close(); QMetaObject::invokeMethod(this, "connectionEstablished", @@ -531,6 +546,48 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody) { Q_ASSERT(activeRequests.find(streamID) != activeRequests.end()); + const quint32 maxFrameSize(clientSetting(Settings::MAX_FRAME_SIZE_ID, + Http2::maxFrameSize)); + + if (pushPromiseEnabled) { + // A real server supporting PUSH_PROMISE will probably first send + // PUSH_PROMISE and then a normal response (to a real request), + // so that a client parsing this response and discovering another + // resource it needs, will _already_ have this additional resource + // in PUSH_PROMISE. + lastPromisedStream += 2; + + writer.start(FrameType::PUSH_PROMISE, FrameFlag::END_HEADERS, streamID); + writer.append(lastPromisedStream); + + HttpHeader pushHeader; + fill_push_header(activeRequests[streamID], pushHeader); + pushHeader.push_back(HeaderField(":method", "GET")); + pushHeader.push_back(HeaderField(":path", pushPath)); + + // Now interesting part, let's make it into 'stream': + activeRequests[lastPromisedStream] = pushHeader; + + HPack::BitOStream ostream(writer.outboundFrame().buffer); + const bool result = encoder.encodeRequest(ostream, pushHeader); + Q_ASSERT(result); + + // Well, it's not HEADERS, it's PUSH_PROMISE with ... HEADERS block. + // Should work. + writer.writeHEADERS(*socket, maxFrameSize); + qDebug() << "server sent a PUSH_PROMISE on" << lastPromisedStream; + + if (responseBody.isEmpty()) + responseBody = QByteArray("I PROMISE (AND PUSH) YOU ..."); + + // Now we send this promised data as a normal response on our reserved + // stream (disabling PUSH_PROMISE for the moment to avoid recursion): + pushPromiseEnabled = false; + sendResponse(lastPromisedStream, false); + pushPromiseEnabled = true; + // Now we'll continue with _normal_ response. + } + writer.start(FrameType::HEADERS, FrameFlag::END_HEADERS, streamID); if (emptyBody) writer.addFlag(FrameFlag::END_STREAM); @@ -544,9 +601,7 @@ void Http2Server::sendResponse(quint32 streamID, bool emptyBody) HPack::BitOStream ostream(writer.outboundFrame().buffer); const bool result = encoder.encodeResponse(ostream, header); Q_ASSERT(result); - Q_UNUSED(result) - const quint32 maxFrameSize(clientSetting(Settings::MAX_FRAME_SIZE_ID, Http2::maxFrameSize)); writer.writeHEADERS(*socket, maxFrameSize); if (!emptyBody) { diff --git a/tests/auto/network/access/http2/http2srv.h b/tests/auto/network/access/http2/http2srv.h index 73b1d80f8e..15a4f212c9 100644 --- a/tests/auto/network/access/http2/http2srv.h +++ b/tests/auto/network/access/http2/http2srv.h @@ -68,6 +68,7 @@ public: ~Http2Server(); // To be called before server started: + void enablePushPromise(bool enabled, const QByteArray &path = QByteArray()); void setResponseBody(const QByteArray &body); // Invokables, since we can call them from the main thread, @@ -157,6 +158,9 @@ private: QByteArray responseBody; bool clearTextHTTP2 = false; + bool pushPromiseEnabled = false; + quint32 lastPromisedStream = 0; + QByteArray pushPath; protected slots: void ignoreErrorSlot(); diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp index 582a103b2e..771ddb01be 100644 --- a/tests/auto/network/access/http2/tst_http2.cpp +++ b/tests/auto/network/access/http2/tst_http2.cpp @@ -44,8 +44,8 @@ #endif // NO_OPENSSL #endif // NO_SSL - #include +#include // At the moment our HTTP/2 imlpementation requires ALPN and this means OpenSSL. #if !defined(QT_NO_OPENSSL) && OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(OPENSSL_NO_TLSEXT) @@ -68,6 +68,7 @@ private slots: void multipleRequests(); void flowControlClientSide(); void flowControlServerSide(); + void pushPromise(); protected slots: // Slots to listen to our in-process server: @@ -90,8 +91,8 @@ private: // small payload. void runEventLoop(int ms = 5000); void stopEventLoop(); - // TODO: different parameters like client/server settings ... - Http2Server *newServer(const Http2Settings &serverSettings); + Http2Server *newServer(const Http2Settings &serverSettings, + const Http2Settings &clientSettings = defaultClientSettings); // Send a get or post request, depending on a payload (empty or not). void sendRequest(int streamNumber, QNetworkRequest::Priority priority = QNetworkRequest::NormalPriority, @@ -105,15 +106,57 @@ private: QTimer timer; int nRequests = 0; + int nSentRequests = 0; int windowUpdates = 0; bool prefaceOK = false; bool serverGotSettingsACK = false; static const Http2Settings defaultServerSettings; + static const Http2Settings defaultClientSettings; }; const Http2Settings tst_Http2::defaultServerSettings{{Http2::Settings::MAX_CONCURRENT_STREAMS_ID, 100}}; +const Http2Settings tst_Http2::defaultClientSettings{{Http2::Settings::MAX_FRAME_SIZE_ID, quint32(Http2::maxFrameSize)}, + {Http2::Settings::ENABLE_PUSH_ID, quint32(0)}}; + +namespace { + +// Our server lives/works on a different thread so we invoke its 'deleteLater' +// instead of simple 'delete'. +struct ServerDeleter +{ + static void cleanup(Http2Server *srv) + { + if (srv) + QMetaObject::invokeMethod(srv, "deleteLater", Qt::QueuedConnection); + } +}; + +using ServerPtr = QScopedPointer; + +struct EnvVarGuard +{ + EnvVarGuard(const char *name, const QByteArray &value) + : varName(name), + prevValue(qgetenv(name)) + { + Q_ASSERT(name); + qputenv(name, value); + } + ~EnvVarGuard() + { + if (prevValue.size()) + qputenv(varName.c_str(), prevValue); + else + qunsetenv(varName.c_str()); + } + + const std::string varName; + const QByteArray prevValue; +}; + +} // unnamed namespace tst_Http2::tst_Http2() : workerThread(new QThread) @@ -146,9 +189,9 @@ void tst_Http2::singleRequest() serverPort = 0; nRequests = 1; - auto srv = newServer(defaultServerSettings); + ServerPtr srv(newServer(defaultServerSettings)); - QMetaObject::invokeMethod(srv, "startServer", Qt::QueuedConnection); + QMetaObject::invokeMethod(srv.data(), "startServer", Qt::QueuedConnection); runEventLoop(); QVERIFY(serverPort != 0); @@ -174,8 +217,6 @@ void tst_Http2::singleRequest() QCOMPARE(reply->error(), QNetworkReply::NoError); QVERIFY(reply->isFinished()); - - QMetaObject::invokeMethod(srv, "deleteLater", Qt::QueuedConnection); } void tst_Http2::multipleRequests() @@ -185,9 +226,9 @@ void tst_Http2::multipleRequests() serverPort = 0; nRequests = 10; - auto srv = newServer(defaultServerSettings); + ServerPtr srv(newServer(defaultServerSettings)); - QMetaObject::invokeMethod(srv, "startServer", Qt::QueuedConnection); + QMetaObject::invokeMethod(srv.data(), "startServer", Qt::QueuedConnection); runEventLoop(); QVERIFY(serverPort != 0); @@ -198,8 +239,6 @@ void tst_Http2::multipleRequests() QNetworkRequest::NormalPriority, QNetworkRequest::LowPriority}; - - for (int i = 0; i < nRequests; ++i) sendRequest(i, priorities[std::rand() % 3]); @@ -208,8 +247,6 @@ void tst_Http2::multipleRequests() QVERIFY(nRequests == 0); QVERIFY(prefaceOK); QVERIFY(serverGotSettingsACK); - - QMetaObject::invokeMethod(srv, "deleteLater", Qt::QueuedConnection); } void tst_Http2::flowControlClientSide() @@ -230,12 +267,12 @@ void tst_Http2::flowControlClientSide() const Http2Settings serverSettings = {{Settings::MAX_CONCURRENT_STREAMS_ID, 3}}; - auto srv = newServer(serverSettings); + ServerPtr srv(newServer(serverSettings)); const QByteArray respond(int(Http2::defaultSessionWindowSize * 50), 'x'); srv->setResponseBody(respond); - QMetaObject::invokeMethod(srv, "startServer", Qt::QueuedConnection); + QMetaObject::invokeMethod(srv.data(), "startServer", Qt::QueuedConnection); runEventLoop(); QVERIFY(serverPort != 0); @@ -249,8 +286,6 @@ void tst_Http2::flowControlClientSide() QVERIFY(prefaceOK); QVERIFY(serverGotSettingsACK); QVERIFY(windowUpdates > 0); - - QMetaObject::invokeMethod(srv, "deleteLater", Qt::QueuedConnection); } void tst_Http2::flowControlServerSide() @@ -270,11 +305,11 @@ void tst_Http2::flowControlServerSide() const Http2Settings serverSettings = {{Settings::MAX_CONCURRENT_STREAMS_ID, 7}}; - auto srv = newServer(serverSettings); + ServerPtr srv(newServer(serverSettings)); const QByteArray payload(int(Http2::defaultSessionWindowSize * 500), 'x'); - QMetaObject::invokeMethod(srv, "startServer", Qt::QueuedConnection); + QMetaObject::invokeMethod(srv.data(), "startServer", Qt::QueuedConnection); runEventLoop(); QVERIFY(serverPort != 0); @@ -287,9 +322,73 @@ void tst_Http2::flowControlServerSide() QVERIFY(nRequests == 0); QVERIFY(prefaceOK); QVERIFY(serverGotSettingsACK); +} - QMetaObject::invokeMethod(srv, "deleteLater", Qt::QueuedConnection); - srv = nullptr; +void tst_Http2::pushPromise() +{ + // We will first send some request, the server should reply and also emulate + // PUSH_PROMISE sending us another response as promised. + using namespace Http2; + + clearHTTP2State(); + + serverPort = 0; + nRequests = 1; + + const EnvVarGuard env("QT_HTTP2_ENABLE_PUSH_PROMISE", "1"); + const Http2Settings clientSettings{{Settings::MAX_FRAME_SIZE_ID, quint32(Http2::maxFrameSize)}, + {Settings::ENABLE_PUSH_ID, quint32(1)}}; + + ServerPtr srv(newServer(defaultServerSettings, clientSettings)); + srv->enablePushPromise(true, QByteArray("/script.js")); + + QMetaObject::invokeMethod(srv.data(), "startServer", Qt::QueuedConnection); + runEventLoop(); + + QVERIFY(serverPort != 0); + + const QString urlAsString((clearTextHTTP2 ? QString("http://127.0.0.1:%1/") + : QString("https://127.0.0.1:%1/")).arg(serverPort)); + const QUrl requestUrl(urlAsString + "index.html"); + + QNetworkRequest request(requestUrl); + request.setAttribute(QNetworkRequest::HTTP2AllowedAttribute, QVariant(true)); + + auto reply = manager.get(request); + connect(reply, &QNetworkReply::finished, this, &tst_Http2::replyFinished); + // Since we're using self-signed certificates, ignore SSL errors: + reply->ignoreSslErrors(); + + runEventLoop(); + + QVERIFY(nRequests == 0); + QVERIFY(prefaceOK); + QVERIFY(serverGotSettingsACK); + + QCOMPARE(reply->error(), QNetworkReply::NoError); + QVERIFY(reply->isFinished()); + + // Now, the most interesting part! + nSentRequests = 0; + nRequests = 1; + // Create an additional request (let's say, we parsed reply and realized we + // need another resource): + + const QUrl promisedUrl(urlAsString + "script.js"); + QNetworkRequest promisedRequest(promisedUrl); + promisedRequest.setAttribute(QNetworkRequest::HTTP2AllowedAttribute, QVariant(true)); + reply = manager.get(promisedRequest); + connect(reply, &QNetworkReply::finished, this, &tst_Http2::replyFinished); + reply->ignoreSslErrors(); + + runEventLoop(); + + // Let's check that NO request was actually made: + QCOMPARE(nSentRequests, 0); + // Decreased by replyFinished(): + QCOMPARE(nRequests, 0); + QCOMPARE(reply->error(), QNetworkReply::NoError); + QVERIFY(reply->isFinished()); } void tst_Http2::serverStarted(quint16 port) @@ -318,12 +417,10 @@ void tst_Http2::stopEventLoop() eventLoop.quit(); } -Http2Server *tst_Http2::newServer(const Http2Settings &serverSettings) +Http2Server *tst_Http2::newServer(const Http2Settings &serverSettings, + const Http2Settings &clientSettings) { using namespace Http2; - // Client's settings are fixed by qhttp2protocolhandler. - const Http2Settings clientSettings = {{Settings::MAX_FRAME_SIZE_ID, quint32(Http2::maxFrameSize)}, - {Settings::ENABLE_PUSH_ID, quint32(0)}}; auto srv = new Http2Server(clearTextHTTP2, serverSettings, clientSettings); using Srv = Http2Server; @@ -397,6 +494,7 @@ void tst_Http2::decompressionFailed(quint32 streamID) void tst_Http2::receivedRequest(quint32 streamID) { + ++nSentRequests; qDebug() << " server got a request on stream" << streamID; Http2Server *srv = qobject_cast(sender()); Q_ASSERT(srv);