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 +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 +--- + 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 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 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)), + this, SLOT(_q_sslErrors(QList)), + 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 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 uploadByteDevice; ++ qint64 uploadByteDevicePosition; + bool uploadDeviceChoking; // if we couldn't readPointer() any data at the moment + QIODevice *outgoingData; + QSharedPointer 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)), 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)), 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)), 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=" < 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 5.4.1-13 +- backport: data corruption in QNetworkAccessManager + * Fri May 01 2015 Rex Dieter - 5.4.1-12 - backport a couple more upstream fixes - introduce -common noarch subpkg, should help multilib issues