From bf9bbfb5fb10b19899a9a80299a7230458dca40e Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Tue, 26 May 2026 02:34:48 +0000 Subject: [PATCH] Folders: Fix slug collision on long ASCII paths #5614 #5615 --- internal/entity/album.go | 4 +- internal/entity/album_test.go | 76 +++++++++++++++++++++++ internal/entity/folder.go | 4 +- internal/entity/folder_album_slug.go | 6 +- internal/entity/folder_album_slug_test.go | 44 +++++++++++++ pkg/txt/slug.go | 30 +++++++++ pkg/txt/slug_test.go | 39 ++++++++++++ 7 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 internal/entity/folder_album_slug_test.go diff --git a/internal/entity/album.go b/internal/entity/album.go index d222b670c..547724ea5 100644 --- a/internal/entity/album.go +++ b/internal/entity/album.go @@ -238,7 +238,7 @@ func NewUserAlbum(albumTitle, albumType, sortOrder, userUid string) *Album { // NewFolderAlbum creates a new album representing a filesystem folder. func NewFolderAlbum(albumTitle, albumPath, albumFilter string) *Album { - albumSlug := txt.Slug(albumPath) + albumSlug := txt.SlugUnique(albumPath) if albumTitle == "" || albumSlug == "" || albumPath == "" || albumFilter == "" { return nil @@ -817,7 +817,7 @@ func (m *Album) UpdateFolder(albumPath, albumFilter, albumTitle string) error { } albumPath = clean.SlashPath(albumPath) - albumSlug := txt.Slug(albumPath) + albumSlug := txt.SlugUnique(albumPath) repairTitle := shouldRepairFolderAlbumTitle(m.AlbumTitle, albumTitle, albumPath, m.AlbumFilter) if albumSlug == "" || albumPath == "" || albumFilter == "" || !m.HasID() { diff --git a/internal/entity/album_test.go b/internal/entity/album_test.go index 04712ff2e..ae0bc7143 100644 --- a/internal/entity/album_test.go +++ b/internal/entity/album_test.go @@ -516,6 +516,82 @@ func TestFindFolderAlbum(t *testing.T) { assert.Equal(t, legacy.ID, album.ID) }) + t.Run("LegacyFallbackForLongAsciiPath", func(t *testing.T) { + unique := txt.Slug(time.Now().UTC().Format(time.RFC3339Nano)) + folderPath := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/" + unique + "/01 Praha, Dresden, Wroclaw" + + legacy := &Album{ + AlbumType: AlbumFolder, + AlbumSlug: legacyFolderAlbumSlug(folderPath), + AlbumPath: "", + AlbumFilter: `path:"` + folderPath + `" public:true`, + CreatedAt: Now(), + UpdatedAt: Now(), + } + legacy.SetTitle("Legacy Folder") + + if err := legacy.Create(); err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + _ = legacy.DeletePermanently() + }) + + album := FindFolderAlbum(folderPath) + if album == nil { + t.Fatal("expected legacy album") + } + + assert.Equal(t, legacy.ID, album.ID) + assert.NotEqual(t, txt.SlugUnique(folderPath), legacy.AlbumSlug) + }) + t.Run("LongAsciiSiblingsDoNotCollide", func(t *testing.T) { + unique := txt.Slug(time.Now().UTC().Format(time.RFC3339Nano)) + base := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/" + unique + "/" + pathA := base + "01 Praha, Dresden, Wroclaw" + pathB := base + "02 Wroclaw, Auschwitz" + filterA := `path:"` + pathA + `" public:true` + filterB := `path:"` + pathB + `" public:true` + + albumA := NewFolderAlbum("01 Praha, Dresden, Wroclaw", pathA, filterA) + if albumA == nil { + t.Fatal("expected albumA") + } + + if err := albumA.Create(); err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + _ = albumA.DeletePermanently() + }) + + albumB := NewFolderAlbum("02 Wroclaw, Auschwitz", pathB, filterB) + if albumB == nil { + t.Fatal("expected albumB") + } + + if err := albumB.Create(); err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + _ = albumB.DeletePermanently() + }) + + assert.NotEqual(t, albumA.AlbumSlug, albumB.AlbumSlug) + + foundA := FindFolderAlbum(pathA) + foundB := FindFolderAlbum(pathB) + + if foundA == nil || foundB == nil { + t.Fatalf("expected both folder albums to resolve, got A=%v B=%v", foundA, foundB) + } + + assert.Equal(t, albumA.ID, foundA.ID) + assert.Equal(t, albumB.ID, foundB.ID) + }) } // TestFindAlbum exercises the related album behavior. diff --git a/internal/entity/folder.go b/internal/entity/folder.go index 03f3308cf..4ad92251e 100644 --- a/internal/entity/folder.go +++ b/internal/entity/folder.go @@ -141,9 +141,9 @@ func (m *Folder) SetValuesFromPath() { } } -// Slug returns a slug based on the folder title. +// Slug returns a collision-resistant slug derived from the folder path. func (m *Folder) Slug() string { - return txt.Slug(m.Path) + return txt.SlugUnique(m.Path) } // RootPath returns the full folder path including root. diff --git a/internal/entity/folder_album_slug.go b/internal/entity/folder_album_slug.go index cb9ad09b7..fedf563d3 100644 --- a/internal/entity/folder_album_slug.go +++ b/internal/entity/folder_album_slug.go @@ -10,6 +10,8 @@ import ( ) // folderAlbumSlugCandidates returns the current and legacy slug candidates for a folder path. +// The first entry is the canonical slug used by new rows; later entries cover legacy formats +// so FindFolderAlbum can resolve rows written before the collision-safe slug landed. func folderAlbumSlugCandidates(albumPath string) []string { albumPath = clean.SlashPath(albumPath) @@ -17,9 +19,9 @@ func folderAlbumSlugCandidates(albumPath string) []string { return nil } - candidates := make([]string, 0, 2) + candidates := make([]string, 0, 3) - for _, value := range []string{txt.Slug(albumPath), legacyFolderAlbumSlug(albumPath)} { + for _, value := range []string{txt.SlugUnique(albumPath), txt.Slug(albumPath), legacyFolderAlbumSlug(albumPath)} { if value == "" { continue } diff --git a/internal/entity/folder_album_slug_test.go b/internal/entity/folder_album_slug_test.go new file mode 100644 index 000000000..25c7403ce --- /dev/null +++ b/internal/entity/folder_album_slug_test.go @@ -0,0 +1,44 @@ +package entity + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/photoprism/photoprism/pkg/txt" +) + +func TestFolderAlbumSlugCandidates(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + assert.Nil(t, folderAlbumSlugCandidates("")) + }) + t.Run("ShortAsciiPath", func(t *testing.T) { + candidates := folderAlbumSlugCandidates("pictures/2024") + assert.Equal(t, []string{"pictures-2024"}, candidates) + }) + t.Run("LongAsciiPathExposesHashAndLegacy", func(t *testing.T) { + path := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/ferie 2008 mellomeuropa/galleri/01 Praha, Dresden, Wroclaw" + candidates := folderAlbumSlugCandidates(path) + + if len(candidates) < 2 { + t.Fatalf("expected at least 2 candidates, got %v", candidates) + } + + assert.Equal(t, txt.SlugUnique(path), candidates[0]) + assert.Contains(t, candidates, legacyFolderAlbumSlug(path)) + assert.NotEqual(t, candidates[0], legacyFolderAlbumSlug(path)) + }) + t.Run("LongAsciiSiblingsDoNotCollide", func(t *testing.T) { + base := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/ferie 2008 mellomeuropa/galleri/" + a := folderAlbumSlugCandidates(base + "01 Praha, Dresden, Wroclaw") + b := folderAlbumSlugCandidates(base + "02 Wroclaw, Auschwitz") + + assert.NotEqual(t, a[0], b[0]) + }) + t.Run("LegacyAndCurrentMatchForShortPath", func(t *testing.T) { + path := "pictures/" + strings.Repeat("x", 4) + candidates := folderAlbumSlugCandidates(path) + assert.Len(t, candidates, 1) + }) +} diff --git a/pkg/txt/slug.go b/pkg/txt/slug.go index 845080807..c1229de61 100644 --- a/pkg/txt/slug.go +++ b/pkg/txt/slug.go @@ -44,6 +44,36 @@ func Slug(s string) string { return Clip(result, ClipSlug) } +// SlugUnique converts a string to a slug like Slug, but always uses a stable +// hash suffix instead of plain truncation when the result would exceed +// ClipSlug runes. Use it where two distinct long inputs sharing a common +// prefix would otherwise collide on the truncated slug (e.g. deeply nested +// folder album paths with identical ancestor segments). +func SlugUnique(s string) string { + s = strings.TrimSpace(s) + + if s == "" || s == "-" { + return s + } + + if s[0] == SlugEncoded && ContainsAlnumLower(s[1:]) { + return Clip(s, ClipSlug) + } + + result := slug.Make(s) + + if result == "" { + result = string(SlugEncoded) + SlugEncoding.EncodeToString([]byte(s)) + return Clip(result, ClipSlug) + } + + if tokens := slugRuneTokens(s); tokens != "" { + result += "-" + tokens + } + + return clipSlugWithHash(result, s) +} + // slugRuneTokens returns stable rune tokens for non-ASCII symbols that slug.Make would drop. func slugRuneTokens(s string) string { tokens := make([]string, 0, 4) diff --git a/pkg/txt/slug_test.go b/pkg/txt/slug_test.go index d68e5f793..b394a2c73 100644 --- a/pkg/txt/slug_test.go +++ b/pkg/txt/slug_test.go @@ -41,6 +41,45 @@ func TestSlug(t *testing.T) { slugA := Slug(base + "Alpha") slugB := Slug(base + "Beta") + assert.NotEqual(t, slugA, slugB) + assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugA)) + assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugB)) + }) + t.Run("LongAsciiInputCollides", func(t *testing.T) { + base := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/ferie 2008 mellomeuropa/galleri/" + slugA := Slug(base + "01 Praha, Dresden, Wroclaw") + slugB := Slug(base + "02 Wroclaw, Auschwitz") + + assert.Equal(t, slugA, slugB) + assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugA)) + }) +} + +func TestSlugUnique(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + assert.Equal(t, "", SlugUnique("")) + }) + t.Run("ShortMatchesSlug", func(t *testing.T) { + assert.Equal(t, Slug("William Henry Gates III"), SlugUnique("William Henry Gates III")) + assert.Equal(t, Slug("PhotoPrism 💎"), SlugUnique("PhotoPrism 💎")) + }) + t.Run("EmojiMatchesSlug", func(t *testing.T) { + assert.Equal(t, Slug("ins/🍷"), SlugUnique("ins/🍷")) + }) + t.Run("LongAsciiInputUsesHashSuffix", func(t *testing.T) { + base := "pictures/Ferie 2008 Mellomeuropa/Galleri-konvertert/bilder/ferie 2008 mellomeuropa/galleri/" + slugA := SlugUnique(base + "01 Praha, Dresden, Wroclaw") + slugB := SlugUnique(base + "02 Wroclaw, Auschwitz") + + assert.NotEqual(t, slugA, slugB) + assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugA)) + assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugB)) + }) + t.Run("LongUnicodeInputUsesHashSuffix", func(t *testing.T) { + base := strings.Repeat("Very Long Prefix 💎 ", 8) + slugA := SlugUnique(base + "Alpha") + slugB := SlugUnique(base + "Beta") + assert.NotEqual(t, slugA, slugB) assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugA)) assert.Equal(t, ClipSlug, utf8.RuneCountInString(slugB))