Skip to content

Commit

Permalink
fix: properly stream uploadData in protocol.handle() (#41359)
Browse files Browse the repository at this point in the history 
* refactor(protocol): extract file stream factory

Increase readability by moving the file stream creation logic out of the
`uploadData` to request body conversion function.

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

* fix: properly flatten streams in `protocol.handle()`

Refs: #39658

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

* fix: `protocol.handle()` filter null origin header

Refs: #40754

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

* fix: remove obsolete TODO comment

Refs: #38929

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

* fix: forward `Blob` parts in `protocol.handle()`

Refs: #40826

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

* fix: explicitly error out on unknown chunk parts

Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Henrik S. Gaßmann <BurningEnlightenment@users.noreply.github.com>
  • Loading branch information
@trop @BurningEnlightenment
trop[bot] and BurningEnlightenment committed Feb 19, 2024
1 parent b87cf56 commit b6db80c
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 12 deletions.
55 changes: 43 additions & 12 deletions lib/browser/api/protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@ function makeStreamFromPipe (pipe: any): ReadableStream {
});
}

function makeStreamFromFileInfo ({
filePath,
offset = 0,
length = -1
}: {
filePath: string;
offset?: number;
length?: number;
}): ReadableStream {
return Readable.toWeb(createReadStream(filePath, {
start: offset,
end: length >= 0 ? offset + length : undefined
}));
}

function convertToRequestBody (uploadData: ProtocolRequest['uploadData']): RequestInit['body'] {
if (!uploadData) return null;
// Optimization: skip creating a stream if the request is just a single buffer.
Expand All @@ -37,30 +52,42 @@ 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 = Readable.toWeb(createReadStream(chunk.filePath, { start: chunk.offset ?? 0, end: chunk.length >= 0 ? chunk.offset + chunk.length : undefined })).getReader();
this.pull!(controller);
if (chunk.type === 'rawData') {
controller.enqueue(chunk.bytes);
} else if (chunk.type === 'file') {
current = makeStreamFromFileInfo(chunk).getReader();
return this.pull!(controller);
} else if (chunk.type === 'stream') {
current = makeStreamFromPipe(chunk.body).getReader();
this.pull!(controller);
return this.pull!(controller);
} else if (chunk.type === 'blob') {
// Note that even though `getBlobData()` is a `Session` API, it doesn't
// actually use the `Session` context. Its implementation solely relies
// on global variables which allows us to implement this feature without
// knowledge of the `Session` associated with the current request by
// always pulling `Blob` data out of the default `Session`.
controller.enqueue(await session.defaultSession.getBlobData(chunk.blobUUID));
} else {
throw new Error(`Unknown upload data chunk type: ${chunk.type}`);
}
}
}
}) as RequestInit['body'];
}

// TODO(codebytere): Use Object.hasOwn() once we update to ECMAScript 2022.
function validateResponse (res: Response) {
if (!res || typeof res !== 'object') return false;

Expand All @@ -85,8 +112,12 @@ Protocol.prototype.handle = function (this: Electron.Protocol, scheme: string, h
const success = register.call(this, scheme, async (preq: ProtocolRequest, cb: any) => {
try {
const body = convertToRequestBody(preq.uploadData);
const headers = new Headers(preq.headers);
if (headers.get('origin') === 'null') {
headers.delete('origin');
}
const req = new Request(preq.url, {
headers: preq.headers,
headers,
method: preq.method,
referrer: preq.referrer,
body,
Expand Down
118 changes: 118 additions & 0 deletions spec/api-protocol-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ 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 streamConsumers from 'node:stream/consumers';
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 +1550,122 @@ 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();
}
});

it('filters an illegal "origin: null" header', async () => {
protocol.handle('http', (req) => {
expect(new Headers(req.headers).get('origin')).to.not.equal('null');
return new Response();
});
defer(() => { protocol.unhandle('http'); });

const filePath = path.join(fixturesPath, 'pages', 'form-with-data.html');
await contents.loadFile(filePath);

const loadPromise = new Promise((resolve, reject) => {
contents.once('did-finish-load', resolve);
contents.once('did-fail-load', (_, errorCode, errorDescription) =>
reject(new Error(`did-fail-load: ${errorCode} ${errorDescription}. See AssertionError for details.`))
);
});
await contents.executeJavaScript(`
const form = document.querySelector('form');
form.action = 'http://cors.invalid';
form.method = 'POST';
form.submit();
`);
await loadPromise;
});

it('does forward Blob chunks', async () => {
// we register the protocol on a separate session to validate the assumption
// that `getBlobData()` indeed returns the blob data from a global variable
const s = session.fromPartition('protocol-handle-forwards-blob-chunks');

s.protocol.handle('cors', async (request) => {
expect(request.body).to.be.an.instanceOf(webStream.ReadableStream);
return new Response(
`hello to ${await streamConsumers.text(request.body as webStream.ReadableStream<Uint8Array>)}`,
{ status: 200 }
);
});
defer(() => { s.protocol.unhandle('cors'); });

const w = new BrowserWindow({ show: false, webPreferences: { session: s } });
await w.webContents.loadFile(path.resolve(fixturesPath, 'pages', 'base-page.html'));
const response = await w.webContents.executeJavaScript(`(async () => {
const body = new Blob(["it's-a ", 'me! ', 'Mario!'], { type: 'text/plain' });
return await (await fetch('cors://url.invalid', { method: 'POST', body })).text();
})()`);
expect(response).to.be.string('hello to it\'s-a me! Mario!');
});

// 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 b6db80c

Please sign in to comment.