From 411e807dcc018afbe9a495767ac4a3700912be6e Mon Sep 17 00:00:00 2001 From: Lincoln Nogueira Date: Thu, 28 Dec 2023 20:49:55 -0300 Subject: [PATCH] chore: use consistent relative paths for resources (#2683) - always store resources with a relative path with forward slashes, which will be transformed as needed when the file is accessed - fix an issue with thumbnail generation on Windows - add several validations for local storage setting - improve front-end error feedback when changing local storage - add migrations to make existing resource paths relative (not needed, but improves database consistency) --- api/resource/resource.go | 9 +++++--- api/v1/resource.go | 22 ++++++++++++------- api/v1/system_setting.go | 19 ++++++++++++++++ .../mysql/migration/prod/0.19/00_resource.sql | 19 ++++++++++++++++ .../migration/prod/0.19/00_resource.sql | 19 ++++++++++++++++ .../migration/prod/0.19/00_resource.sql | 19 ++++++++++++++++ store/resource.go | 6 ++++- .../components/UpdateLocalStorageDialog.tsx | 13 +++++++++-- 8 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 store/db/mysql/migration/prod/0.19/00_resource.sql create mode 100644 store/db/postgres/migration/prod/0.19/00_resource.sql create mode 100644 store/db/sqlite/migration/prod/0.19/00_resource.sql diff --git a/api/resource/resource.go b/api/resource/resource.go index c6e6beec..5c94cc0d 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -6,7 +6,6 @@ import ( "io" "net/http" "os" - "path" "path/filepath" "strings" "sync/atomic" @@ -83,7 +82,11 @@ func (s *Service) streamResource(c echo.Context) error { blob := resource.Blob if resource.InternalPath != "" { - resourcePath := resource.InternalPath + resourcePath := filepath.FromSlash(resource.InternalPath) + if !filepath.IsAbs(resourcePath) { + resourcePath = filepath.Join(s.Profile.Data, resourcePath) + } + src, err := os.Open(resourcePath) if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Failed to open the local resource: %s", resourcePath)).SetInternal(err) @@ -142,7 +145,7 @@ func getOrGenerateThumbnailImage(srcBlob []byte, dstPath string) ([]byte, error) } thumbnailImage := imaging.Resize(src, 512, 0, imaging.Lanczos) - dstDir := path.Dir(dstPath) + dstDir := filepath.Dir(dstPath) if err := os.MkdirAll(dstDir, os.ModePerm); err != nil { return nil, errors.Wrap(err, "failed to create thumbnail dir") } diff --git a/api/v1/resource.go b/api/v1/resource.go index 1abb295e..f8935d1b 100644 --- a/api/v1/resource.go +++ b/api/v1/resource.go @@ -416,17 +416,24 @@ func SaveResourceBlob(ctx context.Context, s *store.Store, create *store.Resourc return errors.Wrap(err, "Failed to unmarshal SystemSettingLocalStoragePathName") } } - filePath := filepath.FromSlash(localStoragePath) - if !strings.Contains(filePath, "{filename}") { - filePath = filepath.Join(filePath, "{filename}") - } - filePath = filepath.Join(s.Profile.Data, replacePathTemplate(filePath, create.Filename)) - dir := filepath.Dir(filePath) + internalPath := localStoragePath + if !strings.Contains(internalPath, "{filename}") { + internalPath = filepath.Join(internalPath, "{filename}") + } + internalPath = replacePathTemplate(internalPath, create.Filename) + internalPath = filepath.ToSlash(internalPath) + create.InternalPath = internalPath + + osPath := filepath.FromSlash(internalPath) + if !filepath.IsAbs(osPath) { + osPath = filepath.Join(s.Profile.Data, osPath) + } + dir := filepath.Dir(osPath) if err = os.MkdirAll(dir, os.ModePerm); err != nil { return errors.Wrap(err, "Failed to create directory") } - dst, err := os.Create(filePath) + dst, err := os.Create(osPath) if err != nil { return errors.Wrap(err, "Failed to create file") } @@ -436,7 +443,6 @@ func SaveResourceBlob(ctx context.Context, s *store.Store, create *store.Resourc return errors.Wrap(err, "Failed to copy file") } - create.InternalPath = filePath return nil } diff --git a/api/v1/system_setting.go b/api/v1/system_setting.go index 505376e5..bd05a0cc 100644 --- a/api/v1/system_setting.go +++ b/api/v1/system_setting.go @@ -3,6 +3,7 @@ package v1 import ( "encoding/json" "net/http" + "path/filepath" "strings" "github.com/labstack/echo/v4" @@ -242,6 +243,24 @@ func (upsert UpsertSystemSettingRequest) Validate() error { if err := json.Unmarshal([]byte(upsert.Value), &value); err != nil { return errors.Errorf(systemSettingUnmarshalError, settingName) } + + trimmedValue := strings.TrimSpace(value) + switch { + case trimmedValue != value: + return errors.New("local storage path must not contain leading or trailing whitespace") + case trimmedValue == "": + return errors.New("local storage path can't be empty") + case strings.Contains(trimmedValue, "\\"): + return errors.New("local storage path must use forward slashes `/`") + case strings.Contains(trimmedValue, "../"): + return errors.New("local storage path is not allowed to contain `../`") + case strings.HasPrefix(trimmedValue, "./"): + return errors.New("local storage path is not allowed to start with `./`") + case filepath.IsAbs(trimmedValue) || trimmedValue[0] == '/': + return errors.New("local storage path must be a relative path") + case !strings.Contains(trimmedValue, "{filename}"): + return errors.New("local storage path must contain `{filename}`") + } case SystemSettingTelegramBotTokenName: if upsert.Value == "" { return nil diff --git a/store/db/mysql/migration/prod/0.19/00_resource.sql b/store/db/mysql/migration/prod/0.19/00_resource.sql new file mode 100644 index 00000000..14ce267a --- /dev/null +++ b/store/db/mysql/migration/prod/0.19/00_resource.sql @@ -0,0 +1,19 @@ +-- Make resource internal_path relative (to MEMOS_DATA) and replace backslash with slash +-- This is a best-effort approach, but even if it fails, it won't break assets from loading +UPDATE resource +SET + internal_path = REPLACE (internal_path, '\\', '/') +WHERE + internal_path LIKE '%assets\\\%'; + +UPDATE resource +SET + internal_path = REPLACE ( + internal_path, + SUBSTR ( + internal_path, + 1, + INSTR (internal_path, '/assets') + ), + '' + ); diff --git a/store/db/postgres/migration/prod/0.19/00_resource.sql b/store/db/postgres/migration/prod/0.19/00_resource.sql new file mode 100644 index 00000000..fb4c1e7d --- /dev/null +++ b/store/db/postgres/migration/prod/0.19/00_resource.sql @@ -0,0 +1,19 @@ +-- Make resource internal_path relative (to MEMOS_DATA) and replace backslash with slash +-- This is a best-effort approach, but even if it fails, it won't break assets from loading +UPDATE resource +SET + internal_path = REPLACE (internal_path, '\', '/') +WHERE + internal_path LIKE '%assets\\%'; + +UPDATE resource +SET + internal_path = REPLACE ( + internal_path, + SUBSTRING( + internal_path + FROM + 1 FOR POSITION('/assets' IN internal_path) + ), + '' + ); diff --git a/store/db/sqlite/migration/prod/0.19/00_resource.sql b/store/db/sqlite/migration/prod/0.19/00_resource.sql new file mode 100644 index 00000000..7cdd0c10 --- /dev/null +++ b/store/db/sqlite/migration/prod/0.19/00_resource.sql @@ -0,0 +1,19 @@ +-- Make resource internal_path relative (to MEMOS_DATA) and replace backslash with slash +-- This is a best-effort approach, but even if it fails, it won't break assets from loading +UPDATE resource +SET + internal_path = REPLACE (internal_path, '\', '/') +WHERE + internal_path LIKE '%assets\%'; + +UPDATE resource +SET + internal_path = REPLACE ( + internal_path, + SUBSTR ( + internal_path, + 1, + INSTR (internal_path, '/assets') + ), + '' + ); diff --git a/store/resource.go b/store/resource.go index 4d997945..c2e2b21f 100644 --- a/store/resource.go +++ b/store/resource.go @@ -95,7 +95,11 @@ func (s *Store) DeleteResource(ctx context.Context, delete *DeleteResource) erro // Delete the local file. if resource.InternalPath != "" { - _ = os.Remove(resource.InternalPath) + resourcePath := filepath.FromSlash(resource.InternalPath) + if !filepath.IsAbs(resourcePath) { + resourcePath = filepath.Join(s.Profile.Data, resourcePath) + } + _ = os.Remove(resourcePath) } // Delete the thumbnail. if util.HasPrefixes(resource.Type, "image/png", "image/jpeg") { diff --git a/web/src/components/UpdateLocalStorageDialog.tsx b/web/src/components/UpdateLocalStorageDialog.tsx index cd4f8ccd..25e24b6f 100644 --- a/web/src/components/UpdateLocalStorageDialog.tsx +++ b/web/src/components/UpdateLocalStorageDialog.tsx @@ -27,12 +27,21 @@ const UpdateLocalStorageDialog: React.FC = (props: Props) => { try { await api.upsertSystemSetting({ name: "local-storage-path", - value: JSON.stringify(path), + value: JSON.stringify(path.trim()), }); await globalStore.fetchSystemStatus(); } catch (error: any) { console.error(error); - toast.error(error.response.data.message); + if (error.response.data.error) { + const errorText = error.response.data.error as string; + const internalIndex = errorText.indexOf("internal="); + if (internalIndex !== -1) { + const internalError = errorText.substring(internalIndex + 9); + toast.error(internalError); + } + } else { + toast.error(error.response.data.message); + } } if (confirmCallback) { confirmCallback();