feat: upgrade to Node 12 (#17838)

* fix: add boringssl backport to support node upgrade * fix: Update node_includes.h, add DCHECK macros * fix: Update node Debug Options parser usage * fix: Fix asar setup * fix: using v8Util in isolated context * fix: make "process" available in preload scripts * fix: use proper options parser and remove setting of _breakFirstLine _breakFirstLine was being set on the process, but that has changed in node 12 and so is no longer needed. Node will handle it properly when --inspect-brk is provided * chore: update node dep sha * fix: process.binding => _linkedBinding in sandboxed isolated preload * fix: make original-fs work with streams * build: override node module version * fix: use _linkedBinding in content_script/init.js * chore: update node ref in DEPS * build: node_module_version should be 73
2019-04-26 18:55:12 +09:00 · 2019-04-26 18:55:12 +09:00 · a4fcc32799
commit a4fcc32799
parent e1acfffaf8
15 changed files with 249 additions and 32 deletions
--- a/2
+++ b/2
@ -12,7 +12,7 @@ vars = {
  'chromium_version':
    '1e9f9a24aa12bea9cf194a82a7e249bd1242ec4f',
  'node_version':
-    '2dc0f8811b2b295c08d797b8a11b030234c98502',
+    '696d8fb66d6f65fc82869d390e0d2078970b1eb4',

  'boto_version': 'f7574aa6cc2c819430c1f05e9a1a1a666ef8169b',
  'pyyaml_version': '3.12',
--- a/atom/browser/node_debugger.cc
+++ b/atom/browser/node_debugger.cc
@ -37,13 +37,13 @@ void NodeDebugger::Start() {
  }

  node::DebugOptions options;
+  node::options_parser::DebugOptionsParser options_parser;
  std::vector<std::string> exec_args;
  std::vector<std::string> v8_args;
  std::vector<std::string> errors;

-  node::options_parser::DebugOptionsParser::instance.Parse(
-      &args, &exec_args, &v8_args, &options,
-      node::options_parser::kDisallowedInEnvironment, &errors);
+  options_parser.Parse(&args, &exec_args, &v8_args, &options,
+                       node::options_parser::kDisallowedInEnvironment, &errors);

  if (!errors.empty()) {
    // TODO(jeremy): what's the appropriate behaviour here?
@ -51,13 +51,6 @@ void NodeDebugger::Start() {
               << base::JoinString(errors, " ");
  }

-  // Set process._debugWaitConnect if --inspect-brk was specified to stop
-  // the debugger on the first line
-  if (options.wait_for_connect()) {
-    mate::Dictionary process(env_->isolate(), env_->process_object());
-    process.Set("_breakFirstLine", true);
-  }
-
  const char* path = "";
  if (inspector->Start(path, options,
                       std::make_shared<node::HostPort>(options.host_port),
--- a/atom/common/api/atom_api_asar.cc
+++ b/atom/common/api/atom_api_asar.cc
@ -117,16 +117,11 @@ class Archive : public mate::Wrappable<Archive> {
  DISALLOW_COPY_AND_ASSIGN(Archive);
 };

-void InitAsarSupport(v8::Isolate* isolate,
-                     v8::Local<v8::Value> source,
-                     v8::Local<v8::Value> require) {
+void InitAsarSupport(v8::Isolate* isolate, v8::Local<v8::Value> require) {
  // Evaluate asar_init.js.
  std::vector<v8::Local<v8::String>> asar_init_params = {
-      node::FIXED_ONE_BYTE_STRING(isolate, "source"),
      node::FIXED_ONE_BYTE_STRING(isolate, "require")};
-
-  std::vector<v8::Local<v8::Value>> asar_init_args = {source, require};
-
+  std::vector<v8::Local<v8::Value>> asar_init_args = {require};
  node::per_process::native_module_loader.CompileAndCall(
      isolate->GetCurrentContext(), "electron/js2c/asar_init",
      &asar_init_params, &asar_init_args, nullptr);
--- a/atom/common/node_includes.h
+++ b/atom/common/node_includes.h
@ -25,6 +25,13 @@
 #pragma push_macro("CHECK_LE")
 #pragma push_macro("CHECK_LT")
 #pragma push_macro("CHECK_NE")
+#pragma push_macro("DCHECK")
+#pragma push_macro("DCHECK_EQ")
+#pragma push_macro("DCHECK_GE")
+#pragma push_macro("DCHECK_GT")
+#pragma push_macro("DCHECK_LE")
+#pragma push_macro("DCHECK_LT")
+#pragma push_macro("DCHECK_NE")
 #pragma push_macro("DISALLOW_COPY_AND_ASSIGN")
 #pragma push_macro("LIKELY")
 #pragma push_macro("NO_RETURN")
@ -38,6 +45,13 @@
 #undef CHECK_LE
 #undef CHECK_LT
 #undef CHECK_NE
+#undef DCHECK
+#undef DCHECK_EQ
+#undef DCHECK_GE
+#undef DCHECK_GT
+#undef DCHECK_LE
+#undef DCHECK_LT
+#undef DCHECK_NE
 #undef DISALLOW_COPY_AND_ASSIGN
 #undef LIKELY
 #undef NO_RETURN
@ -67,6 +81,13 @@
 #pragma pop_macro("CHECK_LE")
 #pragma pop_macro("CHECK_LT")
 #pragma pop_macro("CHECK_NE")
+#pragma pop_macro("DCHECK")
+#pragma pop_macro("DCHECK_EQ")
+#pragma pop_macro("DCHECK_GE")
+#pragma pop_macro("DCHECK_GT")
+#pragma pop_macro("DCHECK_LE")
+#pragma pop_macro("DCHECK_LT")
+#pragma pop_macro("DCHECK_NE")
 #pragma pop_macro("DISALLOW_COPY_AND_ASSIGN")
 #pragma pop_macro("LIKELY")
 #pragma pop_macro("NO_RETURN")
--- a/atom/renderer/api/atom_api_web_frame.cc
+++ b/atom/renderer/api/atom_api_web_frame.cc
@ -372,6 +372,9 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
      blink::WebLocalFrame::kSynchronous;
  args->GetNext(&scriptExecutionType);

+  // Debugging tip: if you see a crash stack trace beginning from this call,
+  // then it is very likely that some exception happened when executing the
+  // "content_script/init.js" script.
  GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
      world_id, &sources.front(), sources.size(), has_user_gesture,
      scriptExecutionType, new ScriptExecutionCallback(std::move(promise)));
--- a/atom/renderer/atom_sandboxed_renderer_client.cc
+++ b/atom/renderer/atom_sandboxed_renderer_client.cc
@ -245,7 +245,7 @@ void AtomSandboxedRendererClient::SetupMainWorldOverrides(
  auto* isolate = context->GetIsolate();

  mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
-  process.SetMethod("binding", GetBinding);
+  process.SetMethod("_linkedBinding", GetBinding);

  std::vector<v8::Local<v8::String>> isolated_bundle_params = {
      node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
@ -267,7 +267,7 @@ void AtomSandboxedRendererClient::SetupExtensionWorldOverrides(
  auto* isolate = context->GetIsolate();

  mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
-  process.SetMethod("binding", GetBinding);
+  process.SetMethod("_linkedBinding", GetBinding);

  std::vector<v8::Local<v8::String>> isolated_bundle_params = {
      node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
--- a/build/args/all.gn
+++ b/build/args/all.gn
@ -2,6 +2,9 @@ is_electron_build = true
 use_jumbo_build = true
 root_extra_deps = [ "//electron" ]

+# Registry of NMVs --> https://github.com/nodejs/node/blob/master/doc/abi_version_registry.json
+node_module_version = 73
+
 v8_promise_internal_field_count = 1
 v8_typed_array_max_size_in_heap = 0
 v8_embedder_string = "-electron.0"
--- a/lib/common/asar.js
+++ b/lib/common/asar.js
@ -1,7 +1,7 @@
 'use strict';

 (function () {
-  const asar = process.binding('atom_common_asar')
+  const asar = process._linkedBinding('atom_common_asar')
  const assert = require('assert')
  const { Buffer } = require('buffer')
  const childProcess = require('child_process')
--- a/lib/common/asar_init.js
+++ b/lib/common/asar_init.js
@ -1,14 +1,6 @@
 'use strict'

-/* global source, require */
-
-// Expose fs module without asar support.
-// NB: Node's 'fs' and 'internal/fs/streams' have a lazy-loaded circular
-// dependency. So to expose the unmodified Node 'fs' functionality here,
-// we have to copy both 'fs' *and* 'internal/fs/streams' and modify the
-// copies to depend on each other instead of on our asarified 'fs' code.
-source['original-fs'].replace("require('internal/fs/streams')", "require('original-fs/streams')")
-source['original-fs/streams'].replace("require('fs')", "require('original-fs')")
+/* global require */

 // Monkey-patch the fs module.
 require('electron/js2c/asar').wrapFsWithAsar(require('fs'))
--- a/lib/content_script/init.js
+++ b/lib/content_script/init.js
@ -4,7 +4,7 @@

 const { EventEmitter } = require('events')

-process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess.binding, 'renderer')
+process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess._linkedBinding, 'renderer')

 const v8Util = process.electronBinding('v8_util')

--- a/lib/isolated_renderer/init.js
+++ b/lib/isolated_renderer/init.js
@ -2,7 +2,7 @@

 /* global nodeProcess, isolatedWorld */

-process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess.binding, 'renderer')
+process.electronBinding = require('@electron/internal/common/atom-binding-setup').electronBindingSetup(nodeProcess._linkedBinding, 'renderer')

 const v8Util = process.electronBinding('v8_util')

--- a/lib/renderer/init.ts
+++ b/lib/renderer/init.ts
@ -4,6 +4,24 @@ import * as path from 'path'

 const Module = require('module')

+// Make sure globals like "process" and "global" are always available in preload
+// scripts even after they are deleted in "loaded" script.
+//
+// Note 1: We rely on a Node patch to actually pass "process" and "global" and
+// other arguments to the wrapper.
+//
+// Note 2: Node introduced a new code path to use native code to wrap module
+// code, which does not work with this hack. However by modifying the
+// "Module.wrapper" we can force Node to use the old code path to wrap module
+// code with JavaScript.
+Module.wrapper = [
+  '(function (exports, require, module, __filename, __dirname, process, global, Buffer) { ' +
+  // By running the code in a new closure, it would be possible for the module
+  // code to override "process" and "Buffer" with local variables.
+  'return function (exports, require, module, __filename, __dirname) { ',
+  '\n}.call(this, exports, require, module, __filename, __dirname); });'
+]
+
 // We modified the original process.argv to let node.js load the
 // init.js, we need to restore it here.
 process.argv.splice(1, 1)
--- a/patches/common/boringssl/.patches
+++ b/patches/common/boringssl/.patches
@ -1,2 +1,3 @@
 expose_ripemd160.patch
 expose_aes-cfb.patch
+support_get_versions_with_get_min_max_proto_version_for_context.patch
--- a/patches/common/boringssl/support_get_versions_with_get_min_max_proto_version_for_context.patch
+++ b/patches/common/boringssl/support_get_versions_with_get_min_max_proto_version_for_context.patch
@ -0,0 +1,187 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Nitish Sakhawalkar <nitsakh@icloud.com>
+Date: Mon, 25 Mar 2019 17:26:59 -0700
+Subject: Support get versions with get_{min,max}_proto_version for context
+
+When building node with boringssl, `SSL_CTX_get_min_proto_version` and
+`SSL_CTX_get_max_proto_version` are used. Openssl exposes those; this
+change adds support for boringssl.
+
+For this to work right in DTLS, we switch conf_{min,max}_version to store wire
+versions, rather than our internal normalized versions.
+
+Change-Id: I282ed224806c41f69e6f166ca97c6cc05ff51f17
+Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/35404
+Reviewed-by: Nitish Sakhawalkar <nitsakh@gmail.com>
+Reviewed-by: David Benjamin <davidben@google.com>
+Commit-Queue: David Benjamin <davidben@google.com>
+
+diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
+index a539b20d3..629f0063b 100644
+--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
+@@ -631,6 +631,12 @@ OPENSSL_EXPORT int SSL_CTX_set_min_proto_version(SSL_CTX *ctx,
+ OPENSSL_EXPORT int SSL_CTX_set_max_proto_version(SSL_CTX *ctx,
+                                                  uint16_t version);
+ 
+// SSL_CTX_get_min_proto_version returns the minimum protocol version for |ctx|
+OPENSSL_EXPORT uint16_t SSL_CTX_get_min_proto_version(SSL_CTX *ctx);
+
+// SSL_CTX_get_max_proto_version returns the maximum protocol version for |ctx|
+OPENSSL_EXPORT uint16_t SSL_CTX_get_max_proto_version(SSL_CTX *ctx);
+
+ // SSL_set_min_proto_version sets the minimum protocol version for |ssl| to
+ // |version|. If |version| is zero, the default minimum version is used. It
+ // returns one on success and zero if |version| is invalid.
+diff --git a/ssl/internal.h b/ssl/internal.h
+index 0df9a5fba..a32122e49 100644
+--- a/ssl/internal.h
+++ b/ssl/internal.h
+@@ -2464,14 +2464,14 @@ struct SSL_CONFIG {
+   // ssl is a non-owning pointer to the parent |SSL| object.
+   SSL *const ssl = nullptr;
+ 
+-  // conf_max_version is the maximum acceptable protocol version configured by
+-  // |SSL_set_max_proto_version|. Note this version is normalized in DTLS and is
+-  // further constrainted by |SSL_OP_NO_*|.
+  // conf_max_version is the maximum acceptable version configured by
+  // |SSL_set_max_proto_version|. Note this version is not normalized in DTLS
+  // and is further constrained by |SSL_OP_NO_*|.
+   uint16_t conf_max_version = 0;
+ 
+-  // conf_min_version is the minimum acceptable protocol version configured by
+-  // |SSL_set_min_proto_version|. Note this version is normalized in DTLS and is
+-  // further constrainted by |SSL_OP_NO_*|.
+  // conf_min_version is the minimum acceptable version configured by
+  // |SSL_set_min_proto_version|. Note this version is not normalized in DTLS
+  // and is further constrained by |SSL_OP_NO_*|.
+   uint16_t conf_min_version = 0;
+ 
+   X509_VERIFY_PARAM *param = nullptr;
+diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
+index f3f792347..af5d7952b 100644
+--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
+@@ -840,8 +840,8 @@ static void ExpectDefaultVersion(uint16_t min_version, uint16_t max_version,
+                                  const SSL_METHOD *(*method)(void)) {
+   bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(method()));
+   ASSERT_TRUE(ctx);
+-  EXPECT_EQ(min_version, ctx->conf_min_version);
+-  EXPECT_EQ(max_version, ctx->conf_max_version);
+  EXPECT_EQ(min_version, SSL_CTX_get_min_proto_version(ctx.get()));
+  EXPECT_EQ(max_version, SSL_CTX_get_max_proto_version(ctx.get()));
+ }
+ 
+ TEST(SSLTest, DefaultVersion) {
+@@ -850,9 +850,9 @@ TEST(SSLTest, DefaultVersion) {
+   ExpectDefaultVersion(TLS1_VERSION, TLS1_VERSION, &TLSv1_method);
+   ExpectDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &TLSv1_1_method);
+   ExpectDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &TLSv1_2_method);
+-  ExpectDefaultVersion(TLS1_1_VERSION, TLS1_2_VERSION, &DTLS_method);
+-  ExpectDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &DTLSv1_method);
+-  ExpectDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &DTLSv1_2_method);
+  ExpectDefaultVersion(DTLS1_VERSION, DTLS1_2_VERSION, &DTLS_method);
+  ExpectDefaultVersion(DTLS1_VERSION, DTLS1_VERSION, &DTLSv1_method);
+  ExpectDefaultVersion(DTLS1_2_VERSION, DTLS1_2_VERSION, &DTLSv1_2_method);
+ }
+ 
+ TEST(SSLTest, CipherProperties) {
+@@ -2617,13 +2617,13 @@ TEST(SSLTest, SetVersion) {
+ 
+   // Zero is the default version.
+   EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), 0));
+-  EXPECT_EQ(TLS1_2_VERSION, ctx->conf_max_version);
+  EXPECT_EQ(TLS1_2_VERSION, SSL_CTX_get_max_proto_version(ctx.get()));
+   EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), 0));
+-  EXPECT_EQ(TLS1_VERSION, ctx->conf_min_version);
+  EXPECT_EQ(TLS1_VERSION, SSL_CTX_get_min_proto_version(ctx.get()));
+ 
+   // TLS 1.3 is available, but not by default.
+   EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_VERSION));
+-  EXPECT_EQ(TLS1_3_VERSION, ctx->conf_max_version);
+  EXPECT_EQ(TLS1_3_VERSION, SSL_CTX_get_max_proto_version(ctx.get()));
+ 
+   // SSL 3.0 is not available.
+   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), SSL3_VERSION));
+@@ -2646,9 +2646,9 @@ TEST(SSLTest, SetVersion) {
+   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), 0x1234));
+ 
+   EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), 0));
+-  EXPECT_EQ(TLS1_2_VERSION, ctx->conf_max_version);
+  EXPECT_EQ(DTLS1_2_VERSION, SSL_CTX_get_max_proto_version(ctx.get()));
+   EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), 0));
+-  EXPECT_EQ(TLS1_1_VERSION, ctx->conf_min_version);
+  EXPECT_EQ(DTLS1_VERSION, SSL_CTX_get_min_proto_version(ctx.get()));
+ }
+ 
+ static const char *GetVersionName(uint16_t version) {
+diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc
+index e6dbc8de6..e25fce6d1 100644
+--- a/ssl/ssl_versions.cc
+++ b/ssl/ssl_versions.cc
+@@ -134,12 +134,12 @@ static bool api_version_to_wire(uint16_t *out, uint16_t version) {
+ static bool set_version_bound(const SSL_PROTOCOL_METHOD *method, uint16_t *out,
+                               uint16_t version) {
+   if (!api_version_to_wire(&version, version) ||
+-      !ssl_method_supports_version(method, version) ||
+-      !ssl_protocol_version_from_wire(out, version)) {
+      !ssl_method_supports_version(method, version)) {
+     OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_SSL_VERSION);
+     return false;
+   }
+ 
+  *out = version;
+   return true;
+ }
+ 
+@@ -147,8 +147,7 @@ static bool set_min_version(const SSL_PROTOCOL_METHOD *method, uint16_t *out,
+                             uint16_t version) {
+   // Zero is interpreted as the default minimum version.
+   if (version == 0) {
+-    // TLS 1.0 does not exist in DTLS.
+-    *out = method->is_dtls ? TLS1_1_VERSION : TLS1_VERSION;
+    *out = method->is_dtls ? DTLS1_VERSION : TLS1_VERSION;
+     return true;
+   }
+ 
+@@ -159,7 +158,7 @@ static bool set_max_version(const SSL_PROTOCOL_METHOD *method, uint16_t *out,
+                             uint16_t version) {
+   // Zero is interpreted as the default maximum version.
+   if (version == 0) {
+-    *out = TLS1_2_VERSION;
+    *out = method->is_dtls ? DTLS1_2_VERSION : TLS1_2_VERSION;
+     return true;
+   }
+ 
+@@ -188,8 +187,14 @@ bool ssl_get_version_range(const SSL_HANDSHAKE *hs, uint16_t *out_min_version,
+     }
+   }
+ 
+-  uint16_t min_version = hs->config->conf_min_version;
+-  uint16_t max_version = hs->config->conf_max_version;
+  uint16_t min_version, max_version;
+  if (!ssl_protocol_version_from_wire(&min_version,
+                                      hs->config->conf_min_version) ||
+      !ssl_protocol_version_from_wire(&max_version,
+                                      hs->config->conf_max_version)) {
+    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+    return false;
+  }
+ 
+   // QUIC requires TLS 1.3.
+   if (hs->ssl->quic_method && min_version < TLS1_3_VERSION) {
+@@ -344,6 +349,14 @@ int SSL_CTX_set_max_proto_version(SSL_CTX *ctx, uint16_t version) {
+   return set_max_version(ctx->method, &ctx->conf_max_version, version);
+ }
+ 
+uint16_t SSL_CTX_get_min_proto_version(SSL_CTX *ctx) {
+  return ctx->conf_min_version;
+}
+
+uint16_t SSL_CTX_get_max_proto_version(SSL_CTX *ctx) {
+  return ctx->conf_max_version;
+}
+
+ int SSL_set_min_proto_version(SSL *ssl, uint16_t version) {
+   if (!ssl->config) {
+     return 0;
--- a/spec/asar-spec.js
+++ b/spec/asar-spec.js
@ -1231,6 +1231,10 @@ describe('asar package', function () {
      })
      child.send('message')
    })
+
+    it('can be used with streams', () => {
+      originalFs.createReadStream(path.join(fixtures, 'asar', 'a.asar'))
+    })
  })

  describe('graceful-fs module', function () {