[GR-33742] Avoiding ClassCastException when an enumerator does not return an array.

PullRequest: js/2146

Author: iamstolis

graal-nodejs/mx.graal-nodejs/com.oracle.truffle.trufflenode/src/com/oracle/truffle/trufflenode/node/ExecuteNativePropertyHandlerNode.java

@@ -321,23 +321,25 @@ private Object executeOwnKeys(Object holder, Object[] arguments) {
         }
         if (indexedHandler != null && indexedHandler.getEnumerator() != 0) {
             DynamicObject ownKeys2 = (DynamicObject) NativeAccess.executePropertyHandlerEnumerator(indexedHandler.getEnumerator(), holder, nativeCallArgs, indexedHandlerData);
-            // indexed handler returns array of numbers but we need an array of property keys
-            long length = JSAbstractArray.arrayGetLength(ownKeys2);
-            for (long i = 0; i < length; i++) {
-                Object key = JSObject.get(ownKeys2, i);
-                JSObject.set(ownKeys2, i, JSRuntime.toString(key));
+            if (JSArray.isJSArray(ownKeys2)) {
+                // indexed handler returns array of numbers but we need an array of property keys
+                long length = JSAbstractArray.arrayGetLength(ownKeys2);
+                for (long i = 0; i < length; i++) {
+                    Object key = JSObject.get(ownKeys2, i);
+                    JSObject.set(ownKeys2, i, JSRuntime.toString(key));
+                }
+                ownKeys = concatArrays(ownKeys, ownKeys2);
             }
-            ownKeys = concatArrays(ownKeys, ownKeys2);
         }
-        if (ownKeys == null) {
+        if (!JSArray.isJSArray(ownKeys)) {
             ownKeys = JSArray.createEmpty(context, getRealm(), 0);
         }
         return ownKeys;
     }
 
     private DynamicObject concatArrays(DynamicObject array1, DynamicObject array2) {
-        if (JSRuntime.isArray(array1)) {
-            if (JSRuntime.isArray(array2)) {
+        if (JSArray.isJSArray(array1)) {
+            if (JSArray.isJSArray(array2)) {
                 List<Object> keys = new ArrayList<>(Arrays.asList(JSArray.toArray(array1)));
                 for (Object key : JSArray.toArray(array2)) {
                     if (!keys.contains(key)) {

graal-nodejs/test/graal/unit/object_template.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -45,6 +45,7 @@
 
 void SimpleAccessorGetter(Local<Name> property, const PropertyCallbackInfo<Value>& info);
 void SimpleAccessorSetter(Local<Name> property, Local<Value> value, const PropertyCallbackInfo<void>& info);
+void EmptyPropertyEnumeratorCallback(const PropertyCallbackInfo<Array>& info) {};
 
 #endif
 
@@ -128,4 +129,38 @@ EXPORT_TO_JS(CheckNamedHandlerWithInternalFields) {
     args.GetReturnValue().Set(expectedCount == actualCount);
 }
 
+EXPORT_TO_JS(CreateWithEmptyIndexedEnumerator) {
+    Isolate* isolate = args.GetIsolate();
+    Local<ObjectTemplate> objectTemplate = ObjectTemplate::New(isolate);
+    IndexedPropertyHandlerConfiguration handler(
+            nullptr, // getter
+            nullptr, // setter
+            nullptr, // query
+            nullptr, // deleter
+            EmptyPropertyEnumeratorCallback // enumerator
+    );
+    objectTemplate->SetHandler(handler);
+
+    Local<Context> context = isolate->GetCurrentContext();
+    Local<Object> instance = objectTemplate->NewInstance(context).ToLocalChecked();
+    args.GetReturnValue().Set(instance);
+}
+
+EXPORT_TO_JS(CreateWithEmptyNamedEnumerator) {
+    Isolate* isolate = args.GetIsolate();
+    Local<ObjectTemplate> objectTemplate = ObjectTemplate::New(isolate);
+    NamedPropertyHandlerConfiguration handler(
+            nullptr, // getter
+            nullptr, // setter
+            nullptr, // query
+            nullptr, // deleter
+            EmptyPropertyEnumeratorCallback // enumerator
+    );
+    objectTemplate->SetHandler(handler);
+
+    Local<Context> context = isolate->GetCurrentContext();
+    Local<Object> instance = objectTemplate->NewInstance(context).ToLocalChecked();
+    args.GetReturnValue().Set(instance);
+}
+
 #undef SUITE

graal-nodejs/test/graal/unit/object_template.js

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -83,6 +83,14 @@ describe('ObjectTemplate', function () {
         it('should create object with the specified internal field count', function () {
             var result = module.ObjectTemplate_CheckNamedHandlerWithInternalFields();
             assert.ok(result);
-        });        
+        });
+        it('should not crash with an empty named enumerator', function () {
+            var obj = module.ObjectTemplate_CreateWithEmptyNamedEnumerator();
+            assert.strictEqual(Object.keys(obj).length, 0);
+        });
+        it('should not crash with an empty indexed enumerator', function () {
+            var obj = module.ObjectTemplate_CreateWithEmptyIndexedEnumerator();
+            assert.strictEqual(Object.keys(obj).length, 0);
+        });
     });
 });