fixup! fixup! src: add napi_create_symbol_for()

Disallow null description Signed-off-by: Darshan Sen <darshan.sen@postman.com>
nodejs · Dec 31, 2021 · 549fb91 · 549fb91
1 parent ab158a7
commit 549fb91
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 18 deletions.
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -1666,20 +1666,14 @@ napi_status napi_create_symbol_for(napi_env env,
                                    napi_value description,
                                    napi_value* result) {
   CHECK_ENV(env);
+  CHECK_ARG(env, description);
   CHECK_ARG(env, result);
 
-  v8::Isolate* isolate = env->isolate;
-
-  v8::Local<v8::Value> desc;
-  if (description == nullptr) {
-    CHECK_NEW_FROM_UTF8(env, desc, "undefined");
-  } else {
-    desc = v8impl::V8LocalValueFromJsValue(description);
-    RETURN_STATUS_IF_FALSE(env, desc->IsString(), napi_string_expected);
-  }
+  v8::Local<v8::Value> desc = v8impl::V8LocalValueFromJsValue(description);
+  RETURN_STATUS_IF_FALSE(env, desc->IsString(), napi_string_expected);
 
   *result = v8impl::JsValueFromV8LocalValue(
-    v8::Symbol::For(isolate, desc.As<v8::String>()));
+    v8::Symbol::For(env->isolate, desc.As<v8::String>()));
 
   return napi_clear_last_error(env);
 }

diff --git a/test/js-native-api/test_properties/test.js b/test/js-native-api/test_properties/test.js
@@ -40,9 +40,6 @@ const propertySymbols = Object.getOwnPropertySymbols(test_object);
 assert.strictEqual(String(propertySymbols[0]), 'Symbol(NameKeySymbol)');
 assert.strictEqual(String(propertySymbols[1]), 'Symbol()');
 assert.strictEqual(propertySymbols[2], Symbol.for('NameKeySymbolFor'));
-assert.strictEqual(propertySymbols[3], Symbol.for());
-assert.strictEqual(propertySymbols[3], Symbol.for(undefined));
-assert.strictEqual(propertySymbols[3], Symbol.for('undefined'));
 
 // The napi_writable attribute should be ignored for accessors.
 const readwriteAccessor1Descriptor =

diff --git a/test/js-native-api/test_properties/test_properties.c b/test/js-native-api/test_properties/test_properties.c
@@ -90,10 +90,6 @@ napi_value Init(napi_env env, napi_value exports) {
   NODE_API_CALL(env,
       napi_create_symbol_for(env, symbol_for_description, &name_symbol_for));
 
-  napi_value name_symbol_for_undefined;
-  NODE_API_CALL(env,
-      napi_create_symbol_for(env, NULL, &name_symbol_for_undefined));
-
   napi_property_descriptor properties[] = {
     { "echo", 0, Echo, 0, 0, 0, napi_enumerable, 0 },
     { "readwriteValue", 0, 0, 0, 0, number, napi_enumerable | napi_writable, 0 },
@@ -103,7 +99,6 @@ napi_value Init(napi_env env, napi_value exports) {
     { NULL, name_symbol, 0, 0, 0, number, napi_enumerable, 0},
     { NULL, name_symbol_undefined, 0, 0, 0, number, napi_enumerable, 0},
     { NULL, name_symbol_for, 0, 0, 0, number, napi_enumerable, 0},
-    { NULL, name_symbol_for_undefined, 0, 0, 0, number, napi_enumerable, 0},
     { "readwriteAccessor1", 0, 0, GetValue, SetValue, 0, napi_default, 0},
     { "readwriteAccessor2", 0, 0, GetValue, SetValue, 0, napi_writable, 0},
     { "readonlyAccessor1", 0, 0, GetValue, NULL, 0, napi_default, 0},

diff --git a/test/js-native-api/test_reference/test.js b/test/js-native-api/test_reference/test.js
@@ -28,6 +28,9 @@ async function runTests() {
   })();
   test_reference.deleteReference();
 
+  assert.throws(() => test_reference.createSymbolForUndefined(),
+                /Invalid argument/);
+
   (() => {
     const value = test_reference.createExternal();
     assert.strictEqual(test_reference.finalizeCount, 0);

diff --git a/test/js-native-api/test_reference/test_reference.c b/test/js-native-api/test_reference/test_reference.c
@@ -64,6 +64,12 @@ static napi_value CreateSymbolFor(napi_env env, napi_callback_info info) {
     return result_symbol;
 }
 
+static napi_value CreateSymbolForUndefined(napi_env env, napi_callback_info info) {
+    napi_value result_symbol;
+    NODE_API_CALL(env, napi_create_symbol_for(env, NULL, &result_symbol));
+    return result_symbol;
+}
+
 static napi_value
 CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
   napi_value result;
@@ -206,6 +212,7 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
     DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
     DECLARE_NODE_API_PROPERTY("createSymbolFor", CreateSymbolFor),
+    DECLARE_NODE_API_PROPERTY("createSymbolForUndefined", CreateSymbolForUndefined),
     DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
     DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
     DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),