Update server url in case of permanent redirection #5972
authorChristian Kamm <mail@ckamm.de>
Fri, 8 Sep 2017 09:59:45 +0000 (11:59 +0200)
committerRoeland Jago Douma <roeland@famdouma.nl>
Thu, 5 Oct 2017 20:01:05 +0000 (22:01 +0200)
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
src/libsync/abstractnetworkjob.cpp
src/libsync/abstractnetworkjob.h
src/libsync/connectionvalidator.cpp
src/libsync/networkjobs.cpp
src/libsync/networkjobs.h

index 4b2886e39b40440dcf14cea49aeeb2e4f1ba31a0..9e6853db81c1b8649b475551f6ec9b73e381e306 100644 (file)
@@ -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);
index 71984a66abb1a9b015da9ad9f6fd0a5d62a2d38d..9c60da8f9ca0166b138335c6c0578ac2a7ef91b2 100644 (file)
@@ -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();
index 36a5113ff633814f6df8ca2c06a4f09c73e273d1..03de2d45884d950aa169cdb42fbdd8a7eb3c8fb4 100644 (file)
@@ -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);
 
index 9eff065dee5572ffe63743dad245c315c9ff913d..f2bae6638c2bc17f56b195f25fd22f2e5c904385 100644 (file)
@@ -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;
     }
index 304bb839d6bc4060c6ba6f0125de493a9640e1c2..c92936e3abadc80a1f01deb05d3ac95ab70d5da4 100644 (file)
@@ -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());
index fb54e95ba0fdc4b34480813cbceddfba3819d788..7992be25ae0ecd2397bc332884b906bee57dc4aa 100644 (file)
@@ -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;
 };