Skip to content

Commit

Permalink
fix: route.continue should not change multipart form data body (#30863)
Browse files Browse the repository at this point in the history
The bug was fixed in #30734.
This PR adds a test and updates interception logic to not send post data
when it is not modified.

Fixes #30788
  • Loading branch information
yury-s committed May 20, 2024
1 parent b67b963 commit 0428964
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
5 changes: 2 additions & 3 deletions packages/playwright-core/src/client/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
if (this._redirectedFrom)
this._redirectedFrom._redirectedTo = this;
this._provisionalHeaders = new RawHeaders(initializer.headers);
this._fallbackOverrides.postDataBuffer = initializer.postData;
this._timing = {
startTime: 0,
domainLookupStart: -1,
Expand All @@ -128,11 +127,11 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
}

postData(): string | null {
return this._fallbackOverrides.postDataBuffer?.toString('utf-8') || null;
return (this._fallbackOverrides.postDataBuffer || this._initializer.postData)?.toString('utf-8') || null;
}

postDataBuffer(): Buffer | null {
return this._fallbackOverrides.postDataBuffer || null;
return this._fallbackOverrides.postDataBuffer || this._initializer.postData || null;
}

postDataJSON(): Object | null {
Expand Down
43 changes: 43 additions & 0 deletions tests/page/page-request-continue.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,3 +477,46 @@ it('should intercept css variable with background url', async ({ page, server })
await page.waitForTimeout(1000);
expect(interceptedRequests).toBe(1);
});

it('continue should not change multipart/form-data body', async ({ page, server, browserName }) => {
it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/19158' });
await page.goto(server.EMPTY_PAGE);
server.setRoute('/upload', (request, response) => {
response.writeHead(200, { 'Content-Type': 'text/plain' });
response.end('done');
});
async function sendFormData() {
const reqPromise = server.waitForRequest('/upload');
const status = await page.evaluate(async () => {
const newFile = new File(['file content'], 'file.txt');
const formData = new FormData();
formData.append('file', newFile);
const response = await fetch('/upload', {
method: 'POST',
credentials: 'include',
body: formData,
});
return response.status;
});
const req = await reqPromise;
expect(status).toBe(200);
return req;
}
const reqBefore = await sendFormData();
await page.route('**/*', async route => {
await route.continue();
});
const reqAfter = await sendFormData();
const fileContent = [
'Content-Disposition: form-data; name=\"file\"; filename=\"file.txt\"',
'Content-Type: application/octet-stream',
'',
'file content',
'------'].join('\r\n');
expect.soft((await reqBefore.postBody).toString('utf8')).toContain(fileContent);
expect.soft((await reqAfter.postBody).toString('utf8')).toContain(fileContent);
// Firefox sends a bit longer boundary.
const expectedLength = browserName === 'firefox' ? '246' : '208';
expect.soft(reqBefore.headers['content-length']).toBe(expectedLength);
expect.soft(reqAfter.headers['content-length']).toBe(expectedLength);
});

0 comments on commit 0428964

Please sign in to comment.