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

process: introduce codeGenerationFromString event #48295

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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
10 changes: 5 additions & 5 deletions deps/v8/include/v8-callbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,12 @@ using FailedAccessCheckCallback = void (*)(Local<Object> target,
* the source to string if necessary. See: ModifyCodeGenerationFromStrings.
*/
using ModifyCodeGenerationFromStringsCallback =
ModifyCodeGenerationFromStringsResult (*)(Local<Context> context,
Local<Value> source);
Maybe<ModifyCodeGenerationFromStringsResult> (*)(Local<Context> context,
Local<Value> source);
using ModifyCodeGenerationFromStringsCallback2 =
ModifyCodeGenerationFromStringsResult (*)(Local<Context> context,
Local<Value> source,
bool is_code_like);
Maybe<ModifyCodeGenerationFromStringsResult> (*)(Local<Context> context,
Local<Value> source,
bool is_code_like);

// --- WebAssembly compilation callbacks ---
using ExtensionCallback = bool (*)(const FunctionCallbackInfo<Value>&);
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/codegen/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2925,10 +2925,10 @@ bool ModifyCodeGenerationFromStrings(Isolate* isolate, Handle<Context> context,
ModifyCodeGenerationFromStringsResult result =
isolate->modify_code_gen_callback()
? isolate->modify_code_gen_callback()(v8::Utils::ToLocal(context),
v8::Utils::ToLocal(*source))
v8::Utils::ToLocal(*source)).FromJust()
: isolate->modify_code_gen_callback2()(v8::Utils::ToLocal(context),
v8::Utils::ToLocal(*source),
is_code_like);
is_code_like).FromJust();
if (result.codegen_allowed && !result.modified_source.IsEmpty()) {
// Use the new source (which might be the same as the old source).
*source =
Expand Down
28 changes: 28 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,34 @@ console.log('This message is displayed first.');
// Process exit event with code: 0
```

### Event: `'codeGenerationFromString'`

<!-- YAML
added: REPLACEME
-->

The `'codeGenerationFromString'` event is emitted when a call is made to `eval(str)`
or `new Function(str)`.

The event handler callback will be invoked with the argument passed to `eval` or
the `Function` constructor.

The call to `eval` or `new Function` will happen after the event is emitted.

```js
process.on('codeGenerationFromString', (code) => {
console.log(`Generating code from string '${code}'`);
});
const item = { foo: 0 };
eval('item.foo++');
```

will log

```text
Generating code from string 'item.foo++'
```

### Event: `'disconnect'`

<!-- YAML
Expand Down
20 changes: 20 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,26 @@ process.emitWarning = emitWarning;
// Note: only after this point are the timers effective
}

setupCodegenFromStringCallback(rawMethods);

function setupCodegenFromStringCallback(bindings) {
if (!bindings.codeGenerationFromStringsAllowed()) {
// Do nothing if code generation from strings is not allowed.
return;
}
const eventName = 'codeGenerationFromString';
process.on('newListener', (event) => {
if (event === eventName && process.listenerCount(eventName) === 0) {
bindings.setEmitCodeGenFromStringEvent(true);
}
});
process.on('removeListener', (event) => {
if (event === eventName && process.listenerCount(eventName) === 0) {
bindings.setEmitCodeGenFromStringEvent(false);
}
});
}

function setupProcessObject() {
const EventEmitter = require('events');
const origProcProto = ObjectGetPrototypeOf(process);
Expand Down
50 changes: 50 additions & 0 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ typedef int mode_t;

namespace node {

using errors::TryCatchScope;
using v8::Array;
using v8::ArrayBuffer;
using v8::CFunction;
Expand Down Expand Up @@ -463,6 +464,49 @@ static void ReallyExit(const FunctionCallbackInfo<Value>& args) {
env->Exit(code);
}

static void CodeGenerationFromStringsAllowed(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Local<Context> context = env->context();
Local<Value> allow_code_gen = context->GetEmbedderData(
ContextEmbedderIndex::kAllowCodeGenerationFromStrings);
bool codegen_allowed =
allow_code_gen->IsUndefined() || allow_code_gen->IsTrue();
args.GetReturnValue().Set(codegen_allowed);
}

static v8::Maybe<v8::ModifyCodeGenerationFromStringsResult> CodeGenCallback(
Local<Context> context,
Local<Value> source,
bool is_code_like) {
Environment* env = Environment::GetCurrent(context);
ProcessEmit(env, "codeGenerationFromString", source);

// returning {true, val} where val.IsEmpty() makes v8
// use the orignal value passed to `eval` which does not impact
// calls as `eval({})`
return v8::Just({true, v8::MaybeLocal<v8::String>()});
}

static void SetEmitCodeGenFromStringEvent(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = args.GetIsolate();
Local<Context> context = env->context();

CHECK(args[0]->IsBoolean());

bool val = args[0]->BooleanValue(args.GetIsolate());
if (val) {
Comment on lines +499 to +500
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool val = args[0]->BooleanValue(args.GetIsolate());
if (val) {
if (args[0]->IsTrue()) {

context->AllowCodeGenerationFromStrings(false);
isolate->SetModifyCodeGenerationFromStringsCallback(CodeGenCallback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would replace the ModifyCodeGenerationFromStrings callback we already have for source maps and also override the isolate setting from embedders. I think we could simply configure a per-environment flag here to decide whether we want to call ProcessEmit in the existing ModifyCodeGenerationFromStrings callback.

I also noticed that AllowCodeGenerationFromStrings() is never set to true in our contexts because we always delegate that decision to the callback, so the fast path is never taken. Not sure if that's intentional though, it seems to me that it should only be set to false when we need the callback (e.g. when source maps are enabled)? @legendecas

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the callback can be installed when needed (either with the cli flag --enable-source-maps or programmatically process.setSourceMapsEnabled(true)). However, we'll need an additional flag to memorize if an embedder's callback is installed in SetIsolateMiscHandlers instead so that the embedder's callback is not overridden.

} else {
// This is enough to disable the handler. V8 will not call it anymore
// until set back to false
context->AllowCodeGenerationFromStrings(true);
}
}

namespace process {

BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
Expand Down Expand Up @@ -600,6 +644,10 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethod(isolate, target, "reallyExit", ReallyExit);
SetMethodNoSideEffect(isolate, target, "uptime", Uptime);
SetMethod(isolate, target, "patchProcessObject", PatchProcessObject);
SetMethod(isolate, target, "codeGenerationFromStringsAllowed",
CodeGenerationFromStringsAllowed);
SetMethod(isolate, target, "setEmitCodeGenFromStringEvent",
SetEmitCodeGenFromStringEvent);
}

static void CreatePerContextProperties(Local<Object> target,
Expand Down Expand Up @@ -637,6 +685,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(ReallyExit);
registry->Register(Uptime);
registry->Register(PatchProcessObject);
registry->Register(CodeGenerationFromStringsAllowed);
registry->Register(SetEmitCodeGenFromStringEvent);
}

} // namespace process
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-process-codegen-from-string.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict';

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

const item = { foo: 0 };
eval('++item.foo');

assert.strictEqual(item.foo, 1);

// Basic eval() test
process.on('codeGenerationFromString', common.mustCall((code) => {
assert.strictEqual(code, 'item.foo++');
}));

eval('item.foo++');
assert.strictEqual(item.foo, 2);

process.removeAllListeners('codeGenerationFromString');

// Basic Function() test
process.on('codeGenerationFromString', common.mustCall((code) => {
assert.strictEqual(code, '(function anonymous(a,b\n) {\nreturn a + b\n})');
}));

const fct = new Function('a', 'b', 'return a + b');
assert.strictEqual(fct(1, 2), 3);

process.removeAllListeners('codeGenerationFromString');