From 1ada03e2d2b00331072607a37cf20a1f16df288f Mon Sep 17 00:00:00 2001 From: Rafael Winterhalter Date: Tue, 21 Apr 2015 15:21:38 +0200 Subject: [PATCH] Added on the fly validation for created types. --- .../description/method/MethodDescription.java | 3 +- .../dynamic/scaffold/TypeWriter.java | 251 +++++++++++++++++- .../java/net/bytebuddy/pool/TypePool.java | 14 +- .../scaffold/TypeWriterDefaultTest.java | 141 ++++++++++ .../auxiliary/TrivialTypeTest.java | 4 + .../test/utility/JavaVersionRule.java | 42 ++- 6 files changed, 440 insertions(+), 15 deletions(-) diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/description/method/MethodDescription.java b/byte-buddy-dep/src/main/java/net/bytebuddy/description/method/MethodDescription.java index 9ccc6366aaa..22481365f42 100644 --- a/byte-buddy-dep/src/main/java/net/bytebuddy/description/method/MethodDescription.java +++ b/byte-buddy-dep/src/main/java/net/bytebuddy/description/method/MethodDescription.java @@ -432,8 +432,7 @@ public boolean isBootstrap(List arguments) { @Override public boolean isDefaultValue() { - return getDeclaringType().isAnnotation() - && !isConstructor() + return !isConstructor() && !isStatic() && getReturnType().isAnnotationReturnType() && getParameters().isEmpty(); diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/TypeWriter.java b/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/TypeWriter.java index 58757897de0..a62348f8b2a 100644 --- a/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/TypeWriter.java +++ b/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/TypeWriter.java @@ -891,6 +891,253 @@ public int hashCode() { */ protected abstract byte[] create(Implementation.Context.ExtractableView implementationContext); + /** + * A class validator that validates that a class only defines members that are appropriate for the sort of the generated class. + */ + protected static class ValidatingClassVisitor extends ClassVisitor { + + /** + * The constraint to assert the members against. The constraint is first defined when the general class information is visited. + */ + private Constraint constraint; + + /** + * Creates a validating class visitor. + * + * @param classVisitor The class visitor to which any calls are delegated to. + */ + protected ValidatingClassVisitor(ClassVisitor classVisitor) { + super(ASM_API_VERSION, classVisitor); + } + + @Override + public void visit(int version, int modifier, String name, String signature, String superName, String[] interfaces) { + ClassFileVersion classFileVersion = new ClassFileVersion(version); + if ((modifier & Opcodes.ACC_ANNOTATION) != 0) { + constraint = classFileVersion.isSupportsDefaultMethods() + ? Constraint.JAVA8_ANNOTATION + : Constraint.ANNOTATION; + } else if ((modifier & Opcodes.ACC_INTERFACE) != 0) { + constraint = classFileVersion.isSupportsDefaultMethods() + ? Constraint.JAVA8_INTERFACE + : Constraint.INTERFACE; + } else if ((modifier & Opcodes.ACC_ABSTRACT) != 0) { + constraint = Constraint.ABSTRACT_CLASS; + } else { + constraint = Constraint.MANIFEST_CLASS; + } + super.visit(version, modifier, name, signature, superName, interfaces); + } + + @Override + public FieldVisitor visitField(int modifiers, String name, String desc, String signature, Object defaultValue) { + constraint.assertField(name, (modifiers & Opcodes.ACC_PUBLIC) != 0, (modifiers & Opcodes.ACC_STATIC) != 0); + return super.visitField(modifiers, name, desc, signature, defaultValue); + } + + @Override + public MethodVisitor visitMethod(int modifiers, String name, String descriptor, String signature, String[] exceptions) { + constraint.assertMethod(name, + (modifiers & Opcodes.ACC_ABSTRACT) != 0, + (modifiers & Opcodes.ACC_PUBLIC) != 0, + (modifiers & Opcodes.ACC_STATIC) != 0); + return new ValidatingMethodVisitor(super.visitMethod(modifiers, name, descriptor, signature, exceptions), name); + } + + @Override + public String toString() { + return "TypeWriter.Default.ValidatingClassVisitor{" + + "constraint=" + constraint + + "}"; + } + + /** + * A method validator for checking default values. + */ + protected class ValidatingMethodVisitor extends MethodVisitor { + + /** + * The name of the method being visited. + */ + private final String name; + + /** + * Creates a validating method visitor. + * + * @param methodVisitor The method visitor to which any calls are delegated to. + * @param name The name of the method being visited. + */ + protected ValidatingMethodVisitor(MethodVisitor methodVisitor, String name) { + super(ASM_API_VERSION, methodVisitor); + this.name = name; + } + + @Override + public AnnotationVisitor visitAnnotationDefault() { + constraint.assertDefault(name); + return super.visitAnnotationDefault(); + } + + @Override + public String toString() { + return "TypeWriter.Default.ValidatingClassVisitor.ValidatingMethodVisitor{" + + "classVisitor=" + ValidatingClassVisitor.this + + ", name='" + name + '\'' + + '}'; + } + } + + /** + * A constraint for members that are legal for a given type. + */ + protected enum Constraint { + + /** + * Constraints for a non-abstract class. + */ + MANIFEST_CLASS("non-abstract class", true, true, true, false, true, false), + + /** + * Constraints for an abstract class. + */ + ABSTRACT_CLASS("abstract class", true, true, true, true, true, false), + + /** + * Constrains for an interface type before Java 8. + */ + INTERFACE("interface", false, false, false, true, false, false), + + /** + * Constrains for an interface type since Java 8. + */ + JAVA8_INTERFACE("interface (Java 8+)", false, false, true, true, true, false), + + /** + * Constrains for an annotation type before Java 8. + */ + ANNOTATION("annotation", false, false, false, true, false, true), + + /** + * Constrains for an annotation type since Java 8. + */ + JAVA8_ANNOTATION("annotation (Java 8+)", false, false, true, true, false, true); + + /** + * A name to represent the type being validated within an error message. + */ + private final String sortName; + + /** + * Determines if a sort allows non-public members. + */ + private final boolean allowsNonPublic; + + /** + * Determines if a sort allows non-static fields. + */ + private final boolean allowsNonStaticFields; + + /** + * Determines if a sort allows static methods. + */ + private final boolean allowsStaticMethods; + + /** + * Determines if a sort allows abstract methods. + */ + private final boolean allowsAbstract; + + /** + * Determines if a sort allows non-abstract methods. + */ + private final boolean allowsNonAbstract; + + /** + * Determines if a sort allows the definition of annotation default values. + */ + private final boolean allowsDefaultValue; + + + /** + * Creates a new constraint. + * + * @param sortName A name to represent the type being validated within an error message. + * @param allowsNonPublic Determines if a sort allows non-public members. + * @param allowsNonStaticFields Determines if a sort allows non-static fields. + * @param allowsStaticMethods Determines if a sort allows static methods. + * @param allowsAbstract Determines if a sort allows abstract methods. + * @param allowsNonAbstract Determines if a sort allows non-abstract methods. + * @param allowsDefaultValue Determines if a sort allows the definition of annotation default values. + */ + Constraint(String sortName, + boolean allowsNonPublic, + boolean allowsNonStaticFields, + boolean allowsStaticMethods, + boolean allowsAbstract, + boolean allowsNonAbstract, + boolean allowsDefaultValue) { + this.sortName = sortName; + this.allowsNonPublic = allowsNonPublic; + this.allowsNonStaticFields = allowsNonStaticFields; + this.allowsStaticMethods = allowsStaticMethods; + this.allowsAbstract = allowsAbstract; + this.allowsNonAbstract = allowsNonAbstract; + this.allowsDefaultValue = allowsDefaultValue; + } + + /** + * Asserts a field for being valid. + * + * @param name The name of the field. + * @param isPublic {@code true} if this field is public. + * @param isStatic {@code true} if this field is static. + */ + protected void assertField(String name, boolean isPublic, boolean isStatic) { + if (!isPublic && !allowsNonPublic) { + throw new IllegalStateException("Cannot define non-public field " + name + " for " + sortName); + } else if (!isStatic && !allowsNonStaticFields) { + throw new IllegalStateException("Cannot define for non-static field " + name + " for " + sortName); + } + } + + /** + * Asserts a method for being valid. + * + * @param name The name of the method. + * @param isAbstract {@code true} if the method is abstract. + * @param isPublic {@code true} if this method is public. + * @param isStatic {@code true} if this method is static. + */ + protected void assertMethod(String name, boolean isAbstract, boolean isPublic, boolean isStatic) { + if (!isPublic && !allowsNonPublic) { + throw new IllegalStateException("Cannot define non-public method " + name + " for " + sortName); + } else if (isStatic && !allowsStaticMethods) { + throw new IllegalStateException("Cannot define static method " + name + " for " + sortName); + } else if (!isStatic && isAbstract && !allowsAbstract) { + throw new IllegalStateException("Cannot define abstract method " + name + " for " + sortName); + } else if (!isAbstract && !allowsNonAbstract) { + throw new IllegalStateException("Cannot define non-abstract method " + name + " + for " + sortName); + } + } + + /** + * Asserts if a default value is legal for a method. + * + * @param name The name of the method. + */ + protected void assertDefault(String name) { + if (!allowsDefaultValue) { + throw new IllegalStateException("Cannot define define default value on " + name + " for " + sortName); + } + } + + @Override + public String toString() { + return "TypeWriter.Default.ValidatingClassVisitor.Constraint." + name(); + } + } + } + /** * A type writer that inlines the created type into an existing class file. * @@ -1004,7 +1251,7 @@ public byte[] create(Implementation.Context.ExtractableView implementationContex private byte[] doCreate(Implementation.Context.ExtractableView implementationContext, byte[] binaryRepresentation) { ClassReader classReader = new ClassReader(binaryRepresentation); ClassWriter classWriter = new ClassWriter(classReader, ASM_MANUAL_FLAG); - classReader.accept(writeTo(classVisitorWrapper.wrap(classWriter), implementationContext), ASM_MANUAL_FLAG); + classReader.accept(writeTo(classVisitorWrapper.wrap(new ValidatingClassVisitor(classWriter)), implementationContext), ASM_MANUAL_FLAG); return classWriter.toByteArray(); } @@ -1474,7 +1721,7 @@ protected ForCreation(TypeDescription instrumentedType, @Override public byte[] create(Implementation.Context.ExtractableView implementationContext) { ClassWriter classWriter = new ClassWriter(ASM_MANUAL_FLAG); - ClassVisitor classVisitor = classVisitorWrapper.wrap(classWriter); + ClassVisitor classVisitor = classVisitorWrapper.wrap(new ValidatingClassVisitor(classWriter)); classVisitor.visit(classFileVersion.getVersionNumber(), instrumentedType.getActualModifiers(!instrumentedType.isInterface()), instrumentedType.getInternalName(), diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/pool/TypePool.java b/byte-buddy-dep/src/main/java/net/bytebuddy/pool/TypePool.java index 8a0505db7ad..c9cb1271d9b 100644 --- a/byte-buddy-dep/src/main/java/net/bytebuddy/pool/TypePool.java +++ b/byte-buddy-dep/src/main/java/net/bytebuddy/pool/TypePool.java @@ -925,12 +925,12 @@ class Default extends AbstractBase { /** * The ASM version that is applied when reading class files. */ - private static final int ASM_VERSION = Opcodes.ASM5; + private static final int ASM_API_VERSION = Opcodes.ASM5; /** * A flag to indicate ASM that no automatic calculations are requested. */ - private static final int ASM_MANUAL = 0; + private static final int ASM_MANUAL_FLAG = 0; /** * The locator to query for finding binary data of a type. @@ -979,7 +979,7 @@ protected Resolution doDescribe(String name) { private TypeDescription parse(byte[] binaryRepresentation) { ClassReader classReader = new ClassReader(binaryRepresentation); TypeExtractor typeExtractor = new TypeExtractor(); - classReader.accept(typeExtractor, ASM_MANUAL); + classReader.accept(typeExtractor, ASM_MANUAL_FLAG); return typeExtractor.toTypeDescription(); } @@ -1350,7 +1350,7 @@ protected class TypeExtractor extends ClassVisitor { * Creates a new type extractor. */ protected TypeExtractor() { - super(ASM_VERSION); + super(ASM_API_VERSION); annotationTokens = new LinkedList(); fieldTokens = new LinkedList(); methodTokens = new LinkedList(); @@ -1528,7 +1528,7 @@ protected class AnnotationExtractor extends AnnotationVisitor { */ protected AnnotationExtractor(AnnotationRegistrant annotationRegistrant, ComponentTypeLocator componentTypeLocator) { - super(ASM_VERSION); + super(ASM_API_VERSION); this.annotationRegistrant = annotationRegistrant; this.componentTypeLocator = componentTypeLocator; } @@ -1734,7 +1734,7 @@ protected FieldExtractor(int modifiers, String internalName, String descriptor, String genericSignature) { - super(ASM_VERSION); + super(ASM_API_VERSION); this.modifiers = modifiers; this.internalName = internalName; this.descriptor = descriptor; @@ -1895,7 +1895,7 @@ protected MethodExtractor(int modifiers, String descriptor, String genericSignature, String[] exceptionName) { - super(ASM_VERSION); + super(ASM_API_VERSION); this.modifiers = modifiers; this.internalName = internalName; this.descriptor = descriptor; diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/dynamic/scaffold/TypeWriterDefaultTest.java b/byte-buddy-dep/src/test/java/net/bytebuddy/dynamic/scaffold/TypeWriterDefaultTest.java index 911334a10f4..13c61da4dc0 100644 --- a/byte-buddy-dep/src/test/java/net/bytebuddy/dynamic/scaffold/TypeWriterDefaultTest.java +++ b/byte-buddy-dep/src/test/java/net/bytebuddy/dynamic/scaffold/TypeWriterDefaultTest.java @@ -1,13 +1,154 @@ package net.bytebuddy.dynamic.scaffold; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.modifier.Ownership; +import net.bytebuddy.description.modifier.TypeManifestation; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.test.utility.JavaVersionRule; import net.bytebuddy.test.utility.ObjectPropertyAssertion; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.MethodRule; + +import java.security.acl.Owner; +import java.util.Collections; public class TypeWriterDefaultTest { + private static final String FOO = "foo", BAR = "bar"; + + @Rule + public MethodRule javaVersionRule = new JavaVersionRule(); + + @Test(expected = IllegalStateException.class) + public void testAbstractMethodOnNonAbstractClassAssertion() throws Exception { + new ByteBuddy() + .subclass(Object.class) + .defineMethod(FOO, String.class, Collections.>emptyList()) + .withoutCode() + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonPublicFieldOnInterfaceAssertion() throws Exception { + new ByteBuddy() + .makeInterface() + .defineField(FOO, String.class, Ownership.STATIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonPublicFieldOnAnnotationAssertion() throws Exception { + new ByteBuddy() + .makeAnnotation() + .defineField(FOO, String.class, Ownership.STATIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonStaticFieldOnInterfaceAssertion() throws Exception { + new ByteBuddy() + .makeInterface() + .defineField(FOO, String.class, Visibility.PUBLIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonStaticFieldOnAnnotationAssertion() throws Exception { + new ByteBuddy() + .makeAnnotation() + .defineField(FOO, String.class, Visibility.PUBLIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonPublicMethodOnInterfaceAssertion() throws Exception { + new ByteBuddy() + .makeInterface() + .defineMethod(FOO, void.class, Collections.>emptyList()) + .withoutCode() + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testNonPublicMethodOnAnnotationAssertion() throws Exception { + new ByteBuddy() + .makeAnnotation() + .defineMethod(FOO, void.class, Collections.>emptyList()) + .withoutCode() + .make(); + } + + @Test(expected = IllegalStateException.class) + @JavaVersionRule.Enforce(value = 8, type = JavaVersionRule.Type.LESS_THEN) + public void testStaticMethodOnInterfaceAssertion() throws Exception { + new ByteBuddy() + .makeInterface() + .defineField(FOO, String.class, Visibility.PUBLIC, Ownership.STATIC) + .make(); + } + + @Test + @JavaVersionRule.Enforce(value = 8) + public void testStaticMethodOnAnnotationAssertionJava8() throws Exception { + new ByteBuddy() + .makeInterface() + .defineField(FOO, String.class, Visibility.PUBLIC, Ownership.STATIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + @JavaVersionRule.Enforce(value = 8, type = JavaVersionRule.Type.LESS_THEN) + public void testStaticMethodOnAnnotationAssertion() throws Exception { + new ByteBuddy() + .makeAnnotation() + .defineField(FOO, String.class, Visibility.PUBLIC, Ownership.STATIC) + .make(); + } + + @Test + @JavaVersionRule.Enforce(value = 8) + public void testStaticMethodOnInterfaceAssertionJava8() throws Exception { + new ByteBuddy() + .makeAnnotation() + .defineField(FOO, String.class, Visibility.PUBLIC, Ownership.STATIC) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testAnnotationOnClassAssertion() throws Exception { + new ByteBuddy() + .subclass(Object.class) + .defineMethod(FOO, String.class, Collections.>emptyList()) + .withDefaultValue(BAR) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testAnnotationOnAbstractClassAssertion() throws Exception { + new ByteBuddy() + .subclass(Object.class) + .modifiers(TypeManifestation.ABSTRACT) + .defineMethod(FOO, String.class, Collections.>emptyList()) + .withDefaultValue(BAR) + .make(); + } + + @Test(expected = IllegalStateException.class) + public void testAnnotationOnInterfaceClassAssertion() throws Exception { + new ByteBuddy() + .subclass(Object.class) + .modifiers(TypeManifestation.INTERFACE) + .defineMethod(FOO, String.class, Collections.>emptyList()) + .withDefaultValue(BAR) + .make(); + } + @Test public void testObjectProperties() throws Exception { ObjectPropertyAssertion.of(TypeWriter.Default.ForCreation.class).apply(); ObjectPropertyAssertion.of(TypeWriter.Default.ForInlining.class).apply(); + ObjectPropertyAssertion.of(TypeWriter.Default.ValidatingClassVisitor.class).applyMutable(); + ObjectPropertyAssertion.of(TypeWriter.Default.ValidatingClassVisitor.ValidatingMethodVisitor.class).applyMutable(); } } diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/implementation/auxiliary/TrivialTypeTest.java b/byte-buddy-dep/src/test/java/net/bytebuddy/implementation/auxiliary/TrivialTypeTest.java index 4779da49e7f..15c10675de4 100644 --- a/byte-buddy-dep/src/test/java/net/bytebuddy/implementation/auxiliary/TrivialTypeTest.java +++ b/byte-buddy-dep/src/test/java/net/bytebuddy/implementation/auxiliary/TrivialTypeTest.java @@ -12,11 +12,14 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.Mockito.when; public class TrivialTypeTest { private static final String FOO = "foo"; + private static final int BAR = 42; + @Rule public TestRule mockitoRule = new MockitoRule(this); @@ -28,6 +31,7 @@ public class TrivialTypeTest { @Test public void testCreation() throws Exception { + when(classFileVersion.getVersionNumber()).thenReturn(BAR); DynamicType dynamicType = TrivialType.INSTANCE.make(FOO, classFileVersion, methodAccessorFactory); assertThat(dynamicType.getTypeDescription().getName(), is(FOO)); assertThat(dynamicType.getTypeDescription().getModifiers(), is(Opcodes.ACC_SYNTHETIC)); diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/test/utility/JavaVersionRule.java b/byte-buddy-dep/src/test/java/net/bytebuddy/test/utility/JavaVersionRule.java index 927cdbf20a0..61dd01d8905 100644 --- a/byte-buddy-dep/src/test/java/net/bytebuddy/test/utility/JavaVersionRule.java +++ b/byte-buddy-dep/src/test/java/net/bytebuddy/test/utility/JavaVersionRule.java @@ -22,9 +22,9 @@ public JavaVersionRule() { @Override public Statement apply(Statement base, FrameworkMethod method, Object target) { Enforce enforce = method.getAnnotation(Enforce.class); - return enforce == null || ClassFileVersion.forKnownJavaVersion(enforce.value()).compareTo(supportedVersion) <= 0 + return enforce == null || enforce.type().matches(ClassFileVersion.forKnownJavaVersion(enforce.value()).compareTo(supportedVersion)) ? base - : new NoOpStatement(method.getAnnotation(Enforce.class).value()); + : new NoOpStatement(enforce.value(), enforce.type()); } @Retention(RetentionPolicy.RUNTIME) @@ -32,19 +32,53 @@ public Statement apply(Statement base, FrameworkMethod method, Object target) { public @interface Enforce { int value(); + + Type type() default Type.AT_LEAST; } private class NoOpStatement extends Statement { private final int requiredVersion; - public NoOpStatement(int requiredVersion) { + private final Type type; + + public NoOpStatement(int requiredVersion, Type type) { this.requiredVersion = requiredVersion; + this.type = type; } @Override public void evaluate() throws Throwable { - Logger.getAnonymousLogger().warning("Ignored test case that requires a Java version " + requiredVersion); + Logger.getAnonymousLogger().warning("Ignored test case that requires a Java version " + type.toMessageString() + " " + requiredVersion); + } + } + + public enum Type { + + AT_LEAST("of at least") { + @Override + protected boolean matches(int comparison) { + return comparison <= 0; + } + }, + + LESS_THEN("less than") { + @Override + protected boolean matches(int comparison) { + return comparison > 0; + } + }; + + protected abstract boolean matches(int comparison); + + private final String message; + + Type(String message) { + this.message = message; + } + + public String toMessageString() { + return message; } } }