fix: use HTTP response headers for URL download filename/extension
URLs without file extensions (e.g. /mcp/photos/23) resulted in temp files with no extension, meaningless display filenames, and fallback to application/octet-stream. Now downloadToTemp returns Content-Type and Content-Disposition from the HTTP response, which are used to: - Add correct extension to temp files from Content-Type - Derive display filename via Content-Disposition > URL basename > fallback - Use response Content-Type as mediaType when magic bytes fail Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dd350eddd6
commit
68b7d6737d
2 changed files with 182 additions and 34 deletions
|
|
@ -124,13 +124,15 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
|
|
||||||
// Handle URL downloads
|
// Handle URL downloads
|
||||||
var resolved string
|
var resolved string
|
||||||
|
var dlResult downloadResult
|
||||||
var isURL bool
|
var isURL bool
|
||||||
if isHTTPURL(path) {
|
if isHTTPURL(path) {
|
||||||
var err error
|
var err error
|
||||||
resolved, err = downloadToTemp(ctx, path)
|
dlResult, err = downloadToTemp(ctx, path)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return ErrorResult(fmt.Sprintf("download failed: %v", err))
|
return ErrorResult(fmt.Sprintf("download failed: %v", err))
|
||||||
}
|
}
|
||||||
|
resolved = dlResult.Path
|
||||||
isURL = true
|
isURL = true
|
||||||
// Do NOT delete the temp file here — MediaStore only stores a path
|
// Do NOT delete the temp file here — MediaStore only stores a path
|
||||||
// reference, so the file must survive until the channel sends it.
|
// reference, so the file must survive until the channel sends it.
|
||||||
|
|
@ -160,13 +162,17 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
|
||||||
filename, _ := args["filename"].(string)
|
filename, _ := args["filename"].(string)
|
||||||
if filename == "" {
|
if filename == "" {
|
||||||
if isURL {
|
if isURL {
|
||||||
filename = filenameFromURL(path)
|
filename = filenameForDownload(path, dlResult.ContentDisposition, dlResult.ContentType)
|
||||||
} else {
|
} else {
|
||||||
filename = filepath.Base(resolved)
|
filename = filepath.Base(resolved)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
mediaType := detectMediaType(resolved)
|
mediaType := detectMediaType(resolved)
|
||||||
|
// If magic bytes and extension both failed, use Content-Type from HTTP response
|
||||||
|
if isURL && mediaType == "application/octet-stream" && dlResult.ContentType != "" {
|
||||||
|
mediaType = dlResult.ContentType
|
||||||
|
}
|
||||||
scope := fmt.Sprintf("tool:send_file:%s:%s", channel, chatID)
|
scope := fmt.Sprintf("tool:send_file:%s:%s", channel, chatID)
|
||||||
|
|
||||||
ref, err := t.mediaStore.Store(resolved, media.MediaMeta{
|
ref, err := t.mediaStore.Store(resolved, media.MediaMeta{
|
||||||
|
|
@ -186,41 +192,57 @@ func isHTTPURL(path string) bool {
|
||||||
return strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://")
|
return strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// downloadResult holds the downloaded temp file path and HTTP response metadata.
|
||||||
|
type downloadResult struct {
|
||||||
|
Path string
|
||||||
|
ContentType string // from Content-Type header (media type only, no params)
|
||||||
|
ContentDisposition string // raw Content-Disposition header
|
||||||
|
}
|
||||||
|
|
||||||
// downloadToTemp downloads a URL to a temporary file under sendFileTempDir().
|
// downloadToTemp downloads a URL to a temporary file under sendFileTempDir().
|
||||||
// The file must persist until the channel has sent it; cleanup is handled by
|
// The file must persist until the channel has sent it; cleanup is handled by
|
||||||
// CleanupTempFiles (periodic) rather than the caller.
|
// CleanupTempFiles (periodic) rather than the caller.
|
||||||
func downloadToTemp(ctx context.Context, rawURL string) (string, error) {
|
func downloadToTemp(ctx context.Context, rawURL string) (downloadResult, error) {
|
||||||
dir := sendFileTempDir()
|
dir := sendFileTempDir()
|
||||||
if err := os.MkdirAll(dir, 0o700); err != nil {
|
if err := os.MkdirAll(dir, 0o700); err != nil {
|
||||||
return "", fmt.Errorf("create temp dir: %w", err)
|
return downloadResult{}, fmt.Errorf("create temp dir: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("build request: %w", err)
|
return downloadResult{}, fmt.Errorf("build request: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
resp, err := http.DefaultClient.Do(req)
|
resp, err := http.DefaultClient.Do(req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("HTTP GET: %w", err)
|
return downloadResult{}, fmt.Errorf("HTTP GET: %w", err)
|
||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
|
|
||||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||||
return "", fmt.Errorf("HTTP %d %s", resp.StatusCode, resp.Status)
|
return downloadResult{}, fmt.Errorf("HTTP %d %s", resp.StatusCode, resp.Status)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Extract Content-Type (media type only)
|
||||||
|
respCT := resp.Header.Get("Content-Type")
|
||||||
|
mediaType, _, _ := mime.ParseMediaType(respCT)
|
||||||
|
|
||||||
|
// Determine file extension: prefer URL path, then Content-Type header
|
||||||
|
ext := extFromURL(rawURL)
|
||||||
|
if ext == "" && mediaType != "" {
|
||||||
|
ext = preferredExtension(mediaType)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Generate a random temp filename to avoid collisions
|
// Generate a random temp filename to avoid collisions
|
||||||
var randBytes [8]byte
|
var randBytes [8]byte
|
||||||
if _, err := rand.Read(randBytes[:]); err != nil {
|
if _, err := rand.Read(randBytes[:]); err != nil {
|
||||||
return "", err
|
return downloadResult{}, err
|
||||||
}
|
}
|
||||||
ext := extFromURL(rawURL)
|
|
||||||
tmpPath := filepath.Join(dir, fmt.Sprintf("dl_%x%s", randBytes, ext))
|
tmpPath := filepath.Join(dir, fmt.Sprintf("dl_%x%s", randBytes, ext))
|
||||||
|
|
||||||
f, err := os.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0o600)
|
f, err := os.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0o600)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf("create temp file: %w", err)
|
return downloadResult{}, fmt.Errorf("create temp file: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
n, copyErr := io.Copy(f, io.LimitReader(resp.Body, sendFileDownloadLimit+1))
|
n, copyErr := io.Copy(f, io.LimitReader(resp.Body, sendFileDownloadLimit+1))
|
||||||
|
|
@ -229,14 +251,18 @@ func downloadToTemp(ctx context.Context, rawURL string) (string, error) {
|
||||||
}
|
}
|
||||||
if copyErr != nil {
|
if copyErr != nil {
|
||||||
os.Remove(tmpPath)
|
os.Remove(tmpPath)
|
||||||
return "", fmt.Errorf("write temp file: %w", copyErr)
|
return downloadResult{}, fmt.Errorf("write temp file: %w", copyErr)
|
||||||
}
|
}
|
||||||
if n > sendFileDownloadLimit {
|
if n > sendFileDownloadLimit {
|
||||||
os.Remove(tmpPath)
|
os.Remove(tmpPath)
|
||||||
return "", fmt.Errorf("download exceeds %d byte limit", sendFileDownloadLimit)
|
return downloadResult{}, fmt.Errorf("download exceeds %d byte limit", sendFileDownloadLimit)
|
||||||
}
|
}
|
||||||
|
|
||||||
return tmpPath, nil
|
return downloadResult{
|
||||||
|
Path: tmpPath,
|
||||||
|
ContentType: mediaType,
|
||||||
|
ContentDisposition: resp.Header.Get("Content-Disposition"),
|
||||||
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// extFromURL extracts a file extension from a URL path, e.g. ".jpg".
|
// extFromURL extracts a file extension from a URL path, e.g. ".jpg".
|
||||||
|
|
@ -248,17 +274,75 @@ func extFromURL(rawURL string) string {
|
||||||
return filepath.Ext(u.Path)
|
return filepath.Ext(u.Path)
|
||||||
}
|
}
|
||||||
|
|
||||||
// filenameFromURL derives a display filename from a URL.
|
// filenameForDownload derives a display filename using Content-Disposition,
|
||||||
func filenameFromURL(rawURL string) string {
|
// the URL path, and Content-Type as progressive fallbacks.
|
||||||
u, err := url.Parse(rawURL)
|
func filenameForDownload(rawURL, contentDisposition, contentType string) string {
|
||||||
if err != nil {
|
// 1. Try Content-Disposition header
|
||||||
return "download"
|
if contentDisposition != "" {
|
||||||
|
_, params, err := mime.ParseMediaType(contentDisposition)
|
||||||
|
if err == nil {
|
||||||
|
if fn := params["filename"]; fn != "" {
|
||||||
|
return fn
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. Try URL path basename
|
||||||
|
u, err := url.Parse(rawURL)
|
||||||
|
if err == nil {
|
||||||
base := filepath.Base(u.Path)
|
base := filepath.Base(u.Path)
|
||||||
if base == "" || base == "." || base == "/" {
|
if base != "" && base != "." && base != "/" {
|
||||||
return "download"
|
// If basename has no extension, try to add one from Content-Type
|
||||||
|
if filepath.Ext(base) == "" && contentType != "" {
|
||||||
|
if ext := preferredExtension(contentType); ext != "" {
|
||||||
|
return base + ext
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return base
|
return base
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 3. Fallback
|
||||||
|
if contentType != "" {
|
||||||
|
if ext := preferredExtension(contentType); ext != "" {
|
||||||
|
return "download" + ext
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return "download"
|
||||||
|
}
|
||||||
|
|
||||||
|
// preferredExtension returns a file extension (with dot) for a MIME type.
|
||||||
|
// Uses a short map of common types for deterministic results, then falls back
|
||||||
|
// to mime.ExtensionsByType.
|
||||||
|
func preferredExtension(mediaType string) string {
|
||||||
|
// mime.ExtensionsByType returns multiple options in undefined order;
|
||||||
|
// hardcode the most common ones for determinism.
|
||||||
|
preferred := map[string]string{
|
||||||
|
"image/png": ".png",
|
||||||
|
"image/jpeg": ".jpg",
|
||||||
|
"image/gif": ".gif",
|
||||||
|
"image/webp": ".webp",
|
||||||
|
"image/svg+xml": ".svg",
|
||||||
|
"image/bmp": ".bmp",
|
||||||
|
"image/tiff": ".tiff",
|
||||||
|
"video/mp4": ".mp4",
|
||||||
|
"video/webm": ".webm",
|
||||||
|
"audio/mpeg": ".mp3",
|
||||||
|
"audio/ogg": ".ogg",
|
||||||
|
"application/pdf": ".pdf",
|
||||||
|
"application/zip": ".zip",
|
||||||
|
"text/plain": ".txt",
|
||||||
|
"text/html": ".html",
|
||||||
|
"application/json": ".json",
|
||||||
|
}
|
||||||
|
if ext, ok := preferred[mediaType]; ok {
|
||||||
|
return ext
|
||||||
|
}
|
||||||
|
exts, err := mime.ExtensionsByType(mediaType)
|
||||||
|
if err == nil && len(exts) > 0 {
|
||||||
|
return exts[0]
|
||||||
|
}
|
||||||
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// CleanupTempFiles removes old temp files from the sendfile temp directory.
|
// CleanupTempFiles removes old temp files from the sendfile temp directory.
|
||||||
|
|
|
||||||
|
|
@ -198,6 +198,7 @@ func TestSendFileTool_URLDownload(t *testing.T) {
|
||||||
|
|
||||||
func TestSendFileTool_URLDownloadDefaultFilename(t *testing.T) {
|
func TestSendFileTool_URLDownloadDefaultFilename(t *testing.T) {
|
||||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "image/png")
|
||||||
w.Write([]byte("data"))
|
w.Write([]byte("data"))
|
||||||
}))
|
}))
|
||||||
defer srv.Close()
|
defer srv.Close()
|
||||||
|
|
@ -212,12 +213,69 @@ func TestSendFileTool_URLDownloadDefaultFilename(t *testing.T) {
|
||||||
if result.IsError {
|
if result.IsError {
|
||||||
t.Fatalf("unexpected error: %s", result.ForLLM)
|
t.Fatalf("unexpected error: %s", result.ForLLM)
|
||||||
}
|
}
|
||||||
// filename should be derived from URL path: "42"
|
// filename should be "42.png" (URL basename + extension from Content-Type)
|
||||||
if !strings.Contains(result.ForLLM, `"42"`) {
|
if !strings.Contains(result.ForLLM, `"42.png"`) {
|
||||||
t.Errorf("expected filename '42' in result, got %q", result.ForLLM)
|
t.Errorf("expected filename '42.png' in result, got %q", result.ForLLM)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSendFileTool_URLDownloadContentDisposition(t *testing.T) {
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "image/jpeg")
|
||||||
|
w.Header().Set("Content-Disposition", `attachment; filename="sunset.jpg"`)
|
||||||
|
w.Write([]byte("data"))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
store := media.NewFileMediaStore()
|
||||||
|
tool := NewSendFileTool(t.TempDir(), false, 0, store)
|
||||||
|
tool.SetContext("telegram", "chat123")
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"path": srv.URL + "/mcp/photos/99",
|
||||||
|
})
|
||||||
|
if result.IsError {
|
||||||
|
t.Fatalf("unexpected error: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
// filename should come from Content-Disposition
|
||||||
|
if !strings.Contains(result.ForLLM, `"sunset.jpg"`) {
|
||||||
|
t.Errorf("expected filename 'sunset.jpg' from Content-Disposition, got %q", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSendFileTool_URLDownloadContentTypeAsMIME(t *testing.T) {
|
||||||
|
// Server returns image/png Content-Type but file has no magic bytes match
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "image/png")
|
||||||
|
w.Write([]byte("not-real-png-data"))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
store := media.NewFileMediaStore()
|
||||||
|
tool := NewSendFileTool(t.TempDir(), false, 0, store)
|
||||||
|
tool.SetContext("telegram", "chat123")
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"path": srv.URL + "/api/image/5",
|
||||||
|
})
|
||||||
|
if result.IsError {
|
||||||
|
t.Fatalf("unexpected error: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
// The media should have been registered — check it resolves
|
||||||
|
if len(result.Media) != 1 {
|
||||||
|
t.Fatalf("expected 1 media ref, got %d", len(result.Media))
|
||||||
|
}
|
||||||
|
resolved, err := store.Resolve(result.Media[0])
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Resolve failed: %v", err)
|
||||||
|
}
|
||||||
|
// Temp file should have .png extension from Content-Type
|
||||||
|
if filepath.Ext(resolved) != ".png" {
|
||||||
|
t.Errorf("expected temp file to have .png extension, got %q", filepath.Base(resolved))
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { os.Remove(resolved) })
|
||||||
|
}
|
||||||
|
|
||||||
func TestSendFileTool_URLDownloadHTTPError(t *testing.T) {
|
func TestSendFileTool_URLDownloadHTTPError(t *testing.T) {
|
||||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
w.WriteHeader(http.StatusNotFound)
|
w.WriteHeader(http.StatusNotFound)
|
||||||
|
|
@ -299,21 +357,27 @@ func TestCleanupTempFiles(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestFilenameFromURL(t *testing.T) {
|
func TestFilenameForDownload(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
|
name string
|
||||||
url string
|
url string
|
||||||
|
contentDisposition string
|
||||||
|
contentType string
|
||||||
want string
|
want string
|
||||||
}{
|
}{
|
||||||
{"https://example.com/photos/cat.jpg", "cat.jpg"},
|
{"url with extension", "https://example.com/photos/cat.jpg", "", "", "cat.jpg"},
|
||||||
{"https://example.com/mcp/photos/42", "42"},
|
{"url no ext + content-type", "https://example.com/mcp/photos/42", "", "image/png", "42.png"},
|
||||||
{"https://example.com/", "download"},
|
{"url no ext no content-type", "https://example.com/mcp/photos/42", "", "", "42"},
|
||||||
{"https://example.com", "download"},
|
{"root path", "https://example.com/", "", "image/jpeg", "download.jpg"},
|
||||||
|
{"root no content-type", "https://example.com", "", "", "download"},
|
||||||
|
{"content-disposition wins", "https://example.com/mcp/photos/42", `attachment; filename="photo.png"`, "image/jpeg", "photo.png"},
|
||||||
|
{"content-disposition inline", "https://example.com/x", `inline; filename="report.pdf"`, "application/pdf", "report.pdf"},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
t.Run(tt.url, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
got := filenameFromURL(tt.url)
|
got := filenameForDownload(tt.url, tt.contentDisposition, tt.contentType)
|
||||||
if got != tt.want {
|
if got != tt.want {
|
||||||
t.Errorf("filenameFromURL(%q) = %q, want %q", tt.url, got, tt.want)
|
t.Errorf("filenameForDownload() = %q, want %q", got, tt.want)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue