From abd16d53264c71837e93d02943723022656dc5f9 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 22:56:04 +0800 Subject: [PATCH] fix: filename truncation for HTML-encoded ampersands in Content-Disposition (#1215) --- internal/protocol/http/fetcher.go | 117 ++++++++++++++- internal/protocol/http/fetcher_test.go | 24 +++ internal/protocol/http/filename_parse_test.go | 138 ++++++++++++++++++ internal/test/httptest.go | 29 ++++ 4 files changed, 303 insertions(+), 5 deletions(-) create mode 100644 internal/protocol/http/filename_parse_test.go diff --git a/internal/protocol/http/fetcher.go b/internal/protocol/http/fetcher.go index 8f552d5b..909eb30e 100644 --- a/internal/protocol/http/fetcher.go +++ b/internal/protocol/http/fetcher.go @@ -631,6 +631,86 @@ func parseFilename(contentDisposition string) string { return parseFilenameFallback(contentDisposition) } +// findParamValueEnd finds the end position of a parameter value in a Content-Disposition header. +// It correctly handles quoted values where semicolons inside quotes should not be treated as delimiters. +// It also handles HTML entities in unquoted values (e.g., & should not be split at the semicolon). +// Returns the end index (exclusive) of the value, or -1 if it extends to the end of the string. +func findParamValueEnd(value string) int { + value = strings.TrimSpace(value) + if len(value) == 0 { + return 0 + } + + // If the value starts with a quote, find the matching closing quote + if value[0] == '"' || value[0] == '\'' { + quote := value[0] + // Find the closing quote, handling escaped quotes + for i := 1; i < len(value); i++ { + if value[i] == quote { + // Check if it's escaped + if i > 0 && value[i-1] == '\\' { + continue + } + // Found closing quote, now look for ; after it + remaining := value[i+1:] + if semiIdx := strings.Index(remaining, ";"); semiIdx != -1 { + return i + 1 + semiIdx + } + return -1 // No semicolon after closing quote + } + } + // No closing quote found, treat rest of string as value + return -1 + } + + // Unquoted value - find the next semicolon that's not part of an HTML entity + // HTML entities have the pattern &...; (e.g., & < > " ') + for i := 0; i < len(value); i++ { + if value[i] == ';' { + // Check if this semicolon is part of an HTML entity + // Look backwards for an & character + isEntity := false + if i > 0 { + // Look for & before this semicolon (within reasonable distance, max 10 chars) + for j := i - 1; j >= 0 && j >= i-10; j-- { + if value[j] == '&' { + // Found &, this semicolon might be part of an HTML entity + // Check if there are only alphanumeric or # between & and ; + entityChars := value[j+1 : i] + if len(entityChars) > 0 && isValidHTMLEntityChars(entityChars) { + isEntity = true + } + break + } + // If we hit whitespace or another special char, stop looking + if value[j] == ' ' || value[j] == '"' || value[j] == '\'' { + break + } + } + } + + if !isEntity { + return i + } + } + } + return -1 // No semicolon, extends to end +} + +// isValidHTMLEntityChars checks if a string contains only valid HTML entity characters +// (alphanumeric and #, typically for entities like & < ' etc.) +func isValidHTMLEntityChars(s string) bool { + if len(s) == 0 { + return false + } + for _, c := range s { + if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '#') { + return false + } + } + return true +} + // parseFilenameExtended parses RFC 5987/RFC 2231 extended parameter format // Format: filename*=charset'language'value (e.g., UTF-8''%E6%B5%8B%E8%AF%95.zip) func parseFilenameExtended(cd string) string { @@ -644,8 +724,9 @@ func parseFilenameExtended(cd string) string { // Extract the value after filename*= value := cd[idx+len("filename*="):] - // Find the end of the value (next ; or end of string) - if endIdx := strings.Index(value, ";"); endIdx != -1 { + // Find the end of the value using proper quote handling + endIdx := findParamValueEnd(value) + if endIdx != -1 { value = value[:endIdx] } value = strings.TrimSpace(value) @@ -677,8 +758,13 @@ func parseFilenameExtended(cd string) string { } // decodeFilenameParam decodes a filename parameter value -// Handles MIME encoded-word, URL encoding, and GBK encoding fallback +// Handles HTML entities, MIME encoded-word, URL encoding, and GBK encoding fallback func decodeFilenameParam(filename string) string { + // First, unescape HTML entities (e.g., & -> &, < -> <, > -> >) + // This must be done before other decoding to handle cases where servers + // HTML-encode special characters in filenames + filename = unescapeHTMLEntities(filename) + // Check if the filename is MIME encoded-word (e.g., =?UTF-8?B?...?=) if strings.HasPrefix(filename, "=?") { decoder := new(mime.WordDecoder) @@ -704,6 +790,26 @@ func decodeFilenameParam(filename string) string { return decoded } +// unescapeHTMLEntities unescapes common HTML entities in filenames +// This handles cases where servers HTML-encode special characters like & to & +func unescapeHTMLEntities(s string) string { + // Common HTML entities that might appear in filenames + replacements := map[string]string{ + "&": "&", + "<": "<", + ">": ">", + """: "\"", + "'": "'", + "'": "'", + } + + result := s + for entity, char := range replacements { + result = strings.ReplaceAll(result, entity, char) + } + return result +} + // gbkDecoder is a reusable GBK decoder for better performance var gbkDecoder = simplifiedchinese.GBK.NewDecoder() @@ -742,8 +848,9 @@ func parseFilenameFallback(cd string) string { // Skip "filename=" prefix value := cd[idx+len("filename="):] - // Find the end of the value - if endIdx := strings.Index(value, ";"); endIdx != -1 { + // Find the end of the value using proper quote handling + endIdx := findParamValueEnd(value) + if endIdx != -1 { value = value[:endIdx] } value = strings.TrimSpace(value) diff --git a/internal/protocol/http/fetcher_test.go b/internal/protocol/http/fetcher_test.go index d6bb59ed..c4a57d83 100644 --- a/internal/protocol/http/fetcher_test.go +++ b/internal/protocol/http/fetcher_test.go @@ -139,6 +139,30 @@ func TestFetcher_Resolve(t *testing.T) { }, }, }, t) + // Test filename with HTML-encoded ampersand (fixes issue with & being truncated) + // Before fix: "查询处理&优化.pptx" -> "查询处理&" (truncated at semicolon) + // After fix: correctly decoded to "查询处理&优化.pptx" + testResolve(test.StartTestCustomServer, "ampersand-encoded", &base.Resource{ + Size: test.BuildSize, + Range: false, + Files: []*base.FileInfo{ + { + Name: "查询处理&优化.pptx", + Size: test.BuildSize, + }, + }, + }, t) + // Test unquoted filename with HTML-encoded ampersand + testResolve(test.StartTestCustomServer, "ampersand-unquoted", &base.Resource{ + Size: test.BuildSize, + Range: false, + Files: []*base.FileInfo{ + { + Name: "test&file.txt", + Size: test.BuildSize, + }, + }, + }, t) // Test URL without file path - should use domain/host as filename listener := test.StartTestRootServer() diff --git a/internal/protocol/http/filename_parse_test.go b/internal/protocol/http/filename_parse_test.go new file mode 100644 index 00000000..7ce406f5 --- /dev/null +++ b/internal/protocol/http/filename_parse_test.go @@ -0,0 +1,138 @@ +package http + +import ( + "testing" +) + +// TestParseFilenameWithAmpersand tests the fix for filenames containing & character +// Issue: filenames with & are HTML-encoded as & and then truncated at the semicolon +func TestParseFilenameWithAmpersand(t *testing.T) { + tests := []struct { + name string + disposition string + want string + }{ + { + name: "quoted filename with &", + disposition: `attachment; filename="查询处理&优化.pptx"`, + want: "查询处理&优化.pptx", + }, + { + name: "unquoted filename with &", + disposition: `attachment; filename=test&file.txt`, + want: "test&file.txt", + }, + { + name: "quoted filename with & and extra params", + disposition: `attachment; filename="test&file.txt"; charset=utf-8`, + want: "test&file.txt", + }, + { + name: "filename with multiple HTML entities", + disposition: `attachment; filename="test&<>.txt"`, + want: "test&<>.txt", + }, + { + name: "normal filename without entities", + disposition: `attachment; filename="normal.txt"`, + want: "normal.txt", + }, + { + name: "filename with actual ampersand (no encoding)", + disposition: `attachment; filename="test&file.txt"`, + want: "test&file.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseFilename(tt.disposition) + if got != tt.want { + t.Errorf("parseFilename() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestFindParamValueEnd tests the helper function for finding parameter value boundaries +func TestFindParamValueEnd(t *testing.T) { + tests := []struct { + name string + value string + want int + }{ + { + name: "quoted value with semicolon inside", + value: `"test&file.txt"; charset=utf-8`, + want: 19, // Position of semicolon after closing quote + }, + { + name: "quoted value without semicolon after", + value: `"test.txt"`, + want: -1, + }, + { + name: "unquoted value with semicolon", + value: `test.txt; charset=utf-8`, + want: 8, + }, + { + name: "unquoted value without semicolon", + value: `test.txt`, + want: -1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := findParamValueEnd(tt.value) + if got != tt.want { + t.Errorf("findParamValueEnd() = %d, want %d", got, tt.want) + } + }) + } +} + +// TestUnescapeHTMLEntities tests HTML entity unescaping +func TestUnescapeHTMLEntities(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "ampersand", + in: "test&file.txt", + want: "test&file.txt", + }, + { + name: "less than and greater than", + in: "<test>.txt", + want: ".txt", + }, + { + name: "quote", + in: "test"file.txt", + want: "test\"file.txt", + }, + { + name: "multiple entities", + in: "a&b<c>d"e", + want: "a&bd\"e", + }, + { + name: "no entities", + in: "normal.txt", + want: "normal.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := unescapeHTMLEntities(tt.in) + if got != tt.want { + t.Errorf("unescapeHTMLEntities() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/test/httptest.go b/internal/test/httptest.go index 2432d8c5..7be34e63 100644 --- a/internal/test/httptest.go +++ b/internal/test/httptest.go @@ -192,6 +192,35 @@ func StartTestCustomServer() net.Listener { defer file.Close() io.Copy(writer, file) }) + // Test endpoint for filename with HTML-encoded ampersand (&) + // This tests the case from the bug report where filenames containing & are + // HTML-encoded as & by the server, causing truncation at the semicolon. + // Example: "查询处理&优化.pptx" -> "查询处理&优化.pptx" + mux.HandleFunc("/ampersand-encoded", func(writer http.ResponseWriter, request *http.Request) { + // Simulate server sending filename with HTML-encoded ampersand + writer.Header().Set("Content-Disposition", `attachment; filename="查询处理&优化.pptx"`) + writer.Header().Set("Content-Type", "application/octet-stream") + writer.Header().Set("Content-Length", fmt.Sprintf("%d", BuildSize)) + file, err := os.Open(BuildFile) + if err != nil { + panic(err) + } + defer file.Close() + io.Copy(writer, file) + }) + // Test endpoint for unquoted filename with HTML-encoded ampersand + // Some servers might send unquoted filenames with HTML entities + mux.HandleFunc("/ampersand-unquoted", func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("Content-Disposition", `attachment; filename=test&file.txt`) + writer.Header().Set("Content-Type", "application/octet-stream") + writer.Header().Set("Content-Length", fmt.Sprintf("%d", BuildSize)) + file, err := os.Open(BuildFile) + if err != nil { + panic(err) + } + defer file.Close() + io.Copy(writer, file) + }) return mux }) }