Do not report OpenSubsonic extra fields for selected legacy clients. This made DSub behave badly. fixes #356

conf/lms.conf

@@ -63,6 +63,9 @@ api-subsonic = true;
 # Main usage is to make auto detections for the 'p' (password) parameter work
 api-subsonic-report-old-server-protocol = ("DSub");
 
+# List of clients for whom open subsonic extensions and extra fields are disabled
+api-open-subsonic-disabled-clients = ("DSub");
+
 # Turn on this option to allow the demo account creation/use
 demo = false;
 

src/libs/subsonic/impl/RequestContext.hpp

@@ -41,6 +41,7 @@ namespace API::Subsonic
         Database::UserId userId;
         ClientInfo clientInfo;
         ProtocolVersion serverProtocolVersion;
+        bool enableOpenSubsonic{ true };
     };
 }
 

src/libs/subsonic/impl/SubsonicResource.cpp

@@ -75,6 +75,18 @@ namespace API::Subsonic
             return res;
         }
 
+        std::unordered_set<std::string> readOpenSubsonicDisabledClients()
+        {
+            std::unordered_set<std::string> res;
+
+            Service<IConfig>::get()->visitStrings("api-open-subsonic-disabled-clients",
+                [&](std::string_view client)
+                {
+                    res.emplace(std::string{ client });
+                }, { "DSub" });
+
+            return res;
+        }
 
         std::string parameterMapToDebugString(const Wt::Http::ParameterMap& parameterMap)
         {
@@ -253,6 +265,7 @@ namespace API::Subsonic
 
     SubsonicResource::SubsonicResource(Db& db)
         : _serverProtocolVersionsByClient{ readConfigProtocolVersions() }
+        , _openSubsonicDisabledClients{ readOpenSubsonicDisabledClients() }
         , _db{ db }
     {
     }
@@ -273,13 +286,13 @@ namespace API::Subsonic
         const ResponseFormat format{ getParameterAs<std::string>(request.getParameterMap(), "f").value_or("xml") == "json" ? ResponseFormat::json : ResponseFormat::xml };
 
         ProtocolVersion protocolVersion{ defaultServerProtocolVersion };
-
+        
         try
         {
             // We need to parse client a soon as possible to make sure to answer with the right protocol version
             protocolVersion = getServerProtocolVersion(getMandatoryParameterAs<std::string>(request.getParameterMap(), "c"));
             RequestContext requestContext{ buildRequestContext(request) };
-
+            
             auto itEntryPoint{ requestEntryPoints.find(requestPath) };
             if (itEntryPoint != requestEntryPoints.end())
             {
@@ -364,8 +377,9 @@ namespace API::Subsonic
         const Wt::Http::ParameterMap& parameters{ request.getParameterMap() };
         const ClientInfo clientInfo{ getClientInfo(parameters) };
         const Database::UserId userId{ authenticateUser(request, clientInfo) };
+        bool enableOpenSubsonic{ _openSubsonicDisabledClients.find(clientInfo.name) == std::cend(_openSubsonicDisabledClients) };
 
-        return { parameters, _db.getTLSSession(), userId, clientInfo, getServerProtocolVersion(clientInfo.name) };
+        return { parameters, _db.getTLSSession(), userId, clientInfo, getServerProtocolVersion(clientInfo.name), enableOpenSubsonic };
     }
 
     Database::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request, const ClientInfo& clientInfo)

src/libs/subsonic/impl/SubsonicResource.hpp

@@ -19,6 +19,7 @@
 #pragma once
 
 #include <string>
+#include <unordered_set>
 #include <unordered_map>
 
 #include <Wt/WResource.h>
@@ -51,6 +52,7 @@ namespace API::Subsonic
             Database::UserId authenticateUser(const Wt::Http::Request& request, const ClientInfo& clientInfo);
 
             const std::unordered_map<std::string, ProtocolVersion> _serverProtocolVersionsByClient;
+            const std::unordered_set<std::string> _openSubsonicDisabledClients;
             Database::Db& _db;
     };
 

src/libs/subsonic/impl/SubsonicResponse.cpp

@@ -148,6 +148,7 @@ namespace API::Subsonic
         }
 
         // OpenSubsonic mandatory fields
+        // No big deal to send them even for legacy clients
         responseNode.setAttribute("type", "lms");
         responseNode.setAttribute("serverVersion", serverVersion);
         responseNode.setAttribute("openSubsonic", true);

src/libs/subsonic/impl/entrypoints/AlbumSongLists.cpp

@@ -37,8 +37,9 @@ namespace API::Subsonic
 {
     using namespace Database;
 
-    namespace {
-        Response handleGetAlbumListRequestCommon(const RequestContext& context, bool id3)
+    namespace
+    {
+        Response handleGetAlbumListRequestCommon(RequestContext& context, bool id3)
         {
             // Mandatory params
             const std::string type{ getMandatoryParameterAs<std::string>(context.parameters, "type") };
@@ -140,7 +141,7 @@ namespace API::Subsonic
             for (const ReleaseId releaseId : releases.results)
             {
                 const Release::pointer release{ Release::find(context.dbSession, releaseId) };
-                albumListNode.addArrayChild("album", createAlbumNode(release, context.dbSession, user, id3));
+                albumListNode.addArrayChild("album", createAlbumNode(context, release, user, id3));
             }
 
             return response;
@@ -162,19 +163,19 @@ namespace API::Subsonic
             for (const ArtistId artistId : feedbackService.getStarredArtists(context.userId, {} /* clusters */, std::nullopt /* linkType */, ArtistSortMethod::BySortName, Range{}).results)
             {
                 if (auto artist{ Artist::find(context.dbSession, artistId) })
-                    starredNode.addArrayChild("artist", createArtistNode(artist, context.dbSession, user, id3));
+                    starredNode.addArrayChild("artist", createArtistNode(context, artist, user, id3));
             }
 
             for (const ReleaseId releaseId : feedbackService.getStarredReleases(context.userId, {} /* clusters */, Range{}).results)
             {
                 if (auto release{ Release::find(context.dbSession, releaseId) })
-                    starredNode.addArrayChild("album", createAlbumNode(release, context.dbSession, user, id3));
+                    starredNode.addArrayChild("album", createAlbumNode(context, release, user, id3));
             }
 
             for (const TrackId trackId : feedbackService.getStarredTracks(context.userId, {} /* clusters */, Range{}).results)
             {
                 if (auto track{ Track::find(context.dbSession, trackId) })
-                    starredNode.addArrayChild("song", createSongNode(track, context.dbSession, user));
+                    starredNode.addArrayChild("song", createSongNode(context, track, user));
             }
 
             return response;
@@ -211,7 +212,7 @@ namespace API::Subsonic
         for (const TrackId trackId : trackIds.results)
         {
             const Track::pointer track{ Track::find(context.dbSession, trackId) };
-            randomSongsNode.addArrayChild("song", createSongNode(track, context.dbSession, user));
+            randomSongsNode.addArrayChild("song", createSongNode(context, track, user));
         }
 
         return response;
@@ -253,7 +254,7 @@ namespace API::Subsonic
         for (const TrackId trackId : trackIds.results)
         {
             const Track::pointer track{ Track::find(context.dbSession, trackId) };
-            songsByGenreNode.addArrayChild("song", createSongNode(track, context.dbSession, user));
+            songsByGenreNode.addArrayChild("song", createSongNode(context, track, user));
         }
 
         return response;

src/libs/subsonic/impl/entrypoints/Bookmarks.cpp

@@ -49,7 +49,7 @@ namespace API::Subsonic
         {
             const TrackBookmark::pointer bookmark{ TrackBookmark::find(context.dbSession, bookmarkId) };
             Response::Node bookmarkNode{ createBookmarkNode(bookmark) };
-            bookmarkNode.addArrayChild("entry", createSongNode(bookmark->getTrack(), context.dbSession, user));
+            bookmarkNode.addArrayChild("entry", createSongNode(context, bookmark->getTrack(), user));
 
             bookmarksNode.addArrayChild("bookmark", std::move(bookmarkNode));
         }

src/libs/subsonic/impl/entrypoints/Browsing.cpp

@@ -82,7 +82,7 @@ namespace API::Subsonic
                 {
                     const Artist::pointer similarArtist{ Artist::find(context.dbSession, similarArtistId) };
                     if (similarArtist)
-                        artistInfoNode.addArrayChild("similarArtist", createArtistNode(similarArtist, context.dbSession, user, id3));
+                        artistInfoNode.addArrayChild("similarArtist", createArtistNode(context, similarArtist, user, id3));
                 }
             }
 
@@ -166,7 +166,7 @@ namespace API::Subsonic
                         throw UserNotAuthorizedError{};
 
                     if (const Artist::pointer artist{ Artist::find(context.dbSession, artistId) })
-                        indexNode.addArrayChild("artist", createArtistNode(artist, context.dbSession, user, id3));
+                        indexNode.addArrayChild("artist", createArtistNode(context, artist, user, id3));
                 }
             }
 
@@ -268,7 +268,7 @@ namespace API::Subsonic
             for (const TrackId trackId : tracks)
             {
                 const Track::pointer track{ Track::find(context.dbSession, trackId) };
-                similarSongsNode.addArrayChild("song", createSongNode(track, context.dbSession, user));
+                similarSongsNode.addArrayChild("song", createSongNode(context, track, user));
             }
 
             return response;
@@ -321,7 +321,7 @@ namespace API::Subsonic
             for (const ArtistId rootArtistId : rootArtistIds.results)
             {
                 const Artist::pointer artist{ Artist::find(context.dbSession, rootArtistId) };
-                directoryNode.addArrayChild("child", createArtistNode(artist, context.dbSession, user, false /* no id3 */));
+                directoryNode.addArrayChild("child", createArtistNode(context, artist, user, false /* no id3 */));
             }
         }
         else if (artistId)
@@ -338,7 +338,7 @@ namespace API::Subsonic
             for (const ReleaseId artistReleaseId : artistReleases.results)
             {
                 const Release::pointer release{ Release::find(context.dbSession, artistReleaseId) };
-                directoryNode.addArrayChild("child", createAlbumNode(release, context.dbSession, user, false /* no id3 */));
+                directoryNode.addArrayChild("child", createAlbumNode(context, release, user, false /* no id3 */));
             }
         }
         else if (releaseId)
@@ -355,7 +355,7 @@ namespace API::Subsonic
             for (const TrackId trackId : tracks.results)
             {
                 const Track::pointer track{ Track::find(context.dbSession, trackId) };
-                directoryNode.addArrayChild("child", createSongNode(track, context.dbSession, user));
+                directoryNode.addArrayChild("child", createSongNode(context, track, user));
             }
         }
         else
@@ -405,13 +405,13 @@ namespace API::Subsonic
             throw UserNotAuthorizedError{};
 
         Response response{ Response::createOkResponse(context.serverProtocolVersion) };
-        Response::Node artistNode{ createArtistNode(artist, context.dbSession, user, true /* id3 */) };
+        Response::Node artistNode{ createArtistNode(context, artist, user, true /* id3 */) };
 
         const auto releases{ Release::find(context.dbSession, Release::FindParameters {}.setArtist(artist->getId())) };
         for (const ReleaseId releaseId : releases.results)
         {
             const Release::pointer release{ Release::find(context.dbSession, releaseId) };
-            artistNode.addArrayChild("album", createAlbumNode(release, context.dbSession, user, true /* id3 */));
+            artistNode.addArrayChild("album", createAlbumNode(context, release, user, true /* id3 */));
         }
 
         response.addNode("artist", std::move(artistNode));
@@ -435,13 +435,13 @@ namespace API::Subsonic
             throw UserNotAuthorizedError{};
 
         Response response{ Response::createOkResponse(context.serverProtocolVersion) };
-        Response::Node albumNode{ createAlbumNode(release, context.dbSession, user, true /* id3 */) };
+        Response::Node albumNode{ createAlbumNode(context, release, user, true /* id3 */) };
 
         const auto tracks{ Track::find(context.dbSession, Track::FindParameters {}.setRelease(id).setSortMethod(TrackSortMethod::Release)) };
         for (const TrackId trackId : tracks.results)
         {
             const Track::pointer track{ Track::find(context.dbSession, trackId) };
-            albumNode.addArrayChild("song", createSongNode(track, context.dbSession, user));
+            albumNode.addArrayChild("song", createSongNode(context, track, user));
         }
 
         response.addNode("album", std::move(albumNode));
@@ -465,7 +465,7 @@ namespace API::Subsonic
             throw UserNotAuthorizedError{};
 
         Response response{ Response::createOkResponse(context.serverProtocolVersion) };
-        response.addNode("song", createSongNode(track, context.dbSession, user));
+        response.addNode("song", createSongNode(context, track, user));
 
         return response;
     }

src/libs/subsonic/impl/entrypoints/Playlists.cpp

@@ -73,7 +73,7 @@ namespace API::Subsonic
 
         auto entries{ tracklist->getEntries() };
         for (const TrackListEntry::pointer& entry : entries)
-            playlistNode.addArrayChild("entry", createSongNode(entry->getTrack(), context.dbSession, user));
+            playlistNode.addArrayChild("entry", createSongNode(context, entry->getTrack(), user));
 
         response.addNode("playlist", std::move(playlistNode));
 

src/libs/subsonic/impl/entrypoints/Searching.cpp

@@ -77,7 +77,7 @@ namespace API::Subsonic
                 for (const ArtistId artistId : artistIds.results)
                 {
                     const auto artist{ Artist::find(context.dbSession, artistId) };
-                    searchResult2Node.addArrayChild("artist", createArtistNode(artist, context.dbSession, user, id3));
+                    searchResult2Node.addArrayChild("artist", createArtistNode(context, artist, user, id3));
                 }
             }
 
@@ -92,7 +92,7 @@ namespace API::Subsonic
                 for (const ReleaseId releaseId : releaseIds.results)
                 {
                     const auto release{ Release::find(context.dbSession, releaseId) };
-                    searchResult2Node.addArrayChild("album", createAlbumNode(release, context.dbSession, user, id3));
+                    searchResult2Node.addArrayChild("album", createAlbumNode(context, release, user, id3));
                 }
             }
 
@@ -106,7 +106,7 @@ namespace API::Subsonic
                 for (const TrackId trackId : trackIds.results)
                 {
                     const auto track{ Track::find(context.dbSession, trackId) };
-                    searchResult2Node.addArrayChild("song", createSongNode(track, context.dbSession, user));
+                    searchResult2Node.addArrayChild("song", createSongNode(context, track, user));
                 }
             }
 

src/libs/subsonic/impl/responses/Album.cpp

@@ -74,7 +74,7 @@ namespace API::Subsonic
         }
     }
 
-    Response::Node createAlbumNode(const Release::pointer& release, Session& dbSession, const User::pointer& user, bool id3)
+    Response::Node createAlbumNode(RequestContext& context, const Release::pointer& release, const User::pointer& user, bool id3)
     {
         Response::Node albumNode;
 
@@ -125,7 +125,7 @@ namespace API::Subsonic
         }
 
         // Report the first GENRE for this track
-        const ClusterType::pointer genreClusterType{ ClusterType::find(dbSession, "GENRE") };
+        const ClusterType::pointer genreClusterType{ ClusterType::find(context.dbSession, "GENRE") };
         if (genreClusterType)
         {
             auto clusters{ release->getClusterGroups({genreClusterType}, 1) };
@@ -136,6 +136,9 @@ namespace API::Subsonic
         if (const Wt::WDateTime dateTime{ Service<Feedback::IFeedbackService>::get()->getStarredDateTime(user->getId(), release->getId()) }; dateTime.isValid())
             albumNode.setAttribute("starred", StringUtils::toISO8601String(dateTime));
 
+        if (!context.enableOpenSubsonic)
+            return albumNode;
+
         // OpenSubsonic specific fields (must always be set)
         if (!id3)
             albumNode.setAttribute("mediaType", "album");
@@ -154,14 +157,14 @@ namespace API::Subsonic
         {
             albumNode.createEmptyArrayValue(field);
 
-            ClusterType::pointer clusterType{ ClusterType::find(dbSession, clusterTypeName) };
+            ClusterType::pointer clusterType{ ClusterType::find(context.dbSession, clusterTypeName) };
             if (clusterType)
             {
                 Cluster::FindParameters params;
                 params.setRelease(release->getId());
                 params.setClusterType(clusterType->getId());
 
-                for (const auto& cluster : Cluster::find(dbSession, params).results)
+                for (const auto& cluster : Cluster::find(context.dbSession, params).results)
                     albumNode.addArrayValue(field, std::get<std::string>(cluster));
             }
         } };
@@ -176,7 +179,7 @@ namespace API::Subsonic
             params.setRelease(release->getId());
             params.setClusterType(genreClusterType->getId());
 
-            for (const auto& cluster : Cluster::find(dbSession, params).results)
+            for (const auto& cluster : Cluster::find(context.dbSession, params).results)
                 albumNode.addArrayChild("genres", createItemGenreNode(std::get<std::string>(cluster)));
         }
 

src/libs/subsonic/impl/responses/Album.hpp

@@ -31,5 +31,5 @@ namespace Database
 
 namespace API::Subsonic
 {
-    Response::Node createAlbumNode(const Database::ObjectPtr<Database::Release>& release, Database::Session& dbSession, const Database::ObjectPtr<Database::User>& user, bool id3);
+    Response::Node createAlbumNode(RequestContext& context, const Database::ObjectPtr<Database::Release>& release, const Database::ObjectPtr<Database::User>& user, bool id3);
 }

src/libs/subsonic/impl/responses/Artist.cpp

@@ -73,7 +73,7 @@ namespace API::Subsonic
         }
     }
 
-    Response::Node createArtistNode(const Artist::pointer& artist, Session& session, const User::pointer& user, bool id3)
+    Response::Node createArtistNode(RequestContext& context, const Artist::pointer& artist, const User::pointer& user, bool id3)
     {
         Response::Node artistNode{ createArtistNode(artist) };
 
@@ -82,7 +82,7 @@ namespace API::Subsonic
 
         if (id3)
         {
-            const auto releases{ Release::find(session, Release::FindParameters {}.setArtist(artist->getId())) };
+            const auto releases{ Release::find(context.dbSession, Release::FindParameters {}.setArtist(artist->getId())) };
             artistNode.setAttribute("albumCount", releases.results.size());
         }
 
@@ -90,22 +90,25 @@ namespace API::Subsonic
             artistNode.setAttribute("starred", StringUtils::toISO8601String(dateTime));
 
         // OpenSubsonic specific fields (must always be set)
-        if (!id3)
-            artistNode.setAttribute("mediaType", "artist");
-
+        if (context.enableOpenSubsonic)
         {
-            std::optional<UUID> mbid{ artist->getMBID() };
-            artistNode.setAttribute("musicBrainzId", mbid ? mbid->getAsString() : "");
+            if (!id3)
+                artistNode.setAttribute("mediaType", "artist");
+
+            {
+                std::optional<UUID> mbid{ artist->getMBID() };
+                artistNode.setAttribute("musicBrainzId", mbid ? mbid->getAsString() : "");
+            }
+
+            artistNode.setAttribute("sortName", artist->getSortName());
+
+            // roles
+            Response::Node roles;
+            artistNode.createEmptyArrayValue("roles");
+            for (const TrackArtistLinkType linkType : TrackArtistLink::findUsedTypes(context.dbSession, artist->getId()))
+                artistNode.addArrayValue("roles", Utils::toString(linkType));
         }
 
-        artistNode.setAttribute("sortName", artist->getSortName());
-
-        // roles
-        Response::Node roles;
-        artistNode.createEmptyArrayValue("roles");
-        for (const TrackArtistLinkType linkType : TrackArtistLink::findUsedTypes(session, artist->getId()))
-            artistNode.addArrayValue("roles", Utils::toString(linkType));
-
         return artistNode;
     }
 

src/libs/subsonic/impl/responses/Artist.hpp

@@ -39,6 +39,6 @@ namespace API::Subsonic
         std::string joinArtistNames(const std::vector<Database::ObjectPtr<Database::Artist>>& artists);
         std::string_view toString(Database::TrackArtistLinkType type);
     }
-    Response::Node createArtistNode(const Database::ObjectPtr<Database::Artist>& artist, Database::Session& session, const Database::ObjectPtr<Database::User>& user, bool id3);
+    Response::Node createArtistNode(RequestContext& context, const Database::ObjectPtr<Database::Artist>& artist, const Database::ObjectPtr<Database::User>& user, bool id3);
     Response::Node createArtistNode(const Database::ObjectPtr<Database::Artist>& artist); // only minimal info
 }

src/libs/subsonic/impl/responses/Song.cpp

@@ -93,7 +93,7 @@ namespace API::Subsonic
         }
     }
 
-    Response::Node createSongNode(const Track::pointer& track, Session& dbSession, const User::pointer& user)
+    Response::Node createSongNode(RequestContext& context, const Track::pointer& track, const User::pointer& user)
     {
         Response::Node trackResponse;
 
@@ -153,7 +153,7 @@ namespace API::Subsonic
             trackResponse.setAttribute("starred", StringUtils::toISO8601String(dateTime));
 
         // Report the first GENRE for this track
-        const ClusterType::pointer genreClusterType{ ClusterType::find(dbSession, "GENRE") };
+        const ClusterType::pointer genreClusterType{ ClusterType::find(context.dbSession, "GENRE") };
         if (genreClusterType)
         {
             auto clusters{ track->getClusterGroups({genreClusterType}, 1) };
@@ -162,6 +162,9 @@ namespace API::Subsonic
         }
 
         // OpenSubsonic specific fields (must always be set)
+        if (!context.enableOpenSubsonic)
+            return trackResponse;
+
         trackResponse.setAttribute("mediaType", "song");
 
         {
@@ -179,9 +182,9 @@ namespace API::Subsonic
             TrackArtistLink::FindParameters params;
             params.setTrack(track->getId());
 
-            for (const TrackArtistLinkId linkId : TrackArtistLink::find(dbSession, params).results)
+            for (const TrackArtistLinkId linkId : TrackArtistLink::find(context.dbSession, params).results)
             {
-                TrackArtistLink::pointer link{ TrackArtistLink::find(dbSession, linkId) };
+                TrackArtistLink::pointer link{ TrackArtistLink::find(context.dbSession, linkId) };
                 // Don't report artists nor release artists as they are set in dedicated fields
                 if (link && link->getType() != TrackArtistLinkType::Artist && link->getType() != TrackArtistLinkType::ReleaseArtist)
                     trackResponse.addArrayChild("contributors", createContributorNode(link));
@@ -196,9 +199,9 @@ namespace API::Subsonic
             params.setTrack(track->getId());
             params.setLinkType(type);
 
-            for (const TrackArtistLinkId linkId : TrackArtistLink::find(dbSession, params).results)
+            for (const TrackArtistLinkId linkId : TrackArtistLink::find(context.dbSession, params).results)
             {
-                TrackArtistLink::pointer link{ TrackArtistLink::find(dbSession, linkId) };
+                TrackArtistLink::pointer link{ TrackArtistLink::find(context.dbSession, linkId) };
                 if (link)
                     trackResponse.addArrayChild(nodeName, createArtistNode(link->getArtist()));
             }
@@ -215,14 +218,14 @@ namespace API::Subsonic
         {
             trackResponse.createEmptyArrayValue(field);
 
-            ClusterType::pointer clusterType{ ClusterType::find(dbSession, clusterTypeName) };
+            ClusterType::pointer clusterType{ ClusterType::find(context.dbSession, clusterTypeName) };
             if (clusterType)
             {
                 Cluster::FindParameters params;
                 params.setTrack(track->getId());
                 params.setClusterType(clusterType->getId());
 
-                for (const auto& cluster : Cluster::find(dbSession, params).results)
+                for (const auto& cluster : Cluster::find(context.dbSession, params).results)
                     trackResponse.addArrayValue(field, std::get<std::string>(cluster));
             }
         } };
@@ -237,7 +240,7 @@ namespace API::Subsonic
             params.setTrack(track->getId());
             params.setClusterType(genreClusterType->getId());
 
-            for (const auto& cluster : Cluster::find(dbSession, params).results)
+            for (const auto& cluster : Cluster::find(context.dbSession, params).results)
                 trackResponse.addArrayChild("genres", createItemGenreNode(std::get<std::string>(cluster)));
         }
 

src/libs/subsonic/impl/responses/Song.hpp

@@ -31,5 +31,5 @@ namespace Database
 
 namespace API::Subsonic
 {
-    Response::Node createSongNode(const Database::ObjectPtr<Database::Track>& track, Database::Session& session, const Database::ObjectPtr<Database::User>& user);
+    Response::Node createSongNode(RequestContext& context, const Database::ObjectPtr<Database::Track>& track, const Database::ObjectPtr<Database::User>& user);
 }