Skip to content

Commit 0c236d1

Browse files
AnnaMagevanlucas
authored andcommitted
src: replace SetNamedPropertyHandler()
The changes introdcued here replace the deprecated v8 method SetNamedPropertyHandler() to SetHandler() in node.cc. Prior to refactoring, the method defined callbacks when accessing object properties defined by Strings and not Symbols. test/parallel/test-v8-interceptStrings-not-Symbols.js demonstrates that this behaviour remained unchanged after refactoring. PR-URL: #9062 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
1 parent 3d4a829 commit 0c236d1

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

src/node.cc

+15-10
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,15 @@ using v8::Locker;
114114
using v8::MaybeLocal;
115115
using v8::Message;
116116
using v8::Name;
117+
using v8::NamedPropertyHandlerConfiguration;
117118
using v8::Null;
118119
using v8::Number;
119120
using v8::Object;
120121
using v8::ObjectTemplate;
121122
using v8::Promise;
122123
using v8::PromiseRejectMessage;
123124
using v8::PropertyCallbackInfo;
125+
using v8::PropertyHandlerFlags;
124126
using v8::ScriptOrigin;
125127
using v8::SealHandleScope;
126128
using v8::String;
@@ -2673,7 +2675,7 @@ static void ProcessTitleSetter(Local<Name> property,
26732675
}
26742676

26752677

2676-
static void EnvGetter(Local<String> property,
2678+
static void EnvGetter(Local<Name> property,
26772679
const PropertyCallbackInfo<Value>& info) {
26782680
Isolate* isolate = info.GetIsolate();
26792681
#ifdef __POSIX__
@@ -2701,7 +2703,7 @@ static void EnvGetter(Local<String> property,
27012703
}
27022704

27032705

2704-
static void EnvSetter(Local<String> property,
2706+
static void EnvSetter(Local<Name> property,
27052707
Local<Value> value,
27062708
const PropertyCallbackInfo<Value>& info) {
27072709
#ifdef __POSIX__
@@ -2722,7 +2724,7 @@ static void EnvSetter(Local<String> property,
27222724
}
27232725

27242726

2725-
static void EnvQuery(Local<String> property,
2727+
static void EnvQuery(Local<Name> property,
27262728
const PropertyCallbackInfo<Integer>& info) {
27272729
int32_t rc = -1; // Not found unless proven otherwise.
27282730
#ifdef __POSIX__
@@ -2748,7 +2750,7 @@ static void EnvQuery(Local<String> property,
27482750
}
27492751

27502752

2751-
static void EnvDeleter(Local<String> property,
2753+
static void EnvDeleter(Local<Name> property,
27522754
const PropertyCallbackInfo<Boolean>& info) {
27532755
#ifdef __POSIX__
27542756
node::Utf8Value key(info.GetIsolate(), property);
@@ -3147,12 +3149,15 @@ void SetupProcessObject(Environment* env,
31473149
// create process.env
31483150
Local<ObjectTemplate> process_env_template =
31493151
ObjectTemplate::New(env->isolate());
3150-
process_env_template->SetNamedPropertyHandler(EnvGetter,
3151-
EnvSetter,
3152-
EnvQuery,
3153-
EnvDeleter,
3154-
EnvEnumerator,
3155-
env->as_external());
3152+
process_env_template->SetHandler(NamedPropertyHandlerConfiguration(
3153+
EnvGetter,
3154+
EnvSetter,
3155+
EnvQuery,
3156+
EnvDeleter,
3157+
EnvEnumerator,
3158+
env->as_external(),
3159+
PropertyHandlerFlags::kOnlyInterceptStrings));
3160+
31563161
Local<Object> process_env =
31573162
process_env_template->NewInstance(env->context()).ToLocalChecked();
31583163
process->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "env"), process_env);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
require('../common');
3+
4+
const assert = require('assert');
5+
6+
// Test that the v8 named property handler intercepts callbacks
7+
// when properties are defined as Strings and NOT for Symbols.
8+
//
9+
// With the kOnlyInterceptStrings flag, manipulating properties via
10+
// Strings is intercepted by the callbacks, while Symbols adopt
11+
// the default global behaviour.
12+
// Removing the kOnlyInterceptStrings flag, adds intercepting to Symbols,
13+
// which causes Type Error at process.env[symbol]=42 due to process.env being
14+
// strongly typed for Strings
15+
// (node::Utf8Value key(info.GetIsolate(), property);).
16+
17+
18+
const symbol = Symbol('sym');
19+
20+
// check if its undefined
21+
assert.strictEqual(process.env[symbol], undefined);
22+
23+
// set a value using a Symbol
24+
process.env[symbol] = 42;
25+
26+
// set a value using a String (call to EnvSetter, node.cc)
27+
process.env['s'] = 42;
28+
29+
//check the values after substitutions
30+
assert.strictEqual(42, process.env[symbol]);
31+
assert.strictEqual('42', process.env['s']);
32+
33+
delete process.env[symbol];
34+
assert.strictEqual(undefined, process.env[symbol]);

0 commit comments

Comments
 (0)