From c7edb89abbb38c432667de15181b4cee6f14577e Mon Sep 17 00:00:00 2001
From: Jeremy Lainé <jeremy.laine@m4x.org>
Date: Sat, 15 Aug 2015 01:17:55 +0200
Subject: correctly receive data immediately following a SOCKS5 message (closes
 #64)

The SOCKS5 client and server would discard any currently buffered received data
when processing a SOCKS5 command as they used readAll(). Now they only consume
the SOCKS5 command's bytes, preserving any buffered data.

Why this situation should occur is a different matter, I would be surprised to
see it happen during file transfer, due to the sequence in which the handshake
is performed.
---
 src/base/QXmppSocks.cpp             | 31 +++++++++++++++----------------
 tests/qxmppsocks/tst_qxmppsocks.cpp | 18 +++++++++++++++---
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/src/base/QXmppSocks.cpp b/src/base/QXmppSocks.cpp
index 905b7a1b..13f040c3 100644
--- a/src/base/QXmppSocks.cpp
+++ b/src/base/QXmppSocks.cpp
@@ -79,26 +79,23 @@ static QByteArray encodeHostAndPort(quint8 type, const QByteArray &host, quint16
     return buffer;
 }
 
-static bool parseHostAndPort(const QByteArray buffer, quint8 &type, QByteArray &host, quint16 &port)
+static bool parseHostAndPort(QDataStream &stream, quint8 &type, QByteArray &host, quint16 &port)
 {
-    if (buffer.size() < 4)
-        return false;
-
-    QDataStream stream(buffer);
     // get host name
     quint8 hostLength;
     stream >> type;
     stream >> hostLength;
-    if (buffer.size() < hostLength + 4)
-    {
+    if (stream.status() != QDataStream::Ok)
+        return false;
+    host.resize(hostLength);
+    if (stream.readRawData(host.data(), hostLength) != hostLength) {
         qWarning("Invalid host length");
         return false;
     }
-    host.resize(hostLength);
-    stream.readRawData(host.data(), hostLength);
+
     // get port
     stream >> port;
-    return true;
+    return stream.status() == QDataStream::Ok;
 }
 
 QXmppSocksClient::QXmppSocksClient(const QString &proxyHost, quint16 proxyPort, QObject *parent)
@@ -166,8 +163,8 @@ void QXmppSocksClient::slotReadyRead()
         disconnect(this, SIGNAL(readyRead()), this, SLOT(slotReadyRead()));
 
         // receive CONNECT response
-        QByteArray buffer = readAll();
-        if (buffer.size() < 6 ||
+        QByteArray buffer = read(3);
+        if (buffer.size() != 3 ||
             buffer.at(0) != SocksVersion ||
             buffer.at(1) != Succeeded ||
             buffer.at(2) != 0)
@@ -181,7 +178,8 @@ void QXmppSocksClient::slotReadyRead()
         quint8 hostType;
         QByteArray hostName;
         quint16 hostPort;
-        if (!parseHostAndPort(buffer.mid(3), hostType, hostName, hostPort))
+        QDataStream stream(this);
+        if (!parseHostAndPort(stream, hostType, hostName, hostPort))
         {
             qWarning("QXmppSocksClient could not parse type/host/port");
             close();
@@ -297,8 +295,8 @@ void QXmppSocksServer::slotReadyRead()
         disconnect(socket, SIGNAL(readyRead()), this, SLOT(slotReadyRead()));
 
         // receive command
-        QByteArray buffer = socket->readAll();
-        if (buffer.size() < 4 ||
+        QByteArray buffer = socket->read(3);
+        if (buffer.size() != 3 ||
             buffer.at(0) != SocksVersion ||
             buffer.at(1) != ConnectCommand ||
             buffer.at(2) != 0x00)
@@ -312,7 +310,8 @@ void QXmppSocksServer::slotReadyRead()
         quint8 hostType;
         QByteArray hostName;
         quint16 hostPort;
-        if (!parseHostAndPort(buffer.mid(3), hostType, hostName, hostPort))
+        QDataStream stream(socket);
+        if (!parseHostAndPort(stream, hostType, hostName, hostPort))
         {
             qWarning("QXmppSocksServer could not parse type/host/port");
             socket->close();
diff --git a/tests/qxmppsocks/tst_qxmppsocks.cpp b/tests/qxmppsocks/tst_qxmppsocks.cpp
index 767e322f..a8ad9564 100644
--- a/tests/qxmppsocks/tst_qxmppsocks.cpp
+++ b/tests/qxmppsocks/tst_qxmppsocks.cpp
@@ -66,16 +66,25 @@ void tst_QXmppSocks::testClient_data()
     QTest::addColumn<bool>("serverHandshakeWorks");
     QTest::addColumn<QByteArray>("serverConnect");
     QTest::addColumn<bool>("serverConnectWorks");
+    QTest::addColumn<QByteArray>("clientReceivedData");
 
     QTest::newRow("no authentication - good connect")
         << QByteArray::fromHex("0500") << true
-        << QByteArray::fromHex("050000030e7777772e676f6f676c652e636f6d0050") << true;
+        << QByteArray::fromHex("050000030e7777772e676f6f676c652e636f6d0050") << true
+        << QByteArray();
+    QTest::newRow("no authentication - good connect and data")
+        << QByteArray::fromHex("0500") << true
+        << QByteArray::fromHex("050000030e7777772e676f6f676c652e636f6d0050001122") << true
+        << QByteArray::fromHex("001122");
+
     QTest::newRow("no authentication - bad connect")
         << QByteArray::fromHex("0500") << true
-        << QByteArray::fromHex("0500") << false;
+        << QByteArray::fromHex("0500") << false
+        << QByteArray();
     QTest::newRow("bad authentication")
         << QByteArray::fromHex("05ff") << false
-        << QByteArray() << false;
+        << QByteArray() << false
+        << QByteArray();
 }
 
 void tst_QXmppSocks::testClient()
@@ -84,6 +93,7 @@ void tst_QXmppSocks::testClient()
     QFETCH(bool, serverHandshakeWorks);
     QFETCH(QByteArray, serverConnect);
     QFETCH(bool, serverConnectWorks);
+    QFETCH(QByteArray, clientReceivedData);
 
     QTcpServer server;
     QVERIFY(server.listen());
@@ -133,6 +143,8 @@ void tst_QXmppSocks::testClient()
 
     QCOMPARE(client.state(), QAbstractSocket::ConnectedState);
     QCOMPARE(m_connectionSocket->state(), QAbstractSocket::ConnectedState);
+    QByteArray received = client.readAll();
+    QCOMPARE(received, clientReceivedData);
 
     // disconnect
     client.disconnectFromHost();
-- 
cgit v1.2.3