From 4c27221295cbe61bb84adefaa58b1f27fc58a6bf Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 16 Aug 2016 16:23:54 +0200 Subject: Refactor Http2::FrameReader/Http2::FrameWriter Introduce new entity: class Http2::Frame with accessors like payloadSize/type/flags/streamID etc. (they actually read /interpret raw bytes from a frame's buffer) instead of duplicating this functionality in reader/writer classes. Delete defaulted members and remove explicitly defined move ctors/operators (not needed actually). Update auto-test ('HTTP/2 server') to use these new classes. Change-Id: Ie3516efbd095704e212142eef9e792323678ccfa Reviewed-by: Edward Welbourne --- src/network/access/http2/http2frames.cpp | 460 ++++++++++++--------------- src/network/access/http2/http2frames_p.h | 108 +++---- src/network/access/http2/http2streams.cpp | 4 + src/network/access/http2/http2streams_p.h | 3 +- src/network/access/qhttp2protocolhandler.cpp | 161 +++++----- src/network/access/qhttp2protocolhandler_p.h | 9 +- 6 files changed, 343 insertions(+), 402 deletions(-) (limited to 'src') diff --git a/src/network/access/http2/http2frames.cpp b/src/network/access/http2/http2frames.cpp index 763d1bb90f..978bee09b1 100644 --- a/src/network/access/http2/http2frames.cpp +++ b/src/network/access/http2/http2frames.cpp @@ -51,46 +51,125 @@ namespace Http2 // HTTP/2 frames are defined by RFC7540, clauses 4 and 6. -FrameStatus validate_frame_header(FrameType type, FrameFlags flags, quint32 payloadSize) +Frame::Frame() + : buffer(frameHeaderSize) { +} + +FrameType Frame::type() const +{ + Q_ASSERT(buffer.size() >= frameHeaderSize); + + if (int(buffer[3]) >= int(FrameType::LAST_FRAME_TYPE)) + return FrameType::LAST_FRAME_TYPE; + + return FrameType(buffer[3]); +} + +quint32 Frame::streamID() const +{ + Q_ASSERT(buffer.size() >= frameHeaderSize); + return qFromBigEndian(&buffer[5]); +} + +FrameFlags Frame::flags() const +{ + Q_ASSERT(buffer.size() >= frameHeaderSize); + return FrameFlags(buffer[4]); +} + +quint32 Frame::payloadSize() const +{ + Q_ASSERT(buffer.size() >= frameHeaderSize); + return buffer[0] << 16 | buffer[1] << 8 | buffer[2]; +} + +uchar Frame::padding() const +{ + Q_ASSERT(validateHeader() == FrameStatus::goodFrame); + + if (!flags().testFlag(FrameFlag::PADDED)) + return 0; + + switch (type()) { + case FrameType::DATA: + case FrameType::PUSH_PROMISE: + case FrameType::HEADERS: + Q_ASSERT(buffer.size() > frameHeaderSize); + return buffer[frameHeaderSize]; + default: + return 0; + } +} + +bool Frame::priority(quint32 *streamID, uchar *weight) const +{ + Q_ASSERT(validatePayload() == FrameStatus::goodFrame); + + if (buffer.size() <= frameHeaderSize) + return false; + + const uchar *src = &buffer[0] + frameHeaderSize; + if (type() == FrameType::HEADERS && flags().testFlag(FrameFlag::PADDED)) + ++src; + + if ((type() == FrameType::HEADERS && flags().testFlag(FrameFlag::PRIORITY)) + || type() == FrameType::PRIORITY) { + if (streamID) + *streamID = qFromBigEndian(src); + if (weight) + *weight = src[4]; + return true; + } + + return false; +} + +FrameStatus Frame::validateHeader() const +{ + // Should be called only on a frame with + // a complete header. + Q_ASSERT(buffer.size() >= frameHeaderSize); + + const auto framePayloadSize = payloadSize(); // 4.2 Frame Size - if (payloadSize > maxPayloadSize) + if (framePayloadSize > maxPayloadSize) return FrameStatus::sizeError; - switch (type) { + switch (type()) { case FrameType::SETTINGS: // SETTINGS ACK can not have any payload. // The payload of a SETTINGS frame consists of zero // or more parameters, each consisting of an unsigned // 16-bit setting identifier and an unsigned 32-bit value. // Thus the payload size must be a multiple of 6. - if (flags.testFlag(FrameFlag::ACK) ? payloadSize : payloadSize % 6) + if (flags().testFlag(FrameFlag::ACK) ? framePayloadSize : framePayloadSize % 6) return FrameStatus::sizeError; break; case FrameType::PRIORITY: // 6.3 PRIORITY - if (payloadSize != 5) + if (framePayloadSize != 5) return FrameStatus::sizeError; break; case FrameType::PING: // 6.7 PING - if (payloadSize != 8) + if (framePayloadSize != 8) return FrameStatus::sizeError; break; case FrameType::GOAWAY: // 6.8 GOAWAY - if (payloadSize < 8) + if (framePayloadSize < 8) return FrameStatus::sizeError; break; case FrameType::RST_STREAM: case FrameType::WINDOW_UPDATE: // 6.4 RST_STREAM, 6.9 WINDOW_UPDATE - if (payloadSize != 4) + if (framePayloadSize != 4) return FrameStatus::sizeError; break; case FrameType::PUSH_PROMISE: // 6.6 PUSH_PROMISE - if (payloadSize < 4) + if (framePayloadSize < 4) return FrameStatus::sizeError; default: // DATA/HEADERS/CONTINUATION will be verified @@ -102,31 +181,37 @@ FrameStatus validate_frame_header(FrameType type, FrameFlags flags, quint32 payl return FrameStatus::goodFrame; } -FrameStatus validate_frame_payload(FrameType type, FrameFlags flags, - quint32 size, const uchar *src) +FrameStatus Frame::validatePayload() const { - Q_ASSERT(!size || src); + // Should be called only on a complete frame with a valid header. + Q_ASSERT(validateHeader() == FrameStatus::goodFrame); // Ignored, 5.1 - if (type == FrameType::LAST_FRAME_TYPE) + if (type() == FrameType::LAST_FRAME_TYPE) return FrameStatus::goodFrame; + auto size = payloadSize(); + Q_ASSERT(buffer.size() >= frameHeaderSize && size == buffer.size() - frameHeaderSize); + + const uchar *src = size ? &buffer[0] + frameHeaderSize : nullptr; + const auto frameFlags = flags(); + switch (type()) { // 6.1 DATA, 6.2 HEADERS - if (type == FrameType::DATA || type == FrameType::HEADERS) { - if (flags.testFlag(FrameFlag::PADDED)) { + case FrameType::DATA: + case FrameType::HEADERS: + if (frameFlags.testFlag(FrameFlag::PADDED)) { if (!size || size < src[0]) return FrameStatus::sizeError; size -= src[0]; } - if (type == FrameType::HEADERS && flags.testFlag(FrameFlag::PRIORITY)) { + if (type() == FrameType::HEADERS && frameFlags.testFlag(FrameFlag::PRIORITY)) { if (size < 5) return FrameStatus::sizeError; } - } - + break; // 6.6 PUSH_PROMISE - if (type == FrameType::PUSH_PROMISE) { - if (flags.testFlag(FrameFlag::PADDED)) { + case FrameType::PUSH_PROMISE: + if (frameFlags.testFlag(FrameFlag::PADDED)) { if (!size || size < src[0]) return FrameStatus::sizeError; size -= src[0]; @@ -134,311 +219,158 @@ FrameStatus validate_frame_payload(FrameType type, FrameFlags flags, if (size < 4) return FrameStatus::sizeError; + break; + default: + break; } return FrameStatus::goodFrame; } -FrameStatus validate_frame_payload(FrameType type, FrameFlags flags, - const std::vector &payload) -{ - const uchar *src = payload.size() ? &payload[0] : nullptr; - return validate_frame_payload(type, flags, quint32(payload.size()), src); -} - -FrameReader::FrameReader(FrameReader &&rhs) - : frameBuffer(std::move(rhs.frameBuffer)) +quint32 Frame::dataSize() const { - type = rhs.type; - rhs.type = FrameType::LAST_FRAME_TYPE; - - flags = rhs.flags; - rhs.flags = FrameFlag::EMPTY; + Q_ASSERT(validatePayload() == FrameStatus::goodFrame); - streamID = rhs.streamID; - rhs.streamID = 0; - - payloadSize = rhs.payloadSize; - rhs.payloadSize = 0; + quint32 size = payloadSize(); + if (const uchar pad = padding()) { + // + 1 one for a byte with padding number itself: + size -= pad + 1; + } - state = rhs.state; - rhs.state = Idle; + if (priority()) + size -= 5; - offset = rhs.offset; - rhs.offset = 0; + return size; } -FrameReader &FrameReader::operator = (FrameReader &&rhs) +const uchar *Frame::dataBegin() const { - frameBuffer = std::move(rhs.frameBuffer); - - type = rhs.type; - rhs.type = FrameType::LAST_FRAME_TYPE; - - flags = rhs.flags; - rhs.flags = FrameFlag::EMPTY; - - streamID = rhs.streamID; - rhs.streamID = 0; - - payloadSize = rhs.payloadSize; - rhs.payloadSize = 0; + Q_ASSERT(validatePayload() == FrameStatus::goodFrame); + if (buffer.size() <= frameHeaderSize) + return nullptr; - state = rhs.state; - rhs.state = Idle; + const uchar *src = &buffer[0] + frameHeaderSize; + if (padding()) + ++src; - offset = rhs.offset; - rhs.offset = 0; + if (priority()) + src += 5; - return *this; + return src; } FrameStatus FrameReader::read(QAbstractSocket &socket) { - if (state != ReadingPayload) { - // Either Idle or ReadingHeader: + if (offset < frameHeaderSize) { if (!readHeader(socket)) return FrameStatus::incompleteFrame; - Q_ASSERT(state == Idle); - - const auto status = validate_frame_header(type, flags, payloadSize); + const auto status = frame.validateHeader(); if (status != FrameStatus::goodFrame) { // No need to read any payload. return status; } - if (Http2PredefinedParameters::maxFrameSize < payloadSize) + if (Http2PredefinedParameters::maxFrameSize < frame.payloadSize()) return FrameStatus::sizeError; - frameBuffer.resize(payloadSize); - offset = 0; - } - - if (frameBuffer.size()) { - if (!readPayload(socket)) - return FrameStatus::incompleteFrame; - Q_ASSERT(state == Idle); - } - - return validate_frame_payload(type, flags, frameBuffer); -} - -bool FrameReader::padded(uchar *pad) const -{ - Q_ASSERT(pad); - - if (!flags.testFlag(FrameFlag::PADDED)) - return false; - - if (type == FrameType::DATA - || type == FrameType::PUSH_PROMISE - || type == FrameType::HEADERS) { - Q_ASSERT(frameBuffer.size() >= 1); - *pad = frameBuffer[0]; - return true; - } - - return false; -} - -bool FrameReader::priority(quint32 *streamID, uchar *weight) const -{ - Q_ASSERT(streamID); - Q_ASSERT(weight); - - if (!frameBuffer.size()) - return false; - - const uchar *src = &frameBuffer[0]; - if (type == FrameType::HEADERS && flags.testFlag(FrameFlag::PADDED)) - ++src; - - if ((type == FrameType::HEADERS && flags.testFlag(FrameFlag::PRIORITY)) - || type == FrameType::PRIORITY) { - *streamID = qFromBigEndian(src); - *weight = src[4]; - return true; - } - - return false; -} - -quint32 FrameReader::dataSize() const -{ - quint32 size = quint32(frameBuffer.size()); - uchar pad = 0; - if (padded(&pad)) { - // + 1 one for a byte with padding number itself: - size -= pad + 1; + frame.buffer.resize(frame.payloadSize() + frameHeaderSize); } - quint32 dummyID = 0; - uchar dummyW = 0; - if (priority(&dummyID, &dummyW)) - size -= 5; - - return size; -} + if (offset < frame.buffer.size() && !readPayload(socket)) + return FrameStatus::incompleteFrame; -const uchar *FrameReader::dataBegin() const -{ - if (!frameBuffer.size()) - return nullptr; + // Reset the offset, our frame can be re-used + // now (re-read): + offset = 0; - const uchar *src = &frameBuffer[0]; - uchar dummyPad = 0; - if (padded(&dummyPad)) - ++src; - - quint32 dummyID = 0; - uchar dummyW = 0; - if (priority(&dummyID, &dummyW)) - src += 5; - - return src; + return frame.validatePayload(); } bool FrameReader::readHeader(QAbstractSocket &socket) { - Q_ASSERT(state != ReadingPayload); - - if (state == Idle) { - offset = 0; - frameBuffer.resize(frameHeaderSize); - state = ReadingHeader; - } - Q_ASSERT(offset < frameHeaderSize); - const auto chunkSize = socket.read(reinterpret_cast(&frameBuffer[offset]), + auto &buffer = frame.buffer; + if (buffer.size() < frameHeaderSize) + buffer.resize(frameHeaderSize); + + const auto chunkSize = socket.read(reinterpret_cast(&buffer[offset]), frameHeaderSize - offset); if (chunkSize > 0) offset += chunkSize; - if (offset < frameHeaderSize) - return false; - - // We got a complete frame header: - state = Idle; - - const uchar *src = &frameBuffer[0]; - - payloadSize = src[0] << 16 | src[1] << 8 | src[2]; - type = FrameType(src[3]); - if (int(type) >= int(FrameType::LAST_FRAME_TYPE)) - type = FrameType::LAST_FRAME_TYPE; // To be ignored, 5.1 - - flags = FrameFlags(src[4]); - streamID = qFromBigEndian(src + 5); - - return true; + return offset == frameHeaderSize; } bool FrameReader::readPayload(QAbstractSocket &socket) { - Q_ASSERT(offset < frameBuffer.size()); - Q_ASSERT(state != ReadingHeader); - - state = ReadingPayload; + Q_ASSERT(offset < frame.buffer.size()); + Q_ASSERT(frame.buffer.size() > frameHeaderSize); + auto &buffer = frame.buffer; // Casts and ugliness - to deal with MSVC. Values are guaranteed to fit into quint32. - const auto residue = qint64(frameBuffer.size() - offset); - const auto chunkSize = socket.read(reinterpret_cast(&frameBuffer[offset]), residue); + const auto chunkSize = socket.read(reinterpret_cast(&buffer[offset]), + qint64(buffer.size() - offset)); if (chunkSize > 0) offset += quint32(chunkSize); - if (offset < frameBuffer.size()) - return false; - - // Complete payload read: - state = Idle; - return true; + return offset == buffer.size(); } - FrameWriter::FrameWriter() { - frameBuffer.reserve(Http2PredefinedParameters::maxFrameSize + - Http2PredefinedParameters::frameHeaderSize); } FrameWriter::FrameWriter(FrameType type, FrameFlags flags, quint32 streamID) { - frameBuffer.reserve(Http2PredefinedParameters::maxFrameSize + - Http2PredefinedParameters::frameHeaderSize); start(type, flags, streamID); } void FrameWriter::start(FrameType type, FrameFlags flags, quint32 streamID) { - frameBuffer.resize(frameHeaderSize); + auto &buffer = frame.buffer; + + buffer.resize(frameHeaderSize); // The first three bytes - payload size, which is 0 for now. - frameBuffer[0] = 0; - frameBuffer[1] = 0; - frameBuffer[2] = 0; + buffer[0] = 0; + buffer[1] = 0; + buffer[2] = 0; - frameBuffer[3] = uchar(type); - frameBuffer[4] = uchar(flags); + buffer[3] = uchar(type); + buffer[4] = uchar(flags); - qToBigEndian(streamID, &frameBuffer[5]); + qToBigEndian(streamID, &buffer[5]); } void FrameWriter::setPayloadSize(quint32 size) { - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - Q_ASSERT(size < maxPayloadSize); + auto &buffer = frame.buffer; - frameBuffer[0] = size >> 16; - frameBuffer[1] = size >> 8; - frameBuffer[2] = size; -} + Q_ASSERT(buffer.size() >= frameHeaderSize); + Q_ASSERT(size < maxPayloadSize); -quint32 FrameWriter::payloadSize() const -{ - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - return frameBuffer[0] << 16 | frameBuffer[1] << 8 | frameBuffer[2]; + buffer[0] = size >> 16; + buffer[1] = size >> 8; + buffer[2] = size; } void FrameWriter::setType(FrameType type) { - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - frameBuffer[3] = uchar(type); -} - -FrameType FrameWriter::type() const -{ - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - return FrameType(frameBuffer[3]); + Q_ASSERT(frame.buffer.size() >= frameHeaderSize); + frame.buffer[3] = uchar(type); } void FrameWriter::setFlags(FrameFlags flags) { - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - frameBuffer[4] = uchar(flags); + Q_ASSERT(frame.buffer.size() >= frameHeaderSize); + frame.buffer[4] = uchar(flags); } void FrameWriter::addFlag(FrameFlag flag) { - setFlags(flags() | flag); -} - -FrameFlags FrameWriter::flags() const -{ - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); - return FrameFlags(frameBuffer[4]); -} - -quint32 FrameWriter::streamID() const -{ - return qFromBigEndian(&frameBuffer[5]); -} - -void FrameWriter::append(uchar val) -{ - frameBuffer.push_back(val); - updatePayloadSize(); + setFlags(frame.flags() | flag); } void FrameWriter::append(const uchar *begin, const uchar *end) @@ -446,65 +378,71 @@ void FrameWriter::append(const uchar *begin, const uchar *end) Q_ASSERT(begin && end); Q_ASSERT(begin < end); - frameBuffer.insert(frameBuffer.end(), begin, end); + frame.buffer.insert(frame.buffer.end(), begin, end); updatePayloadSize(); } void FrameWriter::updatePayloadSize() { - // First, compute size: - const quint32 payloadSize = quint32(frameBuffer.size() - frameHeaderSize); - Q_ASSERT(payloadSize <= maxPayloadSize); - setPayloadSize(payloadSize); + const quint32 size = quint32(frame.buffer.size() - frameHeaderSize); + Q_ASSERT(size <= maxPayloadSize); + setPayloadSize(size); } bool FrameWriter::write(QAbstractSocket &socket) const { - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); + auto &buffer = frame.buffer; + Q_ASSERT(buffer.size() >= frameHeaderSize); // Do some sanity check first: - Q_ASSERT(int(type()) < int(FrameType::LAST_FRAME_TYPE)); - Q_ASSERT(validate_frame_header(type(), flags(), payloadSize()) == FrameStatus::goodFrame); - const auto nWritten = socket.write(reinterpret_cast(&frameBuffer[0]), - frameBuffer.size()); - return nWritten != -1 && size_type(nWritten) == frameBuffer.size(); + Q_ASSERT(int(frame.type()) < int(FrameType::LAST_FRAME_TYPE)); + Q_ASSERT(frame.validateHeader() == FrameStatus::goodFrame); + + const auto nWritten = socket.write(reinterpret_cast(&buffer[0]), + buffer.size()); + return nWritten != -1 && size_type(nWritten) == buffer.size(); } bool FrameWriter::writeHEADERS(QAbstractSocket &socket, quint32 sizeLimit) { - Q_ASSERT(frameBuffer.size() >= frameHeaderSize); + auto &buffer = frame.buffer; + Q_ASSERT(buffer.size() >= frameHeaderSize); if (sizeLimit > quint32(maxPayloadSize)) sizeLimit = quint32(maxPayloadSize); - if (quint32(frameBuffer.size() - frameHeaderSize) <= sizeLimit) { + if (quint32(buffer.size() - frameHeaderSize) <= sizeLimit) { addFlag(FrameFlag::END_HEADERS); updatePayloadSize(); return write(socket); } + // Our HPACK block does not fit into the size limit, remove + // END_HEADERS bit from the first frame, we'll later set + // it on the last CONTINUATION frame: + setFlags(frame.flags() & ~FrameFlags(FrameFlag::END_HEADERS)); // Write a frame's header (not controlled by sizeLimit) and // as many bytes of payload as we can within sizeLimit, // then send CONTINUATION frames, as needed. setPayloadSize(sizeLimit); const quint32 firstChunkSize = frameHeaderSize + sizeLimit; - qint64 written = socket.write(reinterpret_cast(&frameBuffer[0]), + qint64 written = socket.write(reinterpret_cast(&buffer[0]), firstChunkSize); if (written != qint64(firstChunkSize)) return false; - FrameWriter continuationFrame(FrameType::CONTINUATION, FrameFlag::EMPTY, streamID()); + FrameWriter continuationWriter(FrameType::CONTINUATION, FrameFlag::EMPTY, frame.streamID()); quint32 offset = firstChunkSize; - while (offset != frameBuffer.size()) { - const auto chunkSize = std::min(sizeLimit, quint32(frameBuffer.size() - offset)); - if (chunkSize + offset == frameBuffer.size()) - continuationFrame.addFlag(FrameFlag::END_HEADERS); - continuationFrame.setPayloadSize(chunkSize); - if (!continuationFrame.write(socket)) + while (offset != buffer.size()) { + const auto chunkSize = std::min(sizeLimit, quint32(buffer.size() - offset)); + if (chunkSize + offset == buffer.size()) + continuationWriter.addFlag(FrameFlag::END_HEADERS); + continuationWriter.setPayloadSize(chunkSize); + if (!continuationWriter.write(socket)) return false; - written = socket.write(reinterpret_cast(&frameBuffer[offset]), + written = socket.write(reinterpret_cast(&buffer[offset]), chunkSize); if (written != qint64(chunkSize)) return false; @@ -552,6 +490,6 @@ bool FrameWriter::writeDATA(QAbstractSocket &socket, quint32 sizeLimit, return true; } -} +} // Namespace Http2 QT_END_NAMESPACE diff --git a/src/network/access/http2/http2frames_p.h b/src/network/access/http2/http2frames_p.h index c85be57a2e..84ba9c3662 100644 --- a/src/network/access/http2/http2frames_p.h +++ b/src/network/access/http2/http2frames_p.h @@ -68,59 +68,53 @@ class QAbstractSocket; namespace Http2 { -class Q_AUTOTEST_EXPORT FrameReader +struct Q_AUTOTEST_EXPORT Frame { - friend class QT_PREPEND_NAMESPACE(QHttp2ProtocolHandler); - -public: - FrameReader() = default; - - FrameReader(const FrameReader &) = default; - FrameReader(FrameReader &&rhs); - - FrameReader &operator = (const FrameReader &) = default; - FrameReader &operator = (FrameReader &&rhs); - - FrameStatus read(QAbstractSocket &socket); + Frame(); + // 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: + bool priority(quint32 *streamID = nullptr, + uchar *weight = nullptr) const; - bool padded(uchar *pad) const; - bool priority(quint32 *streamID, uchar *weight) const; + FrameStatus validateHeader() const; + FrameStatus validatePayload() const; - // N of 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. const uchar *dataBegin() const; - FrameType type = FrameType::LAST_FRAME_TYPE; - FrameFlags flags = FrameFlag::EMPTY; - quint32 streamID = 0; - quint32 payloadSize = 0; + std::vector buffer; +}; + +class Q_AUTOTEST_EXPORT FrameReader +{ +public: + FrameStatus read(QAbstractSocket &socket); + Frame &inboundFrame() + { + return frame; + } private: bool readHeader(QAbstractSocket &socket); bool readPayload(QAbstractSocket &socket); - enum ReaderState { - Idle, - ReadingHeader, - ReadingPayload - }; - - ReaderState state = Idle; - - // As soon as we got a header, we - // know payload size, offset is - // needed if we do not have enough - // data and will read the next chunk. quint32 offset = 0; - std::vector frameBuffer; + Frame frame; }; class Q_AUTOTEST_EXPORT FrameWriter { - friend class QT_PREPEND_NAMESPACE(QHttp2ProtocolHandler); - public: using payload_type = std::vector; using size_type = payload_type::size_type; @@ -128,19 +122,17 @@ public: FrameWriter(); FrameWriter(FrameType type, FrameFlags flags, quint32 streamID); - void start(FrameType type, FrameFlags flags, quint32 streamID); + Frame &outboundFrame() + { + return frame; + } + // Frame 'builders': + void start(FrameType type, FrameFlags flags, quint32 streamID); void setPayloadSize(quint32 size); - quint32 payloadSize() const; - void setType(FrameType type); - FrameType type() const; - void setFlags(FrameFlags flags); void addFlag(FrameFlag flag); - FrameFlags flags() const; - - quint32 streamID() const; // All append functions also update frame's payload // length. @@ -151,7 +143,11 @@ public: qToBigEndian(val, wired); append(wired, wired + sizeof val); } - void append(uchar val); + void append(uchar val) + { + frame.buffer.push_back(val); + updatePayloadSize(); + } void append(Settings identifier) { append(quint16(identifier)); @@ -163,25 +159,23 @@ public: void append(const uchar *begin, const uchar *end); - // Write 'frameBuffer' as a single frame: + // Write as a single frame: bool write(QAbstractSocket &socket) const; - // Write as a single frame if we can, or write headers and - // CONTINUATION(s) frame(s). + // 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); - // Write either a single DATA frame or several DATA frames - // depending on 'sizeLimit'. Data itself is 'external' to - // FrameWriter, since it's a 'readPointer' from QNonContiguousData. + // 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); - - std::vector &rawFrameBuffer() - { - return frameBuffer; - } - private: void updatePayloadSize(); - std::vector frameBuffer; + Frame frame; }; } diff --git a/src/network/access/http2/http2streams.cpp b/src/network/access/http2/http2streams.cpp index f57f8d8367..660100f5e4 100644 --- a/src/network/access/http2/http2streams.cpp +++ b/src/network/access/http2/http2streams.cpp @@ -49,6 +49,10 @@ QT_BEGIN_NAMESPACE namespace Http2 { +Stream::Stream() +{ +} + Stream::Stream(const HttpMessagePair &message, quint32 id, qint32 sendSize, qint32 recvSize) : httpPair(message), streamID(id), diff --git a/src/network/access/http2/http2streams_p.h b/src/network/access/http2/http2streams_p.h index 5d9a6ab512..8a825a5457 100644 --- a/src/network/access/http2/http2streams_p.h +++ b/src/network/access/http2/http2streams_p.h @@ -73,11 +73,10 @@ struct Q_AUTOTEST_EXPORT Stream closed }; - Stream() = default; + Stream(); Stream(const HttpMessagePair &message, quint32 streamID, qint32 sendSize, qint32 recvSize); - // TODO: check includes!!! QHttpNetworkReply *reply() const; const QHttpNetworkRequest &request() const; QHttpNetworkRequest &request(); diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp index a8de698741..68a00c6837 100644 --- a/src/network/access/qhttp2protocolhandler.cpp +++ b/src/network/access/qhttp2protocolhandler.cpp @@ -171,7 +171,7 @@ void QHttp2ProtocolHandler::_q_receiveReply() Q_ASSERT(m_channel); do { - const auto result = inboundFrame.read(*m_socket); + const auto result = frameReader.read(*m_socket); switch (result) { case FrameStatus::incompleteFrame: return; @@ -185,10 +185,13 @@ void QHttp2ProtocolHandler::_q_receiveReply() Q_ASSERT(result == FrameStatus::goodFrame); - if (continuationExpected && inboundFrame.type != FrameType::CONTINUATION) + inboundFrame = std::move(frameReader.inboundFrame()); + + const auto frameType = inboundFrame.type(); + if (continuationExpected && frameType != FrameType::CONTINUATION) return connectionError(PROTOCOL_ERROR, "CONTINUATION expected"); - switch (inboundFrame.type) { + switch (frameType) { case FrameType::DATA: handleDATA(); break; @@ -289,14 +292,14 @@ bool QHttp2ProtocolHandler::sendClientPreface() return false; // 6.5 SETTINGS - outboundFrame.start(FrameType::SETTINGS, FrameFlag::EMPTY, Http2::connectionStreamID); + frameWriter.start(FrameType::SETTINGS, FrameFlag::EMPTY, Http2::connectionStreamID); // MAX frame size (16 kb), disable PUSH - outboundFrame.append(Settings::MAX_FRAME_SIZE_ID); - outboundFrame.append(quint32(Http2::maxFrameSize)); - outboundFrame.append(Settings::ENABLE_PUSH_ID); - outboundFrame.append(quint32(0)); + frameWriter.append(Settings::MAX_FRAME_SIZE_ID); + frameWriter.append(quint32(Http2::maxFrameSize)); + frameWriter.append(Settings::ENABLE_PUSH_ID); + frameWriter.append(quint32(0)); - if (!outboundFrame.write(*m_socket)) + if (!frameWriter.write(*m_socket)) return false; sessionRecvWindowSize = sessionMaxRecvWindowSize; @@ -319,38 +322,38 @@ bool QHttp2ProtocolHandler::sendSETTINGS_ACK() if (!prefaceSent && !sendClientPreface()) return false; - outboundFrame.start(FrameType::SETTINGS, FrameFlag::ACK, Http2::connectionStreamID); + frameWriter.start(FrameType::SETTINGS, FrameFlag::ACK, Http2::connectionStreamID); - return outboundFrame.write(*m_socket); + return frameWriter.write(*m_socket); } bool QHttp2ProtocolHandler::sendHEADERS(Stream &stream) { using namespace HPack; - outboundFrame.start(FrameType::HEADERS, FrameFlag::PRIORITY, - stream.streamID); + frameWriter.start(FrameType::HEADERS, FrameFlag::PRIORITY | FrameFlag::END_HEADERS, + stream.streamID); if (!stream.data()) { - outboundFrame.addFlag(FrameFlag::END_STREAM); + frameWriter.addFlag(FrameFlag::END_STREAM); stream.state = Stream::halfClosedLocal; } else { stream.state = Stream::open; } - outboundFrame.append(quint32()); // No stream dependency in Qt. - outboundFrame.append(stream.weight()); + frameWriter.append(quint32()); // No stream dependency in Qt. + frameWriter.append(stream.weight()); const auto headers = build_headers(stream.request(), maxHeaderListSize); if (!headers.size()) // nothing fits into maxHeaderListSize return false; // Compress in-place: - BitOStream outputStream(outboundFrame.frameBuffer); + BitOStream outputStream(frameWriter.outboundFrame().buffer); if (!encoder.encodeRequest(outputStream, headers)) return false; - return outboundFrame.writeHEADERS(*m_socket, maxFrameSize); + return frameWriter.writeHEADERS(*m_socket, maxFrameSize); } bool QHttp2ProtocolHandler::sendDATA(Stream &stream) @@ -380,10 +383,10 @@ bool QHttp2ProtocolHandler::sendDATA(Stream &stream) return true; } - outboundFrame.start(FrameType::DATA, FrameFlag::EMPTY, stream.streamID); + frameWriter.start(FrameType::DATA, FrameFlag::EMPTY, stream.streamID); const qint32 bytesWritten = std::min(slot, chunkSize); - if (!outboundFrame.writeDATA(*m_socket, maxFrameSize, src, bytesWritten)) + if (!frameWriter.writeDATA(*m_socket, maxFrameSize, src, bytesWritten)) return false; stream.data()->advanceReadPointer(bytesWritten); @@ -396,9 +399,9 @@ bool QHttp2ProtocolHandler::sendDATA(Stream &stream) } if (replyPrivate->totallyUploadedData == request.contentLength()) { - outboundFrame.start(FrameType::DATA, FrameFlag::END_STREAM, stream.streamID); - outboundFrame.setPayloadSize(0); - outboundFrame.write(*m_socket); + frameWriter.start(FrameType::DATA, FrameFlag::END_STREAM, stream.streamID); + frameWriter.setPayloadSize(0); + frameWriter.write(*m_socket); stream.state = Stream::halfClosedLocal; stream.data()->disconnect(this); removeFromSuspended(stream.streamID); @@ -413,61 +416,61 @@ bool QHttp2ProtocolHandler::sendWINDOW_UPDATE(quint32 streamID, quint32 delta) { Q_ASSERT(m_socket); - outboundFrame.start(FrameType::WINDOW_UPDATE, FrameFlag::EMPTY, streamID); - outboundFrame.append(delta); - return outboundFrame.write(*m_socket); + frameWriter.start(FrameType::WINDOW_UPDATE, FrameFlag::EMPTY, streamID); + frameWriter.append(delta); + return frameWriter.write(*m_socket); } bool QHttp2ProtocolHandler::sendRST_STREAM(quint32 streamID, quint32 errorCode) { Q_ASSERT(m_socket); - outboundFrame.start(FrameType::RST_STREAM, FrameFlag::EMPTY, streamID); - outboundFrame.append(errorCode); - return outboundFrame.write(*m_socket); + frameWriter.start(FrameType::RST_STREAM, FrameFlag::EMPTY, streamID); + frameWriter.append(errorCode); + return frameWriter.write(*m_socket); } bool QHttp2ProtocolHandler::sendGOAWAY(quint32 errorCode) { Q_ASSERT(m_socket); - outboundFrame.start(FrameType::GOAWAY, FrameFlag::EMPTY, connectionStreamID); - outboundFrame.append(quint32(connectionStreamID)); - outboundFrame.append(errorCode); - return outboundFrame.write(*m_socket); + frameWriter.start(FrameType::GOAWAY, FrameFlag::EMPTY, connectionStreamID); + frameWriter.append(quint32(connectionStreamID)); + frameWriter.append(errorCode); + return frameWriter.write(*m_socket); } void QHttp2ProtocolHandler::handleDATA() { - Q_ASSERT(inboundFrame.type == FrameType::DATA); + Q_ASSERT(inboundFrame.type() == FrameType::DATA); - const auto streamID = inboundFrame.streamID; + const auto streamID = inboundFrame.streamID(); if (streamID == connectionStreamID) return connectionError(PROTOCOL_ERROR, "DATA on stream 0x0"); if (!activeStreams.contains(streamID) && !streamWasReset(streamID)) return connectionError(ENHANCE_YOUR_CALM, "DATA on invalid stream"); - if (qint32(inboundFrame.payloadSize) > sessionRecvWindowSize) + if (qint32(inboundFrame.payloadSize()) > sessionRecvWindowSize) return connectionError(FLOW_CONTROL_ERROR, "Flow control error"); - sessionRecvWindowSize -= inboundFrame.payloadSize; + sessionRecvWindowSize -= inboundFrame.payloadSize(); if (activeStreams.contains(streamID)) { auto &stream = activeStreams[streamID]; - if (qint32(inboundFrame.payloadSize) > stream.recvWindow) { + if (qint32(inboundFrame.payloadSize()) > stream.recvWindow) { finishStreamWithError(stream, QNetworkReply::ProtocolInvalidOperationError, QLatin1String("flow control error")); sendRST_STREAM(streamID, FLOW_CONTROL_ERROR); markAsReset(streamID); deleteActiveStream(streamID); } else { - stream.recvWindow -= inboundFrame.payloadSize; + stream.recvWindow -= inboundFrame.payloadSize(); // Uncompress data if needed and append it ... updateStream(stream, inboundFrame); - if (inboundFrame.flags.testFlag(FrameFlag::END_STREAM)) { + if (inboundFrame.flags().testFlag(FrameFlag::END_STREAM)) { finishStream(stream); deleteActiveStream(stream.streamID); } else if (stream.recvWindow < streamInitialRecvWindowSize / 2) { @@ -489,22 +492,23 @@ void QHttp2ProtocolHandler::handleDATA() void QHttp2ProtocolHandler::handleHEADERS() { - Q_ASSERT(inboundFrame.type == FrameType::HEADERS); + Q_ASSERT(inboundFrame.type() == FrameType::HEADERS); - const auto streamID = inboundFrame.streamID; + const auto streamID = inboundFrame.streamID(); if (streamID == connectionStreamID) return connectionError(PROTOCOL_ERROR, "HEADERS on 0x0 stream"); if (!activeStreams.contains(streamID) && !streamWasReset(streamID)) return connectionError(ENHANCE_YOUR_CALM, "HEADERS on invalid stream"); - if (inboundFrame.flags.testFlag(FrameFlag::PRIORITY)) { + const auto flags = inboundFrame.flags(); + if (flags.testFlag(FrameFlag::PRIORITY)) { handlePRIORITY(); if (goingAway) return; } - const bool endHeaders = inboundFrame.flags.testFlag(FrameFlag::END_HEADERS); + const bool endHeaders = flags.testFlag(FrameFlag::END_HEADERS); continuedFrames.clear(); continuedFrames.push_back(std::move(inboundFrame)); if (!endHeaders) { @@ -517,10 +521,10 @@ void QHttp2ProtocolHandler::handleHEADERS() void QHttp2ProtocolHandler::handlePRIORITY() { - Q_ASSERT(inboundFrame.type == FrameType::PRIORITY || - inboundFrame.type == FrameType::HEADERS); + Q_ASSERT(inboundFrame.type() == FrameType::PRIORITY || + inboundFrame.type() == FrameType::HEADERS); - const auto streamID = inboundFrame.streamID; + const auto streamID = inboundFrame.streamID(); if (streamID == connectionStreamID) return connectionError(PROTOCOL_ERROR, "PIRORITY on 0x0 stream"); @@ -544,37 +548,38 @@ void QHttp2ProtocolHandler::handlePRIORITY() void QHttp2ProtocolHandler::handleRST_STREAM() { - Q_ASSERT(inboundFrame.type == FrameType::RST_STREAM); + Q_ASSERT(inboundFrame.type() == FrameType::RST_STREAM); // "RST_STREAM frames MUST be associated with a stream. // If a RST_STREAM frame is received with a stream identifier of 0x0, // the recipient MUST treat this as a connection error (Section 5.4.1) // of type PROTOCOL_ERROR. - if (inboundFrame.streamID == connectionStreamID) + const auto streamID = inboundFrame.streamID(); + if (streamID == connectionStreamID) return connectionError(PROTOCOL_ERROR, "RST_STREAM on 0x0"); - if (!(inboundFrame.streamID & 0x1)) { + if (!(streamID & 0x1)) { // RST_STREAM on a promised stream: // since we do not keep track of such streams, // just ignore. return; } - if (inboundFrame.streamID >= nextID) { + if (streamID >= nextID) { // "RST_STREAM frames MUST NOT be sent for a stream // in the "idle" state. .. the recipient MUST treat this // as a connection error (Section 5.4.1) of type PROTOCOL_ERROR." return connectionError(PROTOCOL_ERROR, "RST_STREAM on idle stream"); } - if (!activeStreams.contains(inboundFrame.streamID)) { + if (!activeStreams.contains(streamID)) { // 'closed' stream, ignore. return; } Q_ASSERT(inboundFrame.dataSize() == 4); - Stream &stream = activeStreams[inboundFrame.streamID]; + Stream &stream = activeStreams[streamID]; finishStreamWithError(stream, qFromBigEndian(inboundFrame.dataBegin())); markAsReset(stream.streamID); deleteActiveStream(stream.streamID); @@ -583,12 +588,12 @@ void QHttp2ProtocolHandler::handleRST_STREAM() void QHttp2ProtocolHandler::handleSETTINGS() { // 6.5 SETTINGS. - Q_ASSERT(inboundFrame.type == FrameType::SETTINGS); + Q_ASSERT(inboundFrame.type() == FrameType::SETTINGS); - if (inboundFrame.streamID != connectionStreamID) + if (inboundFrame.streamID() != connectionStreamID) return connectionError(PROTOCOL_ERROR, "SETTINGS on invalid stream"); - if (inboundFrame.flags.testFlag(FrameFlag::ACK)) { + if (inboundFrame.flags().testFlag(FrameFlag::ACK)) { if (!waitingForSettingsACK) return connectionError(PROTOCOL_ERROR, "unexpected SETTINGS ACK"); waitingForSettingsACK = false; @@ -614,7 +619,7 @@ void QHttp2ProtocolHandler::handleSETTINGS() void QHttp2ProtocolHandler::handlePUSH_PROMISE() { // 6.6 PUSH_PROMISE. - Q_ASSERT(inboundFrame.type == FrameType::PUSH_PROMISE); + Q_ASSERT(inboundFrame.type() == FrameType::PUSH_PROMISE); if (prefaceSent && !waitingForSettingsACK) { // This means, server ACKed our 'NO PUSH', @@ -622,7 +627,7 @@ void QHttp2ProtocolHandler::handlePUSH_PROMISE() return connectionError(PROTOCOL_ERROR, "unexpected PUSH_PROMISE frame"); } - const auto streamID = inboundFrame.streamID; + const auto streamID = inboundFrame.streamID(); if (streamID == connectionStreamID) { return connectionError(PROTOCOL_ERROR, "PUSH_PROMISE with invalid associated stream (0x0)"); @@ -644,7 +649,7 @@ void QHttp2ProtocolHandler::handlePUSH_PROMISE() sendRST_STREAM(reservedID, REFUSE_STREAM); markAsReset(reservedID); - const bool endHeaders = inboundFrame.flags.testFlag(FrameFlag::END_HEADERS); + const bool endHeaders = inboundFrame.flags().testFlag(FrameFlag::END_HEADERS); continuedFrames.clear(); continuedFrames.push_back(std::move(inboundFrame)); @@ -660,30 +665,30 @@ void QHttp2ProtocolHandler::handlePING() { // Since we're implementing a client and not // a server, we only reply to a PING, ACKing it. - Q_ASSERT(inboundFrame.type == FrameType::PING); + Q_ASSERT(inboundFrame.type() == FrameType::PING); Q_ASSERT(m_socket); - if (inboundFrame.streamID != connectionStreamID) + if (inboundFrame.streamID() != connectionStreamID) return connectionError(PROTOCOL_ERROR, "PING on invalid stream"); - if (inboundFrame.flags & FrameFlag::ACK) + if (inboundFrame.flags() & FrameFlag::ACK) return connectionError(PROTOCOL_ERROR, "unexpected PING ACK"); Q_ASSERT(inboundFrame.dataSize() == 8); - outboundFrame.start(FrameType::PING, FrameFlag::ACK, connectionStreamID); - outboundFrame.append(inboundFrame.dataBegin(), inboundFrame.dataBegin() + 8); - outboundFrame.write(*m_socket); + frameWriter.start(FrameType::PING, FrameFlag::ACK, connectionStreamID); + frameWriter.append(inboundFrame.dataBegin(), inboundFrame.dataBegin() + 8); + frameWriter.write(*m_socket); } void QHttp2ProtocolHandler::handleGOAWAY() { // 6.8 GOAWAY - Q_ASSERT(inboundFrame.type == FrameType::GOAWAY); + Q_ASSERT(inboundFrame.type() == FrameType::GOAWAY); // "An endpoint MUST treat a GOAWAY frame with a stream identifier // other than 0x0 as a connection error (Section 5.4.1) of type PROTOCOL_ERROR." - if (inboundFrame.streamID != connectionStreamID) + if (inboundFrame.streamID() != connectionStreamID) return connectionError(PROTOCOL_ERROR, "GOAWAY on invalid stream"); const auto src = inboundFrame.dataBegin(); @@ -736,12 +741,12 @@ void QHttp2ProtocolHandler::handleGOAWAY() void QHttp2ProtocolHandler::handleWINDOW_UPDATE() { - Q_ASSERT(inboundFrame.type == FrameType::WINDOW_UPDATE); + Q_ASSERT(inboundFrame.type() == FrameType::WINDOW_UPDATE); const quint32 delta = qFromBigEndian(inboundFrame.dataBegin()); const bool valid = delta && delta <= quint32(std::numeric_limits::max()); - const auto streamID = inboundFrame.streamID; + const auto streamID = inboundFrame.streamID(); if (streamID == Http2::connectionStreamID) { if (!valid || sum_will_overflow(sessionSendWindowSize, delta)) @@ -772,13 +777,13 @@ void QHttp2ProtocolHandler::handleWINDOW_UPDATE() void QHttp2ProtocolHandler::handleCONTINUATION() { - Q_ASSERT(inboundFrame.type == FrameType::CONTINUATION); + Q_ASSERT(inboundFrame.type() == FrameType::CONTINUATION); Q_ASSERT(continuedFrames.size()); // HEADERS frame must be already in. - if (inboundFrame.streamID != continuedFrames.front().streamID) + if (inboundFrame.streamID() != continuedFrames.front().streamID()) return connectionError(PROTOCOL_ERROR, "CONTINUATION on invalid stream"); - const bool endHeaders = inboundFrame.flags.testFlag(FrameFlag::END_HEADERS); + const bool endHeaders = inboundFrame.flags().testFlag(FrameFlag::END_HEADERS); continuedFrames.push_back(std::move(inboundFrame)); if (!endHeaders) @@ -792,9 +797,9 @@ void QHttp2ProtocolHandler::handleContinuedHEADERS() { Q_ASSERT(continuedFrames.size()); - const auto streamID = continuedFrames[0].streamID; + const auto streamID = continuedFrames[0].streamID(); - if (continuedFrames[0].type == FrameType::HEADERS) { + if (continuedFrames[0].type() == FrameType::HEADERS) { if (activeStreams.contains(streamID)) { Stream &stream = activeStreams[streamID]; if (stream.state != Stream::halfClosedLocal) { @@ -837,11 +842,11 @@ void QHttp2ProtocolHandler::handleContinuedHEADERS() if (!decoder.decodeHeaderFields(inputStream)) return connectionError(COMPRESSION_ERROR, "HPACK decompression failed"); - if (continuedFrames[0].type == FrameType::HEADERS) { + if (continuedFrames[0].type() == FrameType::HEADERS) { if (activeStreams.contains(streamID)) { Stream &stream = activeStreams[streamID]; updateStream(stream, decoder.decodedHeader()); - if (continuedFrames[0].flags & FrameFlag::END_STREAM) { + if (continuedFrames[0].flags() & FrameFlag::END_STREAM) { finishStream(stream); deleteActiveStream(stream.streamID); } @@ -949,9 +954,9 @@ void QHttp2ProtocolHandler::updateStream(Stream &stream, const HPack::HttpHeader emit httpReply->headerChanged(); } -void QHttp2ProtocolHandler::updateStream(Stream &stream, const Http2::FrameReader &frame) +void QHttp2ProtocolHandler::updateStream(Stream &stream, const Frame &frame) { - Q_ASSERT(frame.type == FrameType::DATA); + Q_ASSERT(frame.type() == FrameType::DATA); if (const auto length = frame.dataSize()) { const char *data = reinterpret_cast(frame.dataBegin()); diff --git a/src/network/access/qhttp2protocolhandler_p.h b/src/network/access/qhttp2protocolhandler_p.h index 6804c329b9..92c6851078 100644 --- a/src/network/access/qhttp2protocolhandler_p.h +++ b/src/network/access/qhttp2protocolhandler_p.h @@ -124,7 +124,7 @@ private: bool acceptSetting(Http2::Settings identifier, quint32 newValue); void updateStream(Stream &stream, const HPack::HttpHeader &headers); - void updateStream(Stream &stream, const Http2::FrameReader &dataFrame); + void updateStream(Stream &stream, const Http2::Frame &dataFrame); void finishStream(Stream &stream); // Error code send by a peer (GOAWAY/RST_STREAM): void finishStreamWithError(Stream &stream, quint32 errorCode); @@ -161,12 +161,13 @@ private: // Peer's max frame size. quint32 maxFrameSize = Http2::maxFrameSize; - Http2::FrameReader inboundFrame; - Http2::FrameWriter outboundFrame; + Http2::FrameReader frameReader; + Http2::Frame inboundFrame; + Http2::FrameWriter frameWriter; // Temporary storage to assemble HEADERS' block // from several CONTINUATION frames ... bool continuationExpected = false; - std::vector continuedFrames; + std::vector continuedFrames; // Peer's max number of streams ... quint32 maxConcurrentStreams = Http2::maxConcurrentStreams; -- cgit v1.2.3