fix(server): Content-Disposition carries a non-ASCII or unquotable filename in filename* with an ASCII fallback, so NBSP and zero-width names no longer reach clients as Go escape text (BUG-3190) (#1569)

* fix(server): Content-Disposition carries a non-ASCII or unquotable filename in filename* with an ASCII fallback, so NBSP and zero-width names no longer reach clients as Go escape text (BUG-3190)

* test(server): the Content-Disposition population carries characters the extended form must encode, and the doc comment survives gofmt (BUG-3190)

cmd/pad/bug3190_filename_star_test.go

@@ -0,0 +1,20 @@
+package main
+
+import "testing"
+
+// BUG-3190: the server sends a non-ASCII (or otherwise unquotable) name as an
+// ASCII fallback in filename plus the exact name in filename*. This parser
+// must return the exact name. These are the headers the server emits for
+// those names, byte for byte (internal/server content_disposition_test.go
+// pins the same form).
+func TestBUG3190_parseAttachmentFilename_PrefersFilenameStar(t *testing.T) {
+	for header, want := range map[string]string{
+		`inline; filename="a_b.txt"; filename*=UTF-8''a%C2%A0b.txt`:              "a b.txt",
+		`attachment; filename="__.pdf"; filename*=UTF-8''%E4%BC%9A%E8%AD%B0.pdf`: "会議.pdf",
+		`inline; filename="plain.txt"`:                                           "plain.txt",
+	} {
+		if got := parseAttachmentFilename(header); got != want {
+			t.Errorf("%s: got %q, want %q", header, got, want)
+		}
+	}
+}

internal/cli/bug3190_filename_star_test.go

@@ -0,0 +1,20 @@
+package cli
+
+import "testing"
+
+// BUG-3190: the server sends a non-ASCII (or otherwise unquotable) name as an
+// ASCII fallback in filename plus the exact name in filename*. This parser
+// must return the exact name. These are the headers the server emits for
+// those names, byte for byte (internal/server content_disposition_test.go
+// pins the same form).
+func TestBUG3190_filenameFromContentDisposition_PrefersFilenameStar(t *testing.T) {
+	for header, want := range map[string]string{
+		`inline; filename="a_b.txt"; filename*=UTF-8''a%C2%A0b.txt`:              "a b.txt",
+		`attachment; filename="__.pdf"; filename*=UTF-8''%E4%BC%9A%E8%AD%B0.pdf`: "会議.pdf",
+		`inline; filename="plain.txt"`:                                           "plain.txt",
+	} {
+		if got := filenameFromContentDisposition(header); got != want {
+			t.Errorf("%s: got %q, want %q", header, got, want)
+		}
+	}
+}

internal/mcp/bug3190_filename_star_test.go

@@ -0,0 +1,20 @@
+package mcp
+
+import "testing"
+
+// BUG-3190: the server sends a non-ASCII (or otherwise unquotable) name as an
+// ASCII fallback in filename plus the exact name in filename*. This parser
+// must return the exact name. These are the headers the server emits for
+// those names, byte for byte (internal/server content_disposition_test.go
+// pins the same form).
+func TestBUG3190_parseAttachmentFilename_PrefersFilenameStar(t *testing.T) {
+	for header, want := range map[string]string{
+		`inline; filename="a_b.txt"; filename*=UTF-8''a%C2%A0b.txt`:              "a b.txt",
+		`attachment; filename="__.pdf"; filename*=UTF-8''%E4%BC%9A%E8%AD%B0.pdf`: "会議.pdf",
+		`inline; filename="plain.txt"`:                                           "plain.txt",
+	} {
+		if got := parseAttachmentFilename(header); got != want {
+			t.Errorf("%s: got %q, want %q", header, got, want)
+		}
+	}
+}

internal/server/content_disposition.go

@@ -0,0 +1,80 @@
+package server
+
+import (
+	"strings"
+)
+
+// contentDisposition builds a Content-Disposition header value that carries
+// name faithfully (BUG-3190).
+//
+// A name of printable ASCII with no quote or backslash is written as
+// `filename="name"` alone: byte-identical to the `%q` form every site used,
+// which is the same string for such a name. Any other name gets RFC 6266's
+// two parameters: an ASCII fallback in `filename`, for clients that do not
+// read the extended form, and the exact name in `filename*` (RFC 8187: the
+// UTF-8 charset, an empty language tag, the percent-encoded bytes). A client
+// that reads both
+// prefers `filename*`; Go's mime.ParseMediaType, which the CLI and both MCP
+// transports use, does exactly that.
+//
+// What this replaces was `filename=%q`, which put raw UTF-8 in a parameter
+// defined as ISO-8859-1 and, worse, wrote Go escape TEXT for any rune
+// strconv.IsPrint rejects: a stored "a<NBSP>b.txt" reached every Go client
+// as the characters `a b.txt`, a different name carrying a backslash,
+// which a Windows client reads as a path separator.
+//
+// The caller passes the name it means to serve; sanitising it (the attachment
+// download drops control, quote and backslash runes) stays the caller's job.
+func contentDisposition(disposition, name string) string {
+	if plainHeaderFilename(name) {
+		return disposition + `; filename="` + name + `"`
+	}
+	return disposition + `; filename="` + asciiFilenameFallback(name) + `"; filename*=UTF-8''` + encodeRFC8187(name)
+}
+
+// plainHeaderFilename reports whether name can travel as a quoted-string
+// with no escaping and no loss: printable ASCII, no quote, no backslash.
+func plainHeaderFilename(name string) bool {
+	for i := 0; i < len(name); i++ {
+		c := name[i]
+		if c < 0x20 || c > 0x7e || c == '"' || c == '\\' {
+			return false
+		}
+	}
+	return true
+}
+
+// asciiFilenameFallback replaces every rune plainHeaderFilename would reject
+// with '_', so an ASCII extension survives ("会議.pdf" → "__.pdf").
+func asciiFilenameFallback(name string) string {
+	var b strings.Builder
+	b.Grow(len(name))
+	for _, r := range name {
+		if r >= 0x20 && r <= 0x7e && r != '"' && r != '\\' {
+			b.WriteRune(r)
+		} else {
+			b.WriteByte('_')
+		}
+	}
+	return b.String()
+}
+
+// encodeRFC8187 percent-encodes every byte of s that is not an RFC 8187
+// attr-char (ALPHA / DIGIT / "!#$&+-.^_`|~").
+func encodeRFC8187(s string) string {
+	const hex = "0123456789ABCDEF"
+	var b strings.Builder
+	b.Grow(len(s) * 3)
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		if ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
+			strings.IndexByte("!#$&+-.^_`|~", c) >= 0 {
+			b.WriteByte(c)
+			continue
+		}
+		b.WriteByte('%')
+		b.WriteByte(hex[c>>4])
+		b.WriteByte(hex[c&0x0f])
+	}
+	return b.String()
+}

internal/server/content_disposition_test.go

@@ -0,0 +1,77 @@
+package server
+
+import (
+	"encoding/json"
+	"fmt"
+	"mime"
+	"net/http/httptest"
+	"testing"
+)
+
+// The population: every class a stored name can be in. ASCII that travels
+// plain; ASCII the quoted-string form cannot carry bare; runes strconv.IsPrint
+// rejects (the `%q` corruption); multi-byte scripts; combining marks; emoji;
+// and the characters RFC 8187 encoding and parameter parsing treat specially.
+var contentDispositionNames = []string{
+	"plain.txt", "with space.txt", "semi;colon.txt", "per%cent.txt", "star*.txt", "tick'.txt",
+	`quo"te.txt`, `back\slash.txt`,
+	"a b.txt", "a​b.txt", "a b.txt",
+	"résumé.pdf", "会議.pdf", "é.txt", "📎 clip.png",
+	// Characters the extended form must itself percent-encode, beside a rune
+	// that forces that form: a bare '%' there would decode as an escape.
+	"50% résumé.txt", "a'b é.txt", "é;x=y.txt",
+}
+
+func TestContentDispositionRoundTripsEveryName(t *testing.T) {
+	for _, name := range contentDispositionNames {
+		h := contentDisposition("attachment", name)
+		disp, params, err := mime.ParseMediaType(h)
+		if err != nil || disp != "attachment" || params["filename"] != name {
+			t.Errorf("%q: header %q parsed to (%q, %q, %v)", name, h, disp, params["filename"], err)
+		}
+		for i := 0; i < len(h); i++ {
+			if h[i] < 0x20 || h[i] > 0x7e {
+				t.Errorf("%q: header %q carries a non-ASCII byte", name, h)
+				break
+			}
+		}
+	}
+}
+
+// A name that travels plain is byte-identical to the `%q` form every site
+// used, so this change is invisible for every ASCII name in the product.
+func TestContentDispositionPlainNameUnchanged(t *testing.T) {
+	for _, name := range []string{"plain.txt", "with space.txt", "my-item.pad.md", "ws-export.tar.gz"} {
+		if got, was := contentDisposition("inline", name), fmt.Sprintf(`inline; filename=%q`, name); got != was {
+			t.Errorf("%q: %q, was %q", name, got, was)
+		}
+	}
+	if got := contentDisposition("attachment", "会議.pdf"); got != `attachment; filename="__.pdf"; filename*=UTF-8''%E4%BC%9A%E8%AD%B0.pdf` {
+		t.Errorf("CJK: %q", got)
+	}
+}
+
+// Through the real upload and download doors: the name a Go client parses out
+// of the download header is the name the server stored. NBSP and ZWSP were
+// the measured corruption (BUG-3190 checkpoint 1).
+func TestAttachmentDownloadCarriesTheStoredName(t *testing.T) {
+	srv, slug := testServerWithAttachments(t)
+	for _, name := range []string{"plain.txt", "résumé.txt", "会議.txt", "a b.txt", "a​b.txt", "é.txt", "per%cent.txt"} {
+		rr := doMultipartUpload(srv, slug, name, []byte("hello"))
+		var up struct {
+			ID       string `json:"id"`
+			Filename string `json:"filename"`
+		}
+		if err := json.Unmarshal(rr.Body.Bytes(), &up); err != nil || up.ID == "" {
+			t.Fatalf("%q: upload %d %s", name, rr.Code, rr.Body.String())
+		}
+		req := httptest.NewRequest("GET", "/api/v1/workspaces/"+slug+"/attachments/"+up.ID, nil)
+		req.RemoteAddr = "127.0.0.1:1"
+		dr := httptest.NewRecorder()
+		srv.ServeHTTP(dr, req)
+		_, params, err := mime.ParseMediaType(dr.Header().Get("Content-Disposition"))
+		if err != nil || params["filename"] != up.Filename || up.Filename != name {
+			t.Errorf("sent %q, stored %q, header %q parsed to %q (%v)", name, up.Filename, dr.Header().Get("Content-Disposition"), params["filename"], err)
+		}
+	}
+}

internal/server/handlers_artifact_export.go

@@ -139,8 +139,7 @@ func (s *Server) handleExportItemArtifact(w http.ResponseWriter, r *http.Request
 	}
 
 	w.Header().Set("Content-Type", "text/markdown; charset=utf-8")
-	w.Header().Set("Content-Disposition",
-		fmt.Sprintf("attachment; filename=%q", artifactExportFilename(item)))
+	w.Header().Set("Content-Disposition", contentDisposition("attachment", artifactExportFilename(item)))
 	w.WriteHeader(http.StatusOK)
 	_, _ = w.Write(out)
 }

internal/server/handlers_attachments.go

@@ -852,7 +852,7 @@ func (s *Server) handleGetAttachment(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("X-Pad-Attachment-Derived", derived)
 	}
 	w.Header().Set("Content-Disposition",
-		fmt.Sprintf(`%s; filename=%q`, disposition, sanitizeHeaderFilename(attachments.ServedFilename(att.Filename))))
+		contentDisposition(disposition, sanitizeHeaderFilename(attachments.ServedFilename(att.Filename))))
 
 	// http.ServeContent gives Range, conditional GETs, and 206
 	// responses for free — but it requires an io.ReadSeeker. FSStore

internal/server/handlers_export_bundle.go

@@ -141,8 +141,7 @@ func (s *Server) handleExportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
 	}
 
 	w.Header().Set("Content-Type", "application/gzip")
-	w.Header().Set("Content-Disposition",
-		fmt.Sprintf(`attachment; filename="%s-export.tar.gz"`, ws.Slug))
+	w.Header().Set("Content-Disposition", contentDisposition("attachment", ws.Slug+"-export.tar.gz"))
 	// Bundles are streamed; we don't know the final size up front. No
 	// Content-Length header — http.Server falls through to chunked
 	// transfer-encoding, which the gzip+tar pair handles fine.

internal/server/handlers_workspaces.go

@@ -796,7 +796,7 @@ func (s *Server) handleExportWorkspace(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment; filename="%s-export.json"`, ws.Slug))
+	w.Header().Set("Content-Disposition", contentDisposition("attachment", ws.Slug+"-export.json"))
 	writeJSON(w, http.StatusOK, export)
 }