Do not store the redirected URL in the config file.

Because it may be a temporary URL.
Especially anoying in captive portal

Fix issue #368

src/mirall/application.cpp

@@ -321,7 +321,7 @@ void Application::slotCheckAuthentication()
              this,SLOT(slotAuthCheck(QString,QNetworkReply*)));
 
     qDebug() << "# checking for authentication settings.";
-    ownCloudInfo::instance()->getRequest(QLatin1String("/"), true ); // this call needs to be authenticated.
+    ownCloudInfo::instance()->getWebDAVPath(QLatin1String("/") ); // this call needs to be authenticated.
     // simply GET the webdav root, will fail if credentials are wrong.
     // continue in slotAuthCheck here :-)
 }

src/mirall/connectionvalidator.cpp

@@ -163,7 +163,7 @@ void ConnectionValidator::slotCheckAuthentication()
              this, SLOT(slotAuthCheck(QString,QNetworkReply*)));
 
     qDebug() << "# checking for authentication settings.";
-    ownCloudInfo::instance()->getRequest(QLatin1String("/"), true ); // this call needs to be authenticated.
+    ownCloudInfo::instance()->getWebDAVPath(QLatin1String("/") ); // this call needs to be authenticated.
     // simply GET the webdav root, will fail if credentials are wrong.
     // continue in slotAuthCheck here :-)
 }

src/mirall/folderman.cpp

@@ -18,6 +18,7 @@
 #include "mirall/syncresult.h"
 #include "mirall/inotify.h"
 #include "mirall/theme.h"
+#include "owncloudinfo.h"
 
 #ifdef Q_OS_MAC
 #include <CoreServices/CoreServices.h>
@@ -249,10 +250,8 @@ Folder* FolderMan::setupFolderFromConfigFile(const QString &file) {
     if (!backend.isEmpty()) {
 
         if( backend == QLatin1String("owncloud") ) {
-            MirallConfigFile cfgFile;
-
             // assemble the owncloud url to pass to csync, incl. webdav
-            QString oCUrl = cfgFile.ownCloudUrl( QString::null, true );
+            QString oCUrl = ownCloudInfo::instance()->webdavUrl( );
 
             // cut off the leading slash, oCUrl always has a trailing.
             if( targetPath.startsWith(QLatin1Char('/')) ) {

src/mirall/mirallconfigfile.cpp

@@ -228,19 +228,6 @@ bool MirallConfigFile::writePassword( const QString& passwd, const QString& conn
     return true;
 }
 
-// set the url, called from redirect handling.
-void MirallConfigFile::setOwnCloudUrl( const QString& connection, const QString & url )
-{
-    const QString file = configFile();
-
-    QSettings settings( file, QSettings::IniFormat);
-    settings.setIniCodec( "UTF-8" );
-    settings.beginGroup( connection );
-    settings.setValue( QLatin1String("url"), url );
-
-    settings.sync();
-}
-
 QByteArray MirallConfigFile::caCerts( )
 {
     QSettings settings( configFile(), QSettings::IniFormat );

src/mirall/mirallconfigfile.h

@@ -58,8 +58,6 @@ public:
     QString ownCloudUser( const QString& connection = QString() ) const;
     QString ownCloudUrl( const QString& connection = QString(), bool webdav = false ) const;
 
-    void setOwnCloudUrl(const QString &connection, const QString& );
-
     // the certs do not depend on a connection.
     QByteArray caCerts();
     void setCaCerts( const QByteArray& );

src/mirall/owncloudfolder.cpp

@@ -124,17 +124,16 @@ void ownCloudFolder::setProxy()
 {
     if( _csync_ctx ) {
         /* Store proxy */
-        MirallConfigFile cfgFile;
-        QUrl proxyUrl(cfgFile.ownCloudUrl());
+        QUrl proxyUrl(ownCloudInfo::instance()->webdavUrl());
         QList<QNetworkProxy> proxies = QNetworkProxyFactory::proxyForQuery(proxyUrl);
         // We set at least one in Application
         Q_ASSERT(proxies.count() > 0);
         QNetworkProxy proxy = proxies.first();
         if (proxy.type() == QNetworkProxy::NoProxy) {
-            qDebug() << "Passing NO proxy to csync for" << cfgFile.ownCloudUrl();
+            qDebug() << "Passing NO proxy to csync for" << proxyUrl;
         } else {
             qDebug() << "Passing" << proxy.hostName() << "of proxy type " << proxy.type()
-                     << " to csync for" << cfgFile.ownCloudUrl();
+                     << " to csync for" << proxyUrl;
         }
         int proxyPort = proxy.port();
 
@@ -241,8 +240,7 @@ bool ownCloudFolder::isBusy() const
 QString ownCloudFolder::secondPath() const
 {
     QString re(Folder::secondPath());
-    MirallConfigFile cfg;
-    QString ocUrl = cfg.ownCloudUrl(QString::null, true);
+    QString ocUrl = ownCloudInfo::instance()->webdavUrl();
     if (ocUrl.endsWith(QLatin1Char('/')))
         ocUrl.chop(1);
 

src/mirall/owncloudinfo.cpp

@@ -116,28 +116,32 @@ bool ownCloudInfo::isConfigured()
 
 QNetworkReply *ownCloudInfo::checkInstallation()
 {
+    _redirectCount = 0;
+    MirallConfigFile cfgFile(  _configHandle );
+    QUrl url ( cfgFile.ownCloudUrl( _connection ) +  QLatin1String("status.php") );
     /* No authentication required for this. */
-    return getRequest( QLatin1String("status.php"), false );
+    return getRequest(url);
 }
 
 QNetworkReply* ownCloudInfo::getWebDAVPath( const QString& path )
 {
-    return getRequest( path, true );
+    _redirectCount = 0;
+    QUrl url ( webdavUrl( _connection ) +  path );
+    QNetworkReply *reply = getRequest(url);
+    _directories[reply] = path;
+    return reply;
 }
 
-QNetworkReply* ownCloudInfo::getRequest( const QString& path, bool webdav )
+QNetworkReply* ownCloudInfo::getRequest( const QUrl& url )
 {
-    qDebug() << "Get Request to " << path;
+    qDebug() << "Get Request to " << url;
 
-    MirallConfigFile cfgFile(  _configHandle );
-    QString url = cfgFile.ownCloudUrl( _connection, webdav ) + path;
     QNetworkRequest request;
-    request.setUrl( QUrl( url ) );
+    request.setUrl( url );
     setupHeaders( request, 0 );
 
     QNetworkReply *reply = _manager->get( request );
     connect( reply, SIGNAL(finished()), SLOT(slotReplyFinished()));
-    _directories[reply] = path;
 
     if( !_configHandle.isEmpty() ) {
         qDebug() << "Setting config handle " << _configHandle;
@@ -155,7 +159,7 @@ QNetworkReply* ownCloudInfo::mkdirRequest( const QString& dir )
     qDebug() << "OCInfo Making dir " << dir;
 
     MirallConfigFile cfgFile( _configHandle );
-    QUrl url = QUrl( cfgFile.ownCloudUrl( _connection, true ) + dir );
+    QUrl url = QUrl( webdavUrl(_connection) + dir );
     QHttp::ConnectionMode conMode = QHttp::ConnectionModeHttp;
     if (url.scheme() == "https")
         conMode = QHttp::ConnectionModeHttps;
@@ -320,20 +324,6 @@ QList<QSslCertificate> ownCloudInfo::certificateChain() const
     return _certificateChain;
 }
 
-QUrl ownCloudInfo::redirectUrl(const QUrl& possibleRedirectUrl,
-                               const QUrl& oldRedirectUrl) const {
-    QUrl redirectUrl;
-    /*
-     * Check if the URL is empty and
-     * that we aren't being fooled into a infinite redirect loop.
-     */
-    if(!possibleRedirectUrl.isEmpty() &&
-       possibleRedirectUrl != oldRedirectUrl) {
-        redirectUrl = possibleRedirectUrl;
-    }
-    return redirectUrl;
-}
-
 //
 // There have been problems with the finish-signal coming from the networkmanager.
 // To avoid that, the reply-signals were connected and the data is taken from the
@@ -354,12 +344,17 @@ void ownCloudInfo::slotReplyFinished()
     }
 
     // Detect redirect url
-    QVariant possibleRedirUrl = reply->attribute(QNetworkRequest::RedirectionTargetAttribute);
+    QUrl possibleRedirUrl = reply->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl();
     /* We'll deduct if the redirection is valid in the redirectUrl function */
-    _urlRedirectedTo = redirectUrl( possibleRedirUrl.toUrl(),
-                                    _urlRedirectedTo );
 
-    if(!_urlRedirectedTo.isEmpty()) {
+
+    if (!possibleRedirUrl.isEmpty() && _redirectCount++  > 10) {
+        // Are we in a redirect loop
+        qDebug() << "Redirect loop while redirecting to" << possibleRedirUrl;
+        possibleRedirUrl.clear();
+    }
+
+    if(!possibleRedirUrl.isEmpty()) {
         QString configHandle;
 
         qDebug() << "Redirected to " << possibleRedirUrl;
@@ -374,22 +369,21 @@ void ownCloudInfo::slotReplyFinished()
         QString path = _directories[reply];
         qDebug() << "This path was redirected: " << path;
 
-        MirallConfigFile cfgFile( configHandle );
-        QString newUrl = _urlRedirectedTo.toString();
+        QString newUrl = possibleRedirUrl.toString();
         if( newUrl.endsWith( path )) {
             // cut off the trailing path
             newUrl.chop( path.length() );
-            cfgFile.setOwnCloudUrl( _connection, newUrl );
-
-            qDebug() << "Update the config file url to " << newUrl;
-            getRequest( path, false ); // FIXME: Redirect for webdav!
-            reply->deleteLater();
-            return;
+            _urlRedirectedTo = newUrl;
+            qDebug() << "Updated url to" << newUrl;
         } else {
             qDebug() << "WRN: Path is not part of the redirect URL. NO redirect.";
         }
+        getRequest( possibleRedirUrl );
+        reply->deleteLater();
+        _directories.remove(reply);
+        _configHandleMap.remove(reply);
+        return;
     }
-    _urlRedirectedTo.clear();
 
     // TODO: check if this is always the correct encoding
     const QString version = QString::fromUtf8( reply->readAll() );
@@ -455,15 +449,13 @@ void ownCloudInfo::slotReplyFinished()
         QString dir(QLatin1String("unknown"));
         if( _directories.contains(reply) ) {
             dir = _directories[reply];
-            _directories.remove(reply);
         }
 
         emit ownCloudDirExists( dir, reply );
     }
-    if( _configHandleMap.contains(reply)) {
-        _configHandleMap.remove(reply);
-    }
     reply->deleteLater();
+    _directories.remove(reply);
+    _configHandleMap.remove(reply);
 }
 
 void ownCloudInfo::resetSSLUntrust()
@@ -536,9 +528,7 @@ void ownCloudInfo::setCredentials( const QString& user, const QString& passwd,
 // ============================================================================
 void ownCloudInfo::setupHeaders( QNetworkRequest & req, quint64 size )
 {
-    MirallConfigFile cfgFile(_configHandle );
-
-    QUrl url( cfgFile.ownCloudUrl( QString::null, false ) );
+    QUrl url( webdavUrl() );
     qDebug() << "Setting up host header: " << url.host();
     req.setRawHeader( QByteArray("Host"), url.host().toUtf8() );
     req.setRawHeader( QByteArray("User-Agent"), Utility::userAgentString());
@@ -572,5 +562,14 @@ QNetworkReply* ownCloudInfo::davRequest(const QString& reqVerb,  QNetworkRequest
     }
 }
 #endif
+
+QString ownCloudInfo::webdavUrl(const QString &connection)
+{
+    if (!_urlRedirectedTo.isEmpty())
+        return _urlRedirectedTo.toString();
+
+    MirallConfigFile cfgFile(_configHandle );
+    return cfgFile.ownCloudUrl( connection, true );
 }
 
+}

src/mirall/owncloudinfo.h

@@ -47,12 +47,6 @@ public:
       */
     QNetworkReply* checkInstallation();
 
-    /**
-      * a general GET request to the ownCloud. If the second bool parameter is
-      * true, the WebDAV server is queried.
-      */
-    QNetworkReply* getRequest( const QString&, bool );
-
     /**
       * convenience: GET request to the WebDAV server.
       */
@@ -110,6 +104,12 @@ public:
     void setCredentials( const QString&, const QString&,
                          const QString& configHandle = QString::null );
 
+    /**
+     * returns the owncloud webdav url.
+     * It may be different from the one in the config if there was a HTTP redirection
+     */
+    QString webdavUrl(const QString& connection = QString());
+
 signals:
     // result signal with url- and version string.
     void ownCloudInfoFound( const QString&, const QString&, const QString&, const QString& );
@@ -138,7 +138,11 @@ protected slots:
 private:
     explicit ownCloudInfo();
 
-    QUrl redirectUrl(const QUrl&, const QUrl& ) const;
+    /**
+     * a general GET request to the ownCloud WebDAV.
+     */
+    QNetworkReply* getRequest( const QUrl &url);
+
 
     ~ownCloudInfo();
 
@@ -161,6 +165,7 @@ private:
     int                            _authAttempts;
     QMap<QString, oCICredentials>  _credentials;
     QMutex                         _certChainMutex;
+    int                            _redirectCount;
 };
 
 };

src/mirall/owncloudsetupwizard.cpp

@@ -329,7 +329,7 @@ void OwncloudSetupWizard::checkRemoteFolder()
 
     qDebug() << "# checking for authentication settings.";
     ownCloudInfo::instance()->setCustomConfigHandle(_configHandle);
-    _checkRemoteFolderRequest = ownCloudInfo::instance()->getRequest(_remoteFolder, true ); // this call needs to be authenticated.
+    _checkRemoteFolderRequest = ownCloudInfo::instance()->getWebDAVPath(_remoteFolder ); // this call needs to be authenticated.
     // continue in slotAuthCheckReply
 }