fix: properly stream `uploadData` in `protocol.handle()` (#41359)

* 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 <[email protected]>

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

Refs: electron/electron#39658

Co-authored-by: Henrik S. Gaßmann <[email protected]>

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

Refs: electron/electron#40754

Co-authored-by: Henrik S. Gaßmann <[email protected]>

* fix: remove obsolete TODO comment

Refs: electron/electron#38929

Co-authored-by: Henrik S. Gaßmann <[email protected]>

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

Refs: electron/electron#40826

Co-authored-by: Henrik S. Gaßmann <[email protected]>

* fix: explicitly error out on unknown chunk parts

Co-authored-by: Henrik S. Gaßmann <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Henrik S. Gaßmann <[email protected]>

lib/browser/api/protocol.ts

@@ -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.
@@ -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;
 
@@ -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,

spec/api-protocol-spec.ts

@@ -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';
@@ -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.