8255560: Class::isRecord should check that the current class is final and not abstract

Reviewed-by: mchung, darcy
This commit is contained in:
Chris Hegarty 2020-12-07 11:02:52 +00:00
parent 8e8e584552
commit 5a03e47605
3 changed files with 227 additions and 22 deletions

View File

@ -3662,9 +3662,10 @@ public final class Class<T> implements java.io.Serializable,
* Returns {@code true} if and only if this class is a record class.
*
* <p> The {@linkplain #getSuperclass() direct superclass} of a record
* class is {@code java.lang.Record}. A record class has (possibly zero)
* record components, that is, {@link #getRecordComponents()} returns a
* non-null value.
* class is {@code java.lang.Record}. A record class is {@linkplain
* Modifier#FINAL final}. A record class has (possibly zero) record
* components; {@link #getRecordComponents()} returns a non-null but
* possibly empty value for a record.
*
* <p> Note that class {@link Record} is not a record type and thus invoking
* this method on class {@code Record} returns {@code false}.
@ -3674,7 +3675,9 @@ public final class Class<T> implements java.io.Serializable,
* @since 16
*/
public boolean isRecord() {
return getSuperclass() == java.lang.Record.class && isRecord0();
return getSuperclass() == java.lang.Record.class &&
(this.getModifiers() & Modifier.FINAL) != 0 &&
isRecord0();
}
// Fetches the factory for reflective objects

View File

@ -0,0 +1,189 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8255560
* @summary Class::isRecord should check that the current class is final and not abstract
* @modules java.base/jdk.internal.org.objectweb.asm
* @library /test/lib
* @run testng/othervm IsRecordTest
* @run testng/othervm/java.security.policy=allPermissions.policy IsRecordTest
*/
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.List;
import java.util.Map;
import jdk.internal.org.objectweb.asm.ClassWriter;
import jdk.internal.org.objectweb.asm.Opcodes;
import jdk.test.lib.ByteCodeLoader;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static java.lang.System.out;
import static jdk.internal.org.objectweb.asm.ClassWriter.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
public class IsRecordTest {
@DataProvider(name = "scenarios")
public Object[][] scenarios() {
return new Object[][] {
// isFinal, isAbstract, extendJLR, withRecAttr, expectIsRecord
{ false, false, false, true, false },
{ false, false, true, true, false },
{ false, true, false, true, false },
{ false, true, true, true, false },
{ true, false, false, true, false },
{ true, false, true, true, true },
{ false, false, false, false, false },
{ false, false, true, false, false },
{ false, true, false, false, false },
{ false, true, true, false, false },
{ true, false, false, false, false },
{ true, false, true, false, false },
};
}
/**
* Tests the valid combinations of i) final/non-final, ii) abstract/non-abstract,
* iii) direct subclass of j.l.Record (or not), along with the presence or
* absence of a record attribute.
*/
@Test(dataProvider = "scenarios")
public void testDirectSubClass(boolean isFinal,
boolean isAbstract,
boolean extendsJLR,
boolean withRecordAttr,
boolean expectIsRecord) throws Exception {
out.println("\n--- testDirectSubClass isFinal=%s, isAbstract=%s, extendsJLR=%s, withRecordAttr=%s, expectIsRecord=%s ---"
.formatted(isFinal, isAbstract, extendsJLR, withRecordAttr, expectIsRecord));
List<RecordComponentEntry> rc = null;
if (withRecordAttr)
rc = List.of(new RecordComponentEntry("x", "I"));
String superName = extendsJLR ? "java/lang/Record" : "java/lang/Object";
var classBytes = generateClassBytes("C", isFinal, isAbstract, superName, rc);
Class<?> cls = ByteCodeLoader.load("C", classBytes);
out.println("cls=%s, Record::isAssignable=%s, isRecord=%s"
.formatted(cls, Record.class.isAssignableFrom(cls), cls.isRecord()));
assertEquals(cls.isRecord(), expectIsRecord);
var getRecordComponents = cls.getRecordComponents();
assertTrue(expectIsRecord ? getRecordComponents != null : getRecordComponents == null);
}
/**
* Tests the valid combinations of i) final/non-final, ii) abstract/non-abstract,
* along with the presence or absence of a record attribute, where the class has
* a superclass whose superclass is j.l.Record.
*/
@Test(dataProvider = "scenarios")
public void testIndirectSubClass(boolean isFinal,
boolean isAbstract,
boolean unused1,
boolean withRecordAttr,
boolean unused2) throws Exception {
out.println("\n--- testIndirectSubClass isFinal=%s, isAbstract=%s withRecordAttr=%s ---"
.formatted(isFinal, isAbstract, withRecordAttr));
List<RecordComponentEntry> rc = null;
if (withRecordAttr)
rc = List.of(new RecordComponentEntry("x", "I"));
var supFooClassBytes = generateClassBytes("SupFoo", false, isAbstract, "java/lang/Record", rc);
var subFooClassBytes = generateClassBytes("SubFoo", isFinal, isAbstract, "SupFoo", rc);
var allClassBytes = Map.of("SupFoo", supFooClassBytes,
"SubFoo", subFooClassBytes);
ClassLoader loader = new ByteCodeLoader(allClassBytes, null);
Class<?> supFooCls = loader.loadClass("SupFoo");
Class<?> subFooCls = loader.loadClass("SubFoo");
for (var cls : List.of(supFooCls, subFooCls))
out.println("cls=%s, Record::isAssignable=%s, isRecord=%s"
.formatted(cls, Record.class.isAssignableFrom(cls), cls.isRecord()));
assertFalse(supFooCls.isRecord());
assertFalse(subFooCls.isRecord());
assertEquals(supFooCls.getRecordComponents(), null);
assertEquals(subFooCls.getRecordComponents(), null);
}
/** Tests record-ness properties of traditionally compiled classes. */
@Test
public void testBasicRecords() {
out.println("\n--- testBasicRecords ---");
record EmptyRecord () { }
assertTrue(EmptyRecord.class.isRecord());
assertEquals(EmptyRecord.class.getRecordComponents().length, 0);
record FooRecord (int x) { }
assertTrue(FooRecord.class.isRecord());
assertTrue(FooRecord.class.getRecordComponents() != null);
final record FinalFooRecord (int x) { }
assertTrue(FinalFooRecord.class.isRecord());
assertTrue(FinalFooRecord.class.getRecordComponents() != null);
class A { }
assertFalse(A.class.isRecord());
assertFalse(A.class.getRecordComponents() != null);
final class B { }
assertFalse(B.class.isRecord());
assertFalse(B.class.getRecordComponents() != null);
}
// -- infra
// Generates a class with the given properties.
byte[] generateClassBytes(String className,
boolean isFinal,
boolean isAbstract,
String superName,
List<RecordComponentEntry> components) {
ClassWriter cw = new ClassWriter(COMPUTE_MAXS | COMPUTE_FRAMES);
int access = 0;
if (isFinal)
access = access | Opcodes.ACC_FINAL;
if (isAbstract)
access = access | Opcodes.ACC_ABSTRACT;
cw.visit(Opcodes.V16,
access,
className,
null,
superName,
null);
if (components != null)
components.forEach(rc -> cw.visitRecordComponent(rc.name(), rc.descriptor(), null));
cw.visitEnd();
return cw.toByteArray();
}
record RecordComponentEntry (String name, String descriptor) { }
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -24,6 +24,8 @@
package jdk.test.lib;
import java.security.SecureClassLoader;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
/**
* {@code ByteCodeLoader} can be used for easy loading of byte code already
@ -35,9 +37,14 @@ import java.security.SecureClassLoader;
* @see InMemoryCompiler
*/
public class ByteCodeLoader extends SecureClassLoader {
private final String className;
private final byte[] byteCode;
private volatile Class<?> holder;
private final Map<String,byte[]> classBytesMap;
private final Map<String,Class<?>> cache;
public ByteCodeLoader(Map<String,byte[]> classBytesMap, ClassLoader parent) {
super(parent);
this.classBytesMap = classBytesMap;
cache = new ConcurrentHashMap<>();
}
/**
* Creates a new {@code ByteCodeLoader} ready to load a class with the
@ -48,8 +55,9 @@ public class ByteCodeLoader extends SecureClassLoader {
* @param byteCode The byte code of the class
*/
public ByteCodeLoader(String className, byte[] byteCode) {
this.className = className;
this.byteCode = byteCode;
super();
classBytesMap = Map.of(className, byteCode);
cache = new ConcurrentHashMap<>();
}
/**
@ -59,34 +67,39 @@ public class ByteCodeLoader extends SecureClassLoader {
*
* @param className The name of the class
* @param byteCode The byte code of the class
* @param parent The parent class loader for delegation
*/
public ByteCodeLoader(String className, byte[] byteCode, ClassLoader parent) {
super(parent);
this.className = className;
this.byteCode = byteCode;
this(Map.of(className, byteCode), parent);
}
private static final Object lock = new Object();
@Override
public Class<?> loadClass(String name) throws ClassNotFoundException {
if (!name.equals(className)) {
if (classBytesMap.get(name) == null) {
return super.loadClass(name);
}
if (holder == null) {
synchronized(this) {
if (holder == null) {
holder = findClass(name);
}
Class<?> cls = cache.get(name);
if (cls != null) {
return cls;
}
synchronized (lock) {
cls = cache.get(name);
if (cls == null) {
cls = findClass(name);
cache.put(name, cls);
}
}
return holder;
return cls;
}
@Override
protected Class<?> findClass(String name) throws ClassNotFoundException {
if (!name.equals(className)) {
byte[] byteCode = classBytesMap.get(name);
if (byteCode == null) {
throw new ClassNotFoundException(name);
}
return defineClass(name, byteCode, 0, byteCode.length);
}