Skip to content

Commit

Permalink
fix(trace viewer): do not serve resources with x-unknown content type (
Browse files Browse the repository at this point in the history
…#32219)

`x-unknown` is used as a placeholder for "no content-type" in the har.
We should not send it to the browser, because it is meaningfully
different from not sending `Content-Type` header. For example, Chromium
refuses to interpret stylesheets served with `x-unknown` content type.

Fixes microsoft/playwright-java#1651.
  • Loading branch information
dgozman committed Aug 19, 2024
1 parent 18694f6 commit 5271c26
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
4 changes: 3 additions & 1 deletion packages/trace-viewer/src/snapshotServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ export class SnapshotServer {
contentType = `${contentType}; charset=utf-8`;

const headers = new Headers();
headers.set('Content-Type', contentType);
// "x-unknown" in the har means "no content type".
if (contentType !== 'x-unknown')
headers.set('Content-Type', contentType);
for (const { name, value } of resource.response.headers)
headers.set(name, value);
headers.delete('Content-Encoding');
Expand Down
8 changes: 5 additions & 3 deletions packages/trace-viewer/src/traceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,11 @@ export class TraceModel {

async resourceForSha1(sha1: string): Promise<Blob | undefined> {
const blob = await this._backend.readBlob('resources/' + sha1);
if (!blob)
return;
return new Blob([blob], { type: this._resourceToContentType.get(sha1) || 'application/octet-stream' });
const contentType = this._resourceToContentType.get(sha1);
// "x-unknown" in the har means "no content type".
if (!blob || contentType === undefined || contentType === 'x-unknown')
return blob;
return new Blob([blob], { type: contentType });
}

storage(): SnapshotStorage {
Expand Down
12 changes: 12 additions & 0 deletions tests/library/trace-viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1439,3 +1439,15 @@ test('should show baseURL in metadata pane', {
await traceViewer.showMetadataTab();
await expect(traceViewer.metadataTab).toContainText('baseURL:https://example.com');
});

test('should serve css without content-type', async ({ page, runAndTrace, server }) => {
server.setRoute('/one-style.css', (req, res) => {
res.writeHead(200);
res.end(`body { background: red }`);
});
const traceViewer = await runAndTrace(async () => {
await page.goto(server.PREFIX + '/one-style.html');
});
const snapshotFrame = await traceViewer.snapshotFrame('page.goto');
await expect(snapshotFrame.locator('body')).toHaveCSS('background-color', 'rgb(255, 0, 0)', { timeout: 0 });
});

0 comments on commit 5271c26

Please sign in to comment.