Skip to content

Commit

Permalink
src: restore context default IsCodeGenerationFromStringsAllowed value
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
legendecas committed Aug 21, 2022
1 parent 798a6ed commit 106dc81
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,15 @@ Maybe<bool> InitializeContextRuntime(Local<Context> 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
{
Expand Down Expand Up @@ -665,9 +674,6 @@ Maybe<bool> InitializeContextForSnapshot(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);

context->AllowCodeGenerationFromStrings(false);
context->SetEmbedderData(
ContextEmbedderIndex::kAllowCodeGenerationFromStrings, True(isolate));
context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
True(isolate));

Expand Down
13 changes: 13 additions & 0 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,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() {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
7 changes: 7 additions & 0 deletions test/parallel/test-eval.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

require('../common');
const assert = require('assert');

// Verify that eval is allowed by default.
assert.strictEqual(eval(`'eval'`), 'eval');

0 comments on commit 106dc81

Please sign in to comment.