From caf2003d96b6beaae2865b864f290c0943ab95fa Mon Sep 17 00:00:00 2001 From: Jason Waataja Date: Fri, 20 Jul 2018 17:49:10 -0700 Subject: [PATCH] Fix addnext failing on an empty queue Previously, addnext would try to unconditionally add at index one into a queue. This panicked if the queue was empty. Added two protections, one that checks the index in the InsertTrack function, the other in the addnext command itself to insert at zero if the queue is empty. --- bot/queue.go | 5 +++++ commands/addnext.go | 6 +++++- commands/addnext_test.go | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/bot/queue.go b/bot/queue.go index 505c967..bfd650f 100644 --- a/bot/queue.go +++ b/bot/queue.go @@ -85,6 +85,11 @@ func (q *Queue) InsertTrack(i int, t interfaces.Track) error { q.mutex.Lock() beforeLen := len(q.Queue) + if i < 0 || i > beforeLen { + q.mutex.Unlock() + return errors.New("Adding at invalid index in queue") + } + // An error should never occur here since maxTrackDuration is restricted to // ints. Any error in the configuration will be caught during yaml load. maxTrackDuration, _ := time.ParseDuration(fmt.Sprintf("%ds", diff --git a/commands/addnext.go b/commands/addnext.go index ea9ed30..b5eb075 100644 --- a/commands/addnext.go +++ b/commands/addnext.go @@ -75,7 +75,11 @@ func (c *AddNextCommand) Execute(user *gumble.User, args ...string) (string, boo numAdded := 0 // We must loop backwards here to preserve the track order when inserting tracks. for i := len(allTracks) - 1; i >= 0; i-- { - if err = DJ.Queue.InsertTrack(1, allTracks[i]); err != nil { + insertIndex := 1 + if DJ.Queue.Length() == 0 { + insertIndex = 0 + } + if err = DJ.Queue.InsertTrack(insertIndex, allTracks[i]); err != nil { numTooLong++ } else { numAdded++ diff --git a/commands/addnext_test.go b/commands/addnext_test.go index b06799a..fa750da 100644 --- a/commands/addnext_test.go +++ b/commands/addnext_test.go @@ -6,3 +6,5 @@ */ package commands + +// TODO: Add regression test for bug where addnext fails on an empty queue.