feat: allow some NODE_OPTIONS in packaged apps (#20857)

* feat: allow some NODE_OPTIONS in packaged apps

* fix: correctly detect packaged app

docs/api/environment-variables.md

@@ -44,7 +44,11 @@ Unsupported options are:
 --use-openssl-ca
 ```
 
-`NODE_OPTIONS` are explicitly disallowed in packaged apps.
+`NODE_OPTIONS` are explicitly disallowed in packaged apps, except for the following:
+
+```sh
+--max-http-header-size
+```
 
 ### `GOOGLE_API_KEY`
 

shell/common/node_bindings.cc

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -122,6 +123,61 @@ void stop_and_close_uv_loop(uv_loop_t* loop) {
 
 bool g_is_initialized = false;
 
+bool IsPackagedApp() {
+  base::FilePath exe_path;
+  base::PathService::Get(base::FILE_EXE, &exe_path);
+  base::FilePath::StringType base_name =
+      base::ToLowerASCII(exe_path.BaseName().value());
+
+#if defined(OS_WIN)
+  return base_name != FILE_PATH_LITERAL("electron.exe");
+#else
+  return base_name != FILE_PATH_LITERAL("electron");
+#endif
+}
+
+// Initialize NODE_OPTIONS to pass to Node.js
+void SetNodeOptions(base::Environment* env) {
+  // Options that are unilaterally disallowed
+  const std::set<std::string> disallowed = {
+      "--openssl-config", "--use-bundled-ca", "--use-openssl-ca",
+      "--force-fips", "--enable-fips"};
+
+  // Subset of options allowed in packaged apps
+  const std::set<std::string> allowed_in_packaged = {"--max-http-header-size"};
+
+  if (env->HasVar("NODE_OPTIONS")) {
+    std::string options;
+    env->GetVar("NODE_OPTIONS", &options);
+    std::vector<std::string> parts = base::SplitString(
+        options, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+
+    bool is_packaged_app = IsPackagedApp();
+
+    for (const auto& part : parts) {
+      // Strip off values passed to individual NODE_OPTIONs
+      std::string option = part.substr(0, part.find("="));
+
+      if (is_packaged_app &&
+          allowed_in_packaged.find(option) == allowed_in_packaged.end()) {
+        // Explicitly disallow majority of NODE_OPTIONS in packaged apps
+        LOG(ERROR) << "Most NODE_OPTIONs are not supported in packaged apps."
+                   << " See documentation for more details.";
+        options.erase(options.find(option), part.length());
+      } else if (disallowed.find(option) != disallowed.end()) {
+        // Remove NODE_OPTIONS specifically disallowed for use in Node.js
+        // through Electron owing to constraints like BoringSSL.
+        LOG(ERROR) << "The NODE_OPTION " << option
+                   << " is not supported in Electron";
+        options.erase(options.find(option), part.length());
+      }
+    }
+
+    // overwrite new NODE_OPTIONS without unsupported variables
+    env->SetVar("NODE_OPTIONS", options);
+  }
+}
+
 }  // namespace
 
 namespace electron {
@@ -222,55 +278,7 @@ void NodeBindings::Initialize() {
   const char** exec_argv = nullptr;
 
   std::unique_ptr<base::Environment> env(base::Environment::Create());
-  if (env->HasVar("NODE_OPTIONS")) {
+  SetNodeOptions(env.get());
-    base::FilePath exe_path;
-    base::PathService::Get(base::FILE_EXE, &exe_path);
-#if defined(OS_WIN)
-    std::string path = base::UTF16ToUTF8(exe_path.value());
-#else
-    std::string path = exe_path.value();
-#endif
-    std::transform(path.begin(), path.end(), path.begin(), ::tolower);
-
-#if defined(OS_WIN)
-    const bool is_packaged_app = path == "electron.exe";
-#else
-    const bool is_packaged_app = path == "electron";
-#endif
-
-    // explicitly disallow NODE_OPTIONS in packaged apps
-    if (is_packaged_app) {
-      LOG(WARNING) << "NODE_OPTIONs are not supported in packaged apps";
-      env->SetVar("NODE_OPTIONS", "");
-    } else {
-      const std::vector<std::string> disallowed = {
-          "--openssl-config", "--use-bundled-ca", "--use-openssl-ca",
-          "--force-fips", "--enable-fips"};
-
-      std::string options;
-      env->GetVar("NODE_OPTIONS", &options);
-      std::vector<std::string> parts = base::SplitString(
-          options, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-
-      // parse passed options for unsupported options
-      // and remove them from the options list
-      std::string new_options = options;
-      for (const auto& disallow : disallowed) {
-        for (const auto& part : parts) {
-          if (part.find(disallow) != std::string::npos) {
-            LOG(WARNING) << "The NODE_OPTION" << disallow
-                         << "is not supported in Electron";
-            new_options.erase(new_options.find(part), part.length());
-            break;
-          }
-        }
-      }
-
-      // overwrite new NODE_OPTIONS without unsupported variables
-      if (new_options != options)
-        env->SetVar("NODE_OPTIONS", new_options);
-    }
-  }
 
   // TODO(codebytere): this is going to be deprecated in the near future
   // in favor of Init(std::vector<std::string>* argv,

spec-main/node-spec.ts

@@ -53,6 +53,63 @@ describe('node feature', () => {
     })
   })
 
+  describe('NODE_OPTIONS', () => {
+    let child: childProcess.ChildProcessWithoutNullStreams
+    let exitPromise: Promise<any[]>
+
+    afterEach(async () => {
+      if (child && exitPromise) {
+        const [code, signal] = await exitPromise
+        expect(signal).to.equal(null)
+        expect(code).to.equal(0)
+      } else if (child) {
+        child.kill()
+      }
+    })
+
+    it('fails for options disallowed by Node.js itself', (done) => {
+      const env = Object.assign({}, process.env, { NODE_OPTIONS: '--v8-options' });
+      child = childProcess.spawn(process.execPath, { env })
+
+      function cleanup () {
+        child.stderr.removeListener('data', listener)
+        child.stdout.removeListener('data', listener)
+      }
+
+      let output = ''
+      function listener (data: Buffer) {
+        output += data
+        if (/electron: --v8-options is not allowed in NODE_OPTIONS/m.test(output)) {
+          cleanup()
+          done()
+        }
+      }
+      child.stderr.on('data', listener)
+      child.stdout.on('data', listener)
+    })
+
+    it('disallows crypto-related options', (done) => {
+      const env = Object.assign({}, process.env, { NODE_OPTIONS: '--use-openssl-ca' });
+      child = childProcess.spawn(process.execPath, ['--enable-logging'], { env })
+
+      function cleanup () {
+        child.stderr.removeListener('data', listener)
+        child.stdout.removeListener('data', listener)
+      }
+
+      let output = ''
+      function listener (data: Buffer) {
+        output += data
+        if (/The NODE_OPTION --use-openssl-ca is not supported in Electron/m.test(output)) {
+          cleanup()
+          done()
+        }
+      }
+      child.stderr.on('data', listener)
+      child.stdout.on('data', listener)
+    })
+  })
+
   ifdescribe(features.isRunAsNodeEnabled())('inspector', () => {
     let child: childProcess.ChildProcessWithoutNullStreams
     let exitPromise: Promise<any[]>