From 545993a8aae8c46b42d1d1d411febcb14f4fbcbd Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Dec 2025 10:27:42 -0500 Subject: [PATCH] Fix Windows attachment path encoding in blob storage operations (#1144) * Initial plan * Initial plan for fixing Windows attachment path issue Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> * Fix Windows attachment path encoding issue by normalizing to forward slashes Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> * Refactor path normalization into helper function per code review Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> * Update progress - all checks complete Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> --- .scaffold/go.sum | 14 +++++ backend/go.sum | 8 --- .../data/repo/repo_item_attachments.go | 27 ++++++++- .../data/repo/repo_item_attachments_test.go | 56 +++++++++++++++++++ 4 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 .scaffold/go.sum diff --git a/.scaffold/go.sum b/.scaffold/go.sum new file mode 100644 index 00000000..8d91d370 --- /dev/null +++ b/.scaffold/go.sum @@ -0,0 +1,14 @@ +entgo.io/ent v0.14.5 h1:Rj2WOYJtCkWyFo6a+5wB3EfBRP0rnx1fMk6gGA0UUe4= +entgo.io/ent v0.14.5/go.mod h1:zTzLmWtPvGpmSwtkaayM2cm5m819NdM7z7tYPq3vN0U= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/sysadminsmedia/homebox/backend v0.0.0-20251212183312-2d1d3d927bfd h1:QULUJSgHc4rSlTjb2qYT6FIgwDWFCqEpnYqc/ltsrkk= +github.com/sysadminsmedia/homebox/backend v0.0.0-20251212183312-2d1d3d927bfd/go.mod h1:jB+tPmHtPDM1VnAjah0gvcRfP/s7c+rtQwpA8cvZD/U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/backend/go.sum b/backend/go.sum index 72192aaf..7bc52ea2 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -325,8 +325,6 @@ github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/ github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-runewidth v0.0.9 h1:Lm995f3rfxdpd6TSmuVCHVb/QhupuXlYr8sCI/QdE+0= -github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= github.com/mattn/go-sqlite3 v1.14.32 h1:JD12Ag3oLy1zQA+BNn74xRgaBbdhbNIDYvQUEuuErjs= github.com/mattn/go-sqlite3 v1.14.32/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY= @@ -349,8 +347,6 @@ github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOF github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= github.com/olahol/melody v1.4.0 h1:Pa5SdeZL/zXPi1tJuMAPDbl4n3gQOThSL6G1p4qZ4SI= github.com/olahol/melody v1.4.0/go.mod h1:GgkTl6Y7yWj/HtfD48Q5vLKPVoZOH+Qqgfa7CvJgJM4= -github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= -github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= @@ -393,10 +389,6 @@ github.com/shirou/gopsutil/v4 v4.25.11 h1:X53gB7muL9Gnwwo2evPSE+SfOrltMoR6V3xJAX github.com/shirou/gopsutil/v4 v4.25.11/go.mod h1:EivAfP5x2EhLp2ovdpKSozecVXn1TmuG7SMzs/Wh4PU= github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e h1:MRM5ITcdelLK2j1vwZ3Je0FKVCfqOLp5zO6trqMLYs0= github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e/go.mod h1:XV66xRDqSt+GTGFMVlhk3ULuV0y9ZmzeVGR4mloJI3M= -github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= -github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= -github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spiffe/go-spiffe/v2 v2.6.0 h1:l+DolpxNWYgruGQVV0xsfeya3CsC7m8iBzDnMpsbLuo= github.com/spiffe/go-spiffe/v2 v2.6.0/go.mod h1:gm2SeUoMZEtpnzPNs2Csc0D/gX33k1xIx7lEzqblHEs= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/backend/internal/data/repo/repo_item_attachments.go b/backend/internal/data/repo/repo_item_attachments.go index 0cd2feed..d86a52f2 100644 --- a/backend/internal/data/repo/repo_item_attachments.go +++ b/backend/internal/data/repo/repo_item_attachments.go @@ -97,12 +97,35 @@ func ToItemAttachment(attachment *ent.Attachment) ItemAttachment { } } +// normalizePath converts backslashes to forward slashes and trims slashes from both ends +// This ensures consistent path separators for blob storage which expects forward slashes +func normalizePath(path string) string { + path = strings.ReplaceAll(path, "\\", "/") + return strings.Trim(path, "/") +} + func (r *AttachmentRepo) path(gid uuid.UUID, hash string) string { - return filepath.Join(gid.String(), "documents", hash) + // Always use forward slashes for consistency across platforms + // This ensures paths are stored in the database with forward slashes + return fmt.Sprintf("%s/documents/%s", gid.String(), hash) } func (r *AttachmentRepo) fullPath(relativePath string) string { - return filepath.Join(r.storage.PrefixPath, relativePath) + // Normalize path separators to forward slashes for blob storage + // The blob library expects forward slashes in keys regardless of OS + normalizedRelativePath := normalizePath(relativePath) + + // Always use forward slashes when joining paths for blob storage + if r.storage.PrefixPath == "" { + return normalizedRelativePath + } + normalizedPrefix := normalizePath(r.storage.PrefixPath) + + if normalizedPrefix == "" { + return normalizedRelativePath + } + + return fmt.Sprintf("%s/%s", normalizedPrefix, normalizedRelativePath) } func (r *AttachmentRepo) GetFullPath(relativePath string) string { diff --git a/backend/internal/data/repo/repo_item_attachments_test.go b/backend/internal/data/repo/repo_item_attachments_test.go index 0963a160..cbd839e5 100644 --- a/backend/internal/data/repo/repo_item_attachments_test.go +++ b/backend/internal/data/repo/repo_item_attachments_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/sysadminsmedia/homebox/backend/internal/data/ent" "github.com/sysadminsmedia/homebox/backend/internal/data/ent/attachment" + "github.com/sysadminsmedia/homebox/backend/internal/sys/config" ) func TestAttachmentRepo_Create(t *testing.T) { @@ -281,3 +282,58 @@ func TestAttachmentRepo_SettingPhotoPrimaryStillWorks(t *testing.T) { require.NoError(t, err) assert.False(t, photo1.Primary, "Photo 1 should no longer be primary after setting Photo 2 as primary") } + +func TestAttachmentRepo_PathNormalization(t *testing.T) { + // Test that paths always use forward slashes + repo := &AttachmentRepo{ + storage: config.Storage{ + PrefixPath: ".data", + }, + } + + testGUID := uuid.MustParse("eb6bf410-a1a8-478d-a803-ca3948368a0c") + testHash := "f295eb01-18a9-4631-a797-70bd9623edd4.png" + + // Test path() method - should always return forward slashes + relativePath := repo.path(testGUID, testHash) + assert.Equal(t, "eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png", relativePath) + assert.NotContains(t, relativePath, "\\", "path() should not contain backslashes") + + // Test fullPath() with forward slash input (from database) + fullPath := repo.fullPath("eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png") + assert.Equal(t, ".data/eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png", fullPath) + assert.NotContains(t, fullPath, "\\", "fullPath() should not contain backslashes") + + // Test fullPath() with backslash input (legacy Windows paths from old database) + fullPathWithBackslash := repo.fullPath("eb6bf410-a1a8-478d-a803-ca3948368a0c\\documents\\f295eb01-18a9-4631-a797-70bd9623edd4.png") + assert.Equal(t, ".data/eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png", fullPathWithBackslash) + assert.NotContains(t, fullPathWithBackslash, "\\", "fullPath() should normalize backslashes to forward slashes") + + // Test with Windows-style prefix path + repoWindows := &AttachmentRepo{ + storage: config.Storage{ + PrefixPath: ".data", + }, + } + fullPathWindows := repoWindows.fullPath("eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png") + assert.NotContains(t, fullPathWindows, "\\", "fullPath() should normalize Windows paths") + + // Test empty prefix + repoNoPrefix := &AttachmentRepo{ + storage: config.Storage{ + PrefixPath: "", + }, + } + fullPathNoPrefix := repoNoPrefix.fullPath("eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png") + assert.Equal(t, "eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png", fullPathNoPrefix) + + // Test with single slash prefix (like in tests) + repoSlashPrefix := &AttachmentRepo{ + storage: config.Storage{ + PrefixPath: "/", + }, + } + fullPathSlashPrefix := repoSlashPrefix.fullPath("eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png") + assert.Equal(t, "eb6bf410-a1a8-478d-a803-ca3948368a0c/documents/f295eb01-18a9-4631-a797-70bd9623edd4.png", fullPathSlashPrefix) + assert.NotContains(t, fullPathSlashPrefix, "//", "fullPath() should not have double slashes") +}