From 2eeee81b67ce4fc81793c51bafd2cc1b33076c05 Mon Sep 17 00:00:00 2001
From: Aleksey Lysenko <lysenkoalexmail@gmail.com>
Date: Tue, 3 Jan 2017 22:59:36 +0200
Subject: [PATCH] Fixed possible block clearing in
QWebSocketDataProcessor::process method
In QWebSocketDataProcessor::process() the signals text(binary)MessageReceived
are emitted before clear() method. If signal handler blocks loop
(for example, using QDialog::exec()), clear() will be called only after
resuming loop. It may lead to the data corruption due to the fact that
QWebSocketDataProcessor clearing won't be performed before the new data
arrived.
Task-number: QTBUG-55506
Change-Id: Ib7016a91d3987dec7c1af977b17f86a53568c413
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
---
src/websockets/qwebsocketdataprocessor.cpp | 14 +++--
.../dataprocessor/tst_dataprocessor.cpp | 54 ++++++++++++++++---
2 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/src/websockets/qwebsocketdataprocessor.cpp b/src/websockets/qwebsocketdataprocessor.cpp
index 520ecdc1..d9fc5503 100644
--- a/src/websockets/qwebsocketdataprocessor.cpp
+++ b/src/websockets/qwebsocketdataprocessor.cpp
@@ -182,12 +182,16 @@ void QWebSocketDataProcessor::process(QIODevice *pIoDevice)
}
if (frame.isFinalFrame()) {
- if (m_opCode == QWebSocketProtocol::OpCodeText)
- Q_EMIT textMessageReceived(m_textMessage);
- else
- Q_EMIT binaryMessageReceived(m_binaryMessage);
- clear();
isDone = true;
+ if (m_opCode == QWebSocketProtocol::OpCodeText) {
+ const QString textMessage(m_textMessage);
+ clear();
+ Q_EMIT textMessageReceived(textMessage);
+ } else {
+ const QByteArray binaryMessage(m_binaryMessage);
+ clear();
+ Q_EMIT binaryMessageReceived(binaryMessage);
+ }
}
}
} else {
diff --git a/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp b/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
index 55c27afb..5f6cfba2 100644
--- a/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
+++ b/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
@@ -173,6 +173,8 @@ private Q_SLOTS:
void minimumSizeRequirement();
void minimumSizeRequirement_data();
+ void clearDataBuffers(); // qtbug-55506
+
private:
//helper function that constructs a new row of test data for invalid UTF8 sequences
void invalidUTF8(const char *dataTag, const char *utf8Sequence, bool isCloseFrame);
@@ -243,7 +245,7 @@ void tst_DataProcessor::goodBinaryFrame()
QWebSocketDataProcessor dataProcessor;
QFETCH(QByteArray, payload);
- data.append((char)(FIN | QWebSocketProtocol::OpCodeBinary));
+ data.append(char(FIN | QWebSocketProtocol::OpCodeBinary));
if (payload.length() < 126)
{
@@ -337,7 +339,7 @@ void tst_DataProcessor::goodTextFrame()
QFETCH(QByteArray, payload);
QFETCH(int, size);
- data.append((char)(FIN | QWebSocketProtocol::OpCodeText));
+ data.append(char(FIN | QWebSocketProtocol::OpCodeText));
if (payload.length() < 126)
{
@@ -407,7 +409,7 @@ void tst_DataProcessor::goodControlFrame()
QSignalSpy pingReceivedSpy(&dataProcessor, SIGNAL(pingReceived(QByteArray)));
QSignalSpy pongReceivedSpy(&dataProcessor, SIGNAL(pongReceived(QByteArray)));
- data.append((char)(FIN | QWebSocketProtocol::OpCodePing));
+ data.append(char(FIN | QWebSocketProtocol::OpCodePing));
data.append(QChar::fromLatin1(0));
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
@@ -425,7 +427,7 @@ void tst_DataProcessor::goodControlFrame()
data.clear();
pingReceivedSpy.clear();
pongReceivedSpy.clear();
- data.append((char)(FIN | QWebSocketProtocol::OpCodePong));
+ data.append(char(FIN | QWebSocketProtocol::OpCodePong));
data.append(QChar::fromLatin1(0));
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
@@ -537,7 +539,7 @@ void tst_DataProcessor::goodOpcodes()
QWebSocketDataProcessor dataProcessor;
QFETCH(QWebSocketProtocol::OpCode, opCode);
- data.append((char)(FIN | opCode));
+ data.append(char(FIN | opCode));
data.append(char(0)); //zero length
buffer.setData(data);
@@ -584,7 +586,7 @@ void tst_DataProcessor::goodCloseFrame()
quint16 swapped = qToBigEndian<quint16>(closeCode);
const char *wireRepresentation = static_cast<const char *>(static_cast<const void *>(&swapped));
- data.append((char)(FIN | QWebSocketProtocol::OpCodeClose));
+ data.append(char(FIN | QWebSocketProtocol::OpCodeClose));
if (swapped != 0)
{
data.append(char(payload.length() + 2)).append(wireRepresentation, 2).append(payload);
@@ -800,7 +802,7 @@ void tst_DataProcessor::frameTooSmall()
{
//text frame with final bit not set
- data.append((char)(QWebSocketProtocol::OpCodeText)).append(char(0x0));
+ data.append(char(QWebSocketProtocol::OpCodeText)).append(char(0x0));
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
@@ -842,7 +844,7 @@ void tst_DataProcessor::frameTooSmall()
data.clear();
//text frame with final bit not set
- data.append((char)(QWebSocketProtocol::OpCodeText)).append(char(0x0));
+ data.append(char(QWebSocketProtocol::OpCodeText)).append(char(0x0));
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
dataProcessor.process(&buffer);
@@ -1832,6 +1834,42 @@ void tst_DataProcessor::insertIncompleteSizeFieldTest(quint8 payloadCode, quint8
<< QWebSocketProtocol::CloseCodeGoingAway;
}
+void tst_DataProcessor::clearDataBuffers()
+{
+ const QByteArray binaryData("Hello!");
+ QByteArray data;
+ data.append(char(FIN | QWebSocketProtocol::OpCodeBinary));
+ data.append(char(binaryData.length()));
+ data.append(binaryData);
+
+ QWebSocketDataProcessor dataProcessor;
+ connect(&dataProcessor, &QWebSocketDataProcessor::binaryMessageReceived,
+ [&binaryData](const QByteArray &message)
+ {
+ QCOMPARE(message, binaryData);
+ QEventLoop loop;
+ QTimer::singleShot(2000, &loop, SLOT(quit()));
+ loop.exec();
+ });
+
+ QBuffer buffer;
+ buffer.setData(data);
+ auto processData = [&dataProcessor, &buffer]()
+ {
+ buffer.open(QIODevice::ReadOnly);
+ dataProcessor.process(&buffer);
+ buffer.close();
+ };
+
+ QTimer timer;
+ timer.setSingleShot(true);
+ connect(&timer, &QTimer::timeout, processData);
+
+ timer.start(1000);
+ processData();
+ QTest::qWait(2000);
+}
+
QTEST_MAIN(tst_DataProcessor)
#include "tst_dataprocessor.moc"
--
GitLab