Skip to content

Commit 3eaad41

Browse files
Shane KearnsPasi Pentikäinen
authored andcommitted
Fix regressions due to partial QSslSocket::peek fix
The fix broke HTTPS transactions with chunked encoding. It also broke use of a QSslSocket in unencrypted mode where peek and read calls are mixed. See change 68b1d5c. Change-Id: Ib115b3737b0e4217496f5def10aaaea3c6452ff8 Reviewed-by: Thiago Macieira <[email protected]> (cherry picked from commit 621f189) Reviewed-by: Jaakko Helanti <[email protected]> Reviewed-by: Pasi Pentikäinen <[email protected]>
1 parent 276fb04 commit 3eaad41

File tree

3 files changed

+163
-12
lines changed

3 files changed

+163
-12
lines changed

src/corelib/io/qiodevice_p.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ class QIODevicePrivateLinearBuffer
110110
first += r;
111111
return r;
112112
}
113+
int peek(char* target, int size) {
114+
int r = qMin(size, len);
115+
memcpy(target, first, r);
116+
return r;
117+
}
113118
char* reserve(int size) {
114119
makeSpace(size + len, freeSpaceAtEnd);
115120
char* writePtr = first + len;

src/network/ssl/qsslsocket.cpp

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2249,15 +2249,24 @@ void QSslSocketPrivate::_q_flushReadBuffer()
22492249
qint64 QSslSocketPrivate::peek(char *data, qint64 maxSize)
22502250
{
22512251
if (mode == QSslSocket::UnencryptedMode && !autoStartHandshake) {
2252-
if (plainSocket)
2253-
return plainSocket->peek(data, maxSize);
2254-
else
2252+
//unencrypted mode - do not use QIODevice::peek, as it reads ahead data from the plain socket
2253+
//peek at data already in the QIODevice buffer (from a previous read)
2254+
qint64 r = buffer.peek(data, maxSize);
2255+
if (r == maxSize)
2256+
return r;
2257+
data += r;
2258+
//peek at data in the plain socket
2259+
if (plainSocket) {
2260+
qint64 r2 = plainSocket->peek(data, maxSize - r);
2261+
if (r2 < 0)
2262+
return (r > 0 ? r : r2);
2263+
return r + r2;
2264+
} else {
22552265
return -1;
2266+
}
22562267
} else {
2257-
QByteArray tmp;
2258-
tmp = readBuffer.peek(maxSize);
2259-
memcpy(data, tmp.data(), tmp.length());
2260-
return tmp.length();
2268+
//encrypted mode - the socket engine will read and decrypt data into the QIODevice buffer
2269+
return QTcpSocketPrivate::peek(data, maxSize);
22612270
}
22622271
}
22632272

@@ -2267,14 +2276,21 @@ qint64 QSslSocketPrivate::peek(char *data, qint64 maxSize)
22672276
QByteArray QSslSocketPrivate::peek(qint64 maxSize)
22682277
{
22692278
if (mode == QSslSocket::UnencryptedMode && !autoStartHandshake) {
2279+
//unencrypted mode - do not use QIODevice::peek, as it reads ahead data from the plain socket
2280+
//peek at data already in the QIODevice buffer (from a previous read)
2281+
QByteArray ret;
2282+
ret.reserve(maxSize);
2283+
ret.resize(buffer.peek(ret.data(), maxSize));
2284+
if (ret.length() == maxSize)
2285+
return ret;
2286+
//peek at data in the plain socket
22702287
if (plainSocket)
2271-
return plainSocket->peek(maxSize);
2288+
return ret + plainSocket->peek(maxSize - ret.length());
22722289
else
22732290
return QByteArray();
22742291
} else {
2275-
QByteArray tmp;
2276-
tmp = readBuffer.peek(maxSize);
2277-
return tmp;
2292+
//encrypted mode - the socket engine will read and decrypt data into the QIODevice buffer
2293+
return QTcpSocketPrivate::peek(maxSize);
22782294
}
22792295
}
22802296

tests/auto/qsslsocket/tst_qsslsocket.cpp

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ private slots:
191191
void writeBigChunk();
192192
void blacklistedCertificates();
193193
void qtbug18498_peek();
194+
void qtbug18498_peek2();
194195
void setEmptyDefaultConfiguration();
195196

196197
static void exitLoop()
@@ -2119,7 +2120,7 @@ void tst_QSslSocket::qtbug18498_peek()
21192120
QVERIFY(server.listen(QHostAddress::LocalHost));
21202121
client->connectToHost("127.0.0.1", server.serverPort());
21212122
QVERIFY(client->waitForConnected(5000));
2122-
QVERIFY(server.waitForNewConnection(0));
2123+
QVERIFY(server.waitForNewConnection(1000));
21232124
client->setObjectName("client");
21242125
client->ignoreSslErrors();
21252126

@@ -2159,6 +2160,135 @@ void tst_QSslSocket::qtbug18498_peek()
21592160
QCOMPARE(read_data, data);
21602161
}
21612162

2163+
class SslServer5 : public QTcpServer
2164+
{
2165+
Q_OBJECT
2166+
public:
2167+
SslServer5() : socket(0) {}
2168+
QSslSocket *socket;
2169+
2170+
protected:
2171+
void incomingConnection(int socketDescriptor)
2172+
{
2173+
socket = new QSslSocket;
2174+
socket->setSocketDescriptor(socketDescriptor);
2175+
}
2176+
};
2177+
2178+
void tst_QSslSocket::qtbug18498_peek2()
2179+
{
2180+
QFETCH_GLOBAL(bool, setProxy);
2181+
if (setProxy)
2182+
return;
2183+
2184+
SslServer5 listener;
2185+
QVERIFY(listener.listen(QHostAddress::Any));
2186+
QScopedPointer<QSslSocket> client(new QSslSocket);
2187+
client->connectToHost(QHostAddress::LocalHost, listener.serverPort());
2188+
QVERIFY(client->waitForConnected(5000));
2189+
QVERIFY(listener.waitForNewConnection(1000));
2190+
2191+
QScopedPointer<QSslSocket> server(listener.socket);
2192+
2193+
QVERIFY(server->write("HELLO\r\n", 7));
2194+
QElapsedTimer stopwatch;
2195+
stopwatch.start();
2196+
while (client->bytesAvailable() < 7 && stopwatch.elapsed() < 5000)
2197+
QTest::qWait(100);
2198+
char c;
2199+
QVERIFY(client->peek(&c,1) == 1);
2200+
QCOMPARE(c, 'H');
2201+
QVERIFY(client->read(&c,1) == 1);
2202+
QCOMPARE(c, 'H');
2203+
QByteArray b = client->peek(2);
2204+
QCOMPARE(b, QByteArray("EL"));
2205+
char a[3];
2206+
QVERIFY(client->peek(a, 2) == 2);
2207+
QCOMPARE(a[0], 'E');
2208+
QCOMPARE(a[1], 'L');
2209+
QCOMPARE(client->readAll(), QByteArray("ELLO\r\n"));
2210+
2211+
//check data split between QIODevice and plain socket buffers.
2212+
QByteArray bigblock;
2213+
bigblock.fill('#', QIODEVICE_BUFFERSIZE + 1024);
2214+
QVERIFY(client->write(QByteArray("head")));
2215+
QVERIFY(client->write(bigblock));
2216+
while (server->bytesAvailable() < bigblock.length() + 4 && stopwatch.elapsed() < 5000)
2217+
QTest::qWait(100);
2218+
QCOMPARE(server->read(4), QByteArray("head"));
2219+
QCOMPARE(server->peek(bigblock.length()), bigblock);
2220+
b.reserve(bigblock.length());
2221+
b.resize(server->peek(b.data(), bigblock.length()));
2222+
QCOMPARE(b, bigblock);
2223+
2224+
//check oversized peek
2225+
QCOMPARE(server->peek(bigblock.length() * 3), bigblock);
2226+
b.reserve(bigblock.length() * 3);
2227+
b.resize(server->peek(b.data(), bigblock.length() * 3));
2228+
QCOMPARE(b, bigblock);
2229+
2230+
QCOMPARE(server->readAll(), bigblock);
2231+
2232+
QVERIFY(client->write("STARTTLS\r\n"));
2233+
stopwatch.start();
2234+
// ### Qt5 use QTRY_VERIFY
2235+
while (server->bytesAvailable() < 10 && stopwatch.elapsed() < 5000)
2236+
QTest::qWait(100);
2237+
QVERIFY(server->peek(&c,1) == 1);
2238+
QCOMPARE(c, 'S');
2239+
b = server->peek(3);
2240+
QCOMPARE(b, QByteArray("STA"));
2241+
QCOMPARE(server->read(5), QByteArray("START"));
2242+
QVERIFY(server->peek(a, 3) == 3);
2243+
QCOMPARE(a[0], 'T');
2244+
QCOMPARE(a[1], 'L');
2245+
QCOMPARE(a[2], 'S');
2246+
QCOMPARE(server->readAll(), QByteArray("TLS\r\n"));
2247+
2248+
QFile file(SRCDIR "certs/fluke.key");
2249+
QVERIFY(file.open(QIODevice::ReadOnly));
2250+
QSslKey key(file.readAll(), QSsl::Rsa, QSsl::Pem, QSsl::PrivateKey);
2251+
QVERIFY(!key.isNull());
2252+
server->setPrivateKey(key);
2253+
2254+
QList<QSslCertificate> localCert = QSslCertificate::fromPath(SRCDIR "certs/fluke.cert");
2255+
QVERIFY(!localCert.isEmpty());
2256+
QVERIFY(localCert.first().handle());
2257+
server->setLocalCertificate(localCert.first());
2258+
2259+
server->setProtocol(QSsl::AnyProtocol);
2260+
server->setPeerVerifyMode(QSslSocket::VerifyNone);
2261+
2262+
server->ignoreSslErrors();
2263+
client->ignoreSslErrors();
2264+
2265+
server->startServerEncryption();
2266+
client->startClientEncryption();
2267+
2268+
QVERIFY(server->write("hello\r\n", 7));
2269+
stopwatch.start();
2270+
while (client->bytesAvailable() < 7 && stopwatch.elapsed() < 5000)
2271+
QTest::qWait(100);
2272+
QVERIFY(server->mode() == QSslSocket::SslServerMode && client->mode() == QSslSocket::SslClientMode);
2273+
QVERIFY(client->peek(&c,1) == 1);
2274+
QCOMPARE(c, 'h');
2275+
QVERIFY(client->read(&c,1) == 1);
2276+
QCOMPARE(c, 'h');
2277+
b = client->peek(2);
2278+
QCOMPARE(b, QByteArray("el"));
2279+
QCOMPARE(client->readAll(), QByteArray("ello\r\n"));
2280+
2281+
QVERIFY(client->write("goodbye\r\n"));
2282+
stopwatch.start();
2283+
while (server->bytesAvailable() < 9 && stopwatch.elapsed() < 5000)
2284+
QTest::qWait(100);
2285+
QVERIFY(server->peek(&c,1) == 1);
2286+
QCOMPARE(c, 'g');
2287+
QCOMPARE(server->readAll(), QByteArray("goodbye\r\n"));
2288+
client->disconnectFromHost();
2289+
QVERIFY(client->waitForDisconnected(5000));
2290+
}
2291+
21622292
void tst_QSslSocket::setEmptyDefaultConfiguration()
21632293
{
21642294
// used to produce a crash in QSslConfigurationPrivate::deepCopyDefaultConfiguration, QTBUG-13265

0 commit comments

Comments
 (0)