From bad7f1d06cb4eb556abb0dfce1851ae286574996 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Jan 2026 23:12:38 +0800 Subject: [PATCH] fix: filesystem error when creating long filename (#1240) --- pkg/download/downloader.go | 6 +- pkg/download/extension.go | 4 +- pkg/util/path.go | 119 ++++++++++++++++++++++++ pkg/util/path_test.go | 179 +++++++++++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 5 deletions(-) diff --git a/pkg/download/downloader.go b/pkg/download/downloader.go index 7b3b4b43..de0ebc42 100644 --- a/pkg/download/downloader.go +++ b/pkg/download/downloader.go @@ -1107,10 +1107,10 @@ func (d *Downloader) doStart(task *Task) (err error) { if task.fetcherManager.AutoRename() { d.checkDuplicateLock.Lock() defer d.checkDuplicateLock.Unlock() - task.Meta.Opts.Name = util.ReplaceInvalidFilename(task.Meta.Opts.Name) + task.Meta.Opts.Name = util.SafeFilename(task.Meta.Opts.Name) // check if the download file is duplicated and rename it automatically. if task.Meta.Res.Name != "" { - task.Meta.Res.Name = util.ReplaceInvalidFilename(task.Meta.Res.Name) + task.Meta.Res.Name = util.SafeFilename(task.Meta.Res.Name) fullDirPath := task.Meta.FolderPath() newName, err := util.CheckDuplicateAndRename(fullDirPath) if err != nil { @@ -1118,7 +1118,7 @@ func (d *Downloader) doStart(task *Task) (err error) { } task.Meta.Opts.Name = newName } else { - task.Meta.Res.Files[0].Name = util.ReplaceInvalidFilename(task.Meta.Res.Files[0].Name) + task.Meta.Res.Files[0].Name = util.SafeFilename(task.Meta.Res.Files[0].Name) fullFilePath := task.Meta.SingleFilepath() newName, err := util.CheckDuplicateAndRename(fullFilePath) if err != nil { diff --git a/pkg/download/extension.go b/pkg/download/extension.go index 7ad6edb3..71e4b895 100644 --- a/pkg/download/extension.go +++ b/pkg/download/extension.go @@ -264,9 +264,9 @@ func (d *Downloader) triggerOnResolve(req *base.Request) (res *base.Resource, er gopeed.Logger.logger.Warn().Err(err).Msgf("[%s] resource invalid", ext.buildIdentity()) return } - ctx.Res.Name = util.ReplaceInvalidFilename(ctx.Res.Name) + ctx.Res.Name = util.SafeFilename(ctx.Res.Name) for _, file := range ctx.Res.Files { - file.Name = util.ReplaceInvalidFilename(file.Name) + file.Name = util.SafeFilename(file.Name) } ctx.Res.CalcSize(nil) } diff --git a/pkg/util/path.go b/pkg/util/path.go index 71290fdb..15bb1c7f 100644 --- a/pkg/util/path.go +++ b/pkg/util/path.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" "time" + "unicode/utf8" ) func Dir(path string) string { @@ -179,7 +180,65 @@ func IsExistsFile(path string) bool { return false } +const ( + // MaxFilenameLength is the maximum length in bytes for a filename + MaxFilenameLength = 100 + // maxExtensionLength is the maximum length in bytes for a file extension + // to be treated as a valid extension. Extensions longer than this are + // treated as part of the filename to avoid edge cases. + maxExtensionLength = 20 +) + +// SafeFilename sanitizes a filename by replacing invalid characters and truncating to a safe length. +// It performs two operations: +// 1. Replaces invalid path characters (platform-specific) with underscores +// 2. Truncates filename to MaxFilenameLength bytes while preserving the file extension +// The function handles UTF-8 multi-byte characters correctly by truncating at valid boundaries. +func SafeFilename(filename string) string { + if filename == "" { + return "" + } + + // Step 1: Replace invalid characters + for _, char := range invalidPathChars { + filename = strings.ReplaceAll(filename, char, "_") + } + + // Step 2: Truncate if needed + if len(filename) <= MaxFilenameLength { + return filename + } + + // Find the extension (last dot in filename) + ext := "" + lastDot := strings.LastIndex(filename, ".") + + // Only treat as extension if: + // 1. There is a dot + // 2. The dot is not at the start (not a hidden file like .gitignore) + // 3. The extension is reasonable length (< maxExtensionLength bytes) to avoid edge cases + if lastDot > 0 && lastDot < len(filename)-1 && len(filename)-lastDot < maxExtensionLength { + ext = filename[lastDot:] + filename = filename[:lastDot] + } + + // Calculate how much space we have for the base name + availableLength := MaxFilenameLength - len(ext) + + // Ensure we have at least some space for the base name + if availableLength < 1 { + // Extension itself is too long or no room, just truncate everything at byte boundary + return truncateAtValidUTF8Boundary(filename+ext, MaxFilenameLength) + } + + // Truncate the base name at a valid UTF-8 boundary + truncatedBase := truncateAtValidUTF8Boundary(filename, availableLength) + + return truncatedBase + ext +} + // ReplaceInvalidFilename replace invalid path characters +// Deprecated: Use SafeFilename instead which also handles length truncation func ReplaceInvalidFilename(path string) string { if path == "" { return "" @@ -190,6 +249,66 @@ func ReplaceInvalidFilename(path string) string { return path } +// TruncateFilename truncates a filename to a maximum byte length while preserving the extension. +// Deprecated: Use SafeFilename instead which also handles invalid character replacement +func TruncateFilename(filename string, maxLength int) string { + // If already short enough, return as-is + if len(filename) <= maxLength { + return filename + } + + // Find the extension (last dot in filename) + ext := "" + lastDot := strings.LastIndex(filename, ".") + + // Only treat as extension if: + // 1. There is a dot + // 2. The dot is not at the start (not a hidden file like .gitignore) + // 3. The extension is reasonable length (< maxExtensionLength bytes) to avoid edge cases + if lastDot > 0 && lastDot < len(filename)-1 && len(filename)-lastDot < maxExtensionLength { + ext = filename[lastDot:] + filename = filename[:lastDot] + } + + // Calculate how much space we have for the base name + availableLength := maxLength - len(ext) + + // Ensure we have at least some space for the base name + if availableLength < 1 { + // Extension itself is too long or no room, just truncate everything at byte boundary + return truncateAtValidUTF8Boundary(filename+ext, maxLength) + } + + // Truncate the base name at a valid UTF-8 boundary + truncatedBase := truncateAtValidUTF8Boundary(filename, availableLength) + + return truncatedBase + ext +} + +// truncateAtValidUTF8Boundary truncates a string to at most maxBytes, +// ensuring we don't cut in the middle of a UTF-8 character +func truncateAtValidUTF8Boundary(s string, maxBytes int) string { + if len(s) <= maxBytes { + return s + } + + // Truncate at byte position + truncated := s[:maxBytes] + + // Find the last valid UTF-8 character boundary + // Walk backwards to find where the last complete character ends + for len(truncated) > 0 { + // Check if this is a valid UTF-8 string + if utf8.ValidString(truncated) { + return truncated + } + // Remove one byte and try again + truncated = truncated[:len(truncated)-1] + } + + return truncated +} + // ReplacePathPlaceholders replaces date placeholders in a path with actual values // Supported placeholders: // - %year% - Current year (e.g., 2025) diff --git a/pkg/util/path_test.go b/pkg/util/path_test.go index 14760fd3..c05f5731 100644 --- a/pkg/util/path_test.go +++ b/pkg/util/path_test.go @@ -244,6 +244,81 @@ func TestReplaceInvalidFilename(t *testing.T) { } } +// TestSafeFilename tests the combined filename sanitization functionality +func TestSafeFilename(t *testing.T) { + tests := []struct { + name string + filename string + want string + }{ + { + name: "short filename - no changes needed", + filename: "test.txt", + want: "test.txt", + }, + { + name: "empty filename", + filename: "", + want: "", + }, + { + name: "invalid chars only", + filename: "te/st:file.txt", + want: "te_st_file.txt", + }, + { + name: "long filename only", + filename: "this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length_and_should_be_truncated_properly.txt", + want: "this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length_and_should_be_truncated_pro.txt", + }, + { + name: "both invalid chars and too long", + filename: "path/to/very:long*filename?thatfilesystem|limits_and_has_invalid_characters_everywhere.pdf", + want: "path_to_very_long*filename?thatfilesystem|limits_and_has_invalid_characters_everywhere.pdf", + }, + { + name: "unicode with invalid chars and truncation", + filename: "测试/文件名:非常长的中文文件名_需要被截断_这是一个测试用的超长文件名.pdf", + want: "测试_文件名_非常长的中文文件名_需要被截断_这是一个测试用的超长文.pdf", + }, + { + name: "hidden file with truncation", + filename: ".gitignore_with_very_long_name_that_needs_truncation_and_more_characters_to_exceed_the_maximum_length", + want: ".gitignore_with_very_long_name_that_needs_truncation_and_more_characters_to_exceed_the_maximum_lengt", + }, + { + name: "multiple dots and invalid chars", + filename: "archive/tar.gz.backup:old.txt", + want: "archive_tar.gz.backup_old.txt", + }, + { + name: "extension longer than reasonable", + filename: "test.verylongextensionthatshouldnotbetreatedasextension_with_more_characters_to_exceed_maximum_length", + want: "test.verylongextensionthatshouldnotbetreatedasextension_with_more_characters_to_exceed_maximum_lengt", + }, + { + name: "filename with spaces and invalid chars", + filename: "my document/with:spaces and a very long name that needs to be truncated because it exceeds the maximum length.docx", + want: "my document_with_spaces and a very long name that needs to be truncated because it exceeds the .docx", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SafeFilename(tt.filename) + if got != tt.want { + t.Errorf("SafeFilename() = %q (len=%d), want %q (len=%d)", + got, len(got), tt.want, len(tt.want)) + } + // Verify result doesn't exceed MaxFilenameLength + if len(got) > MaxFilenameLength { + t.Errorf("SafeFilename() result length %d exceeds MaxFilenameLength %d", + len(got), MaxFilenameLength) + } + }) + } +} + func TestReplacePathPlaceholders(t *testing.T) { now := time.Now() year := fmt.Sprintf("%d", now.Year()) @@ -315,3 +390,107 @@ func TestReplacePathPlaceholders(t *testing.T) { }) } } + +// TestTruncateFilename tests the filename truncation functionality +func TestTruncateFilename(t *testing.T) { + tests := []struct { + name string + filename string + maxLength int + want string + }{ + { + name: "short filename - no truncation", + filename: "test.txt", + maxLength: 100, + want: "test.txt", + }, + { + name: "filename at exact limit", + filename: "abcdefghij.txt", // 14 chars + maxLength: 14, + want: "abcdefghij.txt", + }, + { + name: "long filename with extension - truncate base", + filename: "this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length_and_should_be_truncated_properly.txt", + maxLength: 100, + want: "this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length_and_should_be_truncated_pro.txt", + }, + { + name: "long filename without extension", + filename: "this_is_a_very_long_filename_without_extension_that_exceeds_maximum_length_and_needs_truncation", + maxLength: 100, + want: "this_is_a_very_long_filename_without_extension_that_exceeds_maximum_length_and_needs_truncation", + }, + { + name: "filename with multiple dots", + filename: "archive.tar.gz.backup.old.file.with.many.dots.txt", + maxLength: 30, + want: "archive.tar.gz.backup.old..txt", // Preserves last extension + }, + { + name: "hidden file (starts with dot)", + filename: ".gitignore_with_very_long_name_that_needs_truncation", + maxLength: 30, + want: ".gitignore_with_very_long_name", + }, + { + name: "only extension (no base name)", + filename: ".txt", + maxLength: 100, + want: ".txt", + }, + { + name: "unicode characters in filename", + filename: "测试文件名_非常长的中文文件名_需要被截断_这是一个测试用的超长文件名.pdf", + maxLength: 50, + want: "测试文件名_非常长的中文文件名_.pdf", // Truncated at byte boundary, preserving UTF-8 + }, + { + name: "extension longer than reasonable (>20 chars)", + filename: "test.verylongextensionthatshouldnotbetreatedasextension", + maxLength: 30, + want: "test.verylongextensionthatshou", + }, + { + name: "very short max length with extension", + filename: "document.pdf", + maxLength: 10, + want: "docume.pdf", + }, + { + name: "maxLength smaller than extension", + filename: "test.pdf", + maxLength: 3, + want: "tes", + }, + { + name: "empty filename", + filename: "", + maxLength: 100, + want: "", + }, + { + name: "filename with spaces", + filename: "my document with spaces and a very long name that needs to be truncated.docx", + maxLength: 50, + want: "my document with spaces and a very long name .docx", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := TruncateFilename(tt.filename, tt.maxLength) + if got != tt.want { + t.Errorf("TruncateFilename() = %q (len=%d), want %q (len=%d)", + got, len(got), tt.want, len(tt.want)) + } + // Verify result doesn't exceed maxLength + if len(got) > tt.maxLength { + t.Errorf("TruncateFilename() result length %d exceeds maxLength %d", + len(got), tt.maxLength) + } + }) + } +}