Skip to content

Commit e118eda

Browse files
committed
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.
1 parent 798a6ed commit e118eda

4 files changed

+45
-3
lines changed

src/api/environment.cc

+16-3
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,19 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
556556
Isolate* isolate = context->GetIsolate();
557557
HandleScope handle_scope(isolate);
558558

559+
// When `IsCodeGenerationFromStringsAllowed` is true, V8 takes the fast path
560+
// and ignores the ModifyCodeGenerationFromStrings callback. Set it to false
561+
// to delegate the code generation validation to
562+
// node::ModifyCodeGenerationFromStrings.
563+
// The `IsCodeGenerationFromStringsAllowed` can be refreshed by V8 according
564+
// to the runtime flags, propagate the value to the embedder data.
565+
bool is_code_generation_from_strings_allowed =
566+
context->IsCodeGenerationFromStringsAllowed();
567+
context->AllowCodeGenerationFromStrings(false);
568+
context->SetEmbedderData(
569+
ContextEmbedderIndex::kAllowCodeGenerationFromStrings,
570+
is_code_generation_from_strings_allowed ? True(isolate) : False(isolate));
571+
559572
// Delete `Intl.v8BreakIterator`
560573
// https://github.com/nodejs/node/issues/14909
561574
{
@@ -665,11 +678,11 @@ Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
665678
Isolate* isolate = context->GetIsolate();
666679
HandleScope handle_scope(isolate);
667680

668-
context->AllowCodeGenerationFromStrings(false);
669-
context->SetEmbedderData(
670-
ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
681+
// Initialize the default values.
671682
context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
672683
True(isolate));
684+
context->SetEmbedderData(
685+
ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
673686

674687
return InitializePrimordials(context);
675688
}

src/node_snapshotable.cc

+13
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,16 @@ const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() {
956956
)";
957957
}
958958

959+
// Reset context settings that need to be initialized again after
960+
// deserialization.
961+
static void ResetContextSettingsBeforeSnapshot(Local<Context> context) {
962+
// Reset the AllowCodeGenerationFromStrings flag to true (default value) so
963+
// that it can be re-initialized with v8 flag
964+
// --disallow-code-generation-from-strings and recognized in
965+
// node::InitializeContextRuntime.
966+
context->AllowCodeGenerationFromStrings(true);
967+
}
968+
959969
Mutex SnapshotBuilder::snapshot_data_mutex_;
960970

961971
const std::vector<intptr_t>& SnapshotBuilder::CollectExternalReferences() {
@@ -1108,6 +1118,9 @@ int SnapshotBuilder::Generate(SnapshotData* out,
11081118
#endif
11091119
}
11101120

1121+
ResetContextSettingsBeforeSnapshot(base_context);
1122+
ResetContextSettingsBeforeSnapshot(main_context);
1123+
11111124
// Global handles to the contexts can't be disposed before the
11121125
// blob is created. So initialize all the contexts before adding them.
11131126
// TODO(joyeecheung): figure out how to remove this restriction.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Flags: --disallow-code-generation-from-strings
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
7+
// Verify that v8 option --disallow-code-generation-from-strings is still
8+
// respected
9+
assert.throws(() => eval('"eval"'), EvalError);

test/parallel/test-eval.js

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
require('../common');
4+
const assert = require('assert');
5+
6+
// Verify that eval is allowed by default.
7+
assert.strictEqual(eval('"eval"'), 'eval');

0 commit comments

Comments
 (0)