Skip to content

Commit c936027

Browse files
G4brymclaude
andauthored
Fix Content-Disposition header injection in GetObject (G4brym#145)
* Fix Content-Disposition header injection in GetObject The filename in the Content-Disposition header was interpolated directly without sanitization. Filenames containing double quotes could break header parsing or enable header injection. This fix: - Strips non-ASCII characters and replaces double quotes in the ASCII `filename` parameter for compatibility - Adds RFC 5987 `filename*` parameter with proper UTF-8 percent encoding, matching the approach used in getShareLink.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix CI: update test for new Content-Disposition format and add changeset - Update GetObject test to expect the sanitized filename with RFC 5987 filename* parameter added by the header injection fix - Add required changeset for the patch release Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent aa34984 commit c936027

File tree

3 files changed

+13
-3
lines changed

3 files changed

+13
-3
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"r2-explorer": patch
3+
---
4+
5+
Fix Content-Disposition header injection in GetObject by sanitizing filenames (replacing non-ASCII with `_` and double quotes with `'`) and adding RFC 5987 `filename*=UTF-8''...` parameter for proper Unicode support.

packages/worker/src/modules/buckets/getObject.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,14 @@ export class GetObject extends OpenAPIRoute {
5757
object.writeHttpMetadata(headers);
5858
headers.set("etag", object.httpEtag);
5959
headers.set("content-length", object.size.toString());
60+
61+
const fileName = filePath.split("/").pop() || "download";
62+
const asciiFileName = fileName
63+
.replace(/[^\x20-\x7E]/g, "_")
64+
.replace(/"/g, "'");
6065
headers.set(
6166
"Content-Disposition",
62-
`attachment; filename="${filePath.split("/").pop()}"`,
67+
`attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodeURIComponent(fileName)}`,
6368
);
6469

6570
return new Response(object.body, {

packages/worker/tests/integration/object.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ describe("Object Specific Endpoints", () => {
147147
expect(response.headers.get("content-type")).toBe(TEST_OBJECT_CONTENT_TYPE);
148148
expect(response.headers.get("content-length")).toBe(TEST_OBJECT_CONTENT.length.toString());
149149
expect(response.headers.has("etag")).toBe(true);
150-
// Default R2 GetObject includes Content-Disposition: attachment; filename="key"
151-
expect(response.headers.get("content-disposition")).toBe(`attachment; filename="${TEST_OBJECT_KEY}"`);
150+
// Default R2 GetObject includes Content-Disposition with sanitized filename and RFC 5987 filename*
151+
expect(response.headers.get("content-disposition")).toBe(`attachment; filename="${TEST_OBJECT_KEY}"; filename*=UTF-8''${encodeURIComponent(TEST_OBJECT_KEY)}`);
152152

153153
const body = await response.text();
154154
expect(body).toBe(TEST_OBJECT_CONTENT);

0 commit comments

Comments
 (0)