Fixed visibility resolution where the visibility of method arguments …

…and return types as well as field types was not taken into consideration. Explicitly handle array types in visibility resolution.
raphw · Jun 18, 2015 · 6f223d9 · 6f223d9
1 parent f05c59e
commit 6f223d9
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 8 deletions.
diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/description/ByteCodeElement.java b/byte-buddy-dep/src/main/java/net/bytebuddy/description/ByteCodeElement.java
@@ -24,7 +24,9 @@ public interface ByteCodeElement extends NamedElement, ModifierReviewable, Decla
     String getGenericSignature();
 
     /**
-     * Checks if this element is visible from a given type.
+     * Checks if this element is visible from a given type. Methods are only considered visible if their return type and their parameter
+     * types are also visible to the given type. Similarly, fields are only considered visible if the field's type is visible to the
+     * given type. For array types, a type is considered visible only if the component type is visible to the given type.
      *
      * @param typeDescription The type which is checked for its access of this element.
      * @return {@code true} if this element is visible for {@code typeDescription}.

diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/description/field/FieldDescription.java b/byte-buddy-dep/src/main/java/net/bytebuddy/description/field/FieldDescription.java
@@ -48,6 +48,7 @@ public String getGenericSignature() {
         @Override
         public boolean isVisibleTo(TypeDescription typeDescription) {
             return getDeclaringType().isVisibleTo(typeDescription)
+                    && getFieldType().isVisibleTo(typeDescription)
                     && (isPublic()
                     || typeDescription.equals(getDeclaringType())
                     || (isProtected() && getDeclaringType().isAssignableFrom(typeDescription))

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
@@ -305,11 +305,18 @@ public int getAdjustedModifiers(boolean nonAbstract) {
 
         @Override
         public boolean isVisibleTo(TypeDescription typeDescription) {
-            return getDeclaringType().isVisibleTo(typeDescription)
-                    && (isPublic()
+            if (!getDeclaringType().isVisibleTo(typeDescription) || !getReturnType().isVisibleTo(typeDescription)) {
+                return false;
+            }
+            for (TypeDescription parameterType : getParameters().asTypeList()) {
+                if (!parameterType.isVisibleTo(typeDescription)) {
+                    return false;
+                }
+            }
+            return isPublic()
                     || typeDescription.equals(getDeclaringType())
                     || (isProtected() && getDeclaringType().isAssignableFrom(typeDescription))
-                    || (!isPrivate() && typeDescription.isSamePackage(getDeclaringType())));
+                    || (!isPrivate() && typeDescription.isSamePackage(getDeclaringType()));
         }
 
         @Override

diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/description/type/TypeDescription.java b/byte-buddy-dep/src/main/java/net/bytebuddy/description/type/TypeDescription.java
@@ -428,7 +428,9 @@ public boolean isSamePackage(TypeDescription typeDescription) {
 
         @Override
         public boolean isVisibleTo(TypeDescription typeDescription) {
-            return isPublic() || isProtected() || isSamePackage(typeDescription);
+            return isPrimitive() || (isArray()
+                    ? getComponentType().isVisibleTo(typeDescription)
+                    : (isPublic() || isProtected() || isSamePackage(typeDescription)));
         }
 
         @Override
@@ -484,7 +486,9 @@ public String getSourceCodeName() {
          */
         protected String getPackageName() {
             String name = getName();
-            int packageIndex = name.lastIndexOf('.');
+            int packageIndex = isArray()
+                    ? -1
+                    : name.lastIndexOf('.');
             return packageIndex == -1
                     ? null
                     : name.substring(0, packageIndex);

diff --git a/...-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/subclass/ConstructorStrategy.java b/...-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/subclass/ConstructorStrategy.java
@@ -106,8 +106,7 @@ public MethodList extractConstructors(TypeDescription instrumentedType) {
                 TypeDescription superType = instrumentedType.getSupertype();
                 return superType == null
                         ? new MethodList.Empty()
-                        : superType.getDeclaredMethods()
-                        .filter(isConstructor().<MethodDescription>and(isVisibleTo(instrumentedType)));
+                        : superType.getDeclaredMethods().filter(isConstructor().<MethodDescription>and(isVisibleTo(instrumentedType)));
             }
 
             @Override

diff --git a/...buddy-dep/src/test/java/net/bytebuddy/description/field/AbstractFieldDescriptionTest.java b/...buddy-dep/src/test/java/net/bytebuddy/description/field/AbstractFieldDescriptionTest.java
@@ -157,6 +157,10 @@ public void testIsVisibleTo() throws Exception {
                 .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
         assertThat(describe(PackagePrivateType.class.getDeclaredField("privateField"))
                 .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
+        assertThat(describe(PackagePrivateFieldType.class.getDeclaredField("packagePrivateType"))
+                .isVisibleTo(new TypeDescription.ForLoadedType(PackagePrivateFieldType.class)), is(true));
+        assertThat(describe(PackagePrivateFieldType.class.getDeclaredField("packagePrivateType"))
+                .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
     }
 
     @Test
@@ -203,4 +207,9 @@ static class PackagePrivateType {
 
         private Void privateField;
     }
+
+    public static class PackagePrivateFieldType {
+
+        public PackagePrivateType packagePrivateType;
+    }
 }
diff --git a/...ddy-dep/src/test/java/net/bytebuddy/description/method/AbstractMethodDescriptionTest.java b/...ddy-dep/src/test/java/net/bytebuddy/description/method/AbstractMethodDescriptionTest.java
@@ -471,6 +471,14 @@ public void testConstructorIsVisibleTo() throws Exception {
                 .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
         assertThat(describe(PackagePrivateType.class.getDeclaredConstructor(String.class))
                 .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
+        assertThat(describe(MethodVisibilityType.class.getDeclaredMethod("packagePrivateArgument", PackagePrivateType.class))
+                .isVisibleTo(new TypeDescription.ForLoadedType(MethodVisibilityType.class)), is(true));
+        assertThat(describe(MethodVisibilityType.class.getDeclaredMethod("packagePrivateReturnType"))
+                .isVisibleTo(new TypeDescription.ForLoadedType(MethodVisibilityType.class)), is(true));
+        assertThat(describe(MethodVisibilityType.class.getDeclaredMethod("packagePrivateArgument", PackagePrivateType.class))
+                .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
+        assertThat(describe(MethodVisibilityType.class.getDeclaredMethod("packagePrivateReturnType"))
+                .isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
     }
 
     @Test
@@ -643,4 +651,15 @@ private void privateMethod() {
             /* do nothing*/
         }
     }
+
+    public static class MethodVisibilityType {
+
+        public void packagePrivateArgument(PackagePrivateType arg) {
+            /* do nothing */
+        }
+
+        public PackagePrivateType packagePrivateReturnType() {
+            return null;
+        }
+    }
 }
diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/description/type/AbstractTypeDescriptionTest.java b/byte-buddy-dep/src/test/java/net/bytebuddy/description/type/AbstractTypeDescriptionTest.java
@@ -321,6 +321,9 @@ public void testIsVisible() throws Exception {
         assertThat(describe(SamplePackagePrivate.class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
         assertThat(describe(SampleInterface.class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(true));
         assertThat(describe(SampleAnnotation.class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
+        assertThat(describe(int.class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(true));
+        assertThat(describe(SampleInterface[].class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(true));
+        assertThat(describe(SamplePackagePrivate[].class).isVisibleTo(new TypeDescription.ForLoadedType(Object.class)), is(false));
     }
 
     @Test