From e052fda7929a066dfa679e169167c1b381e60443 Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Sun, 9 Aug 2026 16:37:25 +0200 Subject: [PATCH] fix(server): classify library path validation errors instead of a catch-all (#4595) Signed-off-by: junkerderprovinz --- core/library.go | 37 ++++++++++++++++++++++--------------- core/library_test.go | 4 ++-- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/core/library.go b/core/library.go index 365dcbd4c..4537c3632 100644 --- a/core/library.go +++ b/core/library.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io/fs" - "os" "path/filepath" "strconv" "strings" @@ -359,25 +358,14 @@ func (r *libraryRepositoryWrapper) validateLibraryPath(library *model.Library) e fsys, err := fileStore.FS() if err != nil { log.Warn(r.ctx, "Error validating library.path", "path", library.Path, err) - return fmt.Errorf("resources.library.validation.pathInvalid") + return errors.New(classifyLibraryPathError(err)) } - // Check if root directory exists + // Check if root directory exists and is accessible info, err := fs.Stat(fsys, ".") if err != nil { - // Parse the error message to check for "not a directory" log.Warn(r.ctx, "Error stating library.path", "path", library.Path, err) - errStr := err.Error() - if strings.Contains(errStr, "not a directory") || - strings.Contains(errStr, "The directory name is invalid.") { - return fmt.Errorf("resources.library.validation.pathNotDirectory") - } else if os.IsNotExist(err) { - return fmt.Errorf("resources.library.validation.pathNotFound") - } else if os.IsPermission(err) { - return fmt.Errorf("resources.library.validation.pathNotAccessible") - } else { - return fmt.Errorf("resources.library.validation.pathInvalid") - } + return errors.New(classifyLibraryPathError(err)) } if !info.IsDir() { @@ -387,6 +375,25 @@ func (r *libraryRepositoryWrapper) validateLibraryPath(library *model.Library) e return nil } +// classifyLibraryPathError maps a filesystem error from opening or stating a +// library path to the specific i18n validation code, so the UI can show an +// actionable message instead of the generic "invalid path" catch-all. It uses +// errors.Is (not os.IsNotExist/os.IsPermission) because the storage layer wraps +// the underlying error with %w, which those helpers do not unwrap. +func classifyLibraryPathError(err error) string { + switch { + case errors.Is(err, fs.ErrNotExist): + return "resources.library.validation.pathNotFound" + case errors.Is(err, fs.ErrPermission): + return "resources.library.validation.pathNotAccessible" + case strings.Contains(err.Error(), "not a directory") || + strings.Contains(err.Error(), "The directory name is invalid."): + return "resources.library.validation.pathNotDirectory" + default: + return "resources.library.validation.pathInvalid" + } +} + func (s *libraryService) validateLibraryIDs(ctx context.Context, libraryIDs []int) error { if len(libraryIDs) == 0 { return nil diff --git a/core/library_test.go b/core/library_test.go index 175d9c37d..65aa94e18 100644 --- a/core/library_test.go +++ b/core/library_test.go @@ -335,7 +335,7 @@ var _ = Describe("Library Service", func() { Expect(err).To(HaveOccurred()) var validationErr *rest.ValidationError Expect(errors.As(err, &validationErr)).To(BeTrue()) - Expect(validationErr.Errors["path"]).To(Equal("resources.library.validation.pathInvalid")) + Expect(validationErr.Errors["path"]).To(Equal("resources.library.validation.pathNotFound")) }) It("fails when path is a file instead of directory", func() { @@ -415,7 +415,7 @@ var _ = Describe("Library Service", func() { Expect(err).To(HaveOccurred()) var validationErr *rest.ValidationError Expect(errors.As(err, &validationErr)).To(BeTrue()) - Expect(validationErr.Errors["path"]).To(Equal("resources.library.validation.pathInvalid")) + Expect(validationErr.Errors["path"]).To(Equal("resources.library.validation.pathNotFound")) }) It("fails when updated path is a file instead of directory", func() {