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. PR-URL: #44324 Fixes: #44287 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Sep 5, 2022 · 0f4e98b · 0f4e98b
1 parent 3c53548
commit 0f4e98b
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 3 deletions.
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -549,6 +549,19 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
   Isolate* isolate = context->GetIsolate();
   HandleScope handle_scope(isolate);
 
+  // When `IsCodeGenerationFromStringsAllowed` is true, V8 takes the fast path
+  // and ignores the ModifyCodeGenerationFromStrings callback. Set it to false
+  // to delegate the code generation validation to
+  // node::ModifyCodeGenerationFromStrings.
+  // The `IsCodeGenerationFromStringsAllowed` can be refreshed by V8 according
+  // to the runtime flags, propagate the value to the embedder data.
+  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));
+
   if (per_process::cli_options->disable_proto == "") {
     return Just(true);
   }
@@ -641,11 +654,11 @@ Maybe<bool> InitializeMainContextForSnapshot(Local<Context> context) {
   Isolate* isolate = context->GetIsolate();
   HandleScope handle_scope(isolate);
 
-  context->AllowCodeGenerationFromStrings(false);
-  context->SetEmbedderData(
-      ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
+  // Initialize the default values.
   context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
                            True(isolate));
+  context->SetEmbedderData(
+      ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
 
   if (InitializeBaseContextForSnapshot(context).IsNothing()) {
     return Nothing<bool>();

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -855,6 +855,16 @@ const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() {
 )";
 }
 
+// Reset context settings that need to be initialized again after
+// deserialization.
+static void ResetContextSettingsBeforeSnapshot(Local<Context> 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<intptr_t>& SnapshotBuilder::CollectExternalReferences() {
@@ -944,6 +954,7 @@ int SnapshotBuilder::Generate(SnapshotData* out,
     if (base_context.IsEmpty()) {
       return BOOTSTRAP_ERROR;
     }
+    ResetContextSettingsBeforeSnapshot(base_context);
 
     Local<Context> main_context = NewContext(isolate);
     if (main_context.IsEmpty()) {
@@ -1012,6 +1023,8 @@ int SnapshotBuilder::Generate(SnapshotData* out,
                            size_str.c_str());
       }
 #endif
+
+      ResetContextSettingsBeforeSnapshot(main_context);
     }
 
     // Global handles to the contexts can't be disposed before the

diff --git a/test/parallel/test-eval-disallow-code-generation-from-strings.js 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
@@ -0,0 +1,7 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+// Verify that eval is allowed by default.
+assert.strictEqual(eval('"eval"'), 'eval');