Skip to content
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

src: restore context default IsCodeGenerationFromStringsAllowed value #44324

Merged
merged 1 commit into from
Aug 28, 2022
Merged
Show file tree
Hide file tree
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
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);
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
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(
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
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);
legendecas marked this conversation as resolved.
Show resolved Hide resolved
}

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');