summaryrefslogtreecommitdiffstats
path: root/src/network/access/http2
diff options
context:
space:
mode:
authorTimur Pocheptsov <timur.pocheptsov@theqtcompany.com>2016-08-09 19:22:48 +0200
committerTimur Pocheptsov <timur.pocheptsov@theqtcompany.com>2016-08-11 12:45:59 +0000
commit193abdfc0798cf10eac06769b650ac03e86eb55e (patch)
tree92ef3e1e58ea886a953bbb873fc7a9f23b3c9010 /src/network/access/http2
parent615616b0699d98cfb9f4eeb67e005e3226398097 (diff)
http2frame - do not rely on a socket's buffer
- Whenever we have a read notification, read this data, even if it's not enough for a frame's header - otherwise, QNAM can use a socket in an Ubuffered mode and FrameReader can potentially fail to read anything correctly. - Do not call/rely on bytesAvailable and do not try to invoke _q_receiveReply in Qt::QueuedConnection mode, instead try to read until we end up in an incomplete frame or some error. Change-Id: I7f44ba9e34bc64f3e26bd29080f0050da635b3ae Reviewed-by: Alex Trotsenko <alex1973tr@gmail.com> Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Diffstat (limited to 'src/network/access/http2')
-rw-r--r--src/network/access/http2/http2frames.cpp83
-rw-r--r--src/network/access/http2/http2frames_p.h11
2 files changed, 64 insertions, 30 deletions
diff --git a/src/network/access/http2/http2frames.cpp b/src/network/access/http2/http2frames.cpp
index 55e9f93b19..95f00dd286 100644
--- a/src/network/access/http2/http2frames.cpp
+++ b/src/network/access/http2/http2frames.cpp
@@ -148,7 +148,7 @@ FrameStatus validate_frame_payload(FrameType type, FrameFlags flags,
FrameReader::FrameReader(FrameReader &&rhs)
- : framePayload(std::move(rhs.framePayload))
+ : frameBuffer(std::move(rhs.frameBuffer))
{
type = rhs.type;
rhs.type = FrameType::LAST_FRAME_TYPE;
@@ -162,8 +162,8 @@ FrameReader::FrameReader(FrameReader &&rhs)
payloadSize = rhs.payloadSize;
rhs.payloadSize = 0;
- incompleteRead = rhs.incompleteRead;
- rhs.incompleteRead = false;
+ state = rhs.state;
+ rhs.state = Idle;
offset = rhs.offset;
rhs.offset = 0;
@@ -171,7 +171,7 @@ FrameReader::FrameReader(FrameReader &&rhs)
FrameReader &FrameReader::operator = (FrameReader &&rhs)
{
- framePayload = std::move(rhs.framePayload);
+ frameBuffer = std::move(rhs.frameBuffer);
type = rhs.type;
rhs.type = FrameType::LAST_FRAME_TYPE;
@@ -185,8 +185,8 @@ FrameReader &FrameReader::operator = (FrameReader &&rhs)
payloadSize = rhs.payloadSize;
rhs.payloadSize = 0;
- incompleteRead = rhs.incompleteRead;
- rhs.incompleteRead = false;
+ state = rhs.state;
+ rhs.state = Idle;
offset = rhs.offset;
rhs.offset = 0;
@@ -196,10 +196,13 @@ FrameReader &FrameReader::operator = (FrameReader &&rhs)
FrameStatus FrameReader::read(QAbstractSocket &socket)
{
- if (!incompleteRead) {
+ if (state != ReadingPayload) {
+ // Either Idle or ReadingHeader:
if (!readHeader(socket))
return FrameStatus::incompleteFrame;
+ Q_ASSERT(state == Idle);
+
const auto status = validate_frame_header(type, flags, payloadSize);
if (status != FrameStatus::goodFrame) {
// No need to read any payload.
@@ -209,16 +212,17 @@ FrameStatus FrameReader::read(QAbstractSocket &socket)
if (Http2PredefinedParameters::maxFrameSize < payloadSize)
return FrameStatus::sizeError;
- framePayload.resize(payloadSize);
+ frameBuffer.resize(payloadSize);
offset = 0;
}
- if (framePayload.size()) {
+ if (frameBuffer.size()) {
if (!readPayload(socket))
return FrameStatus::incompleteFrame;
+ Q_ASSERT(state == Idle);
}
- return validate_frame_payload(type, flags, framePayload);
+ return validate_frame_payload(type, flags, frameBuffer);
}
bool FrameReader::padded(uchar *pad) const
@@ -231,8 +235,8 @@ bool FrameReader::padded(uchar *pad) const
if (type == FrameType::DATA
|| type == FrameType::PUSH_PROMISE
|| type == FrameType::HEADERS) {
- Q_ASSERT(framePayload.size() >= 1);
- *pad = framePayload[0];
+ Q_ASSERT(frameBuffer.size() >= 1);
+ *pad = frameBuffer[0];
return true;
}
@@ -244,10 +248,10 @@ bool FrameReader::priority(quint32 *streamID, uchar *weight) const
Q_ASSERT(streamID);
Q_ASSERT(weight);
- if (!framePayload.size())
+ if (!frameBuffer.size())
return false;
- const uchar *src = &framePayload[0];
+ const uchar *src = &frameBuffer[0];
if (type == FrameType::HEADERS && flags.testFlag(FrameFlag::PADDED))
++src;
@@ -263,7 +267,7 @@ bool FrameReader::priority(quint32 *streamID, uchar *weight) const
quint32 FrameReader::dataSize() const
{
- quint32 size = quint32(framePayload.size());
+ quint32 size = quint32(frameBuffer.size());
uchar pad = 0;
if (padded(&pad)) {
// + 1 one for a byte with padding number itself:
@@ -280,10 +284,10 @@ quint32 FrameReader::dataSize() const
const uchar *FrameReader::dataBegin() const
{
- if (!framePayload.size())
+ if (!frameBuffer.size())
return nullptr;
- const uchar *src = &framePayload[0];
+ const uchar *src = &frameBuffer[0];
uchar dummyPad = 0;
if (padded(&dummyPad))
++src;
@@ -298,14 +302,30 @@ const uchar *FrameReader::dataBegin() const
bool FrameReader::readHeader(QAbstractSocket &socket)
{
- if (socket.bytesAvailable() < frameHeaderSize)
+ 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<char *>(&frameBuffer[offset]),
+ frameHeaderSize - offset);
+ if (chunkSize > 0)
+ offset += chunkSize;
+
+ if (offset < frameHeaderSize)
return false;
- uchar src[frameHeaderSize] = {};
- socket.read(reinterpret_cast<char*>(src), frameHeaderSize);
+ // We got a complete frame header:
+ state = Idle;
- payloadSize = src[0] << 16 | src[1] << 8 | src[2];
+ 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
@@ -318,16 +338,23 @@ bool FrameReader::readHeader(QAbstractSocket &socket)
bool FrameReader::readPayload(QAbstractSocket &socket)
{
- Q_ASSERT(offset <= framePayload.size());
+ Q_ASSERT(offset < frameBuffer.size());
+ Q_ASSERT(state != ReadingHeader);
+
+ state = ReadingPayload;
// Casts and ugliness - to deal with MSVC. Values are guaranteed to fit into quint32.
- if (const auto residue = std::min(qint64(framePayload.size() - offset), socket.bytesAvailable())) {
- socket.read(reinterpret_cast<char *>(&framePayload[offset]), residue);
- offset += quint32(residue);
- }
+ const auto residue = qint64(frameBuffer.size() - offset);
+ const auto chunkSize = socket.read(reinterpret_cast<char *>(&frameBuffer[offset]), residue);
+ if (chunkSize > 0)
+ offset += quint32(chunkSize);
- incompleteRead = offset < framePayload.size();
- return !incompleteRead;
+ if (offset < frameBuffer.size())
+ return false;
+
+ // Complete payload read:
+ state = Idle;
+ return true;
}
diff --git a/src/network/access/http2/http2frames_p.h b/src/network/access/http2/http2frames_p.h
index 6abed315ca..c85be57a2e 100644
--- a/src/network/access/http2/http2frames_p.h
+++ b/src/network/access/http2/http2frames_p.h
@@ -101,13 +101,20 @@ 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.
- bool incompleteRead = false;
quint32 offset = 0;
- std::vector<uchar> framePayload;
+ std::vector<uchar> frameBuffer;
};
class Q_AUTOTEST_EXPORT FrameWriter