chore: change undocumented protocol.registerProtocol to detect body type (#36595)

* feat: add protocol.registerProtocol

* remove wip handleProtocol code

* lint

* Update shell/browser/net/electron_url_loader_factory.h

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>

* fix

---------

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>

shell/browser/net/electron_url_loader_factory.cc

@@ -4,6 +4,7 @@
 
 #include "shell/browser/net/electron_url_loader_factory.h"
 
+#include <list>
 #include <memory>
 #include <string>
 #include <utility>
@@ -81,6 +82,19 @@ bool ResponseMustBeObject(ProtocolType type) {
   }
 }
 
+bool LooksLikeStream(v8::Isolate* isolate, v8::Local<v8::Value> v) {
+  // the stream loader can handle null and undefined as "empty body". Could
+  // probably be more efficient here but this works.
+  if (v->IsNullOrUndefined())
+    return true;
+  if (!v->IsObject())
+    return false;
+  gin_helper::Dictionary dict(isolate, v.As<v8::Object>());
+  v8::Local<v8::Value> method;
+  return dict.Get("on", &method) && method->IsFunction() &&
+         dict.Get("removeListener", &method) && method->IsFunction();
+}
+
 // Helper to convert value to Dictionary.
 gin::Dictionary ToDict(v8::Isolate* isolate, v8::Local<v8::Value> value) {
   if (!value->IsFunction() && value->IsObject())
@@ -390,36 +404,94 @@ void ElectronURLLoaderFactory::StartLoading(
   }
 
   switch (type) {
+    // DEPRECATED: Soon only |kFree| will be supported!
     case ProtocolType::kBuffer:
-      StartLoadingBuffer(std::move(client), std::move(head), dict);
+      if (response->IsArrayBufferView())
-      break;
+        StartLoadingBuffer(std::move(client), std::move(head),
-    case ProtocolType::kString:
+                           response.As<v8::ArrayBufferView>());
-      StartLoadingString(std::move(client), std::move(head), dict,
+      else if (v8::Local<v8::Value> data; !dict.IsEmpty() &&
-                         args->isolate(), response);
+                                          dict.Get("data", &data) &&
-      break;
+                                          data->IsArrayBufferView())
-    case ProtocolType::kFile:
+        StartLoadingBuffer(std::move(client), std::move(head),
-      StartLoadingFile(std::move(loader), request, std::move(client),
+                           data.As<v8::ArrayBufferView>());
-                       std::move(head), dict, args->isolate(), response);
+      else
-      break;
-    case ProtocolType::kHttp:
-      StartLoadingHttp(std::move(loader), request, std::move(client),
-                       traffic_annotation, dict);
-      break;
-    case ProtocolType::kStream:
-      StartLoadingStream(std::move(loader), std::move(client), std::move(head),
-                         dict);
-      break;
-    case ProtocolType::kFree:
-      ProtocolType protocol_type;
-      if (!gin::ConvertFromV8(args->isolate(), response, &protocol_type)) {
         OnComplete(std::move(client), request_id,
                    network::URLLoaderCompletionStatus(net::ERR_FAILED));
-        return;
-      }
-      StartLoading(std::move(loader), request_id, options, request,
-                   std::move(client), traffic_annotation,
-                   std::move(target_factory), protocol_type, args);
       break;
+    case ProtocolType::kString: {
+      std::string data;
+      if (gin::ConvertFromV8(args->isolate(), response, &data))
+        SendContents(std::move(client), std::move(head), data);
+      else if (!dict.IsEmpty() && dict.Get("data", &data))
+        SendContents(std::move(client), std::move(head), data);
+      else
+        OnComplete(std::move(client), request_id,
+                   network::URLLoaderCompletionStatus(net::ERR_FAILED));
+      break;
+    }
+    case ProtocolType::kFile: {
+      base::FilePath path;
+      if (gin::ConvertFromV8(args->isolate(), response, &path))
+        StartLoadingFile(std::move(client), std::move(loader), std::move(head),
+                         request, path, dict);
+      else if (!dict.IsEmpty() && dict.Get("path", &path))
+        StartLoadingFile(std::move(client), std::move(loader), std::move(head),
+                         request, path, dict);
+      else
+        OnComplete(std::move(client), request_id,
+                   network::URLLoaderCompletionStatus(net::ERR_FAILED));
+      break;
+    }
+    case ProtocolType::kHttp:
+      if (GURL url; !dict.IsEmpty() && dict.Get("url", &url) && url.is_valid())
+        StartLoadingHttp(std::move(client), std::move(loader), request,
+                         traffic_annotation, dict);
+      else
+        OnComplete(std::move(client), request_id,
+                   network::URLLoaderCompletionStatus(net::ERR_FAILED));
+      break;
+    case ProtocolType::kStream:
+      StartLoadingStream(std::move(client), std::move(loader), std::move(head),
+                         dict);
+      break;
+
+    case ProtocolType::kFree: {
+      // Infer the type based on the object given
+      v8::Local<v8::Value> data;
+      if (!dict.IsEmpty() && dict.Has("data"))
+        dict.Get("data", &data);
+      else
+        data = response;
+
+      // |data| can be either a string, a buffer or a stream.
+      if (data->IsArrayBufferView()) {
+        StartLoadingBuffer(std::move(client), std::move(head),
+                           data.As<v8::ArrayBufferView>());
+      } else if (data->IsString()) {
+        SendContents(std::move(client), std::move(head),
+                     gin::V8ToString(args->isolate(), data));
+      } else if (LooksLikeStream(args->isolate(), data)) {
+        StartLoadingStream(std::move(client), std::move(loader),
+                           std::move(head), dict);
+      } else if (!dict.IsEmpty()) {
+        // |data| wasn't specified, so look for |response.url| or
+        // |response.path|.
+        if (GURL url; dict.Get("url", &url))
+          StartLoadingHttp(std::move(client), std::move(loader), request,
+                           traffic_annotation, dict);
+        else if (base::FilePath path; dict.Get("path", &path))
+          StartLoadingFile(std::move(client), std::move(loader),
+                           std::move(head), request, path, dict);
+        else
+          // Don't know what kind of response this is, so fail.
+          OnComplete(std::move(client), request_id,
+                     network::URLLoaderCompletionStatus(net::ERR_FAILED));
+      } else {
+        OnComplete(std::move(client), request_id,
+                   network::URLLoaderCompletionStatus(net::ERR_FAILED));
+      }
+      break;
+    }
   }
 }
 
@@ -427,68 +499,25 @@ void ElectronURLLoaderFactory::StartLoading(
 void ElectronURLLoaderFactory::StartLoadingBuffer(
     mojo::PendingRemote<network::mojom::URLLoaderClient> client,
     network::mojom::URLResponseHeadPtr head,
-    const gin_helper::Dictionary& dict) {
+    v8::Local<v8::ArrayBufferView> buffer) {
-  v8::Local<v8::Value> buffer = dict.GetHandle();
+  SendContents(std::move(client), std::move(head),
-  dict.Get("data", &buffer);
+               std::string(node::Buffer::Data(buffer.As<v8::Value>()),
-  if (!node::Buffer::HasInstance(buffer)) {
+                           node::Buffer::Length(buffer.As<v8::Value>())));
-    mojo::Remote<network::mojom::URLLoaderClient> client_remote(
-        std::move(client));
-    client_remote->OnComplete(
-        network::URLLoaderCompletionStatus(net::ERR_FAILED));
-    return;
-  }
-
-  SendContents(
-      std::move(client), std::move(head),
-      std::string(node::Buffer::Data(buffer), node::Buffer::Length(buffer)));
-}
-
-// static
-void ElectronURLLoaderFactory::StartLoadingString(
-    mojo::PendingRemote<network::mojom::URLLoaderClient> client,
-    network::mojom::URLResponseHeadPtr head,
-    const gin_helper::Dictionary& dict,
-    v8::Isolate* isolate,
-    v8::Local<v8::Value> response) {
-  std::string contents;
-  if (response->IsString()) {
-    contents = gin::V8ToString(isolate, response);
-  } else if (!dict.IsEmpty()) {
-    dict.Get("data", &contents);
-  } else {
-    mojo::Remote<network::mojom::URLLoaderClient> client_remote(
-        std::move(client));
-    client_remote->OnComplete(
-        network::URLLoaderCompletionStatus(net::ERR_FAILED));
-    return;
-  }
-
-  SendContents(std::move(client), std::move(head), std::move(contents));
 }
 
 // static
 void ElectronURLLoaderFactory::StartLoadingFile(
-    mojo::PendingReceiver<network::mojom::URLLoader> loader,
-    network::ResourceRequest request,
     mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+    mojo::PendingReceiver<network::mojom::URLLoader> loader,
     network::mojom::URLResponseHeadPtr head,
-    const gin_helper::Dictionary& dict,
+    const network::ResourceRequest& original_request,
-    v8::Isolate* isolate,
+    const base::FilePath& path,
-    v8::Local<v8::Value> response) {
+    const gin_helper::Dictionary& opts) {
-  base::FilePath path;
+  network::ResourceRequest request = original_request;
-  if (gin::ConvertFromV8(isolate, response, &path)) {
   request.url = net::FilePathToFileURL(path);
-  } else if (!dict.IsEmpty()) {
+  if (!opts.IsEmpty()) {
-    dict.Get("referrer", &request.referrer);
+    opts.Get("referrer", &request.referrer);
-    dict.Get("method", &request.method);
+    opts.Get("method", &request.method);
-    if (dict.Get("path", &path))
-      request.url = net::FilePathToFileURL(path);
-  } else {
-    mojo::Remote<network::mojom::URLLoaderClient> client_remote(
-        std::move(client));
-    client_remote->OnComplete(
-        network::URLLoaderCompletionStatus(net::ERR_FAILED));
-    return;
   }
 
   // Add header to ignore CORS.
@@ -499,9 +528,9 @@ void ElectronURLLoaderFactory::StartLoadingFile(
 
 // static
 void ElectronURLLoaderFactory::StartLoadingHttp(
+    mojo::PendingRemote<network::mojom::URLLoaderClient> client,
     mojo::PendingReceiver<network::mojom::URLLoader> loader,
     const network::ResourceRequest& original_request,
-    mojo::PendingRemote<network::mojom::URLLoaderClient> client,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     const gin_helper::Dictionary& dict) {
   auto request = std::make_unique<network::ResourceRequest>();
@@ -543,8 +572,8 @@ void ElectronURLLoaderFactory::StartLoadingHttp(
 
 // static
 void ElectronURLLoaderFactory::StartLoadingStream(
-    mojo::PendingReceiver<network::mojom::URLLoader> loader,
     mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+    mojo::PendingReceiver<network::mojom::URLLoader> loader,
     network::mojom::URLResponseHeadPtr head,
     const gin_helper::Dictionary& dict) {
   v8::Local<v8::Value> stream;

shell/browser/net/electron_url_loader_factory.h

@@ -135,33 +135,27 @@ class ElectronURLLoaderFactory : public network::SelfDeletingURLLoaderFactory {
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       int32_t request_id,
       const network::URLLoaderCompletionStatus& status);
+
   static void StartLoadingBuffer(
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       network::mojom::URLResponseHeadPtr head,
-      const gin_helper::Dictionary& dict);
+      v8::Local<v8::ArrayBufferView> buffer);
-  static void StartLoadingString(
-      mojo::PendingRemote<network::mojom::URLLoaderClient> client,
-      network::mojom::URLResponseHeadPtr head,
-      const gin_helper::Dictionary& dict,
-      v8::Isolate* isolate,
-      v8::Local<v8::Value> response);
   static void StartLoadingFile(
-      mojo::PendingReceiver<network::mojom::URLLoader> loader,
-      network::ResourceRequest request,
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+      mojo::PendingReceiver<network::mojom::URLLoader> loader,
       network::mojom::URLResponseHeadPtr head,
-      const gin_helper::Dictionary& dict,
+      const network::ResourceRequest& original_request,
-      v8::Isolate* isolate,
+      const base::FilePath& path,
-      v8::Local<v8::Value> response);
+      const gin_helper::Dictionary& opts);
   static void StartLoadingHttp(
+      mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       mojo::PendingReceiver<network::mojom::URLLoader> loader,
       const network::ResourceRequest& original_request,
-      mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
       const gin_helper::Dictionary& dict);
   static void StartLoadingStream(
-      mojo::PendingReceiver<network::mojom::URLLoader> loader,
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+      mojo::PendingReceiver<network::mojom::URLLoader> loader,
       network::mojom::URLResponseHeadPtr head,
       const gin_helper::Dictionary& dict);
 

spec/api-protocol-spec.ts

@@ -19,7 +19,6 @@ const fixturesPath = path.resolve(__dirname, 'fixtures');
 const registerStringProtocol = protocol.registerStringProtocol;
 const registerBufferProtocol = protocol.registerBufferProtocol;
 const registerFileProtocol = protocol.registerFileProtocol;
-const registerHttpProtocol = protocol.registerHttpProtocol;
 const registerStreamProtocol = protocol.registerStreamProtocol;
 const interceptStringProtocol = protocol.interceptStringProtocol;
 const interceptBufferProtocol = protocol.interceptBufferProtocol;
@@ -146,7 +145,11 @@ describe('protocol module', () => {
     });
   });
 
-  describe('protocol.registerStringProtocol', () => {
+  for (const [registerStringProtocol, name] of [
+    [protocol.registerStringProtocol, 'protocol.registerStringProtocol'] as const,
+    [(protocol as any).registerProtocol as typeof protocol.registerStringProtocol, 'protocol.registerProtocol'] as const
+  ]) {
+    describe(name, () => {
       it('sends string as response', async () => {
         registerStringProtocol(protocolName, (request, callback) => callback(text));
         const r = await ajax(protocolName + '://fake-host');
@@ -177,8 +180,13 @@ describe('protocol module', () => {
         await expect(ajax(protocolName + '://fake-host')).to.be.eventually.rejected();
       });
     });
+  }
 
-  describe('protocol.registerBufferProtocol', () => {
+  for (const [registerBufferProtocol, name] of [
+    [protocol.registerBufferProtocol, 'protocol.registerBufferProtocol'] as const,
+    [(protocol as any).registerProtocol as typeof protocol.registerBufferProtocol, 'protocol.registerProtocol'] as const
+  ]) {
+    describe(name, () => {
       const buffer = Buffer.from(text);
       it('sends Buffer as response', async () => {
         registerBufferProtocol(protocolName, (request, callback) => callback(buffer));
@@ -204,13 +212,20 @@ describe('protocol module', () => {
         expect(r.data).to.equal(text);
       });
 
+      if (name !== 'protocol.registerProtocol') {
         it('fails when sending string', async () => {
           registerBufferProtocol(protocolName, (request, callback) => callback(text as any));
           await expect(ajax(protocolName + '://fake-host')).to.be.eventually.rejected();
         });
+      }
     });
+  }
 
-  describe('protocol.registerFileProtocol', () => {
+  for (const [registerFileProtocol, name] of [
+    [protocol.registerFileProtocol, 'protocol.registerFileProtocol'] as const,
+    [(protocol as any).registerProtocol as typeof protocol.registerFileProtocol, 'protocol.registerProtocol'] as const
+  ]) {
+    describe(name, () => {
       const filePath = path.join(fixturesPath, 'test.asar', 'a.asar', 'file1');
       const fileContent = fs.readFileSync(filePath);
       const normalPath = path.join(fixturesPath, 'pages', 'a.html');
@@ -218,14 +233,16 @@ describe('protocol module', () => {
 
       afterEach(closeAllWindows);
 
+      if (name === 'protocol.registerFileProtocol') {
         it('sends file path as response', async () => {
           registerFileProtocol(protocolName, (request, callback) => callback(filePath));
           const r = await ajax(protocolName + '://fake-host');
           expect(r.data).to.equal(String(fileContent));
         });
+      }
 
       it('sets Access-Control-Allow-Origin', async () => {
-      registerFileProtocol(protocolName, (request, callback) => callback(filePath));
+        registerFileProtocol(protocolName, (request, callback) => callback({ path: filePath }));
         const r = await ajax(protocolName + '://fake-host');
         expect(r.data).to.equal(String(fileContent));
         expect(r.headers).to.have.property('access-control-allow-origin', '*');
@@ -278,14 +295,14 @@ describe('protocol module', () => {
       });
 
       it('can send normal file', async () => {
-      registerFileProtocol(protocolName, (request, callback) => callback(normalPath));
+        registerFileProtocol(protocolName, (request, callback) => callback({ path: normalPath }));
         const r = await ajax(protocolName + '://fake-host');
         expect(r.data).to.equal(String(normalContent));
       });
 
       it('fails when sending unexist-file', async () => {
         const fakeFilePath = path.join(fixturesPath, 'test.asar', 'a.asar', 'not-exist');
-      registerFileProtocol(protocolName, (request, callback) => callback(fakeFilePath));
+        registerFileProtocol(protocolName, (request, callback) => callback({ path: fakeFilePath }));
         await expect(ajax(protocolName + '://fake-host')).to.be.eventually.rejected();
       });
 
@@ -294,8 +311,13 @@ describe('protocol module', () => {
         await expect(ajax(protocolName + '://fake-host')).to.be.eventually.rejected();
       });
     });
+  }
 
-  describe('protocol.registerHttpProtocol', () => {
+  for (const [registerHttpProtocol, name] of [
+    [protocol.registerHttpProtocol, 'protocol.registerHttpProtocol'] as const,
+    [(protocol as any).registerProtocol as typeof protocol.registerHttpProtocol, 'protocol.registerProtocol'] as const
+  ]) {
+    describe(name, () => {
       it('sends url as response', async () => {
         const server = http.createServer((req, res) => {
           expect(req.headers.accept).to.not.equal('');
@@ -355,8 +377,13 @@ describe('protocol module', () => {
         ajax(protocolName + '://fake-host').catch(() => {});
       });
     });
+  }
 
-  describe('protocol.registerStreamProtocol', () => {
+  for (const [registerStreamProtocol, name] of [
+    [protocol.registerStreamProtocol, 'protocol.registerStreamProtocol'] as const,
+    [(protocol as any).registerProtocol as typeof protocol.registerStreamProtocol, 'protocol.registerProtocol'] as const
+  ]) {
+    describe(name, () => {
       it('sends Stream as response', async () => {
         registerStreamProtocol(protocolName, (request, callback) => callback(getStream()));
         const r = await ajax(protocolName + '://fake-host');
@@ -506,6 +533,7 @@ describe('protocol module', () => {
         await hasClosedPromise;
       });
     });
+  }
 
   describe('protocol.isProtocolRegistered', () => {
     it('returns false when scheme is not registered', () => {