From b810ce7768904e702b16e7aa6083f014829add3f Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Fri, 8 Sep 2017 11:59:45 +0200 Subject: [PATCH] Update server url in case of permanent redirection #5972 This is the first time the account url may update outside of account setup. Summary of redirection handling: 1. During account setup (wizard) - status.php gets permanently redirected -> adjust url - authed PROPFIND gets *any* redirection -> adjust url 2. During connectivity ping (ConnectionValidator) - status.php gets permanently redirected -> adjust url (new!) All other redirections should be followed transparently and don't update the account url in the settings. --- src/gui/owncloudsetupwizard.cpp | 9 +++------ src/libsync/abstractnetworkjob.cpp | 2 ++ src/libsync/abstractnetworkjob.h | 8 ++++++++ src/libsync/connectionvalidator.cpp | 7 +++++++ src/libsync/networkjobs.cpp | 26 ++++++++++++++++++++++++-- src/libsync/networkjobs.h | 21 +++++++++++++++++++++ 6 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp index 4b2886e39..9e6853db8 100644 --- a/src/gui/owncloudsetupwizard.cpp +++ b/src/gui/owncloudsetupwizard.cpp @@ -196,13 +196,10 @@ void OwncloudSetupWizard::slotOwnCloudFoundAuth(const QUrl &url, const QJsonObje // https://github.com/owncloud/core/pull/27473/files _ocWizard->account()->setServerVersion(serverVersion); - QString p = url.path(); - if (p.endsWith("/status.php")) { + if (url != _ocWizard->account()->url()) { // We might be redirected, update the account - QUrl redirectedUrl = url; - redirectedUrl.setPath(url.path().left(url.path().length() - 11)); - _ocWizard->account()->setUrl(redirectedUrl); - qCInfo(lcWizard) << " was redirected to" << redirectedUrl.toString(); + _ocWizard->account()->setUrl(url); + qCInfo(lcWizard) << " was redirected to" << url.toString(); } DetermineAuthTypeJob *job = new DetermineAuthTypeJob(_ocWizard->account(), this); diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp index 71984a66a..9c60da8f9 100644 --- a/src/libsync/abstractnetworkjob.cpp +++ b/src/libsync/abstractnetworkjob.cpp @@ -182,6 +182,8 @@ void AbstractNetworkJob::slotFinished() } else if (verb.isEmpty()) { qCWarning(lcNetworkJob) << this << "cannot redirect request: could not detect original verb"; } else { + emit redirected(_reply, redirectUrl, _redirectCount - 1); + // Create the redirected request and send it qCInfo(lcNetworkJob) << "Redirecting" << verb << requestedUrl << redirectUrl; resetTimeout(); diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h index 36a5113ff..03de2d458 100644 --- a/src/libsync/abstractnetworkjob.h +++ b/src/libsync/abstractnetworkjob.h @@ -98,6 +98,14 @@ signals: void networkError(QNetworkReply *reply); void networkActivity(); + /** Emitted when a redirect is followed. + * + * \a reply The "please redirect" reply + * \a targetUrl Where to redirect to + * \a redirectCount Counts redirect hops, first is 0. + */ + void redirected(QNetworkReply *reply, const QUrl &targetUrl, int redirectCount); + protected: void setupConnections(QNetworkReply *reply); diff --git a/src/libsync/connectionvalidator.cpp b/src/libsync/connectionvalidator.cpp index 9eff065de..f2bae6638 100644 --- a/src/libsync/connectionvalidator.cpp +++ b/src/libsync/connectionvalidator.cpp @@ -135,6 +135,13 @@ void ConnectionValidator::slotStatusFound(const QUrl &url, const QJsonObject &in << CheckServerJob::versionString(info) << "(" << serverVersion << ")"; + // Update server url in case of redirection + if (_account->url() != url) { + qCInfo(lcConnectionValidator()) << "status.php was redirected to" << url.toString(); + _account->setUrl(url); + _account->wantsAccountSaved(_account.data()); + } + if (!serverVersion.isEmpty() && !setAndCheckServerVersion(serverVersion)) { return; } diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp index 304bb839d..c92936e3a 100644 --- a/src/libsync/networkjobs.cpp +++ b/src/libsync/networkjobs.cpp @@ -397,13 +397,17 @@ namespace { CheckServerJob::CheckServerJob(AccountPtr account, QObject *parent) : AbstractNetworkJob(account, QLatin1String(statusphpC), parent) , _subdirFallback(false) + , _permanentRedirects(0) { setIgnoreCredentialFailure(true); + connect(this, SIGNAL(redirected(QNetworkReply *, QUrl, int)), + SLOT(slotRedirected(QNetworkReply *, QUrl, int))); } void CheckServerJob::start() { - sendRequest("GET", makeAccountUrl(path())); + _serverUrl = account()->url(); + sendRequest("GET", Utility::concatUrlPath(_serverUrl, path())); connect(reply(), SIGNAL(metaDataChanged()), this, SLOT(metaDataChangedSlot())); connect(reply(), SIGNAL(encrypted()), this, SLOT(encryptedSlot())); AbstractNetworkJob::start(); @@ -455,6 +459,24 @@ void CheckServerJob::encryptedSlot() mergeSslConfigurationForSslButton(reply()->sslConfiguration(), account()); } +void CheckServerJob::slotRedirected(QNetworkReply *reply, const QUrl &targetUrl, int redirectCount) +{ + QByteArray slashStatusPhp("/"); + slashStatusPhp.append(statusphpC); + + int httpCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); + QString path = targetUrl.path(); + if ((httpCode == 301 || httpCode == 308) // permanent redirection + && redirectCount == _permanentRedirects // don't apply permanent redirects after a temporary one + && path.endsWith(slashStatusPhp)) { + _serverUrl = targetUrl; + _serverUrl.setPath(path.left(path.size() - slashStatusPhp.size())); + qCInfo(lcCheckServerJob) << "status.php was permanently redirected to" + << targetUrl << "new server url is" << _serverUrl; + ++_permanentRedirects; + } +} + void CheckServerJob::metaDataChangedSlot() { account()->setSslConfiguration(reply()->sslConfiguration()); @@ -499,7 +521,7 @@ bool CheckServerJob::finished() qCInfo(lcCheckServerJob) << "status.php returns: " << status << " " << reply()->error() << " Reply: " << reply(); if (status.object().contains("installed")) { - emit instanceFound(reply()->url(), status.object()); + emit instanceFound(_serverUrl, status.object()); } else { qCWarning(lcCheckServerJob) << "No proper answer on " << reply()->url(); emit instanceNotFound(reply()); diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h index fb54e95ba..7992be25a 100644 --- a/src/libsync/networkjobs.h +++ b/src/libsync/networkjobs.h @@ -241,6 +241,11 @@ public: static bool installed(const QJsonObject &info); signals: + /** Emitted when a status.php was successfully read. + * + * \a url see _serverStatusUrl (does not include "/status.php") + * \a info The status.php reply information + */ void instanceFound(const QUrl &url, const QJsonObject &info); /** Emitted on invalid status.php reply. @@ -248,6 +253,11 @@ signals: * \a reply is never null */ void instanceNotFound(QNetworkReply *reply); + + /** A timeout occurred. + * + * \a url The specific url where the timeout happened. + */ void timeout(const QUrl &url); private: @@ -256,9 +266,20 @@ private: private slots: virtual void metaDataChangedSlot(); virtual void encryptedSlot(); + void slotRedirected(QNetworkReply *reply, const QUrl &targetUrl, int redirectCount); private: bool _subdirFallback; + + /** The permanent-redirect adjusted account url. + * + * Note that temporary redirects or a permanent redirect behind a temporary + * one do not affect this url. + */ + QUrl _serverUrl; + + /** Keep track of how many permanent redirect were applied. */ + int _permanentRedirects; };