Bugfix: Remediation for Panic! at the Parser

[shinterro]

Hi! This project is pretty great, thanks for it.

After scanning an initial set of albums, attempts to bring up my artist list (via both Tempo/Subsonic) would fail with a network error. On reviewing gonic logs, I found the following:

gonic_1  | 2023/12/30 19:12:29 http: panic serving ###.###.###.###:58864: runtime error: index 
out of range [0] with length 0                                                                
gonic_1  | goroutine 89959 [running]:                                                         
gonic_1  | net/http.(*conn).serve.func1()                                                     
gonic_1  |      /usr/local/go/src/net/http/server.go:1868 +0xb9                               
gonic_1  | panic({0xbda260?, 0xc000002a08?})                                                  
gonic_1  |      /usr/local/go/src/runtime/panic.go:920 +0x270                                 
gonic_1  | go.senan.xyz/gonic/server/ctrlsubsonic.lowerUDecOrHash({0x0?, 0xb37460?})          
gonic_1  |      /src/server/ctrlsubsonic/handlers_common.go:517 +0x90                         
gonic_1  | go.senan.xyz/gonic/server/ctrlsubsonic.(*Controller).ServeGetArtists(0xc000756000, 
0xc001932300)                                                                                 
gonic_1  |      /src/server/ctrlsubsonic/handlers_by_tags.go:47 +0x5fe                        
gonic_1  | go.senan.xyz/gonic/server/ctrlsubsonic.New.resp.func37({0xd9ea48, 0xc000392000}, 
0x
...

After some further tests I isolated this down to a single album and artist - N O Ć [EP]' by DON'T//BE// ⚜⚜⚜ - the failure resulting from the double-slashes in the artist name. The current multiParser function (with configured '/' delim) will split the double-slash into an empty string. When this "" reaches the unicode index check (lowerUDecOrHash), it panics with the above and breaks the response.

This PR implements two remediations:

• Adds a function that will delete any/all empty strings from the parse after a delimiter split.
• Adds a general recovery to the index check, as the (now minimized) breakage of an indexed item is likely preferable than loss of the full artist index response for a client.

While the above restores the artist index, this particular artist will still be split into three separate objects (albeit functionally). Perhaps not ideal, but also an extremely niche case that could likely be addressed at a later time. I'd otherwise expect this to be seen if someone had a typo in their tags.

[sentriz]

i think it would make sense to do this after the

for i := range parts {
		parts[i] = strings.TrimSpace(parts[i])
}

below

so that we trim, then delete. that we we find more matches

[shinterro]

Aha, you are correct, thanks!

[sentriz]

i think i would prefer to just check the length of the string is 0 instead of catching all panics

also, i just noticed that the previous implementation has a subtle bug too. it gets the first byte of the string, where really we probably want the first codepoint/rune
(later this may want to be the first grapheme cluster)

how about:

func lowerUDecOrHash(in string) string {
	inRunes := []rune(in)
	if len(inRunes) == 0 {
		return ""
	}
	lower := unicode.ToLower(inRunes[0])
	if !unicode.IsLetter(lower) {
		return "#"
	}
	return string(lower)
}

[sentriz]

thanks for the fix! just added 2 comments

[sentriz]

hey i think the changes were safe enough so i just authored you and commited them. let me know if that's okay. thanks!