Made length checks more robusts + bumped sizes, fixes #496

src/libs/database/impl/Artist.cpp

@@ -183,10 +183,10 @@ namespace lms::db
     } // namespace
 
     Artist::Artist(const std::string& name, const std::optional<core::UUID>& MBID)
-        : _name{ std::string(name, 0, _maxNameLength) }
+        : _MBID{ MBID ? MBID->getAsString() : "" }
-        , _sortName{ _name }
-        , _MBID{ MBID ? MBID->getAsString() : "" }
     {
+        setName(name);
+        _sortName = _name;
     }
 
     Artist::pointer Artist::create(Session& session, const std::string& name, const std::optional<core::UUID>& MBID)
@@ -360,9 +360,19 @@ namespace lms::db
         return res;
     }
 
-    void Artist::setSortName(const std::string& sortName)
+    void Artist::setName(std::string_view name)
     {
-        _sortName = std::string(sortName, 0, _maxNameLength);
+        _name.assign(name, 0, _maxNameLength);
+        if (name.size() > _maxNameLength)
+            LMS_LOG(DB, WARNING, "Artist name too long, truncated to '" << _name << "'");
+    }
+
+    void Artist::setSortName(std::string_view sortName)
+    {
+        _sortName.assign(sortName, 0, _maxNameLength);
+
+        if (sortName.size() > _maxNameLength)
+            LMS_LOG(DB, WARNING, "Artist sort name too long, truncated to '" << _sortName << "'");
     }
 
     void Artist::setImage(ObjectPtr<Image> image)

src/libs/database/impl/Cluster.cpp

@@ -96,9 +96,12 @@ namespace lms::db
     } // namespace
 
     Cluster::Cluster(ObjectPtr<ClusterType> type, std::string_view name)
-        : _name{ std::string{ name, 0, _maxNameLength } }
+        : _name{ name }
         , _clusterType{ getDboPtr(type) }
     {
+        // As we use the name to uniquely identify clusters and cluster types, we must throw (and not truncate)
+        if (name.size() > maxNameLength)
+            throw Exception{ "Cluster name is too long: " + std::string{ name } + "'" };
     }
 
     Cluster::pointer Cluster::create(Session& session, ObjectPtr<ClusterType> type, std::string_view name)
@@ -183,6 +186,9 @@ namespace lms::db
     ClusterType::ClusterType(std::string_view name)
         : _name{ name }
     {
+        // As we use the name to uniquely identify clusters and cluster types, we must throw
+        if (name.size() > maxNameLength)
+            throw Exception{ "ClusterType name is too long: " + std::string{ name } + "'" };
     }
 
     ClusterType::pointer ClusterType::create(Session& session, std::string_view name)
@@ -230,6 +236,9 @@ namespace lms::db
     {
         session.checkReadTransaction();
 
+        if (name.size() > maxNameLength)
+            throw Exception{ "Requested ClusterType name is too long: " + std::string{ name } + "'" };
+
         return utils::fetchQuerySingleResult(session.getDboSession()->find<ClusterType>().where("name = ?").bind(name));
     }
 
@@ -254,6 +263,9 @@ namespace lms::db
         assert(self());
         assert(session());
 
+        if (name.size() > Cluster::maxNameLength)
+            throw Exception{ "Requested Cluster name is too long: " + std::string{ name } + "'" };
+
         return utils::fetchQuerySingleResult(session()->find<Cluster>().where("name = ?").bind(name).where("cluster_type_id = ?").bind(getId()));
     }
 

src/libs/database/impl/Release.cpp

@@ -217,8 +217,11 @@ namespace lms::db
     } // namespace
 
     ReleaseType::ReleaseType(std::string_view name)
-        : _name{ std::string(name, 0, _maxNameLength) }
+        : _name{ name }
     {
+        // As we use the name to uniquely identoify release type, we must throw (and not truncate)
+        if (name.size() > _maxNameLength)
+            throw Exception{ "ReleaseType name is too long: " + std::string{ name } + "'" };
     }
 
     ReleaseType::pointer ReleaseType::create(Session& session, std::string_view name)
@@ -237,6 +240,9 @@ namespace lms::db
     {
         session.checkReadTransaction();
 
+        if (name.size() > _maxNameLength)
+            throw Exception{ "Requeted ReleaseType name is too long: " + std::string{ name } + "'" };
+
         return utils::fetchQuerySingleResult(session.getDboSession()->query<Wt::Dbo::ptr<ReleaseType>>("SELECT r_t from release_type r_t").where("r_t.name = ?").bind(name));
     }
 

src/libs/database/impl/Track.cpp

@@ -377,6 +377,27 @@ namespace lms::db
         _relativeFilePath = filePath;
     }
 
+    void Track::setName(std::string_view name)
+    {
+        _name = std::string{ name, 0, _maxNameLength };
+        if (name.size() > _maxNameLength)
+            LMS_LOG(DB, WARNING, "Track name too long, truncated to '" << _name << "'");
+    }
+
+    void Track::setCopyright(std::string_view copyright)
+    {
+        _copyright = std::string{ copyright, 0, _maxCopyrightLength };
+        if (copyright.size() > _maxCopyrightLength)
+            LMS_LOG(DB, WARNING, "Track copyright too long, truncated to '" << _copyright << "'");
+    }
+
+    void Track::setCopyrightURL(std::string_view copyrightURL)
+    {
+        _copyrightURL = std::string{ copyrightURL, 0, _maxCopyrightURLLength };
+        if (copyrightURL.size() > _maxCopyrightURLLength)
+            LMS_LOG(DB, WARNING, "Track copyright URL too long, truncated to '" << _copyrightURL << "'");
+    }
+
     void Track::clearArtistLinks()
     {
         _trackArtistLinks.clear();

src/libs/database/include/database/Artist.hpp

@@ -150,9 +150,9 @@ namespace lms::db
         // size is the max number of cluster per cluster type
         std::vector<std::vector<ObjectPtr<Cluster>>> getClusterGroups(std::vector<ClusterTypeId> clusterTypeIds, std::size_t size) const;
 
-        void setName(std::string_view name) { _name = name; }
+        void setName(std::string_view name);
         void setMBID(const std::optional<core::UUID>& mbid) { _MBID = mbid ? mbid->getAsString() : ""; }
-        void setSortName(const std::string& sortName);
+        void setSortName(std::string_view sortName);
         void setImage(ObjectPtr<Image> image);
 
         template<class Action>
@@ -168,7 +168,7 @@ namespace lms::db
         }
 
     private:
-        static constexpr std::size_t _maxNameLength{ 256 };
+        static constexpr std::size_t _maxNameLength{ 512 };
 
         friend class Session;
         // Create

src/libs/database/include/database/Cluster.hpp

@@ -43,6 +43,8 @@ namespace lms::db
     class Cluster final : public Object<Cluster, ClusterId>
     {
     public:
+        static constexpr std::size_t maxNameLength{ 512 };
+
         struct FindParameters
         {
             std::optional<Range> range;
@@ -126,8 +128,6 @@ namespace lms::db
         Cluster(ObjectPtr<ClusterType> type, std::string_view name);
         static pointer create(Session& session, ObjectPtr<ClusterType> type, std::string_view name);
 
-        static const std::size_t _maxNameLength = 128;
-
         std::string _name;
         int _trackCount{};
         int _releaseCount{};
@@ -141,6 +141,8 @@ namespace lms::db
     public:
         ClusterType() = default;
 
+        static constexpr std::size_t maxNameLength{ 512 };
+
         // Getters
         static std::size_t getCount(Session& session);
         static RangeResults<ClusterTypeId> findIds(Session& session, std::optional<Range> range = std::nullopt);
@@ -169,8 +171,6 @@ namespace lms::db
         ClusterType(std::string_view name);
         static pointer create(Session& session, std::string_view name);
 
-        static const std::size_t _maxNameLength = 128;
-
         std::string _name;
         Wt::Dbo::collection<Wt::Dbo::ptr<Cluster>> _clusters;
     };

src/libs/database/include/database/Release.hpp

@@ -69,7 +69,7 @@ namespace lms::db
         }
 
     private:
-        static constexpr std::size_t _maxNameLength{ 128 };
+        static constexpr std::size_t _maxNameLength{ 512 };
 
         friend class Session;
         ReleaseType(std::string_view name);
@@ -241,7 +241,7 @@ namespace lms::db
         Wt::WDate getDate(bool original) const;
         std::optional<int> getYear(bool original) const;
 
-        static constexpr std::size_t _maxNameLength{ 256 };
+        static constexpr std::size_t _maxNameLength{ 512 };
 
         std::string _name;
         std::string _sortName;

src/libs/database/include/database/Track.hpp

@@ -204,8 +204,8 @@ namespace lms::db
         void setTrackNumber(std::optional<int> num) { _trackNumber = num; }
         void setDiscNumber(std::optional<int> num) { _discNumber = num; }
         void setTotalTrack(std::optional<int> totalTrack) { _totalTrack = totalTrack; }
-        void setDiscSubtitle(const std::string& name) { _discSubtitle = name; }
+        void setDiscSubtitle(std::string_view name) { _discSubtitle = name; }
-        void setName(const std::string& name) { _name = std::string(name, 0, _maxNameLength); }
+        void setName(std::string_view name);
         void setAbsoluteFilePath(const std::filesystem::path& filePath);
         void setRelativeFilePath(const std::filesystem::path& filePath);
         void setFileSize(std::size_t fileSize) { _fileSize = fileSize; }
@@ -223,8 +223,8 @@ namespace lms::db
         void setHasCover(bool hasCover) { _hasCover = hasCover; }
         void setTrackMBID(const std::optional<core::UUID>& MBID) { _trackMBID = MBID ? MBID->getAsString() : ""; }
         void setRecordingMBID(const std::optional<core::UUID>& MBID) { _recordingMBID = MBID ? MBID->getAsString() : ""; }
-        void setCopyright(const std::string& copyright) { _copyright = std::string(copyright, 0, _maxCopyrightLength); }
+        void setCopyright(std::string_view copyright);
-        void setCopyrightURL(const std::string& copyrightURL) { _copyrightURL = std::string(copyrightURL, 0, _maxCopyrightURLLength); }
+        void setCopyrightURL(std::string_view copyrightURL);
         void setTrackReplayGain(std::optional<float> replayGain) { _trackReplayGain = replayGain; }
         void setReleaseReplayGain(std::optional<float> replayGain) { _releaseReplayGain = replayGain; } // may be by disc!
         void setArtistDisplayName(std::string_view name) { _artistDisplayName = name; }
@@ -318,9 +318,9 @@ namespace lms::db
         friend class Session;
         static pointer create(Session& session);
 
-        static constexpr std::size_t _maxNameLength{ 256 };
+        static constexpr std::size_t _maxNameLength{ 512 };
-        static constexpr std::size_t _maxCopyrightLength{ 256 };
+        static constexpr std::size_t _maxCopyrightLength{ 512 };
-        static constexpr std::size_t _maxCopyrightURLLength{ 256 };
+        static constexpr std::size_t _maxCopyrightURLLength{ 512 };
 
         int _scanVersion{};
         std::optional<int> _trackNumber{};

src/libs/database/include/database/Types.hpp

@@ -25,8 +25,16 @@
 
 #include <Wt/WDate.h>
 
+#include "core/Exception.hpp"
+
 namespace lms::db
 {
+    class Exception : public core::LmsException
+    {
+    public:
+        using LmsException::LmsException;
+    };
+
     // Caution: do not change enum values if they are set!
 
     // Request:

src/libs/database/include/database/User.hpp

@@ -68,8 +68,8 @@ namespace lms::db
             }
         };
 
-        static inline constexpr std::size_t MinNameLength{ 3 };
+        static inline constexpr std::size_t minNameLength{ 3 };
-        static inline constexpr std::size_t MaxNameLength{ 15 };
+        static inline constexpr std::size_t maxNameLength{ 32 };
         static inline constexpr bool defaultSubsonicEnableTranscodingByDefault{ false };
         static inline constexpr TranscodingOutputFormat defaultSubsonicTranscodingOutputFormat{ TranscodingOutputFormat::OGG_OPUS };
         static inline constexpr Bitrate defaultSubsonicTranscodingOutputBitrate{ 128000 };

src/libs/database/test/Cluster.cpp

@@ -35,7 +35,7 @@ namespace lms::db::tests
         ScopedClusterType clusterType{ session, "MyType" };
 
         {
-            auto transaction{ session.createWriteTransaction() };
+            auto transaction{ session.createReadTransaction() };
             EXPECT_EQ(ClusterType::getCount(session), 1);
         }
 
@@ -43,7 +43,7 @@ namespace lms::db::tests
             ScopedCluster cluster{ session, clusterType.lockAndGet(), "MyCluster" };
 
             {
-                auto transaction{ session.createWriteTransaction() };
+                auto transaction{ session.createReadTransaction() };
 
                 EXPECT_EQ(Cluster::getCount(session), 1);
                 EXPECT_EQ(cluster->getType()->getId(), clusterType.getId());
@@ -74,7 +74,7 @@ namespace lms::db::tests
         }
 
         {
-            auto transaction{ session.createWriteTransaction() };
+            auto transaction{ session.createReadTransaction() };
 
             auto clusterTypes{ ClusterType::findOrphanIds(session) };
             ASSERT_EQ(clusterTypes.results.size(), 1);
@@ -84,6 +84,70 @@ namespace lms::db::tests
         }
     }
 
+    TEST_F(DatabaseFixture, Cluster_find)
+    {
+        ScopedClusterType clusterType{ session, "MyType" };
+        ScopedCluster cluster1{ session, clusterType.lockAndGet(), "MyCluster" };
+        ScopedCluster cluster2{ session, clusterType.lockAndGet(), "Mycluster" };
+        ScopedCluster cluster3{ session, clusterType.lockAndGet(), "MyOtherCluster" };
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            EXPECT_EQ(clusterType->getCluster("MyCluster"), cluster1.get());
+            EXPECT_EQ(clusterType->getCluster("Mycluster"), cluster2.get());
+
+            EXPECT_EQ(clusterType->getCluster(" Mycluster"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("Mycluster "), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("mycluster"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("My"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("Cluster"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("MyCluster1"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("MyCluster2"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(""), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(" "), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster("*"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(R"(%)"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(R"(%%)"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(R"(")"), Cluster::pointer{});
+            EXPECT_EQ(clusterType->getCluster(R"("")"), Cluster::pointer{});
+        }
+    }
+
+    TEST_F(DatabaseFixture, Cluster_create)
+    {
+        ScopedClusterType clusterType{ session, "MyType" };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+
+            auto createdCluster{ session.create<Cluster>(clusterType.get(), "Foo") };
+            auto foundCluster{ clusterType->getCluster("Foo") };
+            EXPECT_EQ(createdCluster, foundCluster);
+        }
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+
+            auto createdCluster{ session.create<Cluster>(clusterType.get(), "") };
+            auto foundCluster{ clusterType->getCluster("") };
+            EXPECT_EQ(createdCluster, foundCluster);
+        }
+    }
+
+    TEST_F(DatabaseFixture, Cluster_create_long)
+    {
+        ScopedClusterType clusterType{ session, "MyType" };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+
+            auto createdCluster{ session.create<Cluster>(clusterType.get(), "Alternative Rock; Art Pop; Art Rock; Britpop; Chamber Pop; Electronic Rock; Electronica; Experimental Rock; Neo-Progressive Rock; Foo") };
+            auto foundCluster{ clusterType->getCluster("Alternative Rock; Art Pop; Art Rock; Britpop; Chamber Pop; Electronic Rock; Electronica; Experimental Rock; Neo-Progressive Rock; Foo") };
+            EXPECT_EQ(createdCluster, foundCluster);
+        }
+    }
+
     TEST_F(DatabaseFixture, Cluster_singleTrack)
     {
         ScopedTrack track{ session };

src/lms/ui/common/LoginNameValidator.cpp

@@ -38,8 +38,8 @@ namespace lms::ui
     {
         auto v = std::make_unique<LengthValidator>();
         v->setMandatory(true);
-        v->setMinimumLength(db::User::MinNameLength);
+        v->setMinimumLength(db::User::minNameLength);
-        v->setMaximumLength(db::User::MaxNameLength);
+        v->setMaximumLength(db::User::maxNameLength);
         return v;
     }
 } // namespace lms::ui