Merge pull request #24 from LabyMod/develop

zortax · web-flow · commit fe14b0b28aec · 2020-12-21T22:16:45.000+01:00
Fix issues with member access
diff --git a/LICENSE b/LICENSE
@@ -1,7 +1,7 @@
                    GNU LESSER GENERAL PUBLIC LICENSE
                        Version 3, 29 June 2007
 
- Copyright (C) 2007 LabyMedia. <https://labymedia.com/>
+ Copyright (C) 2020 LabyMedia GmbH. <https://labymedia.com/>
  Everyone is permitted to copy and distribute verbatim copies
  of this license document, but changing it is not allowed.
 
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-0.3.2
+0.3.3
diff --git a/ultralight-java-databind/src/main/java/com/labymedia/ultralight/Databind.java b/ultralight-java-databind/src/main/java/com/labymedia/ultralight/Databind.java
@@ -74,7 +74,7 @@ public JavascriptClass toJavascript(Class<?> javaClass, boolean ignoreAutomaticP
 
             javascriptClass = classCache.put(
                     javaClass.getName(),
-                    DatabindJavascriptClass.create(configuration, conversionUtils, javaClass)
+                    DatabindJavascriptClass.create(configuration, conversionUtils, javaClass, classCache)
                             .bake());
         }
 
diff --git a/ultralight-java-databind/src/main/java/com/labymedia/ultralight/DatabindJavascriptClass.java b/ultralight-java-databind/src/main/java/com/labymedia/ultralight/DatabindJavascriptClass.java
@@ -19,13 +19,14 @@
 
 package com.labymedia.ultralight;
 
+import com.labymedia.ultralight.cache.JavascriptClassCache;
 import com.labymedia.ultralight.call.CallData;
 import com.labymedia.ultralight.javascript.*;
 import com.labymedia.ultralight.javascript.interop.JavascriptInteropException;
 import com.labymedia.ultralight.call.MethodChooser;
-import com.labymedia.ultralight.javascript.*;
 import com.labymedia.ultralight.utils.JavascriptConversionUtils;
 
+import java.lang.invoke.MethodHandles;
 import java.lang.reflect.*;
 import java.util.*;
 
@@ -51,9 +52,14 @@ public final class DatabindJavascriptClass {
      * @param className       The name of the Javascript class
      */
     private DatabindJavascriptClass(
-            DatabindConfiguration configuration, JavascriptConversionUtils conversionUtils, String className) {
+            DatabindConfiguration configuration,
+            JavascriptConversionUtils conversionUtils,
+            String className,
+            JavascriptClass parentClass
+    ) {
         this.definition = new JavascriptClassDefinition()
                 .name(className)
+                .parentClass(parentClass)
                 .attributes(JavascriptClassAttributes.NO_AUTOMATIC_PROTOTYPE);
 
         this.configuration = configuration;
@@ -76,16 +82,16 @@ private void registerCallbacks() {
      *
      * @param constructors The constructors to index
      */
-    private void addConstructors(Constructor<?>... constructors) {
-        this.constructors.addAll(Arrays.asList(constructors));
+    private void addConstructors(Collection<Constructor<?>> constructors) {
+        this.constructors.addAll(constructors);
     }
 
     /**
      * Indexes methods into this class.
      *
      * @param methods The methods to index
      */
-    private void addMethods(Method... methods) {
+    private void addMethods(Collection<Method> methods) {
         for (Method method : methods) {
             String name = method.getName();
 
@@ -108,7 +114,7 @@ private void addMethods(Method... methods) {
      *
      * @param fields The fields to index
      */
-    private void addFields(Field... fields) {
+    private void addFields(Collection<Field> fields) {
         for (Field field : fields) {
             this.fields.put(field.getName(), field);
         }
@@ -146,20 +152,20 @@ private JavascriptObject onCallAsConstructor(
         try {
             // Invoke constructor with constructed arguments
             return context.makeObject(bake(), new Data(method.newInstance(parameters.toArray()), null));
-        } catch (IllegalAccessException exception) {
+        } catch(IllegalAccessException exception) {
             throw new JavascriptInteropException("Unable to access constructor: " + method.getName(), exception);
-        } catch (InvocationTargetException exception) {
+        } catch(InvocationTargetException exception) {
             throw new JavascriptInteropException("Constructor threw an exception", exception);
-        } catch (InstantiationException exception) {
+        } catch(InstantiationException exception) {
             throw new JavascriptInteropException("Unable to create instance", exception);
         }
     }
 
     /**
      * Determines whether a property exists.
      *
-     * @param context The context the check is executed in
-     * @param object The object to check for the property on
+     * @param context      The context the check is executed in
+     * @param object       The object to check for the property on
      * @param propertyName The name of the property to check for
      * @return {@code true} if the property could be found, {@code false} otherwise
      */
@@ -168,23 +174,23 @@ private boolean onHasProperty(JavascriptContext context, JavascriptObject object
         boolean instanceAvailable = ((Data) object.getPrivate()).instance != null;
 
         Field f = fields.get(propertyName);
-        if(f != null && (Modifier.isStatic(f.getModifiers()) || instanceAvailable)) {
+        if (f != null && (Modifier.isStatic(f.getModifiers()) || instanceAvailable)) {
             // Field found and usable
             return true;
         }
 
         Set<Method> methodsWithName = methods.get(propertyName);
-        if(methodsWithName == null || methodsWithName.isEmpty()) {
+        if (methodsWithName == null || methodsWithName.isEmpty()) {
             // No methods available with that name
             return false;
-        } else if(instanceAvailable) {
+        } else if (instanceAvailable) {
             // There is an instance and methods with this name are available
             return true;
         }
 
         // There are methods with this name available, check if any of them is static
-        for(Method method : methodsWithName) {
-            if(Modifier.isStatic(method.getModifiers())) {
+        for (Method method : methodsWithName) {
+            if (Modifier.isStatic(method.getModifiers())) {
                 // Found a static variant
                 return true;
             }
@@ -211,15 +217,15 @@ private JavascriptValue onGetProperty(
         if (field != null) {
             try {
                 return conversionUtils.toJavascript(context, field.get(privateData.instance), field.getType());
-            } catch (IllegalAccessException exception) {
+            } catch(IllegalAccessException exception) {
                 throw new JavascriptInteropException("Unable to access field: " + field.getName(), exception);
             }
         }
 
         Set<Method> methodSet = methods.get(propertyName);
         if (methodSet == null) {
-            // Property does not exist
-            return context.makeUndefined();
+            // Property does not exist, delegate to parent
+            return null;
         }
 
         return context.makeObject(
@@ -253,8 +259,10 @@ private boolean onSetProperty(
         if (field != null) {
             try {
                 field.set(privateData.instance, conversionUtils.fromJavascript(value, field.getType()));
-            } catch (IllegalAccessException exception) {
-                throw new JavascriptInteropException("Unable to access field: " + field.getName(), exception);
+                return true;
+            } catch(IllegalAccessException exception) {
+                throw new JavascriptInteropException("Unable to access field: " + field.getName() +
+                        " (" + exception.getMessage() + ")", exception);
             }
         }
 
@@ -280,30 +288,110 @@ public JavascriptClass bake() {
      * @param configuration   The configuration to use
      * @param conversionUtils The conversion utilities to use for converting objects
      * @param javaClass       The java class to create a binding for
+     * @param classCache      The class cache to retrieve or cache parent classes with
      * @return The created binding
      */
     static DatabindJavascriptClass create(
-            DatabindConfiguration configuration, JavascriptConversionUtils conversionUtils, Class<?> javaClass) {
+            DatabindConfiguration configuration,
+            JavascriptConversionUtils conversionUtils,
+            Class<?> javaClass,
+            JavascriptClassCache classCache
+    ) {
+        Class<?> superClass = javaClass.getSuperclass();
+
+        JavascriptClass parentClass = null;
+        if (superClass != null && javaClass != Object.class) {
+            String parentClassName = superClass.getName();
+
+            if (!classCache.contains(parentClassName)) {
+                DatabindJavascriptClass databindParent = create(
+                        configuration,
+                        conversionUtils,
+                        superClass,
+                        classCache
+                );
+
+                parentClass = classCache.put(parentClassName, databindParent.bake());
+            } else {
+                parentClass = classCache.get(parentClassName);
+            }
+        }
+
         DatabindJavascriptClass javascriptClass = new DatabindJavascriptClass(
-                configuration, conversionUtils, javaClass.getName());
+                configuration, conversionUtils, javaClass.getName(), parentClass);
 
         javascriptClass.registerCallbacks();
-        javascriptClass.addConstructors(javaClass.getDeclaredConstructors());
-        javascriptClass.addMethods(javaClass.getDeclaredMethods());
-        javascriptClass.addFields(javaClass.getDeclaredFields());
+
+        javascriptClass.addConstructors(filterAccessible(javaClass.getConstructors(), javaClass));
+        javascriptClass.addMethods(filterAccessible(javaClass.getMethods(), javaClass));
+        javascriptClass.addFields(filterAccessible(javaClass.getFields(), javaClass));
 
         // Iteratively scan all interfaces
         Queue<Class<?>> toAdd = new LinkedList<>(Arrays.asList(javaClass.getInterfaces()));
         while (!toAdd.isEmpty()) {
             Class<?> iface = toAdd.remove();
             toAdd.addAll(Arrays.asList(iface.getInterfaces()));
 
-            javascriptClass.addMethods(iface.getMethods());
+            javascriptClass.addMethods(filterAccessible(iface.getMethods(), iface));
+            javascriptClass.addFields(filterAccessible(iface.getFields(), iface));
         }
 
         return javascriptClass;
     }
 
+    /**
+     * Filters an array of reflection objects by their accessibility.
+     *
+     * @param objects The objects to filter
+     * @param owner   The wanted owner class
+     * @param <T>     The type of the objects
+     * @return The filtered objects
+     */
+    private static <T extends AccessibleObject & Member> Set<T> filterAccessible(T[] objects, Class<?> owner) {
+        Set<T> accessible = new HashSet<>();
+
+        for (T object : objects) {
+            if (object.getDeclaringClass() != owner || !allPublic(object)) {
+                continue;
+            }
+
+            accessible.add(object);
+        }
+
+        return accessible;
+    }
+
+    /**
+     * Determines if a members is accessible by checking if everything is public.
+     *
+     * @param member The member to check
+     * @return {@code true} if the member is accessible using a public path, {@code false} otherwise
+     */
+    private static boolean allPublic(Member member) {
+        Class<?> classToCheck = null;
+
+        do {
+            if (classToCheck == null) {
+                if ((member.getModifiers() & Modifier.PUBLIC) == 0) {
+                    return false;
+                }
+
+                classToCheck = member.getDeclaringClass();
+            } else {
+                if((classToCheck.getModifiers() & Modifier.PUBLIC) == 0) {
+                    return false;
+                }
+
+                classToCheck = classToCheck.getSuperclass();
+                if(classToCheck == Object.class) {
+                    return true;
+                }
+            }
+        } while (classToCheck != null);
+
+        return true;
+    }
+
     /**
      * Represents the internal state of a Javascript class.
      */
diff --git a/ultralight-java-native/src/main/c/src/platform/managed_javascript_class.cpp b/ultralight-java-native/src/main/c/src/platform/managed_javascript_class.cpp
@@ -198,7 +198,6 @@ namespace ultralight_java {
 
         if(!ret) {
             env->CallVoidMethod(java_lock, runtime.javascript_context_lock.unlock_method);
-            *exception = Util::create_jserror(ctx, "Java function returned null");
             return nullptr;
         }
 
@@ -245,9 +244,8 @@ namespace ultralight_java {
                     java_property_name.get()));
             
         } else {
-            env->ThrowNew(
-                runtime.illegal_argument_exception.clazz,
-                ("Tried to get non existent static property " + getter_index).c_str());
+            env->CallVoidMethod(java_lock, runtime.javascript_context_lock.unlock_method);
+            return nullptr;
         }
 
         if(env->ExceptionCheck()) {
@@ -259,7 +257,6 @@ namespace ultralight_java {
 
         if(!ret) {
             env->CallVoidMethod(java_lock, runtime.javascript_context_lock.unlock_method);
-            *exception = Util::create_jserror(ctx, "Java function returned null");
             return nullptr;
         }
 
@@ -356,9 +353,8 @@ namespace ultralight_java {
                 java_property_name.get(),
                 java_value.get());
         } else {
-            env->ThrowNew(
-                runtime.illegal_state_exception.clazz,
-                ("Tried to set non existent static property " + setter_index).c_str());
+            env->CallVoidMethod(java_lock, runtime.javascript_context_lock.unlock_method);
+            return false;
         }
 
         if(env->ExceptionCheck()) {

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public JavascriptClass toJavascript(Class<?> javaClass, boolean ignoreAutomaticP`
`74`	`74`
`75`	`75`	`javascriptClass = classCache.put(`
`76`	`76`	`javaClass.getName(),`
`77`		`- DatabindJavascriptClass.create(configuration, conversionUtils, javaClass)`
	`77`	`+ DatabindJavascriptClass.create(configuration, conversionUtils, javaClass, classCache)`
`78`	`78`	`.bake());`
`79`	`79`	`}`
`80`	`80`