From f9ae188be38a5e58d76213b0488a8cca4b66e167 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 10 Mar 2026 15:51:49 +0100 Subject: [PATCH] Subsonic API: fixed unwanted termination when bad client parameters are received --- src/libs/subsonic/impl/RequestContext.cpp | 1 - src/libs/subsonic/impl/RequestContext.hpp | 1 - src/libs/subsonic/impl/SubsonicResource.cpp | 141 +++++++++++--------- src/libs/subsonic/impl/SubsonicResource.hpp | 6 +- 4 files changed, 80 insertions(+), 69 deletions(-) diff --git a/src/libs/subsonic/impl/RequestContext.cpp b/src/libs/subsonic/impl/RequestContext.cpp index 8ca811ac..755aac4b 100644 --- a/src/libs/subsonic/impl/RequestContext.cpp +++ b/src/libs/subsonic/impl/RequestContext.cpp @@ -107,5 +107,4 @@ namespace lms::api::subsonic { return _isOpenSubsonicEnabled; } - } // namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/RequestContext.hpp b/src/libs/subsonic/impl/RequestContext.hpp index c5cc1568..fe4c496c 100644 --- a/src/libs/subsonic/impl/RequestContext.hpp +++ b/src/libs/subsonic/impl/RequestContext.hpp @@ -77,5 +77,4 @@ namespace lms::api::subsonic const ProtocolVersion _serverProtocolVersion; const bool _isOpenSubsonicEnabled; }; - } // namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index e9d5f7fe..21df8265 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -36,7 +36,6 @@ #include "services/auth/IPasswordService.hpp" #include "ParameterParsing.hpp" -#include "ProtocolVersion.hpp" #include "RequestContext.hpp" #include "SubsonicResponse.hpp" #include "endpoints/AlbumSongLists.hpp" @@ -291,91 +290,48 @@ namespace lms::api::subsonic const std::size_t requestId{ curRequestId++ }; TLSMonotonicMemoryResourceCleaner memoryResourceCleaner; - LMS_LOG(API_SUBSONIC, DEBUG, "Handling request " << requestId << " '" << request.pathInfo() << "', continuation = " << (request.continuation() ? "true" : "false") << ", params = " << parameterMapToDebugString(request.getParameterMap())); - + constexpr std::string_view optionalSuffix{ ".view" }; std::string requestPath{ request.pathInfo() }; - if (core::stringUtils::stringEndsWith(requestPath, ".view")) - requestPath.resize(requestPath.length() - 5); + if (core::stringUtils::stringEndsWith(requestPath, optionalSuffix)) + requestPath.resize(requestPath.length() - optionalSuffix.size()); - RequestContext requestContext{ request, _db.getTLSSession(), _config }; + LMS_LOG(API_SUBSONIC, DEBUG, "Handling request " << requestId << " to '" << requestPath << " with params = " << parameterMapToDebugString(request.getParameterMap()) << "', continuation = " << (request.continuation() ? "true" : "false")); - // First check for media retrieval endpoints - auto itStreamHandler{ mediaRetrievalHandlers.find(requestPath) }; - if (itStreamHandler != mediaRetrievalHandlers.end()) - { - try - { - LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itStreamHandler->first); - handleMediaRetrievalRequest(itStreamHandler->second, requestContext, request, response); - LMS_LOG(API_SUBSONIC, DEBUG, "Request " << requestId << " '" << requestPath << "' handled!"); - } - catch (const Error& e) - { - LMS_LOG(API_SUBSONIC, ERROR, "Error while processing request '" << requestId << "', code = " << static_cast(e.getCode()) << ", msg = '" << e.getMessage() << "'"); - } - - return; - } - - // Now check other endpoints try { - if (auto itEntryPoint{ requestEntryPoints.find(requestPath) }; itEntryPoint != requestEntryPoints.end()) - { - LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itEntryPoint->first); + if (!handleMediaRetrievalRequest(requestPath, request, response)) + handleRequest(requestPath, request, response); - db::User::pointer user; - if (itEntryPoint->second.authMode == AuthenticationMode::Authenticated) - { - user = getUserFromUserId(_db.getTLSSession(), authenticateUser(request)); - checkUserTypeIsAllowed(user, itEntryPoint->second.allowedUserTypes); - requestContext.setUser(user); - } - - const Response resp{ [&] { - LMS_SCOPED_TRACE_DETAILED("Subsonic", "HandleRequest"); - return itEntryPoint->second.func(requestContext); - }() }; - - { - LMS_SCOPED_TRACE_DETAILED("Subsonic", "WriteResponse"); - - resp.write(response.out(), requestContext.getResponseFormat()); - response.setMimeType(std::string{ ResponseFormatToMimeType(requestContext.getResponseFormat()) }); - } - - LMS_LOG(API_SUBSONIC, DEBUG, "Request " << requestId << " '" << requestPath << "' handled!"); - return; - } - - // do not disclose unhandled commands for unauthenticated users - authenticateUser(request); - - LMS_LOG(API_SUBSONIC, ERROR, "Unhandled command '" << requestPath << "'"); - throw UnknownEntryPointGenericError{}; + LMS_LOG(API_SUBSONIC, DEBUG, "Request " << requestId << " to '" << requestPath << "' handled!"); } catch (const Error& e) { - LMS_LOG(API_SUBSONIC, ERROR, "Error while processing request '" << requestPath << "'" << ", params = [" << parameterMapToDebugString(request.getParameterMap()) << "]" << ", code = " << static_cast(e.getCode()) << ", msg = '" << e.getMessage() << "'"); - Response resp{ Response::createFailedResponse(requestContext.getServerProtocolVersion(), e) }; - resp.write(response.out(), requestContext.getResponseFormat()); - response.setMimeType(std::string{ ResponseFormatToMimeType(requestContext.getResponseFormat()) }); + LMS_LOG(API_SUBSONIC, ERROR, "Error while processing request " << requestId << " to '" << requestPath << "' with params = " << parameterMapToDebugString(request.getParameterMap()) << ": code = " << static_cast(e.getCode()) << ", msg = '" << e.getMessage() << "'"); } } - void SubsonicResource::handleMediaRetrievalRequest(const MediaRetrievalHandlerFunc& handler, RequestContext& requestContext, const Wt::Http::Request& request, Wt::Http::Response& response) + bool SubsonicResource::handleMediaRetrievalRequest(const std::string& requestPath, const Wt::Http::Request& request, Wt::Http::Response& response) { + auto itStreamHandler{ mediaRetrievalHandlers.find(requestPath) }; + if (itStreamHandler == mediaRetrievalHandlers.end()) + return false; + + LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itStreamHandler->first); + try { - // Media retrieval endpoints are always authenticated - // Optimization: no need to reauth user for each continuation + RequestContext requestContext{ request, _db.getTLSSession(), _config }; + + // Media retrieval endpoints are always authenticated but we don't reauth user for a continuation db::User::pointer user; if (!request.continuation()) user = getUserFromUserId(_db.getTLSSession(), authenticateUser(request)); requestContext.setUser(user); - handler(requestContext, request, response); + itStreamHandler->second(requestContext, request, response); + + return true; } catch (const UserNotAuthorizedError&) { @@ -409,6 +365,61 @@ namespace lms::api::subsonic } } + void SubsonicResource::handleRequest(const std::string& requestPath, const Wt::Http::Request& request, Wt::Http::Response& response) + { + auto writeResponse{ [&](const Response& resp, ResponseFormat format) { + LMS_SCOPED_TRACE_DETAILED("Subsonic", "WriteResponse"); + resp.write(response.out(), format); + response.setMimeType(std::string{ ResponseFormatToMimeType(format) }); + } }; + + std::optional requestContext; + try + { + requestContext.emplace(request, _db.getTLSSession(), _config); + } + catch (const Error& e) + { + writeResponse(Response::createFailedResponse(defaultServerProtocolVersion, e), ResponseFormat::xml); + throw; + } + + try + { + if (auto itEntryPoint{ requestEntryPoints.find(requestPath) }; itEntryPoint != requestEntryPoints.end()) + { + LMS_SCOPED_TRACE_OVERVIEW("Subsonic", itEntryPoint->first); + + db::User::pointer user; + if (itEntryPoint->second.authMode == AuthenticationMode::Authenticated) + { + user = getUserFromUserId(_db.getTLSSession(), authenticateUser(request)); + checkUserTypeIsAllowed(user, itEntryPoint->second.allowedUserTypes); + requestContext->setUser(user); + } + + const Response resp{ [&] { + LMS_SCOPED_TRACE_DETAILED("Subsonic", "HandleRequest"); + return itEntryPoint->second.func(*requestContext); + }() }; + + writeResponse(resp, requestContext->getResponseFormat()); + return; + } + // do not disclose unhandled commands for unauthenticated users + authenticateUser(request); + + LMS_LOG(API_SUBSONIC, ERROR, "Unhandled command '" << requestPath << "'"); + throw UnknownEntryPointGenericError{}; + } + catch (const Error& e) + { + Response resp{ Response::createFailedResponse(requestContext->getServerProtocolVersion(), e) }; + writeResponse(resp, requestContext->getResponseFormat()); + throw; + } + } + db::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request) { const auto& parameters{ request.getParameterMap() }; diff --git a/src/libs/subsonic/impl/SubsonicResource.hpp b/src/libs/subsonic/impl/SubsonicResource.hpp index f65c92c3..0ee33fa9 100644 --- a/src/libs/subsonic/impl/SubsonicResource.hpp +++ b/src/libs/subsonic/impl/SubsonicResource.hpp @@ -18,6 +18,8 @@ */ #pragma once +#include + #include #include #include @@ -43,8 +45,8 @@ namespace lms::api::subsonic private: void handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response) override; - using MediaRetrievalHandlerFunc = std::function; - void handleMediaRetrievalRequest(const MediaRetrievalHandlerFunc& handler, RequestContext& requestContext, const Wt::Http::Request& request, Wt::Http::Response& response); + bool handleMediaRetrievalRequest(const std::string& requestPath, const Wt::Http::Request& request, Wt::Http::Response& response); + void handleRequest(const std::string& requestPath, const Wt::Http::Request& request, Wt::Http::Response& response); db::UserId authenticateUser(const Wt::Http::Request& request);