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

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

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}")
+
+		internalPath := localStoragePath
+		if !strings.Contains(internalPath, "{filename}") {
+			internalPath = filepath.Join(internalPath, "{filename}")
 		}
-		filePath = filepath.Join(s.Profile.Data, replacePathTemplate(filePath, create.Filename))
+		internalPath = replacePathTemplate(internalPath, create.Filename)
+		internalPath = filepath.ToSlash(internalPath)
+		create.InternalPath = internalPath
 
-		dir := filepath.Dir(filePath)
+		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
 	}
 

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

store/db/mysql/migration/prod/0.19/00_resource.sql

+-- 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')
+    ),
+    ''
+  );

store/db/postgres/migration/prod/0.19/00_resource.sql

+-- 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)
+    ),
+    ''
+  );

store/db/sqlite/migration/prod/0.19/00_resource.sql

+-- 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')
+    ),
+    ''
+  );

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") {

web/src/components/UpdateLocalStorageDialog.tsx

@@ -27,13 +27,22 @@ const UpdateLocalStorageDialog: React.FC<Props> = (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);
+      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();
     }