sentriz / gonic

music streaming server / free-software subsonic server API implementation
ircs://irc.libera.chat/#gonic
GNU General Public License v3.0
1.49k stars 105 forks source link

Bugfix: Remediation for Panic! at the Parser #448

Closed shinterro closed 6 months ago

shinterro commented 6 months ago

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:

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 commented 6 months ago

thanks for the fix! just added 2 comments

sentriz commented 6 months ago

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