Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix: Remediation for Panic! at the Parser #448

Closed
wants to merge 2 commits into from
Closed

Bugfix: Remediation for Panic! at the Parser #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c03368e
trim out potential empty strings from parsed inputs
shinterro Jan 2, 2024
24e6390
add panic recovery for parsed item index failures
shinterro Jan 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions scanner/scanner.go
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path/filepath"
"regexp"
"slices"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -714,6 +715,10 @@ func parseMulti(parser tagcommon.Info, setting MultiValueSetting, getMulti func(
default:
parts = []string{get(parser)}
}
// trim potential empty strings from delimited results
Copy link
Owner

@sentriz sentriz Jan 2, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, you are correct, thanks!

parts = slices.DeleteFunc(parts, func(s string) bool {
return s == ""
})
for i := range parts {
parts[i] = strings.TrimSpace(parts[i])
}
Expand Down
5 changes: 5 additions & 0 deletions server/ctrlsubsonic/handlers_common.go
View file
Open in desktop
Expand Up @@ -518,6 +518,11 @@ func getMusicFolder(musicPaths []MusicPath, p params.Params) string {
}

func lowerUDecOrHash(in string) string {
defer func() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
}

if err := recover(); err != nil {
log.Println("unable to index parsed object:", err)
}
}()
lower := unicode.ToLower(rune(in[0]))
if !unicode.IsLetter(lower) {
return "#"
Expand Down