diff options
| -rw-r--r-- | src/PlaylistPrint.cxx | 16 | ||||
| -rw-r--r-- | src/PlaylistQueue.cxx | 7 | ||||
| -rw-r--r-- | src/PlaylistSong.cxx | 96 | ||||
| -rw-r--r-- | src/PlaylistSong.hxx | 10 | ||||
| -rw-r--r-- | test/test_translate_song.cxx | 79 |
5 files changed, 86 insertions, 122 deletions
diff --git a/src/PlaylistPrint.cxx b/src/PlaylistPrint.cxx index c984ea224..38d63f4e8 100644 --- a/src/PlaylistPrint.cxx +++ b/src/PlaylistPrint.cxx @@ -156,15 +156,13 @@ playlist_provider_print(Client &client, const char *uri, DetachedSong *song; while ((song = e.NextSong()) != nullptr) { - song = playlist_check_translate_song(song, base_uri.c_str(), - false); - if (song == nullptr) - continue; - - if (detail) - song_print_info(client, *song); - else - song_print_uri(client, *song); + if (playlist_check_translate_song(*song, base_uri.c_str(), + false)) { + if (detail) + song_print_info(client, *song); + else + song_print_uri(client, *song); + } delete song; } diff --git a/src/PlaylistQueue.cxx b/src/PlaylistQueue.cxx index e65608bf4..0a45920e3 100644 --- a/src/PlaylistQueue.cxx +++ b/src/PlaylistQueue.cxx @@ -48,10 +48,11 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e, continue; } - song = playlist_check_translate_song(song, base_uri.c_str(), - secure); - if (song == nullptr) + if (!playlist_check_translate_song(*song, base_uri.c_str(), + secure)) { + delete song; continue; + } PlaylistResult result = dest.AppendSong(pc, std::move(*song)); delete song; diff --git a/src/PlaylistSong.cxx b/src/PlaylistSong.cxx index beb2d186f..bcbdc30be 100644 --- a/src/PlaylistSong.cxx +++ b/src/PlaylistSong.cxx @@ -34,83 +34,70 @@ #include <string.h> static void -merge_song_metadata(DetachedSong &dest, const DetachedSong &base, - const DetachedSong &add) +merge_song_metadata(DetachedSong &add, const DetachedSong &base) { { TagBuilder builder(add.GetTag()); builder.Complement(base.GetTag()); - dest.SetTag(builder.Commit()); + add.SetTag(builder.Commit()); } - dest.SetLastModified(base.GetLastModified()); - dest.SetStartMS(add.GetStartMS()); - dest.SetEndMS(add.GetEndMS()); + add.SetLastModified(base.GetLastModified()); } -static DetachedSong * -apply_song_metadata(DetachedSong *dest, const DetachedSong &src) +static void +apply_song_metadata(DetachedSong &dest, const DetachedSong &src) { - assert(dest != nullptr); - if (!src.GetTag().IsDefined() && src.GetStartMS() == 0 && src.GetEndMS() == 0) - return dest; + return; - DetachedSong *tmp = new DetachedSong(dest->GetURI()); - merge_song_metadata(*tmp, *dest, src); + merge_song_metadata(dest, src); - if (dest->GetTag().IsDefined() && dest->GetTag().time > 0 && + if (dest.GetTag().IsDefined() && dest.GetTag().time > 0 && src.GetStartMS() > 0 && src.GetEndMS() == 0 && - src.GetStartMS() / 1000 < (unsigned)dest->GetTag().time) + src.GetStartMS() / 1000 < (unsigned)dest.GetTag().time) /* the range is open-ended, and the playlist plugin did not know the total length of the song file (e.g. last track on a CUE file); fix it up here */ - tmp->WritableTag().time = - dest->GetTag().time - src.GetStartMS() / 1000; - - delete dest; - return tmp; + dest.WritableTag().time = + dest.GetTag().time - src.GetStartMS() / 1000; } -static DetachedSong * -playlist_check_load_song(const DetachedSong *song, const char *uri) +static bool +playlist_check_load_song(DetachedSong &song) { - DetachedSong *dest; + const char *const uri = song.GetURI(); if (uri_has_scheme(uri)) { - dest = new DetachedSong(uri); + return true; } else if (PathTraitsUTF8::IsAbsolute(uri)) { - dest = new DetachedSong(uri); - if (!dest->Update()) { - delete dest; - return nullptr; - } + DetachedSong tmp(uri); + if (!tmp.Update()) + return false; + + apply_song_metadata(song, tmp); + return true; } else { - dest = DatabaseDetachSong(uri, IgnoreError()); - if (dest == nullptr) - return nullptr; - } + DetachedSong *tmp = DatabaseDetachSong(uri, IgnoreError()); + if (tmp == nullptr) + return false; - return apply_song_metadata(dest, *song); + apply_song_metadata(song, *tmp); + delete tmp; + return true; + } } -DetachedSong * -playlist_check_translate_song(DetachedSong *song, const char *base_uri, +bool +playlist_check_translate_song(DetachedSong &song, const char *base_uri, bool secure) { - const char *uri = song->GetURI(); + const char *const uri = song.GetURI(); - if (uri_has_scheme(uri)) { - if (uri_supported_scheme(uri)) - /* valid remote song */ - return song; - else { - /* unsupported remote song */ - delete song; - return nullptr; - } - } + if (uri_has_scheme(uri)) + /* valid remote song? */ + return uri_supported_scheme(uri); if (base_uri != nullptr && strcmp(base_uri, ".") == 0) /* PathTraitsUTF8::GetParent() returns "." when there @@ -125,25 +112,20 @@ playlist_check_translate_song(DetachedSong *song, const char *base_uri, assert(suffix != nullptr); if (suffix != uri) - uri = suffix; - else if (!secure) { + song.SetURI(std::string(suffix)); + else if (!secure) /* local files must be relative to the music directory when "secure" is enabled */ - delete song; - return nullptr; - } + return false; base_uri = nullptr; } if (base_uri != nullptr) { - song->SetURI(PathTraitsUTF8::Build(base_uri, uri)); + song.SetURI(PathTraitsUTF8::Build(base_uri, uri)); /* repeat the above checks */ return playlist_check_translate_song(song, nullptr, secure); } - DetachedSong *dest = playlist_check_load_song(song, uri); - delete song; - - return dest; + return playlist_check_load_song(song); } diff --git a/src/PlaylistSong.hxx b/src/PlaylistSong.hxx index 826b4d92d..2a47b28db 100644 --- a/src/PlaylistSong.hxx +++ b/src/PlaylistSong.hxx @@ -23,15 +23,15 @@ class DetachedSong; /** - * Verifies the song, returns nullptr if it is unsafe. Translate the - * song to a new song object within the database, if it is a local - * file. The old song object is freed. + * Verifies the song, returns false if it is unsafe. Translate the + * song to a song within the database, if it is a local file. * * @param secure if true, then local files are only allowed if they * are relative to base_uri + * @return true on success, false if the song should not be used */ -DetachedSong * -playlist_check_translate_song(DetachedSong *song, const char *base_uri, +bool +playlist_check_translate_song(DetachedSong &song, const char *base_uri, bool secure); #endif diff --git a/test/test_translate_song.cxx b/test/test_translate_song.cxx index 34af2c1de..6e56e68e5 100644 --- a/test/test_translate_song.cxx +++ b/test/test_translate_song.cxx @@ -186,15 +186,6 @@ ToString(const DetachedSong &song) return result; } -static std::string -ToString(const DetachedSong *song) -{ - if (song == nullptr) - return "nullptr"; - - return ToString(*song); -} - class TranslateSongTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TranslateSongTest); CPPUNIT_TEST(TestAbsoluteURI); @@ -205,76 +196,68 @@ class TranslateSongTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE_END(); void TestAbsoluteURI() { - auto song1 = new DetachedSong("http://example.com/foo.ogg"); - auto song2 = playlist_check_translate_song(song1, "/ignored", false); - CPPUNIT_ASSERT_EQUAL(song1, song2); + DetachedSong song1("http://example.com/foo.ogg"); + auto se = ToString(song1); + CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", false)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); } void TestInsecure() { /* illegal because secure=false */ - auto song1 = new DetachedSong(uri1); - auto song2 = playlist_check_translate_song(song1, nullptr, false); - CPPUNIT_ASSERT_EQUAL((DetachedSong *)nullptr, song2); + DetachedSong song1 (uri1); + CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr, false)); } void TestSecure() { - auto song1 = new DetachedSong(uri1, MakeTag1b()); + DetachedSong song1(uri1, MakeTag1b()); auto s1 = ToString(song1); auto se = ToString(DetachedSong(uri1, MakeTag1c())); - auto song2 = playlist_check_translate_song(song1, "/ignored", true); - CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; + CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", true)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); } void TestInDatabase() { - auto song1 = new DetachedSong("doesntexist"); - auto song2 = playlist_check_translate_song(song1, nullptr, false); - CPPUNIT_ASSERT_EQUAL((DetachedSong *)nullptr, song2); + DetachedSong song1("doesntexist"); + CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr, false)); - song1 = new DetachedSong(uri2, MakeTag2b()); - auto s1 = ToString(song1); + DetachedSong song2(uri2, MakeTag2b()); + auto s1 = ToString(song2); auto se = ToString(DetachedSong(uri2, MakeTag2c())); - song2 = playlist_check_translate_song(song1, nullptr, false); + CPPUNIT_ASSERT(playlist_check_translate_song(song2, nullptr, false)); CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; - song1 = new DetachedSong("/music/foo/bar.ogg", MakeTag2b()); - s1 = ToString(song1); + DetachedSong song3("/music/foo/bar.ogg", MakeTag2b()); + s1 = ToString(song3); se = ToString(DetachedSong(uri2, MakeTag2c())); - song2 = playlist_check_translate_song(song1, nullptr, false); - CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; + CPPUNIT_ASSERT(playlist_check_translate_song(song3, nullptr, false)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song3)); } void TestRelative() { /* map to music_directory */ - auto song1 = new DetachedSong("bar.ogg", MakeTag2b()); + DetachedSong song1("bar.ogg", MakeTag2b()); auto s1 = ToString(song1); auto se = ToString(DetachedSong(uri2, MakeTag2c())); - auto song2 = playlist_check_translate_song(song1, "/music/foo", false); - CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; + CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/music/foo", false)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); /* illegal because secure=false */ - song1 = new DetachedSong("bar.ogg", MakeTag2b()); - song2 = playlist_check_translate_song(song1, "/foo", false); - CPPUNIT_ASSERT_EQUAL((DetachedSong *)nullptr, song2); + DetachedSong song2("bar.ogg", MakeTag2b()); + CPPUNIT_ASSERT(!playlist_check_translate_song(song1, "/foo", false)); /* legal because secure=true */ - song1 = new DetachedSong("bar.ogg", MakeTag1b()); - s1 = ToString(song2); + DetachedSong song3("bar.ogg", MakeTag1b()); + s1 = ToString(song3); se = ToString(DetachedSong(uri1, MakeTag1c())); - song2 = playlist_check_translate_song(song1, "/foo", true); - CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; + CPPUNIT_ASSERT(playlist_check_translate_song(song3, "/foo", true)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song3)); /* relative to http:// */ - song1 = new DetachedSong("bar.ogg", MakeTag2a()); - s1 = ToString(song1); + DetachedSong song4("bar.ogg", MakeTag2a()); + s1 = ToString(song4); se = ToString(DetachedSong("http://example.com/foo/bar.ogg", MakeTag2a())); - song2 = playlist_check_translate_song(song1, "http://example.com/foo", false); - CPPUNIT_ASSERT_EQUAL(se, ToString(song2)); - delete song2; + CPPUNIT_ASSERT(playlist_check_translate_song(song4, "http://example.com/foo", false)); + CPPUNIT_ASSERT_EQUAL(se, ToString(song4)); } }; |