whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
bb10
Timur Pocheptsov 2016-08-09 19:22:48 +02:00
parent 615616b069
commit 193abdfc07
3 changed files with 115 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
src/network/access/http2/http2frames.cpp
View File

@ -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;
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;
}

11
src/network/access/http2/http2frames_p.h
View File

@ -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

106
src/network/access/qhttp2protocolhandler.cpp
View File

@ -170,64 +170,60 @@ void QHttp2ProtocolHandler::_q_receiveReply()
Q_ASSERT(m_socket);
Q_ASSERT(m_channel);
const auto result = inboundFrame.read(*m_socket);
switch (result) {
case FrameStatus::incompleteFrame:
return;
case FrameStatus::protocolError:
return connectionError(PROTOCOL_ERROR, "invalid frame");
case FrameStatus::sizeError:
return connectionError(FRAME_SIZE_ERROR, "invalid frame size");
default:
break;
}
do {
const auto result = inboundFrame.read(*m_socket);
switch (result) {
case FrameStatus::incompleteFrame:
return;
case FrameStatus::protocolError:
return connectionError(PROTOCOL_ERROR, "invalid frame");
case FrameStatus::sizeError:
return connectionError(FRAME_SIZE_ERROR, "invalid frame size");
default:
break;
}
Q_ASSERT(result == FrameStatus::goodFrame);
Q_ASSERT(result == FrameStatus::goodFrame);
if (continuationExpected && inboundFrame.type != FrameType::CONTINUATION)
return connectionError(PROTOCOL_ERROR, "CONTINUATION expected");
if (continuationExpected && inboundFrame.type != FrameType::CONTINUATION)
return connectionError(PROTOCOL_ERROR, "CONTINUATION expected");
switch (inboundFrame.type) {
case FrameType::DATA:
handleDATA();
break;
case FrameType::HEADERS:
handleHEADERS();
break;
case FrameType::PRIORITY:
handlePRIORITY();
break;
case FrameType::RST_STREAM:
handleRST_STREAM();
break;
case FrameType::SETTINGS:
handleSETTINGS();
break;
case FrameType::PUSH_PROMISE:
handlePUSH_PROMISE();
break;
case FrameType::PING:
handlePING();
break;
case FrameType::GOAWAY:
handleGOAWAY();
break;
case FrameType::WINDOW_UPDATE:
handleWINDOW_UPDATE();
break;
case FrameType::CONTINUATION:
handleCONTINUATION();
break;
case FrameType::LAST_FRAME_TYPE:
// 5.1 - ignore unknown frames.
break;
}
if (goingAway && !activeStreams.size())
return;
if (m_socket->bytesAvailable())
QMetaObject::invokeMethod(m_channel, "_q_receiveReply", Qt::QueuedConnection);
switch (inboundFrame.type) {
case FrameType::DATA:
handleDATA();
break;
case FrameType::HEADERS:
handleHEADERS();
break;
case FrameType::PRIORITY:
handlePRIORITY();
break;
case FrameType::RST_STREAM:
handleRST_STREAM();
break;
case FrameType::SETTINGS:
handleSETTINGS();
break;
case FrameType::PUSH_PROMISE:
handlePUSH_PROMISE();
break;
case FrameType::PING:
handlePING();
break;
case FrameType::GOAWAY:
handleGOAWAY();
break;
case FrameType::WINDOW_UPDATE:
handleWINDOW_UPDATE();
break;
case FrameType::CONTINUATION:
handleCONTINUATION();
break;
case FrameType::LAST_FRAME_TYPE:
// 5.1 - ignore unknown frames.
break;
}
} while (!goingAway || activeStreams.size());
}
bool QHttp2ProtocolHandler::sendRequest()