From 8e4272195971c80d05775b53935a63166ce96ac8 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 20 Jun 2013 16:56:25 +0200 Subject: [PATCH] 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 This is a revert of the revert 0bc9b6f44ecdfdaa191cb6ff1c9ee0cb1033e96e With small changes --- src/mirall/application.cpp | 2 +- src/mirall/connectionvalidator.cpp | 2 +- src/mirall/folderman.cpp | 5 +- src/mirall/mirallconfigfile.cpp | 17 +---- src/mirall/mirallconfigfile.h | 4 +- src/mirall/owncloudfolder.cpp | 10 ++- src/mirall/owncloudinfo.cpp | 101 ++++++++++++++++------------- src/mirall/owncloudinfo.h | 19 ++++-- src/mirall/owncloudsetupwizard.cpp | 2 +- 9 files changed, 78 insertions(+), 84 deletions(-) diff --git a/src/mirall/application.cpp b/src/mirall/application.cpp index cc2ec308f..2f6f6b7bc 100644 --- a/src/mirall/application.cpp +++ b/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 :-) } diff --git a/src/mirall/connectionvalidator.cpp b/src/mirall/connectionvalidator.cpp index 5303998ac..f034363de 100644 --- a/src/mirall/connectionvalidator.cpp +++ b/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 :-) } diff --git a/src/mirall/folderman.cpp b/src/mirall/folderman.cpp index 3dfe406f3..d999a8796 100644 --- a/src/mirall/folderman.cpp +++ b/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 @@ -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('/')) ) { diff --git a/src/mirall/mirallconfigfile.cpp b/src/mirall/mirallconfigfile.cpp index e25b6cd6c..cd3c0c860 100644 --- a/src/mirall/mirallconfigfile.cpp +++ b/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 ); @@ -282,9 +269,8 @@ void MirallConfigFile::removeConnection( const QString& connection ) * returns the configured owncloud url if its already configured, otherwise an empty * string. * The returned url always has a trailing hash. - * If webdav is true, the webdav-server url is returned. */ -QString MirallConfigFile::ownCloudUrl( const QString& connection, bool webdav ) const +QString MirallConfigFile::ownCloudUrl( const QString& connection) const { QString con( connection ); if( connection.isEmpty() ) con = defaultConnection(); @@ -296,7 +282,6 @@ QString MirallConfigFile::ownCloudUrl( const QString& connection, bool webdav ) QString url = settings.value( QLatin1String("url") ).toString(); if( ! url.isEmpty() ) { if( ! url.endsWith(QLatin1Char('/'))) url.append(QLatin1String("/")); - if( webdav ) url.append( QLatin1String("remote.php/webdav/") ); } qDebug() << "Returning configured owncloud url: " << url; diff --git a/src/mirall/mirallconfigfile.h b/src/mirall/mirallconfigfile.h index df9139383..3f4002f91 100644 --- a/src/mirall/mirallconfigfile.h +++ b/src/mirall/mirallconfigfile.h @@ -56,9 +56,7 @@ public: void removeConnection( const QString& connection = QString() ); QString ownCloudUser( const QString& connection = QString() ) const; - QString ownCloudUrl( const QString& connection = QString(), bool webdav = false ) const; - - void setOwnCloudUrl(const QString &connection, const QString& ); + QString ownCloudUrl( const QString& connection = QString() ) const; // the certs do not depend on a connection. QByteArray caCerts(); diff --git a/src/mirall/owncloudfolder.cpp b/src/mirall/owncloudfolder.cpp index 076b3f0a1..ad3c7f881 100644 --- a/src/mirall/owncloudfolder.cpp +++ b/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 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); diff --git a/src/mirall/owncloudinfo.cpp b/src/mirall/owncloudinfo.cpp index fd28ee0a8..b7a2d3722 100644 --- a/src/mirall/owncloudinfo.cpp +++ b/src/mirall/owncloudinfo.cpp @@ -27,6 +27,7 @@ #endif #define DEFAULT_CONNECTION QLatin1String("default"); +static const char WEBDAV_PATH[] = "remote.php/webdav/"; namespace Mirall { @@ -116,28 +117,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 +160,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; @@ -226,9 +231,8 @@ QNetworkReply* ownCloudInfo::mkdirRequest( const QString& dir ) { qDebug() << "OCInfo Making dir " << dir; _authAttempts = 0; - MirallConfigFile cfgFile( _configHandle ); QNetworkRequest req; - req.setUrl( QUrl( cfgFile.ownCloudUrl( _connection, true ) + dir ) ); + req.setUrl( QUrl( webdavUrl(_connection) + dir ) ); QNetworkReply *reply = davRequest(QLatin1String("MKCOL"), req, 0); // remember the confighandle used for this request @@ -320,20 +324,6 @@ QList 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; @@ -372,24 +367,28 @@ void ownCloudInfo::slotReplyFinished() } QString path = _directories[reply]; + if (path.isEmpty()) { + path = QLatin1String("status.php"); + } else { + path.prepend( QLatin1String(WEBDAV_PATH) ); + } qDebug() << "This path was redirected: " << path; - MirallConfigFile cfgFile( configHandle ); - QString newUrl = _urlRedirectedTo.toString(); - if( newUrl.endsWith( path )) { + QString newUrl = possibleRedirUrl.toString(); + if( !path.isEmpty() && 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; + getRequest( possibleRedirUrl ); } else { qDebug() << "WRN: Path is not part of the redirect URL. NO redirect."; } + 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 +454,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() @@ -539,9 +536,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( req.url() ); qDebug() << "Setting up host header: " << url.host(); req.setRawHeader( QByteArray("Host"), url.host().toUtf8() ); req.setRawHeader( QByteArray("User-Agent"), Utility::userAgentString()); @@ -575,5 +570,19 @@ QNetworkReply* ownCloudInfo::davRequest(const QString& reqVerb, QNetworkRequest } } #endif + +QString ownCloudInfo::webdavUrl(const QString &connection) +{ + QString url; + + if (!_urlRedirectedTo.isEmpty()) { + url = _urlRedirectedTo.toString(); + } else { + MirallConfigFile cfgFile(_configHandle ); + url = cfgFile.ownCloudUrl( connection ); + } + url.append( QLatin1String( WEBDAV_PATH ) ); + return url; } +} diff --git a/src/mirall/owncloudinfo.h b/src/mirall/owncloudinfo.h index 3f4f9c26a..243ee8529 100644 --- a/src/mirall/owncloudinfo.h +++ b/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 _credentials; QMutex _certChainMutex; + int _redirectCount; }; }; diff --git a/src/mirall/owncloudsetupwizard.cpp b/src/mirall/owncloudsetupwizard.cpp index 6645239e4..ae1927241 100644 --- a/src/mirall/owncloudsetupwizard.cpp +++ b/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 }