summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Chu <ryan.chu@qt.io>2019-06-28 14:37:05 +0200
committerRyan Chu <ryan.chu@qt.io>2019-08-23 15:28:46 +0200
commitb14f5f59a3ae96949e6a33302385a751d6448182 (patch)
tree5594f3816864eaf6757bcadeecebb84a67b8c2e7
parent24894c032719157a2d738f03e0c70d3ff0cf1782 (diff)
downloadqtwebsockets-b14f5f59a3ae96949e6a33302385a751d6448182.tar.gz
Remove waitForReadyRead from QWebSocketFrame::readFrame
Asynchronously process socket frame in QWebSocketDataProcessor::process. If the processing of QWebSocketFrame is not done and waiting for more data, QWebSocketDataProcessor::process will return the control and wait for next readyRead signal to continue processing the unfinished socket frame. QWebSocketDataProcessor::process gets timeout after 5 seconds, and then an errorEncountered signal will be emitted. Fixes: QTBUG-74464 Change-Id: I03b7f874c1c266617e7eadf59c59ae43fa8540ce Reviewed-by: Edward Welbourne <edward.welbourne@qt.io> Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
-rw-r--r--src/websockets/qwebsocket_p.cpp4
-rw-r--r--src/websockets/qwebsocketdataprocessor.cpp22
-rw-r--r--src/websockets/qwebsocketdataprocessor_p.h3
-rw-r--r--src/websockets/qwebsocketframe.cpp62
-rw-r--r--src/websockets/qwebsocketframe_p.h1
-rw-r--r--tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp16
-rw-r--r--tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp36
7 files changed, 103 insertions, 41 deletions
diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp
index 6965731..9b94d04 100644
--- a/src/websockets/qwebsocket_p.cpp
+++ b/src/websockets/qwebsocket_p.cpp
@@ -1173,10 +1173,10 @@ void QWebSocketPrivate::processData()
{
if (!m_pSocket) // disconnected with data still in-bound
return;
- while (m_pSocket->bytesAvailable()) {
+ if (m_pSocket->bytesAvailable()) {
if (state() == QAbstractSocket::ConnectingState) {
if (!m_pSocket->canReadLine())
- break;
+ return;
processHandshake(m_pSocket);
} else {
m_dataProcessor.process(m_pSocket);
diff --git a/src/websockets/qwebsocketdataprocessor.cpp b/src/websockets/qwebsocketdataprocessor.cpp
index ee28969..39287d4 100644
--- a/src/websockets/qwebsocketdataprocessor.cpp
+++ b/src/websockets/qwebsocketdataprocessor.cpp
@@ -87,6 +87,10 @@ QWebSocketDataProcessor::QWebSocketDataProcessor(QObject *parent) :
m_pTextCodec(QTextCodec::codecForName("UTF-8"))
{
clear();
+ // initialize the internal timeout timer
+ waitTimer.setInterval(5000);
+ waitTimer.setSingleShot(true);
+ waitTimer.callOnTimeout(this, &QWebSocketDataProcessor::timeout);
}
/*!
@@ -126,7 +130,13 @@ void QWebSocketDataProcessor::process(QIODevice *pIoDevice)
while (!isDone) {
frame.readFrame(pIoDevice);
- if (Q_LIKELY(frame.isValid())) {
+ if (!frame.isDone()) {
+ // waiting for more data available
+ QObject::connect(pIoDevice, &QIODevice::readyRead,
+ &waitTimer, &QTimer::stop, Qt::UniqueConnection);
+ waitTimer.start();
+ return;
+ } else if (Q_LIKELY(frame.isValid())) {
if (frame.isControlFrame()) {
isDone = processControlFrame(frame);
} else {
@@ -302,4 +312,14 @@ bool QWebSocketDataProcessor::processControlFrame(const QWebSocketFrame &frame)
return mustStopProcessing;
}
+/*!
+ \internal
+ */
+void QWebSocketDataProcessor::timeout()
+{
+ clear();
+ Q_EMIT errorEncountered(QWebSocketProtocol::CloseCodeGoingAway,
+ tr("Timeout when reading data from socket."));
+}
+
QT_END_NAMESPACE
diff --git a/src/websockets/qwebsocketdataprocessor_p.h b/src/websockets/qwebsocketdataprocessor_p.h
index 1d8024e..41226d6 100644
--- a/src/websockets/qwebsocketdataprocessor_p.h
+++ b/src/websockets/qwebsocketdataprocessor_p.h
@@ -55,6 +55,7 @@
#include <QtCore/QByteArray>
#include <QtCore/QString>
#include <QtCore/QTextCodec>
+#include <QTimer>
#include "qwebsocketframe_p.h"
#include "qwebsocketprotocol.h"
#include "qwebsocketprotocol_p.h"
@@ -113,8 +114,10 @@ private:
QTextCodec::ConverterState *m_pConverterState;
QTextCodec *m_pTextCodec;
QWebSocketFrame frame;
+ QTimer waitTimer;
bool processControlFrame(const QWebSocketFrame &frame);
+ void timeout();
};
QT_END_NAMESPACE
diff --git a/src/websockets/qwebsocketframe.cpp b/src/websockets/qwebsocketframe.cpp
index 5b10e02..11373a7 100644
--- a/src/websockets/qwebsocketframe.cpp
+++ b/src/websockets/qwebsocketframe.cpp
@@ -188,7 +188,7 @@ void QWebSocketFrame::swap(QWebSocketFrame &other)
*/
QWebSocketProtocol::CloseCode QWebSocketFrame::closeCode() const
{
- return m_closeCode;
+ return isDone() ? m_closeCode : QWebSocketProtocol::CloseCodeGoingAway;
}
/*!
@@ -196,7 +196,7 @@ QWebSocketProtocol::CloseCode QWebSocketFrame::closeCode() const
*/
QString QWebSocketFrame::closeReason() const
{
- return m_closeReason;
+ return isDone() ? m_closeReason : tr("Waiting for more data from socket.");
}
/*!
@@ -289,67 +289,63 @@ void QWebSocketFrame::clear()
*/
bool QWebSocketFrame::isValid() const
{
- return m_isValid;
+ return isDone() && m_isValid;
}
-#define WAIT_FOR_MORE_DATA(returnState) \
- { needMoreData = true; \
- m_processingState = (returnState); }
+/*!
+ \internal
+ */
+bool QWebSocketFrame::isDone() const
+{
+ return m_processingState == PS_DISPATCH_RESULT;
+}
/*!
\internal
*/
void QWebSocketFrame::readFrame(QIODevice *pIoDevice)
{
- bool isDone = false;
- while (!isDone)
+ while (true)
{
- bool needMoreData = false;
switch (m_processingState) {
case PS_READ_HEADER:
m_processingState = readFrameHeader(pIoDevice);
- if (m_processingState == PS_WAIT_FOR_MORE_DATA)
- WAIT_FOR_MORE_DATA(PS_READ_HEADER);
+ if (m_processingState == PS_WAIT_FOR_MORE_DATA) {
+ m_processingState = PS_READ_HEADER;
+ return;
+ }
break;
case PS_READ_PAYLOAD_LENGTH:
m_processingState = readFramePayloadLength(pIoDevice);
- if (m_processingState == PS_WAIT_FOR_MORE_DATA)
- WAIT_FOR_MORE_DATA(PS_READ_PAYLOAD_LENGTH);
+ if (m_processingState == PS_WAIT_FOR_MORE_DATA) {
+ m_processingState = PS_READ_PAYLOAD_LENGTH;
+ return;
+ }
break;
case PS_READ_MASK:
m_processingState = readFrameMask(pIoDevice);
- if (m_processingState == PS_WAIT_FOR_MORE_DATA)
- WAIT_FOR_MORE_DATA(PS_READ_MASK);
+ if (m_processingState == PS_WAIT_FOR_MORE_DATA) {
+ m_processingState = PS_READ_MASK;
+ return;
+ }
break;
case PS_READ_PAYLOAD:
m_processingState = readFramePayload(pIoDevice);
- if (m_processingState == PS_WAIT_FOR_MORE_DATA)
- WAIT_FOR_MORE_DATA(PS_READ_PAYLOAD);
+ if (m_processingState == PS_WAIT_FOR_MORE_DATA) {
+ m_processingState = PS_READ_PAYLOAD;
+ return;
+ }
break;
case PS_DISPATCH_RESULT:
- m_processingState = PS_DISPATCH_RESULT;
- isDone = true;
- break;
+ return;
default:
Q_UNREACHABLE();
- break;
- }
-
- if (needMoreData) {
- // TODO: waitForReadyRead should really be changed
- // now, when a WebSocket is used in a GUI thread
- // the GUI will hang for at most 5 seconds
- // maybe, a QStateMachine should be used
- if (!pIoDevice->waitForReadyRead(5000)) {
- setError(QWebSocketProtocol::CloseCodeGoingAway,
- tr("Timeout when reading data from socket."));
- m_processingState = PS_DISPATCH_RESULT;
- }
+ return;
}
}
}
diff --git a/src/websockets/qwebsocketframe_p.h b/src/websockets/qwebsocketframe_p.h
index be79a6e..e2b4e9f 100644
--- a/src/websockets/qwebsocketframe_p.h
+++ b/src/websockets/qwebsocketframe_p.h
@@ -101,6 +101,7 @@ public:
void clear();
bool isValid() const;
+ bool isDone() const;
void readFrame(QIODevice *pIoDevice);
diff --git a/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp b/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
index f9a91d5..705ea39 100644
--- a/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
+++ b/tests/auto/websockets/dataprocessor/tst_dataprocessor.cpp
@@ -193,7 +193,7 @@ private:
//sequences
void nonCharacterSequence(const char *sequence);
- void doTest();
+ void doTest(int timeout = 0);
void doCloseFrameTest();
QString opCodeToString(quint8 opCode);
@@ -744,6 +744,7 @@ void tst_DataProcessor::frameTooSmall()
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(closeSpy.count(), 0);
QCOMPARE(pingMessageSpy.count(), 0);
@@ -776,6 +777,7 @@ void tst_DataProcessor::frameTooSmall()
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(closeSpy.count(), 0);
QCOMPARE(pingMessageSpy.count(), 0);
@@ -808,6 +810,8 @@ void tst_DataProcessor::frameTooSmall()
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
+
buffer.close();
data.clear();
@@ -820,6 +824,7 @@ void tst_DataProcessor::frameTooSmall()
SIGNAL(errorEncountered(QWebSocketProtocol::CloseCode,QString)));
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(closeSpy.count(), 0);
QCOMPARE(pingMessageSpy.count(), 0);
@@ -849,6 +854,7 @@ void tst_DataProcessor::frameTooSmall()
buffer.open(QIODevice::ReadOnly);
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(closeSpy.count(), 0);
QCOMPARE(pingMessageSpy.count(), 0);
@@ -877,6 +883,7 @@ void tst_DataProcessor::frameTooSmall()
buffer.open(QIODevice::ReadOnly);
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), 7000);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(closeSpy.count(), 0);
QCOMPARE(pingMessageSpy.count(), 0);
@@ -1400,7 +1407,7 @@ void tst_DataProcessor::incompletePayload_data()
void tst_DataProcessor::incompletePayload()
{
- doTest();
+ doTest(7000);
}
void tst_DataProcessor::incompleteSizeField_data()
@@ -1430,13 +1437,13 @@ void tst_DataProcessor::incompleteSizeField_data()
void tst_DataProcessor::incompleteSizeField()
{
- doTest();
+ doTest(7000);
}
//////////////////////////////////////////////////////////////////////////////////////////
/// HELPER FUNCTIONS
//////////////////////////////////////////////////////////////////////////////////////////
-void tst_DataProcessor::doTest()
+void tst_DataProcessor::doTest(int timeout)
{
QFETCH(quint8, firstByte);
QFETCH(quint8, secondByte);
@@ -1465,6 +1472,7 @@ void tst_DataProcessor::doTest()
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
dataProcessor.process(&buffer);
+ QTRY_VERIFY_WITH_TIMEOUT(errorSpy.count(), timeout);
QCOMPARE(errorSpy.count(), 1);
QCOMPARE(textMessageSpy.count(), 0);
QCOMPARE(binaryMessageSpy.count(), 0);
diff --git a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
index 2bb5d16..19af815 100644
--- a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
+++ b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
@@ -457,12 +457,46 @@ void tst_QWebSocket::tst_sendTextMessage()
QVERIFY(isLastFrame);
socket.close();
+ socketConnectedSpy.clear();
+ textMessageReceived.clear();
+ textFrameReceived.clear();
- //QTBUG-36762: QWebSocket emits multiplied signals when socket was reopened
+ // QTBUG-74464 QWebsocket doesn't receive text (binary) message with size > 32 kb
+ socket.open(url);
+
+ QTRY_COMPARE(socketConnectedSpy.count(), 1);
+ QCOMPARE(socketError.count(), 0);
+ QCOMPARE(socket.state(), QAbstractSocket::ConnectedState);
+ arguments = serverConnectedSpy.takeFirst();
+ urlConnected = arguments.at(0).toUrl();
+ QCOMPARE(urlConnected, url);
+ QCOMPARE(socket.bytesToWrite(), 0);
+
+ // transmit a long text message with 64 kb
+ QString longString(65536, 'a');
+ socket.sendTextMessage(longString);
+ QVERIFY(socket.bytesToWrite() > longString.length());
+ QVERIFY(textMessageReceived.wait());
+ QCOMPARE(socket.bytesToWrite(), 0);
+
+ QCOMPARE(textMessageReceived.count(), 1);
+ QCOMPARE(binaryMessageReceived.count(), 0);
+ QCOMPARE(binaryFrameReceived.count(), 0);
+ arguments = textMessageReceived.takeFirst();
+ messageReceived = arguments.at(0).toString();
+ QCOMPARE(messageReceived.length(), longString.length());
+ QCOMPARE(messageReceived, longString);
+
+ arguments = textFrameReceived.takeLast();
+ isLastFrame = arguments.at(1).toBool();
+ QVERIFY(isLastFrame);
+
+ socket.close();
socketConnectedSpy.clear();
textMessageReceived.clear();
textFrameReceived.clear();
+ //QTBUG-36762: QWebSocket emits multiplied signals when socket was reopened
socket.open(QUrl(QStringLiteral("ws://") + echoServer.hostAddress().toString() +
QStringLiteral(":") + QString::number(echoServer.port())));