diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp index 514983390f8bb..6fe6d662b4c80 100644 --- a/src/libsync/abstractnetworkjob.cpp +++ b/src/libsync/abstractnetworkjob.cpp @@ -38,7 +38,7 @@ Q_LOGGING_CATEGORY(lcNetworkJob, "nextcloud.sync.networkjob", QtInfoMsg) // If not set, it is overwritten by the Application constructor with the value from the config int AbstractNetworkJob::httpTimeout = qEnvironmentVariableIntValue("OWNCLOUD_TIMEOUT"); -bool AbstractNetworkJob::enableTimeout = false; +bool AbstractNetworkJob::enableTimeout = true; AbstractNetworkJob::AbstractNetworkJob(const AccountPtr &account, const QString &path, QObject *parent) : QObject(parent) @@ -48,19 +48,6 @@ AbstractNetworkJob::AbstractNetworkJob(const AccountPtr &account, const QString { // Since we hold a QSharedPointer to the account, this makes no sense. (issue #6893) ASSERT(account != parent); - - _timer.setSingleShot(true); - _timer.setInterval((httpTimeout ? httpTimeout : 300) * 1000); // default to 5 minutes. - connect(&_timer, &QTimer::timeout, this, &AbstractNetworkJob::slotTimeout); - - connect(this, &AbstractNetworkJob::networkActivity, this, &AbstractNetworkJob::resetTimeout); - - // Network activity on the propagator jobs (GET/PUT) keeps all requests alive. - // This is a workaround for OC instances which only support one - // parallel up and download - if (_account) { - connect(_account.data(), &Account::propagatorNetworkActivity, this, &AbstractNetworkJob::resetTimeout); - } } void AbstractNetworkJob::setReply(QNetworkReply *reply) @@ -75,14 +62,7 @@ void AbstractNetworkJob::setReply(QNetworkReply *reply) void AbstractNetworkJob::setTimeout(qint64 msec) { - _timer.start(msec); -} - -void AbstractNetworkJob::resetTimeout() -{ - qint64 interval = _timer.interval(); - _timer.stop(); - _timer.start(interval); + _timeoutDuration = msec; } void AbstractNetworkJob::setIgnoreCredentialFailure(bool ignore) @@ -112,17 +92,15 @@ void AbstractNetworkJob::setupConnections(QNetworkReply *reply) connect(reply, &QNetworkReply::redirected, this, [reply, this] (const QUrl &url) { emit redirected(reply, url, 0);}); } -QNetworkReply *AbstractNetworkJob::addTimer(QNetworkReply *reply) -{ - reply->setProperty("timer", QVariant::fromValue(&_timer)); - return reply; -} - QNetworkReply *AbstractNetworkJob::sendRequest(const QByteArray &verb, const QUrl &url, QNetworkRequest req, QIODevice *requestBody) { + if (AbstractNetworkJob::enableTimeout) { + req.setTransferTimeout(_timeoutDuration); + } + auto reply = _account->sendRawRequest(verb, url, req, requestBody); _requestBody = requestBody; if (_requestBody) { @@ -137,6 +115,10 @@ QNetworkReply *AbstractNetworkJob::sendRequest(const QByteArray &verb, QNetworkRequest req, const QByteArray &requestBody) { + if (AbstractNetworkJob::enableTimeout) { + req.setTransferTimeout(_timeoutDuration); + } + auto reply = _account->sendRawRequest(verb, url, req, requestBody); _requestBody = nullptr; adoptRequest(reply); @@ -148,6 +130,10 @@ QNetworkReply *AbstractNetworkJob::sendRequest(const QByteArray &verb, QNetworkRequest req, QHttpMultiPart *requestBody) { + if (AbstractNetworkJob::enableTimeout) { + req.setTransferTimeout(_timeoutDuration); + } + auto reply = _account->sendRawRequest(verb, url, req, requestBody); _requestBody = nullptr; adoptRequest(reply); @@ -156,7 +142,6 @@ QNetworkReply *AbstractNetworkJob::sendRequest(const QByteArray &verb, void AbstractNetworkJob::adoptRequest(QNetworkReply *reply) { - addTimer(reply); setReply(reply); setupConnections(reply); newReplyHook(reply); @@ -172,9 +157,21 @@ QUrl AbstractNetworkJob::makeDavUrl(const QString &relativePath) const return Utility::concatUrlPath(_account->davUrl(), relativePath); } +void AbstractNetworkJob::onTimedOut() +{ + if (reply()) { + reply()->abort(); + } else { + deleteLater(); + } +} + void AbstractNetworkJob::slotFinished() { - _timer.stop(); + if (_reply->error() == QNetworkReply::TimeoutError) { + qCWarning(lcNetworkJob) << "TimeoutError" << errorString(); + onTimedOut(); + } if (_reply->error() == QNetworkReply::SslHandshakeFailedError) { qCWarning(lcNetworkJob) << "SslHandshakeFailedError: " << errorString() << " : can be caused by a webserver wanting SSL client certificates"; @@ -195,7 +192,6 @@ void AbstractNetworkJob::slotFinished() qCInfo(lcNetworkJob) << "HTTP2 resending" << _reply->request().url(); _http2ResendCount++; - resetTimeout(); if (_requestBody) { if(!_requestBody->isOpen()) _requestBody->open(QIODevice::ReadOnly); @@ -265,7 +261,6 @@ void AbstractNetworkJob::slotFinished() // Create the redirected request and send it qCInfo(lcNetworkJob) << "Redirecting" << verb << requestedUrl << redirectUrl; - resetTimeout(); if (_requestBody) { if(!_requestBody->isOpen()) { // Avoid the QIODevice::seek (QBuffer): The device is not open warning message @@ -321,10 +316,6 @@ QByteArray AbstractNetworkJob::requestId() QString AbstractNetworkJob::errorString() const { - if (_timedout) { - return tr("The server took too long to respond. Check your connection and try syncing again. If it still doesn’t work, reach out to your server administrator."); - } - if (!reply()) { return tr("An unexpected error occurred. Please try syncing again or contact your server administrator if the issue continues."); } @@ -373,8 +364,6 @@ AbstractNetworkJob::~AbstractNetworkJob() void AbstractNetworkJob::start() { - _timer.start(); - const QUrl url = account()->url(); const QString displayUrl = QStringLiteral("%1://%2%3").arg(url.scheme()).arg(url.host()).arg(url.path()); @@ -382,27 +371,6 @@ void AbstractNetworkJob::start() qCInfo(lcNetworkJob) << metaObject()->className() << "created for" << displayUrl << "+" << path() << parentMetaObjectName; } -void AbstractNetworkJob::slotTimeout() -{ - // TODO: workaround, find cause of https://github.com/nextcloud/desktop/issues/7184 - if (!AbstractNetworkJob::enableTimeout) { - return; - } - - _timedout = true; - qCWarning(lcNetworkJob) << "Network job timeout" << (reply() ? reply()->request().url() : path()); - onTimedOut(); -} - -void AbstractNetworkJob::onTimedOut() -{ - if (reply()) { - reply()->abort(); - } else { - deleteLater(); - } -} - QString AbstractNetworkJob::replyStatusString() { Q_ASSERT(reply()); if (reply()->error() == QNetworkReply::NoError) { @@ -579,7 +547,6 @@ void AbstractNetworkJob::retry() QUrl requestedUrl = req.url(); QByteArray verb = HttpLogger::requestVerb(*_reply); qCInfo(lcNetworkJob) << "Restarting" << verb << requestedUrl; - resetTimeout(); if (_requestBody) { _requestBody->seek(0); } diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h index e65cf06edd6f4..b74fc7b5057fb 100644 --- a/src/libsync/abstractnetworkjob.h +++ b/src/libsync/abstractnetworkjob.h @@ -70,8 +70,7 @@ class OWNCLOUDSYNC_EXPORT AbstractNetworkJob : public QObject /* Content of the X-Request-ID header. (Only set after the request is sent) */ QByteArray requestId(); - [[nodiscard]] qint64 timeoutMsec() const { return _timer.interval(); } - [[nodiscard]] bool timedOut() const { return _timedout; } + [[nodiscard]] qint64 timeoutMsec() const { return _timeoutDuration; } /** Returns an error message, if any. */ [[nodiscard]] virtual QString errorString() const; @@ -112,7 +111,6 @@ class OWNCLOUDSYNC_EXPORT AbstractNetworkJob : public QObject public slots: void setTimeout(qint64 msec); - void resetTimeout(); signals: /** Emitted on network error. * @@ -191,7 +189,6 @@ public slots: virtual void onTimedOut(); QByteArray _responseTimestamp; - bool _timedout = false; // set to true when the timeout slot is received // Automatically follows redirects. Note that this only works for // GET requests that don't set up any HTTP body or other flags. @@ -201,19 +198,17 @@ public slots: private slots: void slotFinished(); - void slotTimeout(); protected: AccountPtr _account; private: - QNetworkReply *addTimer(QNetworkReply *reply); bool _ignoreCredentialFailure = false; QPointer _reply; // (QPointer because the NetworkManager may be destroyed before the jobs at exit) QString _path; - QTimer _timer; int _redirectCount = 0; int _http2ResendCount = 0; + qint64 _timeoutDuration = 300000; // Set by the xyzRequest() functions and needed to be able to redirect // requests, should it be required. diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 5011eb1084fe5..8bcc88a638758 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -102,7 +102,6 @@ void PollJob::start() QUrl finalUrl = QUrl::fromUserInput(accountUrl.scheme() + QLatin1String("://") + accountUrl.authority() + (path().startsWith('/') ? QLatin1String("") : QLatin1String("/")) + path()); sendRequest("GET", finalUrl); - connect(reply(), &QNetworkReply::downloadProgress, this, &AbstractNetworkJob::resetTimeout, Qt::UniqueConnection); AbstractNetworkJob::start(); }