Repository: syncthing/syncthing (language: go)

Issue:
Versions path does not honor tilde (~) shortcut
Hi,

It seems the folder path honors the ~ shortcut, but the versions path does not. This results in rather unexpected behavior. Should the ~ logic not be hooked on both places?

## Situation
Vanilla Syncthing running in a docker container.

## Folder config
Folder path: `~/myfolder/data`
Versions path: `~/myfolder/versions`

## Expected behavior:
- Synced data in `/var/syncthing/myfolder/data`
- Remote deleted data in `/var/syncthing/myfolder/versions`

## Observed behavior:
- Synced data in `/var/syncthing/myfolder/data`
- Remote deleted data in `/var/syncthing/myfolder/data/~/myfolder/versions`

## Additional information
- Syncthing version: v1.26.1 `Gold Grasshopper` (latest as of writing this ticket)
 - OS context: New Linux kernel + [official docker image](https://hub.docker.com/r/syncthing/syncthing/)
Versions path does not honor tilde (~) shortcut
Hi,

It seems the folder path honors the ~ shortcut, but the versions path does not. This results in rather unexpected behavior. Should the ~ logic not be hooked on both places?

## Situation
Vanilla Syncthing running in a docker container.

## Folder config
Folder path: `~/myfolder/data`
Versions path: `~/myfolder/versions`

## Expected behavior:
- Synced data in `/var/syncthing/myfolder/data`
- Remote deleted data in `/var/syncthing/myfolder/versions`

## Observed behavior:
- Synced data in `/var/syncthing/myfolder/data`
- Remote deleted data in `/var/syncthing/myfolder/data/~/myfolder/versions`

## Additional information
- Syncthing version: v1.26.1 `Gold Grasshopper` (latest as of writing this ticket)
 - OS context: New Linux kernel + [official docker image](https://hub.docker.com/r/syncthing/syncthing/)


Discussion / hints:
Probably should. 
Probably should. 

Fix patch:
diff --git a/lib/versioner/util.go b/lib/versioner/util.go
index cb7773da370..a0bcfba1e7e 100644
--- a/lib/versioner/util.go
+++ b/lib/versioner/util.go
@@ -262,10 +262,20 @@ func versionerFsFromFolderCfg(cfg config.FolderConfiguration) (versionsFs fs.Fil
 	folderFs := cfg.Filesystem(nil)
 	if cfg.Versioning.FSPath == "" {
 		versionsFs = fs.NewFilesystem(folderFs.Type(), filepath.Join(folderFs.URI(), DefaultPath))
-	} else if cfg.Versioning.FSType == fs.FilesystemTypeBasic && !filepath.IsAbs(cfg.Versioning.FSPath) {
-		// We only know how to deal with relative folders for basic filesystems, as that's the only one we know
+	} else if cfg.Versioning.FSType == fs.FilesystemTypeBasic {
+		// Expand any leading tildes for basic filesystems,
+		// before checking for absolute paths.
+		path, err := fs.ExpandTilde(cfg.Versioning.FSPath)
+		if err != nil {
+			path = cfg.Versioning.FSPath
+		}
+		// We only know how to deal with relative folders for
+		// basic filesystems, as that's the only one we know
 		// how to check if it's absolute or relative.
-		versionsFs = fs.NewFilesystem(cfg.Versioning.FSType, filepath.Join(folderFs.URI(), cfg.Versioning.FSPath))
+		if !filepath.IsAbs(path) {
+			path = filepath.Join(folderFs.URI(), path)
+		}
+		versionsFs = fs.NewFilesystem(cfg.Versioning.FSType, path)
 	} else {
 		versionsFs = fs.NewFilesystem(cfg.Versioning.FSType, cfg.Versioning.FSPath)
 	}


Test patch:
diff --git a/lib/versioner/simple_test.go b/lib/versioner/simple_test.go
index fe92478e7d0..3b3abb4362e 100644
--- a/lib/versioner/simple_test.go
+++ b/lib/versioner/simple_test.go
@@ -8,7 +8,9 @@ package versioner
 
 import (
 	"math"
+	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -93,3 +95,67 @@ func TestSimpleVersioningVersionCount(t *testing.T) {
 		time.Sleep(time.Second)
 	}
 }
+
+func TestPathTildes(t *testing.T) {
+	// Test that folder and version paths with leading tildes are expanded
+	// to the user's home directory. (issue #9241)
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	if vn := filepath.VolumeName(home); vn != "" {
+		// Legacy Windows home stuff
+		t.Setenv("HomeDrive", vn)
+		t.Setenv("HomePath", strings.TrimPrefix(home, vn))
+	}
+	os.Mkdir(filepath.Join(home, "folder"), 0o755)
+
+	cfg := config.FolderConfiguration{
+		FilesystemType: fs.FilesystemTypeBasic,
+		Path:           "~/folder",
+		Versioning: config.VersioningConfiguration{
+			FSPath: "~/versions",
+			FSType: fs.FilesystemTypeBasic,
+			Params: map[string]string{
+				"keep": "2",
+			},
+		},
+	}
+	fs := cfg.Filesystem(nil)
+	v := newSimple(cfg)
+
+	const testPath = "test"
+
+	f, err := fs.Create(testPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	if err := v.Archive(testPath); err != nil {
+		t.Fatal(err)
+	}
+
+	// Check that there are no entries in the folder directory; this is
+	// specifically to check that there is no directory named "~" there.
+	names, err := fs.DirNames(".")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(names) != 0 {
+		t.Fatalf("found %d files in folder dir, want 0", len(names))
+	}
+
+	// Check that the versions directory contains one file that begins with
+	// our test path.
+	des, err := os.ReadDir(filepath.Join(home, "versions"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, de := range des {
+		names = append(names, de.Name())
+	}
+	if len(names) != 1 {
+		t.Fatalf("found %d files in versions dir, want 1", len(names))
+	}
+	if got := names[0]; !strings.HasPrefix(got, testPath) {
+		t.Fatalf("found versioned file %q, want one that begins with %q", got, testPath)
+	}
+}