From a6d1fb93dec9d1112bdcac92fd01cad02791f6ed Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Mon, 18 Mar 2024 21:26:45 +0100
Subject: [PATCH] fix(trace-viewer): encode attachment filenames as UTF-8
 (#29993)

Fixes https://github.com/microsoft/playwright/issues/29967

Tested in Firefox, Chromium, and Safari. This now leads to "good
attachment names" in Chromium and Safari, for Firefox, it won't produce
attachments, it will open them inline, but this is not a regression, was
before like that already.

See here for the spec:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition#filename_2

---------

Signed-off-by: Max Schmitt <max@schmitt.mx>
---
 packages/trace-viewer/src/sw.ts                |  2 +-
 .../ui-mode-test-attachments.spec.ts           | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/packages/trace-viewer/src/sw.ts b/packages/trace-viewer/src/sw.ts
index 2b4de9c201..841056f457 100644
--- a/packages/trace-viewer/src/sw.ts
+++ b/packages/trace-viewer/src/sw.ts
@@ -159,7 +159,7 @@ function downloadHeadersForAttachment(traceModel: TraceModel, sha1: string): Hea
   if (!attachment)
     return;
   const headers = new Headers();
-  headers.set('Content-Disposition', `attachment; filename="${attachment.name}"`);
+  headers.set('Content-Disposition', `attachment; filename="attachment"; filename*=UTF-8''${encodeURIComponent(attachment.name)}`);
   if (attachment.contentType)
     headers.set('Content-Type', attachment.contentType);
   return headers;
diff --git a/tests/playwright-test/ui-mode-test-attachments.spec.ts b/tests/playwright-test/ui-mode-test-attachments.spec.ts
index d5971bbe2c..71b67e3707 100644
--- a/tests/playwright-test/ui-mode-test-attachments.spec.ts
+++ b/tests/playwright-test/ui-mode-test-attachments.spec.ts
@@ -24,6 +24,7 @@ test('should contain text attachment', async ({ runUITest }) => {
       import { test } from '@playwright/test';
       test('attach test', async () => {
         await test.info().attach('note', { path: __filename });
+        await test.info().attach('🎭', { body: 'hi tester!', contentType: 'text/plain' });
       });
     `,
   });
@@ -31,12 +32,17 @@ test('should contain text attachment', async ({ runUITest }) => {
   await page.getByTitle('Run all').click();
   await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
   await page.getByText('Attachments').click();
-  await page.getByText('attach "note"', { exact: true }).click();
-  const downloadPromise = page.waitForEvent('download');
-  await page.getByRole('link', { name: 'note' }).click();
-  const download = await downloadPromise;
-  expect(download.suggestedFilename()).toBe('note');
-  expect((await readAllFromStream(await download.createReadStream())).toString()).toContain('attach test');
+  for (const { name, content } of [
+    { name: 'note', content: 'attach test' },
+    { name: '🎭', content: 'hi tester!' }
+  ]) {
+    await page.getByText(`attach "${name}"`, { exact: true }).click();
+    const downloadPromise = page.waitForEvent('download');
+    await page.getByRole('link', { name: name }).click();
+    const download = await downloadPromise;
+    expect(download.suggestedFilename()).toBe(name);
+    expect((await readAllFromStream(await download.createReadStream())).toString()).toContain(content);
+  }
 });
 
 test('should contain binary attachment', async ({ runUITest }) => {