Skip to content

Commit c00d469

Browse files
committed
Fix crash if playlist sort item count is out of bounds.
API docs say it's safe to pass out-of-bounds values, so it should behave in that way.
1 parent b9f1c16 commit c00d469

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

src/playlist/Playlist.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,16 @@ auto Playlist::Shuffle() const -> bool
162162
void Playlist::Sort(uint32_t startIndex, uint32_t count,
163163
Playlist::SortPredicate predicate, Playlist::SortOrder order)
164164
{
165+
if (startIndex >= m_items.size())
166+
{
167+
return;
168+
}
169+
170+
if (startIndex + count >= m_items.size())
171+
{
172+
count = m_items.size() - startIndex;
173+
}
174+
165175
m_presetHistory.clear();
166176

167177
std::sort(m_items.begin() + startIndex,

tests/playlist/PlaylistTest.cpp

+46
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,52 @@ TEST(projectMPlaylistPlaylist, SortFilenameOnlyDescending)
411411
}
412412

413413

414+
TEST(projectMPlaylistPlaylist, SortOutOfBoundsStart)
415+
{
416+
Playlist playlist;
417+
EXPECT_TRUE(playlist.AddItem("/some/PresetZ.milk", Playlist::InsertAtEnd, false));
418+
EXPECT_TRUE(playlist.AddItem("/some/PresetA.milk", Playlist::InsertAtEnd, false));
419+
EXPECT_TRUE(playlist.AddItem("/some/other/PresetC.milk", Playlist::InsertAtEnd, false));
420+
EXPECT_TRUE(playlist.AddItem("/yet/another/PresetD.milk", Playlist::InsertAtEnd, false));
421+
422+
ASSERT_EQ(playlist.Size(), 4);
423+
424+
playlist.Sort(std::numeric_limits<uint32_t>::max(), 1, Playlist::SortPredicate::FilenameOnly, Playlist::SortOrder::Ascending);
425+
426+
ASSERT_EQ(playlist.Size(), 4);
427+
428+
const auto& items = playlist.Items();
429+
ASSERT_EQ(items.size(), 4);
430+
EXPECT_EQ(items.at(0).Filename(), "/some/PresetZ.milk");
431+
EXPECT_EQ(items.at(1).Filename(), "/some/PresetA.milk");
432+
EXPECT_EQ(items.at(2).Filename(), "/some/other/PresetC.milk");
433+
EXPECT_EQ(items.at(3).Filename(), "/yet/another/PresetD.milk");
434+
}
435+
436+
437+
TEST(projectMPlaylistPlaylist, SortOutOfBoundsCount)
438+
{
439+
Playlist playlist;
440+
EXPECT_TRUE(playlist.AddItem("/some/PresetZ.milk", Playlist::InsertAtEnd, false));
441+
EXPECT_TRUE(playlist.AddItem("/some/PresetA.milk", Playlist::InsertAtEnd, false));
442+
EXPECT_TRUE(playlist.AddItem("/some/other/PresetC.milk", Playlist::InsertAtEnd, false));
443+
EXPECT_TRUE(playlist.AddItem("/yet/another/PresetD.milk", Playlist::InsertAtEnd, false));
444+
445+
ASSERT_EQ(playlist.Size(), 4);
446+
447+
playlist.Sort(0, std::numeric_limits<uint32_t>::max(), Playlist::SortPredicate::FilenameOnly, Playlist::SortOrder::Ascending);
448+
449+
ASSERT_EQ(playlist.Size(), 4);
450+
451+
const auto& items = playlist.Items();
452+
ASSERT_EQ(items.size(), 4);
453+
EXPECT_EQ(items.at(0).Filename(), "/some/PresetA.milk");
454+
EXPECT_EQ(items.at(1).Filename(), "/some/other/PresetC.milk");
455+
EXPECT_EQ(items.at(2).Filename(), "/yet/another/PresetD.milk");
456+
EXPECT_EQ(items.at(3).Filename(), "/some/PresetZ.milk");
457+
}
458+
459+
414460
TEST(projectMPlaylistPlaylist, NextPresetIndexEmptyPlaylist)
415461
{
416462
Playlist playlist;

0 commit comments

Comments
 (0)