fix: only convert input files for browser collocated with server (#27873)

Reference #27452
Fixes #27792
This commit is contained in:
Yury Semikhatsky 2023-11-01 08:40:12 -07:00 committed by GitHub
parent 38115d121b
commit 36c4c24f8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 102 additions and 16 deletions

View File

@ -151,6 +151,10 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
this.tracing._tracesDir = browserOptions.tracesDir;
}
_isLocalBrowserOnServer(): boolean {
return this._initializer.isLocalBrowserOnServer;
}
private _onPage(page: Page): void {
this._pages.add(page);
this.emit(Events.BrowserContext.Page, page);

View File

@ -263,31 +263,65 @@ type InputFilesList = {
localPaths?: string[];
streams?: channels.WritableStreamChannel[];
};
const filePayloadSizeLimit = 50 * 1024 * 1024;
function filePayloadExceedsSizeLimit(payloads: FilePayload[]) {
return payloads.reduce((size, item) => size + (item.buffer ? item.buffer.byteLength : 0), 0) >= filePayloadSizeLimit;
}
async function filesExceedSizeLimit(files: string[]) {
const sizes = await Promise.all(files.map(async file => (await fs.promises.stat(file)).size));
return sizes.reduce((total, size) => total + size, 0) >= filePayloadSizeLimit;
}
async function readFilesIntoBuffers(items: string[]): Promise<SetInputFilesFiles> {
const filePayloads: SetInputFilesFiles = await Promise.all((items as string[]).map(async item => {
return {
name: path.basename(item),
buffer: await fs.promises.readFile(item),
lastModifiedMs: (await fs.promises.stat(item)).mtimeMs,
};
}));
return filePayloads;
}
export async function convertInputFiles(files: string | FilePayload | string[] | FilePayload[], context: BrowserContext): Promise<InputFilesList> {
const items: (string | FilePayload)[] = Array.isArray(files) ? files.slice() : [files];
if (items.some(item => typeof item === 'string')) {
if (!items.every(item => typeof item === 'string'))
throw new Error('File paths cannot be mixed with buffers');
if (context._connection.isRemote()) {
const streams: channels.WritableStreamChannel[] = await Promise.all((items as string[]).map(async item => {
const lastModifiedMs = (await fs.promises.stat(item)).mtimeMs;
const { writableStream: stream } = await context._channel.createTempFile({ name: path.basename(item), lastModifiedMs });
const writable = WritableStream.from(stream);
await pipelineAsync(fs.createReadStream(item), writable.stream());
return stream;
}));
return { streams };
if (context._isLocalBrowserOnServer()) {
const streams: channels.WritableStreamChannel[] = await Promise.all((items as string[]).map(async item => {
const lastModifiedMs = (await fs.promises.stat(item)).mtimeMs;
const { writableStream: stream } = await context._channel.createTempFile({ name: path.basename(item), lastModifiedMs });
const writable = WritableStream.from(stream);
await pipelineAsync(fs.createReadStream(item), writable.stream());
return stream;
}));
return { streams };
}
if (await filesExceedSizeLimit(items as string[]))
throw new Error('Cannot transfer files larger than 50Mb to a browser not co-located with the server');
return { files: await readFilesIntoBuffers(items as string[]) };
}
return { localPaths: items.map(f => path.resolve(f as string)) as string[] };
if (context._isLocalBrowserOnServer())
return { localPaths: items.map(f => path.resolve(f as string)) as string[] };
if (await filesExceedSizeLimit(items as string[]))
throw new Error('Cannot transfer files larger than 50Mb to a browser not co-located with the server');
return { files: await readFilesIntoBuffers(items as string[]) };
}
const payloads = items as FilePayload[];
const sizeLimit = 50 * 1024 * 1024;
const totalBufferSizeExceedsLimit = payloads.reduce((size, item) => size + (item.buffer ? item.buffer.byteLength : 0), 0) > sizeLimit;
if (totalBufferSizeExceedsLimit)
throw new Error('Cannot set buffer larger than 50Mb, please write it to a file and pass its path instead.');
if (filePayloadExceedsSizeLimit(payloads)) {
let error = 'Cannot set buffer larger than 50Mb';
if (context._isLocalBrowserOnServer())
error += ', please write it to a file and pass its path instead.';
throw new Error(error);
}
return { files: payloads };
}

View File

@ -762,6 +762,7 @@ scheme.ElectronApplicationWaitForEventInfoResult = tType('EventTargetWaitForEven
scheme.AndroidDeviceWaitForEventInfoResult = tType('EventTargetWaitForEventInfoResult');
scheme.BrowserContextInitializer = tObject({
isChromium: tBoolean,
isLocalBrowserOnServer: tBoolean,
requestContext: tChannel(['APIRequestContext']),
tracing: tChannel(['Tracing']),
});
@ -1560,6 +1561,7 @@ scheme.FrameSetInputFilesParams = tObject({
name: tString,
mimeType: tOptional(tString),
buffer: tBinary,
lastModifiedMs: tOptional(tNumber),
})),
timeout: tOptional(tNumber),
noWaitAfter: tOptional(tBoolean),
@ -1933,6 +1935,7 @@ scheme.ElementHandleSetInputFilesParams = tObject({
name: tString,
mimeType: tOptional(tString),
buffer: tBinary,
lastModifiedMs: tOptional(tNumber),
})),
timeout: tOptional(tNumber),
noWaitAfter: tOptional(tBoolean),

View File

@ -65,6 +65,7 @@ export abstract class Browser extends SdkObject {
readonly _idToVideo = new Map<string, { context: BrowserContext, artifact: Artifact }>();
private _contextForReuse: { context: BrowserContext, hash: string } | undefined;
_closeReason: string | undefined;
_isCollocatedWithServer: boolean = true;
constructor(parent: SdkObject, options: BrowserOptions) {
super(parent, 'browser');

View File

@ -120,6 +120,7 @@ export class Chromium extends BrowserType {
validateBrowserContextOptions(persistent, browserOptions);
progress.throwIfAborted();
const browser = await CRBrowser.connect(this.attribution.playwright, chromeTransport, browserOptions);
browser._isCollocatedWithServer = false;
browser.on(Browser.Events.Disconnected, doCleanup);
return browser;
}

View File

@ -59,6 +59,8 @@ export class CRBrowser extends Browser {
const connection = new CRConnection(transport, options.protocolLogger, options.browserLogsCollector);
const browser = new CRBrowser(parent, connection, options);
browser._devtools = devtools;
if (browser.isClank())
browser._isCollocatedWithServer = false;
const session = connection.rootSession;
if ((options as any).__testHookOnConnectToBrowser)
await (options as any).__testHookOnConnectToBrowser();

View File

@ -53,6 +53,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
super(parentScope, context, 'BrowserContext', {
isChromium: context._browser.options.isChromium,
isLocalBrowserOnServer: context._browser._isCollocatedWithServer,
requestContext,
tracing,
});
@ -177,6 +178,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
}
async createTempFile(params: channels.BrowserContextCreateTempFileParams): Promise<channels.BrowserContextCreateTempFileResult> {
if (!this._context._browser._isCollocatedWithServer)
throw new Error('Cannot create temp file: the browser is not co-located with the server');
const dir = this._context._browser.options.artifactsDir;
const tmpDir = path.join(dir, 'upload-' + createGuid());
await fs.promises.mkdir(tmpDir);

View File

@ -597,6 +597,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
name: payload.name,
mimeType: payload.mimeType || mime.getType(payload.name) || 'application/octet-stream',
buffer: payload.buffer.toString('base64'),
lastModifiedMs: payload.lastModifiedMs
});
}
}

View File

@ -824,7 +824,7 @@ export class InjectedScript {
return 'done';
}
setInputFiles(node: Node, payloads: { name: string, mimeType: string, buffer: string }[]) {
setInputFiles(node: Node, payloads: { name: string, mimeType: string, buffer: string, lastModifiedMs?: number }[]) {
if (node.nodeType !== Node.ELEMENT_NODE)
return 'Node is not of type HTMLElement';
const element: Element | undefined = node as Element;
@ -837,7 +837,7 @@ export class InjectedScript {
const files = payloads.map(file => {
const bytes = Uint8Array.from(atob(file.buffer), c => c.charCodeAt(0));
return new File([bytes], file.name, { type: file.mimeType });
return new File([bytes], file.name, { type: file.mimeType, lastModified: file.lastModifiedMs });
});
const dt = new DataTransfer();
for (const file of files)

View File

@ -76,6 +76,7 @@ export type FilePayload = {
name: string,
mimeType: string,
buffer: string,
lastModifiedMs?: number,
};
export type MediaType = 'screen' | 'print' | 'no-override';

View File

@ -1407,6 +1407,7 @@ export interface EventTargetEvents {
// ----------- BrowserContext -----------
export type BrowserContextInitializer = {
isChromium: boolean,
isLocalBrowserOnServer: boolean,
requestContext: APIRequestContextChannel,
tracing: TracingChannel,
};
@ -2795,6 +2796,7 @@ export type FrameSetInputFilesParams = {
name: string,
mimeType?: string,
buffer: Binary,
lastModifiedMs?: number,
}[],
timeout?: number,
noWaitAfter?: boolean,
@ -3425,6 +3427,7 @@ export type ElementHandleSetInputFilesParams = {
name: string,
mimeType?: string,
buffer: Binary,
lastModifiedMs?: number,
}[],
timeout?: number,
noWaitAfter?: boolean,

View File

@ -1012,6 +1012,7 @@ BrowserContext:
initializer:
isChromium: boolean
isLocalBrowserOnServer: boolean
requestContext: APIRequestContext
tracing: Tracing
@ -2118,6 +2119,7 @@ Frame:
name: string
mimeType: string?
buffer: binary
lastModifiedMs: number?
timeout: number?
noWaitAfter: boolean?
flags:
@ -2686,6 +2688,7 @@ ElementHandle:
name: string
mimeType: string?
buffer: binary
lastModifiedMs: number?
timeout: number?
noWaitAfter: boolean?
flags:

View File

@ -470,3 +470,33 @@ test('should allow tracing over cdp session', async ({ browserType, trace }, tes
await browserServer.close();
}
});
test('setInputFiles should preserve lastModified timestamp', async ({ browserType, asset }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27452' });
const port = 9339 + test.info().workerIndex;
const browserServer = await browserType.launch({
args: ['--remote-debugging-port=' + port]
});
try {
const cdpBrowser = await browserType.connectOverCDP({
endpointURL: `http://127.0.0.1:${port}/`,
});
const [context] = cdpBrowser.contexts();
const page = await context.newPage();
await page.setContent(`<input type=file multiple=true/>`);
const input = page.locator('input');
const files = ['file-to-upload.txt', 'file-to-upload-2.txt'];
await input.setInputFiles(files.map(f => asset(f)));
expect(await input.evaluate(e => [...(e as HTMLInputElement).files].map(f => f.name))).toEqual(files);
const timestamps = await input.evaluate(e => [...(e as HTMLInputElement).files].map(f => f.lastModified));
const expectedTimestamps = files.map(file => Math.round(fs.statSync(asset(file)).mtimeMs));
// On Linux browser sometimes reduces the timestamp by 1ms: 1696272058110.0715 -> 1696272058109 or even
// rounds it to seconds in WebKit: 1696272058110 -> 1696272058000.
for (let i = 0; i < timestamps.length; i++)
expect(Math.abs(timestamps[i] - expectedTimestamps[i]), `expected: ${expectedTimestamps}; actual: ${timestamps}`).toBeLessThan(1000);
await cdpBrowser.close();
} finally {
await browserServer.close();
}
});