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 24, 2022
1 parent ab89024 commit 38b97cc
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 3 deletions.
19 changes: 16 additions & 3 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,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);
}
Expand Down Expand Up @@ -648,11 +661,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>();
Expand Down
13 changes: 13 additions & 0 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,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 @@ -1053,6 +1063,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()) {
Expand Down Expand Up @@ -1121,6 +1132,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
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 38b97cc

Please sign in to comment.