Skip to content

Commit 007aaf1

Browse files
committed
fix: support UTF-8 attachment filenames
1 parent 7e8e932 commit 007aaf1

8 files changed

Lines changed: 129 additions & 8 deletions

File tree

DESIGN.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,13 @@ Required headers:
131131

132132
- `Authorization: Bearer <token>`
133133
- `Content-Length: <bytes>`
134-
- `X-Filename: <display filename>`
134+
- `X-Filename: <display filename or percent-encoded UTF-8 filename>`
135135

136136
Optional headers:
137137

138138
- `Content-Type`; defaults to `application/octet-stream`
139139
- `X-Alt-Text`; used when generating image Markdown
140+
- `X-Filename-Encoding: percent`; set when `X-Filename` is percent-encoded UTF-8
140141

141142
`Content-Length` is mandatory so the service can reject an upload before reading its body if insufficient quota remains.
142143

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ curl --fail-with-body \
172172
"$GITHUB_ATTACHMENTS_URL/v1/attachments"
173173
```
174174

175+
For non-ASCII filenames, percent-encode the UTF-8 filename in `X-Filename` and add
176+
`X-Filename-Encoding: percent`. The included CLI and agent skill do this automatically.
177+
175178
See [openapi.yaml](openapi.yaml) for the complete contract.
176179

177180
## Technical overview

bin/github-attach.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ async function main() {
3434
"content-length": String(bytes.byteLength),
3535
"content-type":
3636
CONTENT_TYPES.get(extname(filename).toLowerCase()) ?? "application/octet-stream",
37-
"x-filename": filename,
37+
"x-filename": encodeURIComponent(filename),
38+
"x-filename-encoding": "percent",
3839
};
3940
if (options.alt !== undefined) {
4041
headers["x-alt-text"] = options.alt;

openapi.yaml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ paths:
4040
- name: X-Filename
4141
in: header
4242
required: true
43-
schema: { type: string, maxLength: 200 }
43+
description: Raw filename, or percent-encoded UTF-8 when X-Filename-Encoding is percent. Decoded filenames are limited to 200 Unicode characters.
44+
schema: { type: string, maxLength: 2400 }
45+
- name: X-Filename-Encoding
46+
in: header
47+
required: false
48+
description: Set when X-Filename contains percent-encoded UTF-8.
49+
schema: { type: string, enum: [percent] }
4450
- name: X-Alt-Text
4551
in: header
4652
required: false

skills/attach-github-pr-files/scripts/upload.sh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,15 @@ fi
5757

5858
output=$(mktemp)
5959
trap 'rm -f "$output"' EXIT HUP INT TERM
60+
filename=$(basename "$file")
61+
encoded_filename=$(node -e 'process.stdout.write(encodeURIComponent(process.argv[1]))' "$filename")
6062

6163
set -- \
6264
--fail-with-body --silent --show-error \
6365
--request POST \
6466
--header "Authorization: Bearer $token" \
65-
--header "X-Filename: $(basename "$file")" \
67+
--header "X-Filename: $encoded_filename" \
68+
--header "X-Filename-Encoding: percent" \
6669
--header "Content-Type: $(file --brief --mime-type -- "$file")" \
6770
--data-binary "@$file" \
6871
--output "$output"

src/upload.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export async function handleUpload(request: Request, env: Env): Promise<Response
2525
}
2626

2727
const limits = readLimits(env);
28-
const filename = sanitizeFilename(request.headers.get("x-filename"));
28+
const filename = sanitizeFilename(decodeFilenameHeader(request.headers));
2929
if (filename === null) {
3030
return errorResponse(400, "invalid_filename", "X-Filename must contain a valid filename.");
3131
}
@@ -164,6 +164,23 @@ function sanitizeFilename(raw: string | null): string | null {
164164
return sanitized.length > 0 && sanitized !== "." && sanitized !== ".." ? sanitized : null;
165165
}
166166

167+
function decodeFilenameHeader(headers: Headers): string | null {
168+
const raw = headers.get("x-filename");
169+
const encoding = headers.get("x-filename-encoding");
170+
if (encoding === null) {
171+
return raw;
172+
}
173+
if (encoding !== "percent" || raw === null) {
174+
return null;
175+
}
176+
177+
try {
178+
return decodeURIComponent(raw);
179+
} catch {
180+
return null;
181+
}
182+
}
183+
167184
function sanitizeAltText(raw: string | null, filename: string): string {
168185
const fallback = filename.replace(/\.[^.]+$/, "") || filename;
169186
if (raw === null) {

test-node/cli.test.mjs

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ const CLI = resolve("bin/github-attach.mjs");
1010
const FIXTURE = resolve("test/fixtures/cli-fixture.txt");
1111
const SKILL_UPLOADER = resolve("skills/attach-github-pr-files/scripts/upload.sh");
1212
const EXPECTED_MARKDOWN = "[cli-fixture.txt](https://example.test/a/id/cli-fixture.txt)\n";
13+
const UNICODE_FILENAME = "日本語.txt";
14+
const UNICODE_MARKDOWN = `[${UNICODE_FILENAME}](https://example.test/a/id/${encodeURIComponent(UNICODE_FILENAME)})\n`;
1315

1416
test("CLI uploads raw bytes and prints only Markdown", async (context) => {
1517
const serviceUrl = await startServer(context, { alt: "CLI fixture", token: "test-token" });
@@ -70,6 +72,29 @@ test("CLI reads the user-level service profile", async (context) => {
7072
assert.equal(result.stdout, EXPECTED_MARKDOWN);
7173
});
7274

75+
test("CLI percent-encodes UTF-8 filenames", async (context) => {
76+
const repository = await temporaryRepository(context);
77+
const file = join(repository, UNICODE_FILENAME);
78+
await writeFile(file, "unicode attachment\n");
79+
const serviceUrl = await startServer(context, {
80+
alt: "Unicode CLI",
81+
body: "unicode attachment\n",
82+
filename: UNICODE_FILENAME,
83+
markdown: UNICODE_MARKDOWN.trim(),
84+
token: "unicode-cli-token",
85+
});
86+
87+
const result = await run(process.execPath, [CLI, file, "--alt", "Unicode CLI"], {
88+
...process.env,
89+
GITHUB_ATTACHMENTS_TOKEN: "unicode-cli-token",
90+
GITHUB_ATTACHMENTS_URL: serviceUrl,
91+
});
92+
93+
assert.equal(result.stderr, "");
94+
assert.equal(result.exitCode, 0);
95+
assert.equal(result.stdout, UNICODE_MARKDOWN);
96+
});
97+
7398
test("skill uploader uses the repository override", async (context) => {
7499
const serviceUrl = await startServer(context, { alt: "Skill", token: "skill-repo-token" });
75100
const repository = await temporaryRepository(context);
@@ -94,6 +119,34 @@ test("skill uploader uses the repository override", async (context) => {
94119
assert.equal(result.stdout, EXPECTED_MARKDOWN);
95120
});
96121

122+
test("skill uploader percent-encodes UTF-8 filenames", async (context) => {
123+
const repository = await temporaryRepository(context);
124+
const file = join(repository, UNICODE_FILENAME);
125+
await writeFile(file, "unicode attachment\n");
126+
const serviceUrl = await startServer(context, {
127+
alt: "Unicode skill",
128+
body: "unicode attachment\n",
129+
filename: UNICODE_FILENAME,
130+
markdown: UNICODE_MARKDOWN.trim(),
131+
token: "unicode-skill-token",
132+
});
133+
134+
const result = await run(
135+
SKILL_UPLOADER,
136+
[file, "Unicode skill"],
137+
{
138+
...process.env,
139+
GITHUB_ATTACHMENTS_TOKEN: "unicode-skill-token",
140+
GITHUB_ATTACHMENTS_URL: serviceUrl,
141+
},
142+
repository,
143+
);
144+
145+
assert.equal(result.stderr, "");
146+
assert.equal(result.exitCode, 0);
147+
assert.equal(result.stdout, UNICODE_MARKDOWN);
148+
});
149+
97150
test("skill uploader reads the user-level service profile", async (context) => {
98151
const serviceUrl = await startServer(context, { alt: "Skill user", token: "skill-user-token" });
99152
const repository = await temporaryRepository(context);
@@ -142,18 +195,23 @@ async function startServer(context, expected) {
142195
assert.equal(request.method, "POST");
143196
assert.equal(request.url, "/v1/attachments");
144197
assert.equal(request.headers.authorization, `Bearer ${expected.token}`);
145-
assert.equal(request.headers["x-filename"], "cli-fixture.txt");
198+
const filename = expected.filename ?? "cli-fixture.txt";
199+
assert.equal(request.headers["x-filename"], encodeURIComponent(filename));
200+
assert.equal(request.headers["x-filename-encoding"], "percent");
146201
assert.equal(request.headers["x-alt-text"], expected.alt);
147202
assert.equal(request.headers["content-type"], "text/plain");
148203

149204
const chunks = [];
150205
for await (const chunk of request) {
151206
chunks.push(chunk);
152207
}
153-
assert.equal(Buffer.concat(chunks).toString("utf8"), "attachment CLI fixture\n");
208+
assert.equal(
209+
Buffer.concat(chunks).toString("utf8"),
210+
expected.body ?? "attachment CLI fixture\n",
211+
);
154212

155213
response.writeHead(201, { "content-type": "application/json" });
156-
response.end(JSON.stringify({ markdown: EXPECTED_MARKDOWN.trim() }));
214+
response.end(JSON.stringify({ markdown: expected.markdown ?? EXPECTED_MARKDOWN.trim() }));
157215
});
158216
server.listen(0, "127.0.0.1");
159217
await once(server, "listening");

test/upload.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,38 @@ describe("attachment upload API", () => {
109109
expect(new Uint8Array(await served.arrayBuffer())).toEqual(PNG);
110110
});
111111

112+
it("decodes percent-encoded UTF-8 filenames", async () => {
113+
const filename = "日本語.png";
114+
const response = await upload(PNG, {
115+
filename: encodeURIComponent(filename),
116+
filenameEncoding: "percent",
117+
});
118+
const body = await response.json<UploadBody>();
119+
120+
expect(response.status).toBe(201);
121+
expect(body.filename).toBe(filename);
122+
expect(body.url).toContain(`/${encodeURIComponent(filename)}`);
123+
124+
const served = await exports.default.fetch(new Request(body.url));
125+
expect(served.status).toBe(200);
126+
});
127+
112128
it("validates filenames, body length, and image size", async () => {
113129
const badFilename = await upload(PNG, { filename: ".." });
114130
expect(badFilename.status).toBe(400);
115131

132+
const unknownFilenameEncoding = await upload(PNG, {
133+
filename: "test.png",
134+
filenameEncoding: "base64",
135+
});
136+
expect(unknownFilenameEncoding.status).toBe(400);
137+
138+
const malformedEncodedFilename = await upload(PNG, {
139+
filename: "bad%encoding.png",
140+
filenameEncoding: "percent",
141+
});
142+
expect(malformedEncodedFilename.status).toBe(400);
143+
116144
const missingLength = await authenticatedFetch("/v1/attachments", {
117145
method: "POST",
118146
headers: {
@@ -142,6 +170,7 @@ async function upload(
142170
authorization?: string | null;
143171
contentType?: string;
144172
filename?: string;
173+
filenameEncoding?: string;
145174
} = {},
146175
): Promise<Response> {
147176
const headers: Record<string, string> = {
@@ -152,6 +181,9 @@ async function upload(
152181
if (options.authorization !== null) {
153182
headers.authorization = options.authorization ?? `Bearer ${TOKEN}`;
154183
}
184+
if (options.filenameEncoding !== undefined) {
185+
headers["x-filename-encoding"] = options.filenameEncoding;
186+
}
155187
if (options.altText !== undefined) {
156188
headers["x-alt-text"] = options.altText;
157189
}

0 commit comments

Comments
 (0)