rdieter pushed to qt5-qtbase (f22). "backport: data corruption in QNetworkAccessManager"
notifications at fedoraproject.org
notifications at fedoraproject.org
Tue May 5 18:15:30 UTC 2015
>From e36caa29b262eaa3c96b33df838a5e8341148c49 Mon Sep 17 00:00:00 2001
From: Rex Dieter <rdieter at math.unl.edu>
Date: Tue, 5 May 2015 13:14:32 -0500
Subject: backport: data corruption in QNetworkAccessManager
diff --git a/0248-QNAM-Fix-upload-corruptions-when-server-closes-conne.patch b/0248-QNAM-Fix-upload-corruptions-when-server-closes-conne.patch
new file mode 100644
index 0000000..3ef7cd7
--- /dev/null
+++ b/0248-QNAM-Fix-upload-corruptions-when-server-closes-conne.patch
@@ -0,0 +1,692 @@
+From cff39fba10ffc10ee4dcfdc66ff6528eb26462d3 Mon Sep 17 00:00:00 2001
+From: Markus Goetz <markus at woboq.com>
+Date: Fri, 10 Apr 2015 14:09:53 +0200
+Subject: [PATCH 248/257] QNAM: Fix upload corruptions when server closes
+ connection
+
+This patch fixes several upload corruptions if the server closes the connection
+while/before we send data into it. They happen inside multiple places in the HTTP
+layer and are explained in the comments.
+Corruptions are:
+* The upload byte device has an in-flight signal with pending upload data, if
+it gets reset (because server closes the connection) then the re-send of the
+request was sometimes taking this stale in-flight pending upload data.
+* Because some signals were DirectConnection and some were QueuedConnection, there
+was a chance that a direct signal overtakes a queued signal. The state machine
+then sent data down the socket which was buffered there (and sent later) although
+it did not match the current state of the state machine when it was actually sent.
+* A socket was seen as being able to have requests sent even though it was not
+encrypted yet. This relates to the previous corruption where data is stored inside
+the socket's buffer and then sent later.
+
+The included auto test produces all fixed corruptions, I detected no regressions
+via the other tests.
+This code also adds a bit of sanity checking to protect from possible further
+problems.
+
+[ChangeLog][QtNetwork] Fix HTTP(s) upload corruption when server closes connection
+
+Change-Id: I54c883925ec897050941498f139c4b523030432e
+Reviewed-by: Peter Hartmann <peter-qt at hartmann.tk>
+---
+ src/corelib/io/qnoncontiguousbytedevice.cpp | 18 +++
+ src/corelib/io/qnoncontiguousbytedevice_p.h | 4 +
+ .../access/qhttpnetworkconnectionchannel.cpp | 35 ++++-
+ .../access/qhttpnetworkconnectionchannel_p.h | 2 +
+ src/network/access/qhttpprotocolhandler.cpp | 7 +
+ src/network/access/qhttpthreaddelegate_p.h | 36 ++++-
+ src/network/access/qnetworkreplyhttpimpl.cpp | 25 ++-
+ src/network/access/qnetworkreplyhttpimpl_p.h | 7 +-
+ .../access/qnetworkreply/tst_qnetworkreply.cpp | 175 ++++++++++++++++++++-
+ 9 files changed, 279 insertions(+), 30 deletions(-)
+
+diff --git a/src/corelib/io/qnoncontiguousbytedevice.cpp b/src/corelib/io/qnoncontiguousbytedevice.cpp
+index 11510a8..760ca3d 100644
+--- a/src/corelib/io/qnoncontiguousbytedevice.cpp
++++ b/src/corelib/io/qnoncontiguousbytedevice.cpp
+@@ -236,6 +236,11 @@ qint64 QNonContiguousByteDeviceByteArrayImpl::size()
+ return byteArray->size();
+ }
+
++qint64 QNonContiguousByteDeviceByteArrayImpl::pos()
++{
++ return currentPosition;
++}
++
+ QNonContiguousByteDeviceRingBufferImpl::QNonContiguousByteDeviceRingBufferImpl(QSharedPointer<QRingBuffer> rb)
+ : QNonContiguousByteDevice(), currentPosition(0)
+ {
+@@ -273,6 +278,11 @@ bool QNonContiguousByteDeviceRingBufferImpl::atEnd()
+ return currentPosition >= size();
+ }
+
++qint64 QNonContiguousByteDeviceRingBufferImpl::pos()
++{
++ return currentPosition;
++}
++
+ bool QNonContiguousByteDeviceRingBufferImpl::reset()
+ {
+ if (resetDisabled)
+@@ -406,6 +416,14 @@ qint64 QNonContiguousByteDeviceIoDeviceImpl::size()
+ return device->size() - initialPosition;
+ }
+
++qint64 QNonContiguousByteDeviceIoDeviceImpl::pos()
++{
++ if (device->isSequential())
++ return -1;
++
++ return device->pos();
++}
++
+ QByteDeviceWrappingIoDevice::QByteDeviceWrappingIoDevice(QNonContiguousByteDevice *bd) : QIODevice((QObject*)0)
+ {
+ byteDevice = bd;
+diff --git a/src/corelib/io/qnoncontiguousbytedevice_p.h b/src/corelib/io/qnoncontiguousbytedevice_p.h
+index c05ae11..4d7b7b0 100644
+--- a/src/corelib/io/qnoncontiguousbytedevice_p.h
++++ b/src/corelib/io/qnoncontiguousbytedevice_p.h
+@@ -61,6 +61,7 @@ public:
+ virtual const char* readPointer(qint64 maximumLength, qint64 &len) = 0;
+ virtual bool advanceReadPointer(qint64 amount) = 0;
+ virtual bool atEnd() = 0;
++ virtual qint64 pos() { return -1; }
+ virtual bool reset() = 0;
+ void disableReset();
+ bool isResetDisabled() { return resetDisabled; }
+@@ -106,6 +107,7 @@ public:
+ bool atEnd();
+ bool reset();
+ qint64 size();
++ qint64 pos() Q_DECL_OVERRIDE;
+ protected:
+ QByteArray* byteArray;
+ qint64 currentPosition;
+@@ -121,6 +123,7 @@ public:
+ bool atEnd();
+ bool reset();
+ qint64 size();
++ qint64 pos() Q_DECL_OVERRIDE;
+ protected:
+ QSharedPointer<QRingBuffer> ringBuffer;
+ qint64 currentPosition;
+@@ -138,6 +141,7 @@ public:
+ bool atEnd();
+ bool reset();
+ qint64 size();
++ qint64 pos() Q_DECL_OVERRIDE;
+ protected:
+ QIODevice* device;
+ QByteArray* currentReadBuffer;
+diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
+index 9f63280..49c6793 100644
+--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
++++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
+@@ -106,15 +106,19 @@ void QHttpNetworkConnectionChannel::init()
+ socket->setProxy(QNetworkProxy::NoProxy);
+ #endif
+
++ // We want all signals (except the interactive ones) be connected as QueuedConnection
++ // because else we're falling into cases where we recurse back into the socket code
++ // and mess up the state. Always going to the event loop (and expecting that when reading/writing)
++ // is safer.
+ QObject::connect(socket, SIGNAL(bytesWritten(qint64)),
+ this, SLOT(_q_bytesWritten(qint64)),
+- Qt::DirectConnection);
++ Qt::QueuedConnection);
+ QObject::connect(socket, SIGNAL(connected()),
+ this, SLOT(_q_connected()),
+- Qt::DirectConnection);
++ Qt::QueuedConnection);
+ QObject::connect(socket, SIGNAL(readyRead()),
+ this, SLOT(_q_readyRead()),
+- Qt::DirectConnection);
++ Qt::QueuedConnection);
+
+ // The disconnected() and error() signals may already come
+ // while calling connectToHost().
+@@ -143,13 +147,13 @@ void QHttpNetworkConnectionChannel::init()
+ // won't be a sslSocket if encrypt is false
+ QObject::connect(sslSocket, SIGNAL(encrypted()),
+ this, SLOT(_q_encrypted()),
+- Qt::DirectConnection);
++ Qt::QueuedConnection);
+ QObject::connect(sslSocket, SIGNAL(sslErrors(QList<QSslError>)),
+ this, SLOT(_q_sslErrors(QList<QSslError>)),
+ Qt::DirectConnection);
+ QObject::connect(sslSocket, SIGNAL(encryptedBytesWritten(qint64)),
+ this, SLOT(_q_encryptedBytesWritten(qint64)),
+- Qt::DirectConnection);
++ Qt::QueuedConnection);
+
+ if (ignoreAllSslErrors)
+ sslSocket->ignoreSslErrors();
+@@ -186,8 +190,11 @@ void QHttpNetworkConnectionChannel::close()
+ // pendingEncrypt must only be true in between connected and encrypted states
+ pendingEncrypt = false;
+
+- if (socket)
++ if (socket) {
++ // socket can be 0 since the host lookup is done from qhttpnetworkconnection.cpp while
++ // there is no socket yet.
+ socket->close();
++ }
+ }
+
+
+@@ -353,6 +360,14 @@ bool QHttpNetworkConnectionChannel::ensureConnection()
+ }
+ return false;
+ }
++
++ // This code path for ConnectedState
++ if (pendingEncrypt) {
++ // Let's only be really connected when we have received the encrypted() signal. Else the state machine seems to mess up
++ // and corrupt the things sent to the server.
++ return false;
++ }
++
+ return true;
+ }
+
+@@ -659,6 +674,12 @@ bool QHttpNetworkConnectionChannel::isSocketReading() const
+ void QHttpNetworkConnectionChannel::_q_bytesWritten(qint64 bytes)
+ {
+ Q_UNUSED(bytes);
++ if (ssl) {
++ // In the SSL case we want to send data from encryptedBytesWritten signal since that one
++ // is the one going down to the actual network, not only into some SSL buffer.
++ return;
++ }
++
+ // bytes have been written to the socket. write even more of them :)
+ if (isSocketWriting())
+ sendRequest();
+@@ -734,7 +755,7 @@ void QHttpNetworkConnectionChannel::_q_connected()
+
+ // ### FIXME: if the server closes the connection unexpectedly, we shouldn't send the same broken request again!
+ //channels[i].reconnectAttempts = 2;
+- if (pendingEncrypt) {
++ if (ssl || pendingEncrypt) { // FIXME: Didn't work properly with pendingEncrypt only, we should refactor this into an EncrypingState
+ #ifndef QT_NO_SSL
+ if (connection->sslContext().isNull()) {
+ // this socket is making the 1st handshake for this connection,
+diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
+index 692c0e6..231fe11 100644
+--- a/src/network/access/qhttpnetworkconnectionchannel_p.h
++++ b/src/network/access/qhttpnetworkconnectionchannel_p.h
+@@ -83,6 +83,8 @@ typedef QPair<QHttpNetworkRequest, QHttpNetworkReply*> HttpMessagePair;
+ class QHttpNetworkConnectionChannel : public QObject {
+ Q_OBJECT
+ public:
++ // TODO: Refactor this to add an EncryptingState (and remove pendingEncrypt).
++ // Also add an Unconnected state so IdleState does not have double meaning.
+ enum ChannelState {
+ IdleState = 0, // ready to send request
+ ConnectingState = 1, // connecting to host
+diff --git a/src/network/access/qhttpprotocolhandler.cpp b/src/network/access/qhttpprotocolhandler.cpp
+index 28e10f7..3357948 100644
+--- a/src/network/access/qhttpprotocolhandler.cpp
++++ b/src/network/access/qhttpprotocolhandler.cpp
+@@ -368,6 +368,13 @@ bool QHttpProtocolHandler::sendRequest()
+ // nothing to read currently, break the loop
+ break;
+ } else {
++ if (m_channel->written != uploadByteDevice->pos()) {
++ // Sanity check. This was useful in tracking down an upload corruption.
++ qWarning() << "QHttpProtocolHandler: Internal error in sendRequest. Expected to write at position" << m_channel->written << "but read device is at" << uploadByteDevice->pos();
++ Q_ASSERT(m_channel->written == uploadByteDevice->pos());
++ m_connection->d_func()->emitReplyError(m_socket, m_reply, QNetworkReply::ProtocolFailure);
++ return false;
++ }
+ qint64 currentWriteSize = m_socket->write(readPointer, currentReadSize);
+ if (currentWriteSize == -1 || currentWriteSize != currentReadSize) {
+ // socket broke down
+diff --git a/src/network/access/qhttpthreaddelegate_p.h b/src/network/access/qhttpthreaddelegate_p.h
+index 1661082..b553409 100644
+--- a/src/network/access/qhttpthreaddelegate_p.h
++++ b/src/network/access/qhttpthreaddelegate_p.h
+@@ -187,6 +187,7 @@ protected:
+ QByteArray m_dataArray;
+ bool m_atEnd;
+ qint64 m_size;
++ qint64 m_pos; // to match calls of haveDataSlot with the expected position
+ public:
+ QNonContiguousByteDeviceThreadForwardImpl(bool aE, qint64 s)
+ : QNonContiguousByteDevice(),
+@@ -194,7 +195,8 @@ public:
+ m_amount(0),
+ m_data(0),
+ m_atEnd(aE),
+- m_size(s)
++ m_size(s),
++ m_pos(0)
+ {
+ }
+
+@@ -202,6 +204,11 @@ public:
+ {
+ }
+
++ qint64 pos() Q_DECL_OVERRIDE
++ {
++ return m_pos;
++ }
++
+ const char* readPointer(qint64 maximumLength, qint64 &len)
+ {
+ if (m_amount > 0) {
+@@ -229,11 +236,10 @@ public:
+
+ m_amount -= a;
+ m_data += a;
++ m_pos += a;
+
+- // To main thread to inform about our state
+- emit processedData(a);
+-
+- // FIXME possible optimization, already ask user thread for some data
++ // To main thread to inform about our state. The m_pos will be sent as a sanity check.
++ emit processedData(m_pos, a);
+
+ return true;
+ }
+@@ -250,10 +256,21 @@ public:
+ {
+ m_amount = 0;
+ m_data = 0;
++ m_dataArray.clear();
++
++ if (wantDataPending) {
++ // had requested the user thread to send some data (only 1 in-flight at any moment)
++ wantDataPending = false;
++ }
+
+ // Communicate as BlockingQueuedConnection
+ bool b = false;
+ emit resetData(&b);
++ if (b) {
++ // the reset succeeded, we're at pos 0 again
++ m_pos = 0;
++ // the HTTP code will anyway abort the request if !b.
++ }
+ return b;
+ }
+
+@@ -264,8 +281,13 @@ public:
+
+ public slots:
+ // From user thread:
+- void haveDataSlot(QByteArray dataArray, bool dataAtEnd, qint64 dataSize)
++ void haveDataSlot(qint64 pos, QByteArray dataArray, bool dataAtEnd, qint64 dataSize)
+ {
++ if (pos != m_pos) {
++ // Sometimes when re-sending a request in the qhttpnetwork* layer there is a pending haveData from the
++ // user thread on the way to us. We need to ignore it since it is the data for the wrong(later) chunk.
++ return;
++ }
+ wantDataPending = false;
+
+ m_dataArray = dataArray;
+@@ -285,7 +307,7 @@ signals:
+
+ // to main thread:
+ void wantData(qint64);
+- void processedData(qint64);
++ void processedData(qint64 pos, qint64 amount);
+ void resetData(bool *b);
+ };
+
+diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp
+index 4ce7303..974a101 100644
+--- a/src/network/access/qnetworkreplyhttpimpl.cpp
++++ b/src/network/access/qnetworkreplyhttpimpl.cpp
+@@ -424,6 +424,7 @@ QNetworkReplyHttpImplPrivate::QNetworkReplyHttpImplPrivate()
+ , synchronous(false)
+ , state(Idle)
+ , statusCode(0)
++ , uploadByteDevicePosition(false)
+ , uploadDeviceChoking(false)
+ , outgoingData(0)
+ , bytesUploaded(-1)
+@@ -863,9 +864,9 @@ void QNetworkReplyHttpImplPrivate::postRequest()
+ q, SLOT(uploadByteDeviceReadyReadSlot()),
+ Qt::QueuedConnection);
+
+- // From main thread to user thread:
+- QObject::connect(q, SIGNAL(haveUploadData(QByteArray,bool,qint64)),
+- forwardUploadDevice, SLOT(haveDataSlot(QByteArray,bool,qint64)), Qt::QueuedConnection);
++ // From user thread to http thread:
++ QObject::connect(q, SIGNAL(haveUploadData(qint64,QByteArray,bool,qint64)),
++ forwardUploadDevice, SLOT(haveDataSlot(qint64,QByteArray,bool,qint64)), Qt::QueuedConnection);
+ QObject::connect(uploadByteDevice.data(), SIGNAL(readyRead()),
+ forwardUploadDevice, SIGNAL(readyRead()),
+ Qt::QueuedConnection);
+@@ -873,8 +874,8 @@ void QNetworkReplyHttpImplPrivate::postRequest()
+ // From http thread to user thread:
+ QObject::connect(forwardUploadDevice, SIGNAL(wantData(qint64)),
+ q, SLOT(wantUploadDataSlot(qint64)));
+- QObject::connect(forwardUploadDevice, SIGNAL(processedData(qint64)),
+- q, SLOT(sentUploadDataSlot(qint64)));
++ QObject::connect(forwardUploadDevice,SIGNAL(processedData(qint64, qint64)),
++ q, SLOT(sentUploadDataSlot(qint64,qint64)));
+ QObject::connect(forwardUploadDevice, SIGNAL(resetData(bool*)),
+ q, SLOT(resetUploadDataSlot(bool*)),
+ Qt::BlockingQueuedConnection); // this is the only one with BlockingQueued!
+@@ -1268,12 +1269,22 @@ void QNetworkReplyHttpImplPrivate::replySslConfigurationChanged(const QSslConfig
+ void QNetworkReplyHttpImplPrivate::resetUploadDataSlot(bool *r)
+ {
+ *r = uploadByteDevice->reset();
++ if (*r) {
++ // reset our own position which is used for the inter-thread communication
++ uploadByteDevicePosition = 0;
++ }
+ }
+
+ // Coming from QNonContiguousByteDeviceThreadForwardImpl in HTTP thread
+-void QNetworkReplyHttpImplPrivate::sentUploadDataSlot(qint64 amount)
++void QNetworkReplyHttpImplPrivate::sentUploadDataSlot(qint64 pos, qint64 amount)
+ {
++ if (uploadByteDevicePosition + amount != pos) {
++ // Sanity check, should not happen.
++ error(QNetworkReply::UnknownNetworkError, "");
++ return;
++ }
+ uploadByteDevice->advanceReadPointer(amount);
++ uploadByteDevicePosition += amount;
+ }
+
+ // Coming from QNonContiguousByteDeviceThreadForwardImpl in HTTP thread
+@@ -1298,7 +1309,7 @@ void QNetworkReplyHttpImplPrivate::wantUploadDataSlot(qint64 maxSize)
+ QByteArray dataArray(data, currentUploadDataLength);
+
+ // Communicate back to HTTP thread
+- emit q->haveUploadData(dataArray, uploadByteDevice->atEnd(), uploadByteDevice->size());
++ emit q->haveUploadData(uploadByteDevicePosition, dataArray, uploadByteDevice->atEnd(), uploadByteDevice->size());
+ }
+
+ void QNetworkReplyHttpImplPrivate::uploadByteDeviceReadyReadSlot()
+diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h
+index 77d9c5a..1940922 100644
+--- a/src/network/access/qnetworkreplyhttpimpl_p.h
++++ b/src/network/access/qnetworkreplyhttpimpl_p.h
+@@ -120,7 +120,7 @@ public:
+
+ Q_PRIVATE_SLOT(d_func(), void resetUploadDataSlot(bool *r))
+ Q_PRIVATE_SLOT(d_func(), void wantUploadDataSlot(qint64))
+- Q_PRIVATE_SLOT(d_func(), void sentUploadDataSlot(qint64))
++ Q_PRIVATE_SLOT(d_func(), void sentUploadDataSlot(qint64,qint64))
+ Q_PRIVATE_SLOT(d_func(), void uploadByteDeviceReadyReadSlot())
+ Q_PRIVATE_SLOT(d_func(), void emitReplyUploadProgress(qint64, qint64))
+ Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose())
+@@ -144,7 +144,7 @@ signals:
+
+ void startHttpRequestSynchronously();
+
+- void haveUploadData(QByteArray dataArray, bool dataAtEnd, qint64 dataSize);
++ void haveUploadData(const qint64 pos, QByteArray dataArray, bool dataAtEnd, qint64 dataSize);
+ };
+
+ class QNetworkReplyHttpImplPrivate: public QNetworkReplyPrivate
+@@ -195,6 +195,7 @@ public:
+ // upload
+ QNonContiguousByteDevice* createUploadByteDevice();
+ QSharedPointer<QNonContiguousByteDevice> uploadByteDevice;
++ qint64 uploadByteDevicePosition;
+ bool uploadDeviceChoking; // if we couldn't readPointer() any data at the moment
+ QIODevice *outgoingData;
+ QSharedPointer<QRingBuffer> outgoingDataBuffer;
+@@ -283,7 +284,7 @@ public:
+ // From QNonContiguousByteDeviceThreadForwardImpl in HTTP thread:
+ void resetUploadDataSlot(bool *r);
+ void wantUploadDataSlot(qint64);
+- void sentUploadDataSlot(qint64);
++ void sentUploadDataSlot(qint64, qint64);
+
+ // From user's QNonContiguousByteDevice
+ void uploadByteDeviceReadyReadSlot();
+diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+index 3ccedf6..d2edf67 100644
+--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
++++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+@@ -457,6 +457,10 @@ private Q_SLOTS:
+
+ void putWithRateLimiting();
+
++#ifndef QT_NO_SSL
++ void putWithServerClosingConnectionImmediately();
++#endif
++
+ // NOTE: This test must be last!
+ void parentingRepliesToTheApp();
+ private:
+@@ -4718,18 +4722,22 @@ void tst_QNetworkReply::ioPostToHttpNoBufferFlag()
+ class SslServer : public QTcpServer {
+ Q_OBJECT
+ public:
+- SslServer() : socket(0) {};
++ SslServer() : socket(0), m_ssl(true) {}
+ void incomingConnection(qintptr socketDescriptor) {
+ QSslSocket *serverSocket = new QSslSocket;
+ serverSocket->setParent(this);
+
+ if (serverSocket->setSocketDescriptor(socketDescriptor)) {
++ connect(serverSocket, SIGNAL(readyRead()), this, SLOT(readyReadSlot()));
++ if (!m_ssl) {
++ emit newPlainConnection(serverSocket);
++ return;
++ }
+ QString testDataDir = QFileInfo(QFINDTESTDATA("rfc3252.txt")).absolutePath();
+ if (testDataDir.isEmpty())
+ testDataDir = QCoreApplication::applicationDirPath();
+
+ connect(serverSocket, SIGNAL(encrypted()), this, SLOT(encryptedSlot()));
+- connect(serverSocket, SIGNAL(readyRead()), this, SLOT(readyReadSlot()));
+ serverSocket->setProtocol(QSsl::AnyProtocol);
+ connect(serverSocket, SIGNAL(sslErrors(QList<QSslError>)), serverSocket, SLOT(ignoreSslErrors()));
+ serverSocket->setLocalCertificate(testDataDir + "/certs/server.pem");
+@@ -4740,11 +4748,12 @@ public:
+ }
+ }
+ signals:
+- void newEncryptedConnection();
++ void newEncryptedConnection(QSslSocket *s);
++ void newPlainConnection(QSslSocket *s);
+ public slots:
+ void encryptedSlot() {
+ socket = (QSslSocket*) sender();
+- emit newEncryptedConnection();
++ emit newEncryptedConnection(socket);
+ }
+ void readyReadSlot() {
+ // for the incoming sockets, not the server socket
+@@ -4753,6 +4762,7 @@ public slots:
+
+ public:
+ QSslSocket *socket;
++ bool m_ssl;
+ };
+
+ // very similar to ioPostToHttpUploadProgress but for SSL
+@@ -4780,7 +4790,7 @@ void tst_QNetworkReply::ioPostToHttpsUploadProgress()
+ QNetworkReplyPtr reply(manager.post(request, sourceFile));
+
+ QSignalSpy spy(reply.data(), SIGNAL(uploadProgress(qint64,qint64)));
+- connect(&server, SIGNAL(newEncryptedConnection()), &QTestEventLoop::instance(), SLOT(exitLoop()));
++ connect(&server, SIGNAL(newEncryptedConnection(QSslSocket*)), &QTestEventLoop::instance(), SLOT(exitLoop()));
+ connect(reply, SIGNAL(sslErrors(QList<QSslError>)), reply.data(), SLOT(ignoreSslErrors()));
+
+ // get the request started and the incoming socket connected
+@@ -4788,7 +4798,7 @@ void tst_QNetworkReply::ioPostToHttpsUploadProgress()
+ QVERIFY(!QTestEventLoop::instance().timeout());
+ QTcpSocket *incomingSocket = server.socket;
+ QVERIFY(incomingSocket);
+- disconnect(&server, SIGNAL(newEncryptedConnection()), &QTestEventLoop::instance(), SLOT(exitLoop()));
++ disconnect(&server, SIGNAL(newEncryptedConnection(QSslSocket*)), &QTestEventLoop::instance(), SLOT(exitLoop()));
+
+
+ incomingSocket->setReadBufferSize(1*1024);
+@@ -7905,6 +7915,159 @@ void tst_QNetworkReply::putWithRateLimiting()
+ }
+
+
++#ifndef QT_NO_SSL
++
++class PutWithServerClosingConnectionImmediatelyHandler: public QObject
++{
++ Q_OBJECT
++public:
++ bool m_parsedHeaders;
++ QByteArray m_receivedData;
++ QByteArray m_expectedData;
++ QSslSocket *m_socket;
++ PutWithServerClosingConnectionImmediatelyHandler(QSslSocket *s, QByteArray expected) :m_parsedHeaders(false), m_expectedData(expected), m_socket(s)
++ {
++ m_socket->setParent(this);
++ connect(m_socket, SIGNAL(readyRead()), SLOT(readyReadSlot()));
++ connect(m_socket, SIGNAL(disconnected()), SLOT(disconnectedSlot()));
++ }
++signals:
++ void correctFileUploadReceived();
++ void corruptFileUploadReceived();
++
++public slots:
++ void closeDelayed() {
++ m_socket->close();
++ }
++
++ void readyReadSlot()
++ {
++ QByteArray data = m_socket->readAll();
++ m_receivedData += data;
++ if (!m_parsedHeaders && m_receivedData.contains("\r\n\r\n")) {
++ m_parsedHeaders = true;
++ QTimer::singleShot(qrand()%10, this, SLOT(closeDelayed())); // simulate random network latency
++ // This server simulates a web server connection closing, e.g. because of Apaches MaxKeepAliveRequests or KeepAliveTimeout
++ // In this case QNAM needs to re-send the upload data but it had a bug which then corrupts the upload
++ // This test catches that.
++ }
++
++ }
++ void disconnectedSlot()
++ {
++ if (m_parsedHeaders) {
++ //qDebug() << m_receivedData.left(m_receivedData.indexOf("\r\n\r\n"));
++ m_receivedData = m_receivedData.mid(m_receivedData.indexOf("\r\n\r\n")+4); // check only actual data
++ }
++ if (m_receivedData.length() > 0 && !m_expectedData.startsWith(m_receivedData)) {
++ // We had received some data but it is corrupt!
++ qDebug() << "CORRUPT" << m_receivedData.count();
++
++ // Use this to track down the pattern of the corruption and conclude the source
++// QFile a("/tmp/corrupt");
++// a.open(QIODevice::WriteOnly);
++// a.write(m_receivedData);
++// a.close();
++
++// QFile b("/tmp/correct");
++// b.open(QIODevice::WriteOnly);
++// b.write(m_expectedData);
++// b.close();
++ //exit(1);
++ emit corruptFileUploadReceived();
++ } else {
++ emit correctFileUploadReceived();
++ }
++ }
++};
++
++class PutWithServerClosingConnectionImmediatelyServer: public SslServer
++{
++ Q_OBJECT
++public:
++ int m_correctUploads;
++ int m_corruptUploads;
++ int m_repliesFinished;
++ int m_expectedReplies;
++ QByteArray m_expectedData;
++ PutWithServerClosingConnectionImmediatelyServer() : SslServer(), m_correctUploads(0), m_corruptUploads(0), m_repliesFinished(0), m_expectedReplies(0)
++ {
++ QObject::connect(this, SIGNAL(newEncryptedConnection(QSslSocket*)), this, SLOT(createHandlerForConnection(QSslSocket*)));
++ QObject::connect(this, SIGNAL(newPlainConnection(QSslSocket*)), this, SLOT(createHandlerForConnection(QSslSocket*)));
++ }
++
++public slots:
++ void createHandlerForConnection(QSslSocket* s) {
++ PutWithServerClosingConnectionImmediatelyHandler *handler = new PutWithServerClosingConnectionImmediatelyHandler(s, m_expectedData);
++ handler->setParent(this);
++ QObject::connect(handler, SIGNAL(correctFileUploadReceived()), this, SLOT(increaseCorrect()));
++ QObject::connect(handler, SIGNAL(corruptFileUploadReceived()), this, SLOT(increaseCorrupt()));
++ }
++ void increaseCorrect() {
++ m_correctUploads++;
++ }
++ void increaseCorrupt() {
++ m_corruptUploads++;
++ }
++ void replyFinished() {
++ m_repliesFinished++;
++ if (m_repliesFinished == m_expectedReplies) {
++ QTestEventLoop::instance().exitLoop();
++ }
++ }
++};
++
++
++
++void tst_QNetworkReply::putWithServerClosingConnectionImmediately()
++{
++ const int numUploads = 40;
++ qint64 wantedSize = 512*1024; // 512 kB
++ QByteArray sourceFile;
++ for (int i = 0; i < wantedSize; ++i) {
++ sourceFile += (char)'a' +(i%26);
++ }
++ bool withSsl = false;
++
++ for (int s = 0; s <= 1; s++) {
++ withSsl = (s == 1);
++ // Test also needs to run several times because of 9c2ecf89
++ for (int j = 0; j < 20; j++) {
++ // emulate a minimal https server
++ PutWithServerClosingConnectionImmediatelyServer server;
++ server.m_ssl = withSsl;
++ server.m_expectedData = sourceFile;
++ server.m_expectedReplies = numUploads;
++ server.listen(QHostAddress(QHostAddress::LocalHost), 0);
++
++ for (int i = 0; i < numUploads; i++) {
++ // create the request
++ QUrl url = QUrl(QString("http%1://127.0.0.1:%2/file=%3").arg(withSsl ? "s" : "").arg(server.serverPort()).arg(i));
++ QNetworkRequest request(url);
++ QNetworkReply *reply = manager.put(request, sourceFile);
++ connect(reply, SIGNAL(sslErrors(QList<QSslError>)), reply, SLOT(ignoreSslErrors()));
++ connect(reply, SIGNAL(finished()), &server, SLOT(replyFinished()));
++ reply->setParent(&server);
++ }
++
++ // get the request started and the incoming socket connected
++ QTestEventLoop::instance().enterLoop(10);
++
++ //qDebug() << "correct=" << server.m_correctUploads << "corrupt=" << server.m_corruptUploads << "expected=" <<numUploads;
++
++ // Sanity check because ecause of 9c2ecf89 most replies will error out but we want to make sure at least some of them worked
++ QVERIFY(server.m_correctUploads > 5);
++ // Because actually important is that we don't get any corruption:
++ QCOMPARE(server.m_corruptUploads, 0);
++
++ server.close();
++ }
++ }
++
++
++}
++
++#endif
+
+ // NOTE: This test must be last testcase in tst_qnetworkreply!
+ void tst_QNetworkReply::parentingRepliesToTheApp()
+--
+2.4.0
+
diff --git a/qt5-qtbase.spec b/qt5-qtbase.spec
index a209f1e..27247ba 100644
--- a/qt5-qtbase.spec
+++ b/qt5-qtbase.spec
@@ -37,7 +37,7 @@
Summary: Qt5 - QtBase components
Name: qt5-qtbase
Version: 5.4.1
-Release: 12%{?dist}
+Release: 13%{?dist}
# See LGPL_EXCEPTIONS.txt, for exception details
License: LGPLv2 with exceptions or GPLv3 with exceptions
@@ -105,6 +105,7 @@ Patch294: 0094-Fix-Meta-.-shortcuts-on-XCB.patch
Patch332: 0132-Call-ofono-nm-Registered-delayed-in-constructor-othe.patch
Patch336: 0136-Make-sure-there-s-a-scene-before-using-it.patch
Patch440: 0240-QLockFile-fix-deadlock-when-the-lock-file-is-corrupt.patch
+Patch448: 0248-QNAM-Fix-upload-corruptions-when-server-closes-conne.patch
# http://lists.qt-project.org/pipermail/announce/2015-February/000059.html
# CVE-2015-0295
Patch349: 0149-Fix-a-division-by-zero-when-processing-malformed-BMP.patch
@@ -380,6 +381,7 @@ rm -fv mkspecs/linux-g++*/qmake.conf.multilib-optflags
%patch400 -p1 -b .0200
%patch401 -p1 -b .0201
%patch440 -p1 -b .0240
+%patch448 -p1 -b .0248
# drop -fexceptions from $RPM_OPT_FLAGS
RPM_OPT_FLAGS=`echo $RPM_OPT_FLAGS | sed 's|-fexceptions||g'`
@@ -897,6 +899,9 @@ fi
%changelog
+* Tue May 05 2015 Rex Dieter <rdieter at fedoraproject.org> 5.4.1-13
+- backport: data corruption in QNetworkAccessManager
+
* Fri May 01 2015 Rex Dieter <rdieter at fedoraproject.org> - 5.4.1-12
- backport a couple more upstream fixes
- introduce -common noarch subpkg, should help multilib issues
--
cgit v0.10.2
http://pkgs.fedoraproject.org/cgit/qt5-qtbase.git/commit/?h=f22&id=e36caa29b262eaa3c96b33df838a5e8341148c49
More information about the scm-commits
mailing list