From a4fcc32799d0496b025df344bdfdcd64223dbf7e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 26 Apr 2019 18:55:12 +0900 Subject: [PATCH] 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 --- DEPS | 2 +- atom/browser/node_debugger.cc | 13 +- atom/common/api/atom_api_asar.cc | 9 +- atom/common/node_includes.h | 21 ++ atom/renderer/api/atom_api_web_frame.cc | 3 + .../atom_sandboxed_renderer_client.cc | 4 +- build/args/all.gn | 3 + lib/common/asar.js | 2 +- lib/common/asar_init.js | 10 +- lib/content_script/init.js | 2 +- lib/isolated_renderer/init.js | 2 +- lib/renderer/init.ts | 18 ++ patches/common/boringssl/.patches | 1 + ...et_min_max_proto_version_for_context.patch | 187 ++++++++++++++++++ spec/asar-spec.js | 4 + 15 files changed, 249 insertions(+), 32 deletions(-) create mode 100644 patches/common/boringssl/support_get_versions_with_get_min_max_proto_version_for_context.patch diff --git a/DEPS b/DEPS index 4816feed99b4..d2f0d027313d 100644 --- a/DEPS +++ b/DEPS @@ -12,7 +12,7 @@ vars = { 'chromium_version': '1e9f9a24aa12bea9cf194a82a7e249bd1242ec4f', 'node_version': - '2dc0f8811b2b295c08d797b8a11b030234c98502', + '696d8fb66d6f65fc82869d390e0d2078970b1eb4', 'boto_version': 'f7574aa6cc2c819430c1f05e9a1a1a666ef8169b', 'pyyaml_version': '3.12', diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index fba5228bf5a6..1a47e81fdd96 100644 --- 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 exec_args; std::vector v8_args; std::vector 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(options.host_port), diff --git a/atom/common/api/atom_api_asar.cc b/atom/common/api/atom_api_asar.cc index be839f3675c8..d17a55dd67af 100644 --- 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 { DISALLOW_COPY_AND_ASSIGN(Archive); }; -void InitAsarSupport(v8::Isolate* isolate, - v8::Local source, - v8::Local require) { +void InitAsarSupport(v8::Isolate* isolate, v8::Local require) { // Evaluate asar_init.js. std::vector> asar_init_params = { - node::FIXED_ONE_BYTE_STRING(isolate, "source"), node::FIXED_ONE_BYTE_STRING(isolate, "require")}; - - std::vector> asar_init_args = {source, require}; - + std::vector> asar_init_args = {require}; node::per_process::native_module_loader.CompileAndCall( isolate->GetCurrentContext(), "electron/js2c/asar_init", &asar_init_params, &asar_init_args, nullptr); diff --git a/atom/common/node_includes.h b/atom/common/node_includes.h index f809bcfbfc9b..da60a41da278 100644 --- 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") diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc index 01bee988701a..677d40a45d80 100644 --- a/atom/renderer/api/atom_api_web_frame.cc +++ b/atom/renderer/api/atom_api_web_frame.cc @@ -372,6 +372,9 @@ v8::Local 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))); diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc index f5cc7527f946..80396f45e77c 100644 --- 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> 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> isolated_bundle_params = { node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"), diff --git a/build/args/all.gn b/build/args/all.gn index 35ab1881b309..6138f0f54dcd 100644 --- 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" diff --git a/lib/common/asar.js b/lib/common/asar.js index 08e20426e564..5aaf4a820838 100644 --- 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') diff --git a/lib/common/asar_init.js b/lib/common/asar_init.js index e525319455b1..0d6d145b1104 100644 --- 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')) diff --git a/lib/content_script/init.js b/lib/content_script/init.js index f10942afc951..4753657797ed 100644 --- 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') diff --git a/lib/isolated_renderer/init.js b/lib/isolated_renderer/init.js index 4df0730ddb99..b73e06a33dcf 100644 --- 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') diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index 32e28bca8864..e06931fe2765 100644 --- 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) diff --git a/patches/common/boringssl/.patches b/patches/common/boringssl/.patches index 07c69fae2e1b..18089e202d94 100644 --- 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 diff --git 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 new file mode 100644 index 000000000000..f229596c4481 --- /dev/null +++ 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 +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 +Reviewed-by: David Benjamin +Commit-Queue: David Benjamin + +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 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; diff --git a/spec/asar-spec.js b/spec/asar-spec.js index db323dc704ec..7d0ebfc7e1ee 100644 --- 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 () {