From 106dc8164fdb63b2fba27099eb4ff67fd48a686d Mon Sep 17 00:00:00 2001 From: legendecas Date: Fri, 19 Aug 2022 21:42:39 +0800 Subject: [PATCH] src: restore context default IsCodeGenerationFromStringsAllowed value Context's default IsCodeGenerationFromStringsAllowed value can be changed by v8 flag `--disallow-code-generation-from-strings`. Restore the value at runtime when delegating the code generation validation to `node::ModifyCodeGenerationFromStrings`. The context's settings are serialized in the snapshot. Reset the setting values to its default values before the serialization so that it can be correctly re-initialized after deserialization at runtime. --- src/api/environment.cc | 12 +++++++++--- src/node_snapshotable.cc | 13 +++++++++++++ ...st-eval-disallow-code-generation-from-strings.js | 9 +++++++++ test/parallel/test-eval.js | 7 +++++++ 4 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-eval-disallow-code-generation-from-strings.js create mode 100644 test/parallel/test-eval.js diff --git a/src/api/environment.cc b/src/api/environment.cc index a59f70e8139890..b1669c65a691d0 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -556,6 +556,15 @@ Maybe InitializeContextRuntime(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); + // Delegate the code generation validation to + // node::ModifyCodeGenerationFromStrings. + bool is_code_generation_from_strings_allowed = + context->IsCodeGenerationFromStringsAllowed(); + context->AllowCodeGenerationFromStrings(false); + context->SetEmbedderData( + ContextEmbedderIndex::kAllowCodeGenerationFromStrings, + is_code_generation_from_strings_allowed ? True(isolate) : False(isolate)); + // Delete `Intl.v8BreakIterator` // https://github.com/nodejs/node/issues/14909 { @@ -665,9 +674,6 @@ Maybe InitializeContextForSnapshot(Local context) { Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); - context->AllowCodeGenerationFromStrings(false); - context->SetEmbedderData( - ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate)); context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration, True(isolate)); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 4399b7a3cc132d..dcfa47e7bcaa6b 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -956,6 +956,16 @@ const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() { )"; } +// Reset context settings that need to be initialized again after +// deserialization. +static void ResetContextSettingsBeforeSnapshot(Local context) { + // Reset the AllowCodeGenerationFromStrings flag to true (default value) so + // that it can be re-initialized with v8 flag + // --disallow-code-generation-from-strings and recognized in + // node::InitializeContextRuntime. + context->AllowCodeGenerationFromStrings(true); +} + Mutex SnapshotBuilder::snapshot_data_mutex_; const std::vector& SnapshotBuilder::CollectExternalReferences() { @@ -1108,6 +1118,9 @@ int SnapshotBuilder::Generate(SnapshotData* out, #endif } + ResetContextSettingsBeforeSnapshot(base_context); + ResetContextSettingsBeforeSnapshot(main_context); + // Global handles to the contexts can't be disposed before the // blob is created. So initialize all the contexts before adding them. // TODO(joyeecheung): figure out how to remove this restriction. diff --git a/test/parallel/test-eval-disallow-code-generation-from-strings.js b/test/parallel/test-eval-disallow-code-generation-from-strings.js new file mode 100644 index 00000000000000..d701dbea299655 --- /dev/null +++ b/test/parallel/test-eval-disallow-code-generation-from-strings.js @@ -0,0 +1,9 @@ +// Flags: --disallow-code-generation-from-strings +'use strict'; + +require('../common'); +const assert = require('assert'); + +// Verify that v8 option --disallow-code-generation-from-strings is still +// respected +assert.throws(() => eval(`'eval'`), EvalError); diff --git a/test/parallel/test-eval.js b/test/parallel/test-eval.js new file mode 100644 index 00000000000000..66eab0fa670199 --- /dev/null +++ b/test/parallel/test-eval.js @@ -0,0 +1,7 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// Verify that eval is allowed by default. +assert.strictEqual(eval(`'eval'`), 'eval');