Skip to content

Commit 8a91616

Browse files
AnnaMagMylesBorins
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 cf5a00e commit 8a91616

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
@@ -115,13 +115,15 @@ using v8::Locker;
115115
using v8::MaybeLocal;
116116
using v8::Message;
117117
using v8::Name;
118+
using v8::NamedPropertyHandlerConfiguration;
118119
using v8::Null;
119120
using v8::Number;
120121
using v8::Object;
121122
using v8::ObjectTemplate;
122123
using v8::Promise;
123124
using v8::PromiseRejectMessage;
124125
using v8::PropertyCallbackInfo;
126+
using v8::PropertyHandlerFlags;
125127
using v8::ScriptOrigin;
126128
using v8::SealHandleScope;
127129
using v8::String;
@@ -2719,7 +2721,7 @@ static void ProcessTitleSetter(Local<Name> property,
27192721
}
27202722

27212723

2722-
static void EnvGetter(Local<String> property,
2724+
static void EnvGetter(Local<Name> property,
27232725
const PropertyCallbackInfo<Value>& info) {
27242726
Isolate* isolate = info.GetIsolate();
27252727
#ifdef __POSIX__
@@ -2747,7 +2749,7 @@ static void EnvGetter(Local<String> property,
27472749
}
27482750

27492751

2750-
static void EnvSetter(Local<String> property,
2752+
static void EnvSetter(Local<Name> property,
27512753
Local<Value> value,
27522754
const PropertyCallbackInfo<Value>& info) {
27532755
#ifdef __POSIX__
@@ -2768,7 +2770,7 @@ static void EnvSetter(Local<String> property,
27682770
}
27692771

27702772

2771-
static void EnvQuery(Local<String> property,
2773+
static void EnvQuery(Local<Name> property,
27722774
const PropertyCallbackInfo<Integer>& info) {
27732775
int32_t rc = -1; // Not found unless proven otherwise.
27742776
#ifdef __POSIX__
@@ -2794,7 +2796,7 @@ static void EnvQuery(Local<String> property,
27942796
}
27952797

27962798

2797-
static void EnvDeleter(Local<String> property,
2799+
static void EnvDeleter(Local<Name> property,
27982800
const PropertyCallbackInfo<Boolean>& info) {
27992801
#ifdef __POSIX__
28002802
node::Utf8Value key(info.GetIsolate(), property);
@@ -3221,12 +3223,15 @@ void SetupProcessObject(Environment* env,
32213223
// create process.env
32223224
Local<ObjectTemplate> process_env_template =
32233225
ObjectTemplate::New(env->isolate());
3224-
process_env_template->SetNamedPropertyHandler(EnvGetter,
3225-
EnvSetter,
3226-
EnvQuery,
3227-
EnvDeleter,
3228-
EnvEnumerator,
3229-
env->as_external());
3226+
process_env_template->SetHandler(NamedPropertyHandlerConfiguration(
3227+
EnvGetter,
3228+
EnvSetter,
3229+
EnvQuery,
3230+
EnvDeleter,
3231+
EnvEnumerator,
3232+
env->as_external(),
3233+
PropertyHandlerFlags::kOnlyInterceptStrings));
3234+
32303235
Local<Object> process_env =
32313236
process_env_template->NewInstance(env->context()).ToLocalChecked();
32323237
process->Set(env->env_string(), 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)