Unified modifiers for fields generated by Byte Buddy's interceptors t…

…o be synthetic and public in order to avoid requiring accessibility what could break security managers.

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/FieldAccessor.java

@@ -16,6 +16,7 @@
 import net.bytebuddy.implementation.bytecode.member.MethodVariableAccess;
 import net.bytebuddy.utility.ByteBuddyCommons;
 import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
 
 import static net.bytebuddy.matcher.ElementMatchers.*;
 import static net.bytebuddy.utility.ByteBuddyCommons.*;
@@ -869,6 +870,9 @@ public static PreparationHandler of(String name, TypeDescription typeDescription
 
                 @Override
                 public InstrumentedType prepare(InstrumentedType instrumentedType) {
+                    if (!instrumentedType.isClassType() && ((modifiers & Opcodes.ACC_PUBLIC) == 0 || (modifiers & Opcodes.ACC_STATIC) == 0)) {
+                        throw new IllegalStateException("Cannot define a non-public, non-static field for " + instrumentedType);
+                    }
                     return instrumentedType.withField(new FieldDescription.Token(name, modifiers, typeDescription));
                 }
 

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/FixedValue.java

@@ -418,8 +418,8 @@ public Implementation withAssigner(Assigner assigner, Assigner.Typing typing) {
         @Override
         public InstrumentedType prepare(InstrumentedType instrumentedType) {
             return instrumentedType
-                    .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC, fieldType))
-                    .withInitializer(LoadedTypeInitializer.ForStaticField.nonAccessible(fieldName, fixedValue));
+                    .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC, fieldType))
+                    .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(fieldName, fixedValue));
         }
 
         @Override

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/Forwarding.java

@@ -217,7 +217,10 @@ enum ForInstanceField implements PreparationHandler {
 
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType, String fieldName, TypeDescription fieldType) {
-                return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_PRIVATE, fieldType));
+                if (!instrumentedType.isClassType()) {
+                    throw new IllegalStateException("Cannot define instance field '" + fieldName + "' for " + instrumentedType);
+                }
+                return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC, fieldType));
             }
 
             @Override
@@ -243,7 +246,9 @@ enum ForStaticField implements PreparationHandler {
 
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType, String fieldName, TypeDescription fieldType) {
-                return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC, fieldType));
+                return instrumentedType.withField(new FieldDescription.Token(fieldName,
+                        Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
+                        fieldType));
             }
 
             @Override
@@ -279,8 +284,8 @@ public ForStaticInstance(Object target) {
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType, String fieldName, TypeDescription fieldType) {
                 return instrumentedType
-                        .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC, fieldType))
-                        .withInitializer(new LoadedTypeInitializer.ForStaticField<Object>(fieldName, target, true));
+                        .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, fieldType))
+                        .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(fieldName, target));
             }
 
             @Override
@@ -327,9 +332,7 @@ private Appender(StackManipulation delegateLoadingInstruction) {
         }
 
         @Override
-        public Size apply(MethodVisitor methodVisitor,
-                          Context implementationContext,
-                          MethodDescription instrumentedMethod) {
+        public Size apply(MethodVisitor methodVisitor, Context implementationContext, MethodDescription instrumentedMethod) {
             if (!instrumentedMethod.isInvokableOn(fieldType)) {
                 throw new IllegalArgumentException("Cannot forward " + instrumentedMethod + " to " + fieldType);
             } else if (instrumentedMethod.isStatic()) {

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/InvocationHandlerAdapter.java

@@ -36,6 +36,11 @@ public abstract class InvocationHandlerAdapter implements Implementation {
      */
     private static final boolean NO_CACHING = false;
 
+    /**
+     * Indicates that a {@link java.lang.reflect.Method} should be cached.
+     */
+    protected static final boolean CACHING = true;
+
     /**
      * The prefix for field that are created for storing the instrumented value.
      */
@@ -234,17 +239,14 @@ protected static class ForStaticDelegation extends InvocationHandlerAdapter impl
          * @param assigner          The assigner to apply when defining this implementation.
          * @param invocationHandler The invocation handler to which all method calls are delegated.
          */
-        protected ForStaticDelegation(String fieldName,
-                                      boolean cacheMethods,
-                                      Assigner assigner,
-                                      InvocationHandler invocationHandler) {
+        protected ForStaticDelegation(String fieldName, boolean cacheMethods, Assigner assigner, InvocationHandler invocationHandler) {
             super(fieldName, cacheMethods, assigner);
             this.invocationHandler = invocationHandler;
         }
 
         @Override
         public AssignerConfigurable withMethodCache() {
-            return new ForStaticDelegation(fieldName, true, assigner, invocationHandler);
+            return new ForStaticDelegation(fieldName, CACHING, assigner, invocationHandler);
         }
 
         @Override
@@ -255,8 +257,8 @@ public Implementation withAssigner(Assigner assigner) {
         @Override
         public InstrumentedType prepare(InstrumentedType instrumentedType) {
             return instrumentedType
-                    .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_STATIC, INVOCATION_HANDLER_TYPE))
-                    .withInitializer(LoadedTypeInitializer.ForStaticField.nonAccessible(fieldName, invocationHandler));
+                    .withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC, INVOCATION_HANDLER_TYPE))
+                    .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(fieldName, invocationHandler));
         }
 
         @Override
@@ -305,9 +307,7 @@ protected Appender(TypeDescription instrumentedType) {
             }
 
             @Override
-            public Size apply(MethodVisitor methodVisitor,
-                              Context implementationContext,
-                              MethodDescription instrumentedMethod) {
+            public Size apply(MethodVisitor methodVisitor, Context implementationContext, MethodDescription instrumentedMethod) {
                 return ForStaticDelegation.this.apply(methodVisitor,
                         implementationContext,
                         instrumentedMethod,
@@ -367,7 +367,7 @@ protected ForInstanceDelegation(String fieldName, boolean cacheMethods, Assigner
 
         @Override
         public AssignerConfigurable withMethodCache() {
-            return new ForInstanceDelegation(fieldName, true, assigner);
+            return new ForInstanceDelegation(fieldName, CACHING, assigner);
         }
 
         @Override
@@ -377,7 +377,7 @@ public Implementation withAssigner(Assigner assigner) {
 
         @Override
         public InstrumentedType prepare(InstrumentedType instrumentedType) {
-            return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_PUBLIC, INVOCATION_HANDLER_TYPE));
+            return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC, INVOCATION_HANDLER_TYPE));
         }
 
         @Override

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/InvokeDynamic.java

@@ -1531,11 +1531,6 @@ class ForStaticField implements ArgumentProvider {
                  */
                 private static final String FIELD_PREFIX = "invokeDynamic";
 
-                /**
-                 * The field modifier for the randomly created fields.
-                 */
-                private static final int FIELD_MODIFIER = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC;
-
                 /**
                  * The value that is stored in the static field.
                  */
@@ -1587,8 +1582,8 @@ public Resolved resolve(TypeDescription instrumentedType, MethodDescription inst
                 @Override
                 public InstrumentedType prepare(InstrumentedType instrumentedType) {
                     return instrumentedType
-                            .withField(new FieldDescription.Token(name, FIELD_MODIFIER, typeDescription))
-                            .withInitializer(LoadedTypeInitializer.ForStaticField.nonAccessible(name, value));
+                            .withField(new FieldDescription.Token(name, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, typeDescription))
+                            .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(name, value));
                 }
 
                 @Override
@@ -1618,11 +1613,6 @@ public String toString() {
              */
             class ForInstanceField implements ArgumentProvider {
 
-                /**
-                 * The modifier for the generated instance fields.
-                 */
-                private static final int FIELD_MODIFIER = Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC;
-
                 /**
                  * The name of the field.
                  */
@@ -1655,7 +1645,7 @@ public Resolved resolve(TypeDescription instrumentedType, MethodDescription inst
 
                 @Override
                 public InstrumentedType prepare(InstrumentedType instrumentedType) {
-                    return instrumentedType.withField(new FieldDescription.Token(fieldName, FIELD_MODIFIER, fieldType));
+                    return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC, fieldType));
                 }
 
                 @Override

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/MethodCall.java

@@ -664,11 +664,6 @@ class ForStaticField implements TargetHandler {
              */
             private static final String FIELD_PREFIX = "invocationTarget";
 
-            /**
-             * The modifiers of the static field to store the instance.
-             */
-            private static final int FIELD_MODIFIERS = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC;
-
             /**
              * The target on which the method is to be invoked.
              */
@@ -697,8 +692,10 @@ public StackManipulation resolve(MethodDescription methodDescription, TypeDescri
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType) {
                 return instrumentedType
-                        .withField(new FieldDescription.Token(fieldName, FIELD_MODIFIERS, new TypeDescription.ForLoadedType(target.getClass())))
-                        .withInitializer(LoadedTypeInitializer.ForStaticField.nonAccessible(fieldName, target));
+                        .withField(new FieldDescription.Token(fieldName,
+                                Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
+                                new TypeDescription.ForLoadedType(target.getClass())))
+                        .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(fieldName, target));
             }
 
             @Override
@@ -726,11 +723,6 @@ public String toString() {
          */
         class ForInstanceField implements TargetHandler {
 
-            /**
-             * The modifiers of the field.
-             */
-            private static final int FIELD_MODIFIERS = Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC;
-
             /**
              * The name of the field.
              */
@@ -764,7 +756,10 @@ public StackManipulation resolve(MethodDescription methodDescription, TypeDescri
 
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType) {
-                return instrumentedType.withField(new FieldDescription.Token(fieldName, FIELD_MODIFIERS, fieldType));
+                if (!instrumentedType.isClassType()) {
+                    throw new IllegalStateException("Cannot define non-static field '" + fieldName + "' on " + instrumentedType);
+                }
+                return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC, fieldType));
             }
 
             @Override
@@ -1001,11 +996,6 @@ class ForStaticField implements ArgumentLoader {
              */
             private static final String FIELD_PREFIX = "methodCall";
 
-            /**
-             * The modifier of the field.
-             */
-            private static final int FIELD_MODIFIER = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC;
-
             /**
              * The value to be stored in the field.
              */
@@ -1084,7 +1074,9 @@ public StackManipulation resolve(TypeDescription instrumentedType,
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType) {
                 return instrumentedType
-                        .withField(new FieldDescription.Token(fieldName, FIELD_MODIFIER, new TypeDescription.ForLoadedType(value.getClass())))
+                        .withField(new FieldDescription.Token(fieldName,
+                                Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC,
+                                new TypeDescription.ForLoadedType(value.getClass())))
                         .withInitializer(new LoadedTypeInitializer.ForStaticField<Object>(fieldName, value, true));
             }
 
@@ -1113,11 +1105,6 @@ public String toString() {
          */
         class ForInstanceField implements ArgumentLoader {
 
-            /**
-             * The modifier to be applied to the instance field.
-             */
-            private static final int MODIFIERS = Opcodes.ACC_PRIVATE | Opcodes.ACC_SYNTHETIC;
-
             /**
              * The type of the field.
              */
@@ -1153,15 +1140,17 @@ public StackManipulation resolve(TypeDescription instrumentedType,
                         FieldAccess.forField(instrumentedType.getDeclaredFields().filter(named(fieldName)).getOnly()).getter(),
                         assigner.assign(fieldType, targetType, typing));
                 if (!stackManipulation.isValid()) {
-                    throw new IllegalStateException("Cannot assign field " + fieldName + " of type "
-                            + fieldType + " to " + targetType);
+                    throw new IllegalStateException("Cannot assign field " + fieldName + " of type " + fieldType + " to " + targetType);
                 }
                 return stackManipulation;
             }
 
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType) {
-                return instrumentedType.withField(new FieldDescription.Token(fieldName, MODIFIERS, fieldType));
+                if (!instrumentedType.isClassType()) {
+                    throw new IllegalStateException("Cannot define non-static field '" + fieldName + "' for " + instrumentedType);
+                }
+                return instrumentedType.withField(new FieldDescription.Token(fieldName, Opcodes.ACC_SYNTHETIC | Opcodes.ACC_PUBLIC, fieldType));
             }
 
             @Override

byte-buddy-dep/src/main/java/net/bytebuddy/implementation/MethodDelegation.java

@@ -805,11 +805,6 @@ public String toString() {
          */
         class ForStaticField implements ImplementationDelegate {
 
-            /**
-             * The modifier to be assigned to a static field interceptor.
-             */
-            private static final int FIELD_MODIFIERS = Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC;
-
             /**
              * The name prefix for the {@code static} field that is containing the delegation target.
              */
@@ -839,8 +834,10 @@ public ForStaticField(Object delegate, String fieldName) {
             @Override
             public InstrumentedType prepare(InstrumentedType instrumentedType) {
                 return instrumentedType
-                        .withField(new FieldDescription.Token(fieldName, FIELD_MODIFIERS, new TypeDescription.ForLoadedType(delegate.getClass())))
-                        .withInitializer(LoadedTypeInitializer.ForStaticField.nonAccessible(fieldName, delegate));
+                        .withField(new FieldDescription.Token(fieldName,
+                                Opcodes.ACC_SYNTHETIC | Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC,
+                                new TypeDescription.ForLoadedType(delegate.getClass())))
+                        .withInitializer(LoadedTypeInitializer.ForStaticField.accessible(fieldName, delegate));
             }
 
             @Override

byte-buddy-dep/src/test/java/net/bytebuddy/implementation/FieldAccessorExceptionTest.java

@@ -1,9 +1,16 @@
 package net.bytebuddy.implementation;
 
+import net.bytebuddy.description.modifier.Ownership;
+import net.bytebuddy.description.modifier.Visibility;
+import net.bytebuddy.dynamic.scaffold.InstrumentedType;
 import org.junit.Test;
 
+import static org.mockito.Mockito.mock;
+
 public class FieldAccessorExceptionTest extends AbstractImplementationTest {
 
+    private static final String FOO = "foo";
+
     @Test(expected = IllegalArgumentException.class)
     public void testFinalFieldSetter() throws Exception {
         implement(Foo.class, FieldAccessor.ofBeanProperty());
@@ -19,6 +26,16 @@ public void testFieldNoBeanMethodName() throws Exception {
         implement(Qux.class, FieldAccessor.ofBeanProperty());
     }
 
+    @Test(expected = IllegalStateException.class)
+    public void testFieldNonStaticOnInterface() throws Exception {
+        FieldAccessor.ofField(FOO).defineAs(String.class, Visibility.PUBLIC).prepare(mock(InstrumentedType.class));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testFieldNonPublicOnInterface() throws Exception {
+        FieldAccessor.ofField(FOO).defineAs(String.class, Ownership.STATIC).prepare(mock(InstrumentedType.class));
+    }
+
     @SuppressWarnings("unused")
     public static class Foo {
 

byte-buddy-dep/src/test/java/net/bytebuddy/implementation/FieldAccessorPreparationTest.java

@@ -35,8 +35,10 @@ public void setUp() throws Exception {
 
     @Test
     public void testPreparationDefineField() throws Exception {
+        when(instrumentedType.isClassType()).thenReturn(true);
         assertThat(FieldAccessor.ofField(FOO).defineAs(TYPE).prepare(instrumentedType), is(instrumentedType));
         verify(instrumentedType).withField(new FieldDescription.Token(FOO, NO_MODIFIERS, new TypeDescription.ForLoadedType(TYPE)));
+        verify(instrumentedType).isClassType();
         verifyNoMoreInteractions(instrumentedType);
     }