fix(server): keep download names intact and sanitize filename*

Rework ContentDispositionAttachment after review. Names with no ASCII
letters now fall back to download.<ext> instead of a bare extension
(東京.mp3 gave filename="mp3"). filename* is built from the same
sanitized name as the ASCII fallback, so path separators, reserved
characters, control and bidi characters, and invalid UTF-8 no longer
reach it. The ASCII fallback transliterates accents and typographic
punctuation (Legião -> Legiao, She’s -> She's) through the existing
sanitize.Accents and str.Clear helpers, keeps leading dots, and only
trims trailing ones. Names are capped at 255 bytes, keeping the
extension.

Pure ASCII names now get only the quoted filename parameter, so the
header for them matches the previous output byte for byte. filename*
is encoded with mime.FormatMediaType instead of a hand-written RFC 5987
encoder. Adds tests for the M3U export and Subsonic download headers.

server/nativeapi/playlists_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/deluan/rest"
+	"github.com/go-chi/chi/v5"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/consts"
@@ -183,6 +184,37 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
 	})
 })
 
+var _ = Describe("handleExportPlaylist", func() {
+	export := func(name string) *httptest.ResponseRecorder {
+		r := chi.NewRouter()
+		r.Get("/playlist/{playlistId}", handleExportPlaylist(&mockPlaylistsService{
+			playlist: &model.Playlist{ID: "pls-1", Name: name},
+		}))
+		w := httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest("GET", "/playlist/pls-1", nil))
+		return w
+	}
+
+	It("names the download after the playlist", func() {
+		w := export("Road Trip")
+
+		Expect(w.Code).To(Equal(http.StatusOK))
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="Road Trip.m3u"`))
+	})
+
+	It("does not let the playlist name inject a second filename parameter", func() {
+		w := export(`party"; filename="evil.html`)
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="party_; filename=_evil.html.m3u"`))
+	})
+
+	It("keeps non-ASCII names in filename*", func() {
+		w := export("Кино")
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="download.m3u"; filename*=utf-8''%D0%9A%D0%B8%D0%BD%D0%BE.m3u`))
+	})
+})
+
 var _ = Describe("writePlaylistError", func() {
 	DescribeTable("maps a service error to an HTTP status",
 		func(err error, expected int) {
@@ -231,6 +263,7 @@ func (m *mockPlaylistTrackRepo) Read(id string) (any, error) {
 type mockPlaylistsService struct {
 	playlists.Playlists
 	tracksRepo    rest.Repository
+	playlist      *model.Playlist
 	removeImageFn func(ctx context.Context, id string) error
 	setImageFn    func(ctx context.Context, id string, reader io.Reader, ext string) error
 }
@@ -249,6 +282,13 @@ func (m *mockPlaylistsService) SetImage(ctx context.Context, id string, reader i
 	return model.ErrNotFound
 }
 
+func (m *mockPlaylistsService) GetWithTracks(_ context.Context, _ string) (*model.Playlist, error) {
+	if m.playlist == nil {
+		return nil, model.ErrNotFound
+	}
+	return m.playlist, nil
+}
+
 func (m *mockPlaylistsService) TracksRepository(_ context.Context, _ string, _ bool) rest.Repository {
 	return m.tracksRepo
 }

server/public/handle_downloads_test.go

@@ -74,7 +74,7 @@ var _ = Describe("handleDownloads", func() {
 		w := makeRequest("abc123")
 
 		Expect(w.Code).To(Equal(http.StatusOK))
-		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="My Mixtape.zip"; filename*=UTF-8''My%20Mixtape.zip`))
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="My Mixtape.zip"`))
 		Expect(w.Header().Get("Content-Type")).To(Equal("application/zip"))
 		Expect(archiver.called).To(BeTrue())
 		Expect(w.Body.String()).To(Equal("zip-contents"))
@@ -85,7 +85,7 @@ var _ = Describe("handleDownloads", func() {
 
 		w := makeRequest("abc123")
 
-		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="abc123.zip"; filename*=UTF-8''abc123.zip`))
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="abc123.zip"`))
 	})
 
 	It("sanitizes characters that are unsafe in a filename", func() {
@@ -93,7 +93,15 @@ var _ = Describe("handleDownloads", func() {
 
 		w := makeRequest("abc123")
 
-		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="AC_DC_ Live_ 1979.zip"; filename*=UTF-8''AC%2FDC%3A%20Live%2C%201979.zip`))
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="AC_DC_ Live_ 1979.zip"`))
+	})
+
+	It("sanitizes the UTF-8 filename* as well", func() {
+		shareIs(&model.Share{ID: "abc123", Description: "Sigur Rós/Live", Downloadable: true})
+
+		w := makeRequest("abc123")
+
+		Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="Sigur Ros_Live.zip"; filename*=utf-8''Sigur%20R%C3%B3s_Live.zip`))
 	})
 
 	It("does not let the share description inject a second filename parameter", func() {
@@ -105,7 +113,7 @@ var _ = Describe("handleDownloads", func() {
 		Expect(disposition).ToNot(ContainSubstring(`filename="evil.html`))
 		_, params, err := mime.ParseMediaType(disposition)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(params["filename"]).To(Equal(`mix"; filename="evil.html.zip`))
+		Expect(params["filename"]).To(Equal(`mix_; filename=_evil.html.zip`))
 	})
 
 	It("returns 403 without invoking the archiver when the share is not downloadable", func() {

server/subsonic/e2e/subsonic_media_retrieval_test.go

@@ -106,7 +106,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 	})
 
 	Describe("Download", func() {
-		var trackID string
+		var trackID, trackTitle string
 
 		BeforeAll(func() {
 			// All test tracks are mp3 at 320kbps
@@ -114,6 +114,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).ToNot(BeEmpty())
 			trackID = songs[0].ID
+			trackTitle = songs[0].Title
 		})
 
 		It("returns error when id parameter is missing", func() {
@@ -143,6 +144,7 @@ var _ = Describe("Media Retrieval Endpoints", Ordered, func() {
 			Expect(w.Code).To(Equal(http.StatusOK))
 			Expect(streamerSpy.LastRequest.Format).To(Equal("opus"))
 			Expect(streamerSpy.LastRequest.BitRate).To(Equal(128))
+			Expect(w.Header().Get("Content-Disposition")).To(Equal(`attachment; filename="` + trackTitle + `.opus"`))
 		})
 
 		It("returns error when downloads are disabled", func() {

utils/str/content_disposition.go

@@ -1,88 +1,73 @@
 package str
 
 import (
+	"cmp"
 	"fmt"
+	"mime"
+	"path"
 	"strings"
+	"unicode"
+	"unicode/utf8"
+
+	"github.com/deluan/sanitize"
 )
 
-// fallbackFilename is used when sanitizing leaves nothing usable (e.g. a name made
-// entirely of non-ASCII runes). Clients that understand filename* still get the real
-// name; this is only the ASCII fallback.
-const fallbackFilename = "download"
+const maxFilenameBytes = 255
 
-// ContentDispositionAttachment builds a Content-Disposition header value (RFC 6266)
-// for an attachment download named filename.
-//
-// Filenames here are derived from user-controlled data — playlist names, album and
-// artist names, and titles read from media file tags — so they must never be
-// interpolated into the header as-is. A name containing a double quote closes the
-// quoted-string early and the remainder is parsed as additional parameters, so a
-// playlist named
-//
-//	party"; filename="evil.html
-//
-// would produce `attachment; filename="party"; filename="evil.html.m3u"`, letting
-// whoever chose the name control what the browser saves the download as. Go's
-// net/http rewrites CR and LF in header values to spaces, so response splitting is
-// not reachable this way, but parameter injection is.
-//
-// The `filename` parameter is reduced to a quoted, ASCII-only token and the original
-// UTF-8 name is preserved in an RFC 5987 `filename*` parameter, which RFC 6266 §4.3
-// requires clients to prefer when both are present. Without `filename*`, sanitizing
-// to ASCII would mangle every non-Latin name.
+// ContentDispositionAttachment builds an RFC 6266 attachment header value for a user-controlled filename.
+// Non-ASCII names also get a filename* parameter, which clients prefer over the ASCII fallback.
 func ContentDispositionAttachment(filename string) string {
-	return fmt.Sprintf("attachment; filename=%q; filename*=UTF-8''%s",
-		asciiFilename(filename), encodeRFC5987(filename))
+	stem, ext := splitFilename(filename)
+	header := fmt.Sprintf("attachment; filename=%q", joinFilename(toASCII(stem), toASCII(ext)))
+	name := joinFilename(stem, ext)
+	if isASCII(name) {
+		return header
+	}
+	// FormatMediaType emits a percent-encoded filename* for non-ASCII values
+	extended := mime.FormatMediaType("attachment", map[string]string{"filename": name})
+	return header + strings.TrimPrefix(extended, "attachment")
 }
 
-// asciiFilename reduces filename to a token that is safe inside a quoted-string: the
-// characters SanitizeFilename already strips (including the double quote), plus
-// commas, control characters and everything outside printable ASCII. Leading and
-// trailing spaces and dots are trimmed as well, since they are not valid in Windows
-// filenames.
-func asciiFilename(filename string) string {
-	var sb strings.Builder
-	sb.Grow(len(filename))
-	for _, r := range SanitizeFilename(filename) {
+func splitFilename(filename string) (stem, ext string) {
+	name := strings.Map(func(r rune) rune {
+		if unicode.IsControl(r) || unicode.Is(unicode.Bidi_Control, r) {
+			return -1
+		}
+		return r
+	}, SanitizeFilename(strings.ToValidUTF8(filename, "_")))
+	ext = path.Ext(name)
+	stem = strings.TrimSuffix(name, ext)
+	if limit := max(maxFilenameBytes-len(ext), 0); len(stem) > limit {
+		for limit > 0 && !utf8.RuneStart(stem[limit]) {
+			limit--
+		}
+		stem = stem[:limit]
+	}
+	return stem, ext
+}
+
+func joinFilename(stem, ext string) string {
+	stem = cmp.Or(strings.TrimSpace(stem), "download")
+	return strings.TrimRight(stem+ext, " .")
+}
+
+func toASCII(s string) string {
+	return strings.Map(func(r rune) rune {
 		switch {
 		case r == ',':
-			// Some clients split the header value on commas
-			sb.WriteByte('_')
+			return '_'
 		case r >= ' ' && r <= '~':
-			sb.WriteRune(r)
+			return r
 		}
-	}
-	name := strings.Trim(sb.String(), " .")
-	if name == "" {
-		return fallbackFilename
-	}
-	return name
+		return -1
+	}, sanitize.Accents(SanitizeFilename(Clear(s))))
 }
 
-// encodeRFC5987 percent-encodes filename for use as an ext-value, escaping every byte
-// outside the attr-char set defined in RFC 5987 §3.2.1.
-func encodeRFC5987(filename string) string {
-	const upperhex = "0123456789ABCDEF"
-	var sb strings.Builder
-	sb.Grow(len(filename))
-	for i := 0; i < len(filename); i++ {
-		if c := filename[i]; isAttrChar(c) {
-			sb.WriteByte(c)
-		} else {
-			sb.WriteByte('%')
-			sb.WriteByte(upperhex[c>>4])
-			sb.WriteByte(upperhex[c&0x0F])
+func isASCII(s string) bool {
+	for i := 0; i < len(s); i++ {
+		if s[i] >= utf8.RuneSelf {
+			return false
 		}
 	}
-	return sb.String()
-}
-
-// isAttrChar reports whether c is an attr-char: RFC 5987 §3.2.1 defines it as
-// ALPHA / DIGIT / "!" / "#" / "$" / "&" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
-func isAttrChar(c byte) bool {
-	switch {
-	case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9':
-		return true
-	}
-	return strings.IndexByte("!#$&+-.^_`|~", c) >= 0
+	return true
 }

utils/str/content_disposition_test.go

@@ -1,10 +1,7 @@
 package str_test
 
 import (
-	"bufio"
 	"mime"
-	"net/http"
-	"net/http/httptest"
 	"regexp"
 	"strings"
 
@@ -15,16 +12,14 @@ import (
 
 var asciiFilenameRe = regexp.MustCompile(`(?:^|; )filename="([^"]*)"`)
 
-// asciiName returns the plain (quoted, ASCII) filename parameter, the one used by
-// clients that don't understand filename*.
+// asciiName returns the quoted filename parameter, the one used by clients that ignore filename*.
 func asciiName(filename string) string {
 	m := asciiFilenameRe.FindAllStringSubmatch(str.ContentDispositionAttachment(filename), -1)
 	ExpectWithOffset(1, m).To(HaveLen(1), "expected exactly one quoted filename parameter")
 	return m[0][1]
 }
 
-// decodedName returns what a client sees after parsing the header, which per
-// RFC 6266 §4.3 means the filename* value wins when both are present.
+// decodedName returns the name an RFC 6266 client picks, preferring filename* when present.
 func decodedName(filename string) string {
 	disp, params, err := mime.ParseMediaType(str.ContentDispositionAttachment(filename))
 	ExpectWithOffset(1, err).ToNot(HaveOccurred())
@@ -34,75 +29,82 @@ func decodedName(filename string) string {
 
 var _ = Describe("ContentDispositionAttachment", func() {
 	Describe("parameter injection", func() {
-		// Without escaping, this name yields:
-		//   attachment; filename="party"; filename="evil.html.m3u"
 		const attack = `party"; filename="evil.html.m3u`
 
 		It("does not let a quote in the name open a second parameter", func() {
-			Expect(asciiName(attack)).To(Equal("party_; filename=_evil.html.m3u"))
+			Expect(str.ContentDispositionAttachment(attack)).To(Equal(`attachment; filename="party_; filename=_evil.html.m3u"`))
 		})
 
-		It("keeps the header parseable, with the attack contained in the value", func() {
-			Expect(decodedName(attack)).To(Equal(attack))
+		It("keeps the header parseable", func() {
+			Expect(decodedName(attack)).To(Equal("party_; filename=_evil.html.m3u"))
 		})
 
-		It("emits no quotes beyond the ones delimiting the ASCII name", func() {
-			Expect(strings.Count(str.ContentDispositionAttachment(attack), `"`)).To(Equal(2))
-		})
-
-		It("survives a round trip through net/http", func() {
-			rec := httptest.NewRecorder()
-			rec.Header().Set("Content-Disposition", str.ContentDispositionAttachment(`x"; filename="y.exe`))
-			rec.WriteHeader(http.StatusOK)
-
-			var sb strings.Builder
-			w := bufio.NewWriter(&sb)
-			Expect(rec.Result().Write(w)).To(Succeed())
-			Expect(w.Flush()).To(Succeed())
-
-			Expect(sb.String()).ToNot(ContainSubstring(`filename="y.exe"`))
+		It("does not let a quote in a non-ASCII name inject either", func() {
+			const utf8Attack = `東京"; filename="evil.html.m3u`
+			header := str.ContentDispositionAttachment(utf8Attack)
+			Expect(strings.Count(header, `"`)).To(Equal(2))
+			Expect(decodedName(utf8Attack)).To(Equal("東京_; filename=_evil.html.m3u"))
 		})
 	})
 
-	Describe("ASCII filename parameter", func() {
-		It("leaves a name that is already safe untouched", func() {
-			Expect(asciiName("My Playlist.m3u")).To(Equal("My Playlist.m3u"))
+	Describe("ASCII names", func() {
+		It("sends only the quoted filename, unchanged", func() {
+			Expect(str.ContentDispositionAttachment("My Playlist.m3u")).To(Equal(`attachment; filename="My Playlist.m3u"`))
 		})
 
-		It("strips path separators and control characters", func() {
-			Expect(asciiName("../../etc/passwd\x00\t")).To(Equal("_.._etc_passwd"))
+		It("keeps leading dots", func() {
+			Expect(asciiName("...And Justice for All.zip")).To(Equal("...And Justice for All.zip"))
 		})
 
-		It("replaces commas, which some clients treat as value separators", func() {
-			Expect(asciiName("Bowie, David.zip")).To(Equal("Bowie_ David.zip"))
+		It("trims surrounding spaces and trailing dots", func() {
+			Expect(asciiName("  Greatest Hits  .zip")).To(Equal("Greatest Hits.zip"))
+			Expect(asciiName("Loose End. ")).To(Equal("Loose End"))
 		})
 
-		It("drops non-ASCII runes", func() {
-			Expect(asciiName("Legião Urbana.zip")).To(Equal("Legio Urbana.zip"))
+		It("replaces path separators and reserved characters", func() {
+			Expect(asciiName("AC/DC: Live, 1979?.zip")).To(Equal("AC_DC_ Live_ 1979_.zip"))
 		})
 
-		It("falls back to a placeholder when nothing ASCII survives", func() {
+		It("drops control characters", func() {
+			Expect(asciiName("line\r\nbreak\x00\t.mp3")).To(Equal("linebreak.mp3"))
+		})
+	})
+
+	Describe("non-ASCII names", func() {
+		It("transliterates accents in the ASCII fallback", func() {
+			Expect(asciiName("Legião Urbana.zip")).To(Equal("Legiao Urbana.zip"))
+		})
+
+		It("converts typographic punctuation in the ASCII fallback", func() {
+			Expect(asciiName("She’s a Woman — Live.mp3")).To(Equal("She's a Woman - Live.mp3"))
+			Expect(asciiName("“Heroes”.mp3")).To(Equal("_Heroes_.mp3"))
+			Expect(decodedName("She’s a Woman.mp3")).To(Equal("She’s a Woman.mp3"))
+		})
+
+		It("keeps the extension when no ASCII letters survive", func() {
+			Expect(asciiName("東京.mp3")).To(Equal("download.mp3"))
+			Expect(asciiName("Кино.m3u")).To(Equal("download.m3u"))
 			Expect(asciiName("東京")).To(Equal("download"))
 		})
 
-		It("trims leading and trailing dots and spaces", func() {
-			Expect(asciiName("  ..hidden..  ")).To(Equal("hidden"))
+		DescribeTable("filename* carries the sanitized UTF-8 name",
+			func(filename, expected string) {
+				Expect(decodedName(filename)).To(Equal(expected))
+			},
+			Entry("accented", "Legião Urbana.zip", "Legião Urbana.zip"),
+			Entry("CJK", "東京.zip", "東京.zip"),
+			Entry("emoji", "🎵 mix.zip", "🎵 mix.zip"),
+			Entry("comma and percent", "Sigur Rós, 100%.zip", "Sigur Rós, 100%.zip"),
+			Entry("path separators", "Sigur Rós/Live: Heima.zip", "Sigur Rós_Live_ Heima.zip"),
+			Entry("control characters", "Sigur Rós\r\n\x00.zip", "Sigur Rós.zip"),
+			Entry("bidi override", "Björk\u202Eexe.mp3", "Björkexe.mp3"),
+			Entry("invalid UTF-8", "Bj\xf6rk Café.mp3", "Bj_rk Café.mp3"),
+		)
+
+		It("caps the name length and keeps the extension", func() {
+			name := decodedName(strings.Repeat("東", 300) + ".zip")
+			Expect(len(name)).To(BeNumerically("<=", 255))
+			Expect(name).To(HaveSuffix("東.zip"))
 		})
 	})
-
-	Describe("filename* parameter", func() {
-		DescribeTable("round-trips the original name",
-			func(filename string) {
-				Expect(decodedName(filename)).To(Equal(filename))
-			},
-			Entry("plain ASCII", "My Playlist.m3u"),
-			Entry("accented", "Legião Urbana.zip"),
-			Entry("CJK", "東京.zip"),
-			Entry("emoji", "🎵 mix.zip"),
-			Entry("comma", "Bowie, David.zip"),
-			Entry("quote", `12" Mixes.zip`),
-			Entry("percent", "100% Hits.zip"),
-			Entry("semicolon", "a;b.zip"),
-		)
-	})
 })