This is an automated email from the ASF dual-hosted git repository. sunlan pushed a commit to branch GROOVY-10747 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit a737971b1d0bde7d12cba3efd3eedc37b848e03c Author: Daniel Sun <[email protected]> AuthorDate: Sun Sep 4 15:39:13 2022 +0800 GROOVY-10747: Fix illegal access for object clone on JDK16+ --- .../java/org/apache/groovy/runtime/ObjectUtil.java | 85 ++++++++++++++++++++++ .../org/codehaus/groovy/runtime/InvokerHelper.java | 15 +++- .../org/codehaus/groovy/vmplugin/v8/Selector.java | 12 ++- src/test/groovy/bugs/Groovy9103.groovy | 33 ++++++++- 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/groovy/runtime/ObjectUtil.java b/src/main/java/org/apache/groovy/runtime/ObjectUtil.java new file mode 100644 index 0000000000..f0f47c000c --- /dev/null +++ b/src/main/java/org/apache/groovy/runtime/ObjectUtil.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.groovy.runtime; + +import org.codehaus.groovy.GroovyBugError; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Method; + +/** + * Util for object's operations with checks + * @since 4.0.5 + */ +public class ObjectUtil { + /** + * Clone the specified object + * + * @param object the object to clone + * @return the cloned object + * @param <T> the object type + * @throws Throwable some exception or error + * @since 4.0.5 + */ + @SuppressWarnings("unchecked") + public static <T> T cloneObject(T object) throws Throwable { + if (null == object) return null; + if (!(object instanceof Cloneable)) throw new CloneNotSupportedException(object.getClass().getName()); + + final Method cloneMethod = object.getClass().getMethod("clone"); + final MethodHandle cloneMethodHandle = LOOKUP.in(object.getClass()).unreflect(cloneMethod); + + return (T) cloneMethodHandle.invokeWithArguments(object); + } + + /** + * Returns the method handle of {@link ObjectUtil#cloneObject(Object)} + * + * @return the method handle + * @since 4.0.5 + */ + public static MethodHandle getCloneObjectMethodHandle() { + return MethodHandleHolder.CLONE_OBJECT_METHOD_HANDLE; + } + + private static class MethodHandleHolder { + private static final MethodHandle CLONE_OBJECT_METHOD_HANDLE; + static { + final Class<ObjectUtil> objectUtilClass = ObjectUtil.class; + Method cloneObjectMethod; + try { + cloneObjectMethod = objectUtilClass.getDeclaredMethod("cloneObject", Object.class); + } catch (NoSuchMethodException e) { + throw new GroovyBugError("Failed to find `cloneObject` method in class `" + objectUtilClass.getName() + "`", e); + } + + try { + CLONE_OBJECT_METHOD_HANDLE = LOOKUP.in(objectUtilClass).unreflect(cloneObjectMethod); + } catch (IllegalAccessException e) { + throw new GroovyBugError("Failed to create method handle for " + cloneObjectMethod); + } + } + private MethodHandleHolder() {} + } + + private ObjectUtil() {} + + private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); +} diff --git a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java index 40672f84c0..69d77eea97 100644 --- a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java +++ b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java @@ -33,6 +33,8 @@ import groovy.lang.Script; import groovy.lang.SpreadMap; import groovy.lang.SpreadMapEvaluatingException; import groovy.lang.Tuple; +import org.apache.groovy.internal.util.UncheckedThrow; +import org.apache.groovy.runtime.ObjectUtil; import org.codehaus.groovy.reflection.ClassInfo; import org.codehaus.groovy.runtime.metaclass.MetaClassRegistryImpl; import org.codehaus.groovy.runtime.metaclass.MissingMethodExecutionFailed; @@ -586,10 +588,6 @@ public class InvokerHelper { // it's an instance; check if it's a Java one if (!(object instanceof GroovyObject)) { - if ("clone".equals(methodName) && object.getClass().isArray() && (null == arguments || arguments.getClass().isArray() && 0 == ((Object[]) arguments).length)) { - return ArrayUtil.cloneArray((Object[]) object); - } - return invokePojoMethod(object, methodName, arguments); } @@ -598,6 +596,15 @@ public class InvokerHelper { } static Object invokePojoMethod(Object object, String methodName, Object arguments) { + if ("clone".equals(methodName) && (null == arguments || arguments.getClass().isArray() && 0 == ((Object[]) arguments).length)) { + if (object.getClass().isArray()) return ArrayUtil.cloneArray((Object[]) object); + try { + return ObjectUtil.cloneObject(object); + } catch (Throwable t) { + UncheckedThrow.rethrow(t); + } + } + MetaClass metaClass = InvokerHelper.getMetaClass(object); return metaClass.invokeMethod(object, methodName, asArray(arguments)); } diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java index ed502974c7..ef34caf430 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java @@ -32,6 +32,7 @@ import groovy.lang.MetaMethod; import groovy.lang.MetaProperty; import groovy.lang.MissingMethodException; import groovy.transform.Internal; +import org.apache.groovy.runtime.ObjectUtil; import org.codehaus.groovy.GroovyBugError; import org.codehaus.groovy.reflection.CachedField; import org.codehaus.groovy.reflection.CachedMethod; @@ -680,11 +681,14 @@ public abstract class Selector { final Class<?> declaringClass = m.getDeclaringClass(); if (declaringClass == Class.class && parameterCount == 1 && methodName.equals("forName")) { return MethodHandles.insertArguments(CLASS_FOR_NAME, 1, true, sender.getClassLoader()); - } else if (declaringClass == Object.class && parameterCount == 0 && methodName.equals("clone") && null != args && 1 == args.length && args[0].getClass().isArray()) { - return ArrayUtil.getCloneArrayMethodHandle(); - } else { - return LOOKUP.unreflect(m); + } else if (declaringClass == Object.class && parameterCount == 0 && methodName.equals("clone") && null != args && 1 == args.length) { + if (args[0].getClass().isArray()) { + return ArrayUtil.getCloneArrayMethodHandle(); + } + return ObjectUtil.getCloneObjectMethodHandle(); } + + return LOOKUP.unreflect(m); } /** diff --git a/src/test/groovy/bugs/Groovy9103.groovy b/src/test/groovy/bugs/Groovy9103.groovy index 721344f0c6..9e58032556 100644 --- a/src/test/groovy/bugs/Groovy9103.groovy +++ b/src/test/groovy/bugs/Groovy9103.groovy @@ -21,8 +21,8 @@ package groovy.bugs import org.junit.Test import static groovy.test.GroovyAssert.assertScript +import static groovy.test.GroovyAssert.shouldFail -// TODO: add JVM option `--illegal-access=deny` when all warnings fixed final class Groovy9103 { @Test @@ -78,4 +78,35 @@ final class Groovy9103 { assert nums == cloned ''' } + + @Test // GROOVY-10747 + void testClone5() { + ['Object', 'Dolly'].each { typeName -> + assertScript """ + class Dolly implements Cloneable { + String name + + public ${typeName} clone() { + return super.clone() + } + } + + def dolly = new Dolly(name: "The Sheep") + def cloned = dolly.clone() + assert cloned instanceof Dolly + """ + } + } + + @Test // GROOVY-10747 + void testClone6() { + shouldFail(CloneNotSupportedException, ''' + class Dolly { + String name + } + + def dolly = new Dolly(name: "The Sheep") + dolly.clone() + ''') + } }
