mirror of
https://github.com/nextcloud/desktop.git
synced 2024-11-26 23:28:14 +03:00
691 lines
28 KiB
Diff
691 lines
28 KiB
Diff
From cff39fba10ffc10ee4dcfdc66ff6528eb26462d3 Mon Sep 17 00:00:00 2001
|
|
From: Markus Goetz <markus@woboq.com>
|
|
Date: Fri, 10 Apr 2015 14:09:53 +0200
|
|
Subject: [PATCH] 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@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()
|
|
--
|
|
1.9.1
|
|
|