Skip to content

src: restore context default IsCodeGenerationFromStringsAllowed value #44324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 28, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
@@ -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);
}
@@ -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>();
13 changes: 13 additions & 0 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
@@ -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() {
@@ -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()) {
@@ -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
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');