fix: falsey behavior in route.continue, page.post, testInfo.attach (#11421)
In several of the Playwright APIs, falsey values were not handled correctly. This changeset adds tests (and some fixes): - route.continue: If options.postData was the empty string, the continue failed to override the post data. - page.post (application/json with options.data: false|''|0|null): Raw falsey values were getting dropped (i.e. you can't do the equivalent of curl --header application/json … -d 'false'). This has been fixed with most values across all browsers, but an additional fix is needed for 'null' which the channel serializer treats extra specially. - testInfo.attach: This didn't get reported as an error when options.path was the empty string, but should have been. #11413 (and its fix #11414) inspired this search as they are the same class of bug.
This commit is contained in:
parent
a5bc2efc18
commit
64e7557fb9
|
|
@ -78,7 +78,7 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
|
||||||
if (this._redirectedFrom)
|
if (this._redirectedFrom)
|
||||||
this._redirectedFrom._redirectedTo = this;
|
this._redirectedFrom._redirectedTo = this;
|
||||||
this._provisionalHeaders = new RawHeaders(initializer.headers);
|
this._provisionalHeaders = new RawHeaders(initializer.headers);
|
||||||
this._postData = initializer.postData ? Buffer.from(initializer.postData, 'base64') : null;
|
this._postData = initializer.postData !== undefined ? Buffer.from(initializer.postData, 'base64') : null;
|
||||||
this._timing = {
|
this._timing = {
|
||||||
startTime: 0,
|
startTime: 0,
|
||||||
domainLookupStart: -1,
|
domainLookupStart: -1,
|
||||||
|
|
|
||||||
|
|
@ -123,7 +123,7 @@ export class RouteDispatcher extends Dispatcher<Route, channels.RouteChannel> im
|
||||||
url: params.url,
|
url: params.url,
|
||||||
method: params.method,
|
method: params.method,
|
||||||
headers: params.headers,
|
headers: params.headers,
|
||||||
postData: params.postData ? Buffer.from(params.postData, 'base64') : undefined,
|
postData: params.postData !== undefined ? Buffer.from(params.postData, 'base64') : undefined,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -579,7 +579,7 @@ function isJsonParsable(value: any) {
|
||||||
|
|
||||||
function serializePostData(params: channels.APIRequestContextFetchParams, headers: { [name: string]: string }): Buffer | undefined {
|
function serializePostData(params: channels.APIRequestContextFetchParams, headers: { [name: string]: string }): Buffer | undefined {
|
||||||
assert((params.postData ? 1 : 0) + (params.jsonData ? 1 : 0) + (params.formData ? 1 : 0) + (params.multipartData ? 1 : 0) <= 1, `Only one of 'data', 'form' or 'multipart' can be specified`);
|
assert((params.postData ? 1 : 0) + (params.jsonData ? 1 : 0) + (params.formData ? 1 : 0) + (params.multipartData ? 1 : 0) <= 1, `Only one of 'data', 'form' or 'multipart' can be specified`);
|
||||||
if (params.jsonData) {
|
if (params.jsonData !== undefined) {
|
||||||
const json = isJsonParsable(params.jsonData) ? params.jsonData : JSON.stringify(params.jsonData);
|
const json = isJsonParsable(params.jsonData) ? params.jsonData : JSON.stringify(params.jsonData);
|
||||||
headers['content-type'] ??= 'application/json';
|
headers['content-type'] ??= 'application/json';
|
||||||
return Buffer.from(json, 'utf8');
|
return Buffer.from(json, 'utf8');
|
||||||
|
|
@ -599,7 +599,7 @@ function serializePostData(params: channels.APIRequestContextFetchParams, header
|
||||||
}
|
}
|
||||||
headers['content-type'] ??= formData.contentTypeHeader();
|
headers['content-type'] ??= formData.contentTypeHeader();
|
||||||
return formData.finish();
|
return formData.finish();
|
||||||
} else if (params.postData) {
|
} else if (params.postData !== undefined) {
|
||||||
headers['content-type'] ??= 'application/octet-stream';
|
headers['content-type'] ??= 'application/octet-stream';
|
||||||
return Buffer.from(params.postData, 'base64');
|
return Buffer.from(params.postData, 'base64');
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -204,7 +204,7 @@ export class Dispatcher {
|
||||||
name: a.name,
|
name: a.name,
|
||||||
path: a.path,
|
path: a.path,
|
||||||
contentType: a.contentType,
|
contentType: a.contentType,
|
||||||
body: typeof a.body !== 'undefined' ? Buffer.from(a.body, 'base64') : undefined
|
body: a.body !== undefined ? Buffer.from(a.body, 'base64') : undefined
|
||||||
}));
|
}));
|
||||||
result.status = params.status;
|
result.status = params.status;
|
||||||
test.expectedStatus = params.expectedStatus;
|
test.expectedStatus = params.expectedStatus;
|
||||||
|
|
|
||||||
|
|
@ -267,7 +267,7 @@ export class WorkerRunner extends EventEmitter {
|
||||||
attach: async (name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}) => {
|
attach: async (name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}) => {
|
||||||
if ((options.path !== undefined ? 1 : 0) + (options.body !== undefined ? 1 : 0) !== 1)
|
if ((options.path !== undefined ? 1 : 0) + (options.body !== undefined ? 1 : 0) !== 1)
|
||||||
throw new Error(`Exactly one of "path" and "body" must be specified`);
|
throw new Error(`Exactly one of "path" and "body" must be specified`);
|
||||||
if (options.path) {
|
if (options.path !== undefined) {
|
||||||
const hash = calculateSha1(options.path);
|
const hash = calculateSha1(options.path);
|
||||||
const dest = testInfo.outputPath('attachments', hash + path.extname(options.path));
|
const dest = testInfo.outputPath('attachments', hash + path.extname(options.path));
|
||||||
await fs.promises.mkdir(path.dirname(dest), { recursive: true });
|
await fs.promises.mkdir(path.dirname(dest), { recursive: true });
|
||||||
|
|
|
||||||
|
|
@ -212,3 +212,28 @@ it('should support the times parameter with route matching', async ({ context, p
|
||||||
await page.goto(server.EMPTY_PAGE);
|
await page.goto(server.EMPTY_PAGE);
|
||||||
expect(intercepted).toHaveLength(1);
|
expect(intercepted).toHaveLength(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should overwrite post body with empty string', async ({ context, server, page, browserName }) => {
|
||||||
|
await context.route('**/empty.html', route => {
|
||||||
|
route.continue({
|
||||||
|
postData: '',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
const [req] = await Promise.all([
|
||||||
|
server.waitForRequest('/empty.html'),
|
||||||
|
page.setContent(`
|
||||||
|
<script>
|
||||||
|
(async () => {
|
||||||
|
await fetch('${server.EMPTY_PAGE}', {
|
||||||
|
method: 'POST',
|
||||||
|
body: 'original',
|
||||||
|
});
|
||||||
|
})()
|
||||||
|
</script>
|
||||||
|
`),
|
||||||
|
]);
|
||||||
|
|
||||||
|
const body = (await req.postBody).toString();
|
||||||
|
expect(body).toBe('');
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -266,12 +266,17 @@ it('should remove content-length from reidrected post requests', async ({ playwr
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
||||||
const serialization = [
|
const serialization: [string, any][] = [
|
||||||
['object', { 'foo': 'bar' }],
|
['object', { 'foo': 'bar' }],
|
||||||
['array', ['foo', 'bar', 2021]],
|
['array', ['foo', 'bar', 2021]],
|
||||||
['string', 'foo'],
|
['string', 'foo'],
|
||||||
|
['string (falsey)', ''],
|
||||||
['bool', true],
|
['bool', true],
|
||||||
|
['bool (false)', false],
|
||||||
['number', 2021],
|
['number', 2021],
|
||||||
|
['number (falsey)', 0],
|
||||||
|
['null', null],
|
||||||
|
['literal string undefined', 'undefined'],
|
||||||
];
|
];
|
||||||
for (const [type, value] of serialization) {
|
for (const [type, value] of serialization) {
|
||||||
const stringifiedValue = JSON.stringify(value);
|
const stringifiedValue = JSON.stringify(value);
|
||||||
|
|
|
||||||
|
|
@ -103,6 +103,19 @@ test(`testInfo.attach errors`, async ({ runInlineTest }) => {
|
||||||
expect(result.exitCode).toBe(1);
|
expect(result.exitCode).toBe(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test(`testInfo.attach errors with empty path`, async ({ runInlineTest }) => {
|
||||||
|
const result = await runInlineTest({
|
||||||
|
'a.test.js': `
|
||||||
|
const { test } = pwt;
|
||||||
|
test('fail', async ({}, testInfo) => {
|
||||||
|
await testInfo.attach('name', { path: '' });
|
||||||
|
});
|
||||||
|
`,
|
||||||
|
}, { reporter: 'line', workers: 1 });
|
||||||
|
expect(stripAscii(result.output)).toMatch(/Error: ENOENT: no such file or directory, copyfile ''/);
|
||||||
|
expect(result.exitCode).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
test(`testInfo.attach error in fixture`, async ({ runInlineTest }) => {
|
test(`testInfo.attach error in fixture`, async ({ runInlineTest }) => {
|
||||||
const result = await runInlineTest({
|
const result = await runInlineTest({
|
||||||
'a.test.js': `
|
'a.test.js': `
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue