Allow Subsonic to follow redirects when logging in, as described in issue 3747

Fixes issue #3747

src/internet/subsonicservice.cpp

@@ -33,6 +33,8 @@ const char* SubsonicService::kApiClientName = "Clementine";
 const char* SubsonicService::kSongsTable = "subsonic_songs";
 const char* SubsonicService::kFtsTable = "subsonic_songs_fts";
 
+const int SubsonicService::kMaxRedirects = 10;
+
 SubsonicService::SubsonicService(Application* app, InternetModel* parent)
   : InternetService(kServiceName, app, parent, parent),
     network_(new QNetworkAccessManager(this)),
@@ -46,7 +48,8 @@ SubsonicService::SubsonicService(Application* app, InternetModel* parent)
     library_filter_(NULL),
     library_sort_model_(new QSortFilterProxyModel(this)),
     total_song_count_(0),
-    login_state_(LoginState_OtherError) {
+    login_state_(LoginState_OtherError),
+    redirect_count_(0) {
   app_->player()->RegisterUrlHandler(url_handler_);
 
   connect(scanner_, SIGNAL(ScanFinished()),
@@ -148,7 +151,7 @@ void SubsonicService::ReloadSettings() {
   QSettings s;
   s.beginGroup(kSettingsGroup);
 
-  server_ = s.value("server").toString();
+  UpdateServer(s.value("server").toString());
   username_ = s.value("username").toString();
   password_ = s.value("password").toString();
   usesslv3_ = s.value("usesslv3").toBool();
@@ -157,7 +160,7 @@ void SubsonicService::ReloadSettings() {
 }
 
 bool SubsonicService::IsConfigured() const {
-  return !server_.isEmpty() &&
+  return !configured_server_.isEmpty() &&
          !username_.isEmpty() &&
          !password_.isEmpty();
 }
@@ -181,7 +184,7 @@ void SubsonicService::Login() {
 
 void SubsonicService::Login(
     const QString& server, const QString& username, const QString& password, const bool& usesslv3) {
-  server_ = server;
+  UpdateServer(server);
   username_ = username;
   password_ = password;
   usesslv3_ = usesslv3;
@@ -196,7 +199,7 @@ void SubsonicService::Ping() {
 }
 
 QUrl SubsonicService::BuildRequestUrl(const QString& view) const {
-  QUrl url(server_ + "/rest/" + view + ".view");
+  QUrl url(working_server_ + "/rest/" + view + ".view");
   url.addQueryItem("v", kApiVersion);
   url.addQueryItem("c", kApiClientName);
   url.addQueryItem("u", username_);
@@ -204,6 +207,16 @@ QUrl SubsonicService::BuildRequestUrl(const QString& view) const {
   return url;
 }
 
+QUrl SubsonicService::ScrubUrl(const QUrl& url) {
+  QUrl return_url(url);
+  QString path = url.path();
+  int rest_location = path.lastIndexOf("/rest", -1, Qt::CaseInsensitive);
+  if (rest_location >= 0) {
+    return_url.setPath(path.left(rest_location));
+  }
+  return return_url;
+}
+
 QNetworkReply* SubsonicService::Send(const QUrl& url) {
   QNetworkRequest request(url);
   // Don't try and check the authenticity of the SSL certificate - it'll almost
@@ -271,8 +284,32 @@ void SubsonicService::OnPingFinished(QNetworkReply* reply) {
     QXmlStreamReader reader(reply);
     reader.readNextStartElement();
     QStringRef status = reader.attributes().value("status");
+    int http_status_code = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     if (status == "ok") {
       login_state_ = LoginState_Loggedin;
+    } else if (http_status_code >= 300 && http_status_code <= 399) {
+      // Received a redirect status code, follow up on it.
+      QUrl redirect_url =
+          reply->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl();
+      if (redirect_url.isEmpty()) {
+        qLog(Debug) << "Received HTTP code " << http_status_code << ", but no URL";
+        login_state_ = LoginState_RedirectNoUrl;
+      } else {
+        redirect_count_++;
+        qLog(Debug) << "Redirect receieved to "
+                    << redirect_url.toString(QUrl::RemoveQuery)
+                    << ", current redirect count is "
+                    << redirect_count_;
+        if (redirect_count_ <= kMaxRedirects) {
+          working_server_ = ScrubUrl(redirect_url).toString(QUrl::RemoveQuery);
+          Ping();
+          // To avoid the LoginStateChanged, as it will come from the recursive request.
+          return;
+        } else {
+          // Redirect limit exceeded
+          login_state_ = LoginState_RedirectLimitExceeded;
+        }
+      }
     } else {
       reader.readNextStartElement();
       int error = reader.attributes().value("code").toString().toInt();
@@ -309,6 +346,12 @@ void SubsonicService::ShowConfig() {
   app_->OpenSettingsDialogAtPage(SettingsDialog::Page_Subsonic);
 }
 
+void SubsonicService::UpdateServer(const QString& server) {
+  configured_server_ = server;
+  working_server_ = server;
+  redirect_count_ = 0;
+}
+
 
 const int SubsonicLibraryScanner::kAlbumChunkSize = 500;
 const int SubsonicLibraryScanner::kConcurrentRequests = 8;

src/internet/subsonicservice.h

@@ -38,6 +38,8 @@ class SubsonicService : public InternetService
     LoginState_Timeout,
     LoginState_SslError,
     LoginState_IncompleteCredentials,
+    LoginState_RedirectLimitExceeded,
+    LoginState_RedirectNoUrl,
   };
 
   enum ApiError {
@@ -66,21 +68,27 @@ class SubsonicService : public InternetService
   bool IsConfigured() const;
 
   QStandardItem* CreateRootItem();
-  void LazyPopulate(QStandardItem *item);
+  void LazyPopulate(QStandardItem* item);
-  void ShowContextMenu(const QPoint &global_pos);
+  void ShowContextMenu(const QPoint& global_pos);
   QWidget* HeaderWidget() const;
   void ReloadSettings();
 
   void Login();
   void Login(
-      const QString &server, const QString &username, const QString &password, const bool &usesslv3);
+      const QString& server,
+      const QString& username,
+      const QString& password,
+      const bool& usesslv3);
+
   LoginState login_state() const { return login_state_; }
 
   // Subsonic API methods
   void Ping();
 
   QUrl BuildRequestUrl(const QString& view) const;
-  // Convenience function to reduce QNetworkRequest/QSslConfiguration boilerplate
+  // Scrubs the part of the path that we re-add in BuildRequestUrl().
+  static QUrl ScrubUrl(const QUrl& url);
+  // Convenience function to reduce QNetworkRequest/QSslConfiguration boilerplate.
   QNetworkReply* Send(const QUrl& url);
 
   static const char* kServiceName;
@@ -91,11 +99,15 @@ class SubsonicService : public InternetService
   static const char* kSongsTable;
   static const char* kFtsTable;
 
+  static const int kMaxRedirects;
+
  signals:
   void LoginStateChanged(SubsonicService::LoginState newstate);
 
  private:
   void EnsureMenuCreated();
+  // Update configured and working server state
+  void UpdateServer(const QString& server);
 
   QNetworkAccessManager* network_;
   SubsonicUrlHandler* url_handler_;
@@ -113,12 +125,15 @@ class SubsonicService : public InternetService
   int total_song_count_;
 
   // Configuration
-  QString server_;
+  // The server that shows up in the GUI (use UpdateServer() to update)
+  QString configured_server_;
   QString username_;
   QString password_;
   bool usesslv3_;
 
   LoginState login_state_;
+  QString working_server_; // The actual server, possibly post-redirect
+  int redirect_count_;
 
  private slots:
   void UpdateTotalSongCount(int count);

src/internet/subsonicsettingspage.cpp

@@ -127,6 +127,16 @@ void SubsonicSettingsPage::LoginStateChanged(SubsonicService::LoginState newstat
     ui_->login_state->SetAccountTypeText(tr("Incomplete configuration, please ensure all fields are populated."));
     break;
 
+  case SubsonicService::LoginState_RedirectLimitExceeded:
+    ui_->login_state->SetAccountTypeText(tr("Redirect limit exceeded, verify server configuration."));
+    break;
+
+  case SubsonicService::LoginState_RedirectNoUrl:
+    ui_->login_state->SetAccountTypeText(tr("HTTP 3xx status code received without URL, "
+                                            "verify server configuration."));
+    break;
+
+
   default:
     break;
   }
@@ -141,12 +151,8 @@ void SubsonicSettingsPage::ServerEditingFinished() {
     return;
   }
 
-  // A direct paste of the server URL will probably include the trailing index.view, so remove it
+  // If the user specified a /rest location, remove it since we're going to re-add it later
-  if (url.path().endsWith("index.view")) {
+  url = SubsonicService::ScrubUrl(url);
-    QString newpath = url.path();
-    newpath.chop(10);
-    url.setPath(newpath);
-  }
 
   ui_->server->setText(url.toString());
   qLog(Debug) << "URL fixed:" << input << "to" << url;