Skip to content

Commit

Permalink
fix: properly flatten streams in protocol.handle()
Browse files Browse the repository at this point in the history
Refs: #39658

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>
  • Loading branch information
trop[bot] and BurningEnlightenment committed Feb 16, 2024
1 parent 62a6e4b commit 77f7884
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 8 deletions.
18 changes: 10 additions & 8 deletions lib/browser/api/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,27 @@ function convertToRequestBody (uploadData: ProtocolRequest['uploadData']): Reque
const chunks = [...uploadData] as any[]; // TODO: types are wrong
let current: ReadableStreamDefaultReader | null = null;
return new ReadableStream({
pull (controller) {
async pull (controller) {
if (current) {
current.read().then(({ done, value }) => {
const { done, value } = await current.read();
// (done => value === undefined) as per WHATWG spec
if (done) {
current = null;
return this.pull!(controller);
} else {
controller.enqueue(value);
if (done) current = null;
}, (err) => {
controller.error(err);
});
}
} else {
if (!chunks.length) { return controller.close(); }
const chunk = chunks.shift()!;
if (chunk.type === 'rawData') {
controller.enqueue(chunk.bytes);
} else if (chunk.type === 'file') {
current = makeStreamFromFileInfo(chunk).getReader();
this.pull!(controller);
return this.pull!(controller);
} else if (chunk.type === 'stream') {
current = makeStreamFromPipe(chunk.body).getReader();
this.pull!(controller);
return this.pull!(controller);
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions spec/api-protocol-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as http from 'node:http';
import * as fs from 'node:fs';
import * as qs from 'node:querystring';
import * as stream from 'node:stream';
import * as webStream from 'node:stream/web';
import { EventEmitter, once } from 'node:events';
import { closeAllWindows, closeWindow } from './lib/window-helpers';
import { WebmGenerator } from './lib/video-helpers';
Expand Down Expand Up @@ -1548,6 +1549,74 @@ describe('protocol module', () => {
}
});

it('does not emit undefined chunks into the request body stream when uploading a stream', async () => {
protocol.handle('cors', async (request) => {
expect(request.body).to.be.an.instanceOf(webStream.ReadableStream);
for await (const value of request.body as webStream.ReadableStream<Uint8Array>) {
expect(value).to.not.be.undefined();
}
return new Response(undefined, { status: 200 });
});
defer(() => { protocol.unhandle('cors'); });

await contents.loadFile(path.resolve(fixturesPath, 'pages', 'base-page.html'));
contents.on('console-message', (e, level, message) => console.log(message));
const ok = await contents.executeJavaScript(`(async () => {
function wait(milliseconds) {
return new Promise((resolve) => setTimeout(resolve, milliseconds));
}
const stream = new ReadableStream({
async start(controller) {
await wait(4);
controller.enqueue('This ');
await wait(4);
controller.enqueue('is ');
await wait(4);
controller.enqueue('a ');
await wait(4);
controller.enqueue('slow ');
await wait(4);
controller.enqueue('request.');
controller.close();
}
}).pipeThrough(new TextEncoderStream());
return (await fetch('cors://url.invalid', { method: 'POST', body: stream, duplex: 'half' })).ok;
})()`);
expect(ok).to.be.true();
});

it('does not emit undefined chunks into the request body stream when uploading a file', async () => {
protocol.handle('cors', async (request) => {
expect(request.body).to.be.an.instanceOf(webStream.ReadableStream);
for await (const value of request.body as webStream.ReadableStream<Uint8Array>) {
expect(value).to.not.be.undefined();
}
return new Response(undefined, { status: 200 });
});
defer(() => { protocol.unhandle('cors'); });

await contents.loadFile(path.resolve(fixturesPath, 'pages', 'file-input.html'));
const { debugger: debug } = contents;
debug.attach();
try {
const { root: { nodeId } } = await debug.sendCommand('DOM.getDocument');
const { nodeId: inputNodeId } = await debug.sendCommand('DOM.querySelector', { nodeId, selector: 'input' });
await debug.sendCommand('DOM.setFileInputFiles', {
files: [path.join(fixturesPath, 'cat-spin.mp4')],
nodeId: inputNodeId
});
const ok = await contents.executeJavaScript(`(async () => {
const formData = new FormData();
formData.append("data", document.getElementById("file").files[0]);
return (await fetch('cors://url.invalid', { method: 'POST', body: formData })).ok;
})()`);
expect(ok).to.be.true();
} finally {
debug.detach();
}
});

// TODO(nornagon): this test doesn't pass on Linux currently, investigate.
ifit(process.platform !== 'linux')('is fast', async () => {
// 128 MB of spaces.
Expand Down

0 comments on commit 77f7884

Please sign in to comment.