]> source.dussan.org Git - aspectj.git/commitdiff
Initial cut at bug 535086 - pertypewithin and non vis types
authorAndy Clement <aclement@pivotal.io>
Fri, 25 May 2018 19:49:56 +0000 (12:49 -0700)
committerAndy Clement <aclement@pivotal.io>
Fri, 25 May 2018 19:49:56 +0000 (12:49 -0700)
In this version unless you specify an aspect is privileged then the
pertypewithin clause will not match types not visible from the aspect
(private types or default vis types in another package)

Debating whether to change this to not require privileged.

org.aspectj.matcher/src/org/aspectj/weaver/MissingResolvedTypeWithKnownSignature.java
org.aspectj.matcher/src/org/aspectj/weaver/PerTypeWithinTargetTypeMunger.java
org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java
org.aspectj.matcher/src/org/aspectj/weaver/UnresolvedType.java
org.aspectj.matcher/src/org/aspectj/weaver/patterns/PerTypeWithin.java
tests/src/org/aspectj/systemtest/AllTests19.java
weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java

index 18b2cdff22b99d70952e09030530f21e45c6f90c..da61ffd8b208fb8a687dc811d35d906136b0bc80 100644 (file)
@@ -114,18 +114,22 @@ public class MissingResolvedTypeWithKnownSignature extends ResolvedType {
        public ISourceContext getSourceContext() {
                return new ISourceContext() {
 
+                       @Override
                        public ISourceLocation makeSourceLocation(IHasPosition position) {
                                return null;
                        }
 
+                       @Override
                        public ISourceLocation makeSourceLocation(int line, int offset) {
                                return null;
                        }
 
+                       @Override
                        public int getOffset() {
                                return 0;
                        }
 
+                       @Override
                        public void tidy() {
                        }
 
@@ -151,7 +155,7 @@ public class MissingResolvedTypeWithKnownSignature extends ResolvedType {
                        return isAssignableFrom(other);
                }
        }
-
+       
        /*
         * (non-Javadoc)
         * 
@@ -168,6 +172,7 @@ public class MissingResolvedTypeWithKnownSignature extends ResolvedType {
         * 
         * @see org.aspectj.weaver.AnnotatedElement#hasAnnotation(org.aspectj.weaver.UnresolvedType)
         */
+       @Override
        public boolean hasAnnotation(UnresolvedType ofType) {
                raiseCantFindType(WeaverMessages.CANT_FIND_TYPE_ANNOTATION);
                return false;
index 036ab9b7c7f41fb15e69bec95a63cde4638616f7..ad549417a93801a314f1983e28d122bfee46a6fc 100644 (file)
@@ -29,6 +29,7 @@ public class PerTypeWithinTargetTypeMunger extends ResolvedTypeMunger {
                this.testPointcut = testPointcut;
        }
 
+       @Override
        public boolean equals(Object other) {
                if (!(other instanceof PerTypeWithinTargetTypeMunger)) {
                        return false;
@@ -40,6 +41,7 @@ public class PerTypeWithinTargetTypeMunger extends ResolvedTypeMunger {
 
        private volatile int hashCode = 0;
 
+       @Override
        public int hashCode() {
                if (hashCode == 0) {
                        int result = 17;
@@ -50,6 +52,7 @@ public class PerTypeWithinTargetTypeMunger extends ResolvedTypeMunger {
                return hashCode;
        }
 
+       @Override
        public void write(CompressingDataOutputStream s) throws IOException {
                throw new RuntimeException("shouldn't be serialized");
        }
@@ -65,8 +68,9 @@ public class PerTypeWithinTargetTypeMunger extends ResolvedTypeMunger {
        // This is a lexical within() so if you say PerTypeWithin(Test) and matchType is an
        // inner type (e.g. Test$NestedType) then it should match successfully
        // Does not match if the target is an interface
+       @Override
        public boolean matches(ResolvedType matchType, ResolvedType aspectType) {
-               return isWithinType(matchType).alwaysTrue() && !matchType.isInterface();
+               return isWithinType(matchType).alwaysTrue() && !matchType.isInterface() && (matchType.canBeSeenBy(aspectType) || aspectType.isPrivilegedAspect());
        }
 
        private FuzzyBoolean isWithinType(ResolvedType type) {
@@ -78,5 +82,5 @@ public class PerTypeWithinTargetTypeMunger extends ResolvedTypeMunger {
                }
                return FuzzyBoolean.NO;
        }
-
+       
 }
index daccec105c8e9b9eb3e764bf3062e32f97d49069..98400ebdc4812308c12ff052fb101312e00e11ee 100644 (file)
@@ -59,6 +59,8 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
        private static int TypeHierarchyCompleteBit = 0x0010;
        private static int GroovyObjectInitialized = 0x0020;
        private static int IsGroovyObject = 0x0040;
+       private static int IsPrivilegedBitInitialized = 0x0080;
+       private static int IsPrivilegedAspect = 0x0100;
 
        protected ResolvedType(String signature, World world) {
                super(signature);
@@ -70,6 +72,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                this.world = world;
        }
 
+       @Override
        public int getSize() {
                return 1;
        }
@@ -107,6 +110,18 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 
        public abstract int getModifiers();
 
+       public boolean canBeSeenBy(ResolvedType from) {
+               int targetMods = getModifiers();
+               if (Modifier.isPublic(targetMods)) {
+                       return true;
+               }
+               if (Modifier.isPrivate(targetMods)) {
+                       return false;
+               }
+               // isProtected() or isDefault()
+               return getPackageName().equals(from.getPackageName());
+       }
+
        // return true if this resolved type couldn't be found (but we know it's name maybe)
        public boolean isMissing() {
                return false;
@@ -124,10 +139,12 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                }
        }
 
+       @Override
        public ResolvedType[] getAnnotationTypes() {
                return EMPTY_RESOLVED_TYPE_ARRAY;
        }
 
+       @Override
        public AnnotationAJ getAnnotationOfType(UnresolvedType ofType) {
                return null;
        }
@@ -197,6 +214,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
        public Iterator<ResolvedMember> getFields() {
                final Iterators.Filter<ResolvedType> dupFilter = Iterators.dupFilter();
                Iterators.Getter<ResolvedType, ResolvedType> typeGetter = new Iterators.Getter<ResolvedType, ResolvedType>() {
+                       @Override
                        public Iterator<ResolvedType> get(ResolvedType o) {
                                return dupFilter.filter(o.getDirectSupertypes());
                        }
@@ -230,6 +248,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
         * An Iterators.Getter that returns an iterator over all methods declared on some resolved type.
         */
        private static class MethodGetter implements Iterators.Getter<ResolvedType, ResolvedMember> {
+               @Override
                public Iterator<ResolvedMember> get(ResolvedType type) {
                        return Iterators.array(type.getDeclaredMethods());
                }
@@ -239,6 +258,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
         * An Iterators.Getter that returns an iterator over all pointcuts declared on some resolved type.
         */
        private static class PointcutGetter implements Iterators.Getter<ResolvedType, ResolvedMember> {
+               @Override
                public Iterator<ResolvedMember> get(ResolvedType o) {
                        return Iterators.array(o.getDeclaredPointcuts());
                }
@@ -248,6 +268,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 
        // Getter that returns all declared methods for a type through an iterator - including intertype declarations
        private static class MethodGetterIncludingItds implements Iterators.Getter<ResolvedType, ResolvedMember> {
+               @Override
                public Iterator<ResolvedMember> get(ResolvedType type) {
                        ResolvedMember[] methods = type.getDeclaredMethods();
                        if (type.interTypeMungers != null) {
@@ -280,6 +301,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
         * An Iterators.Getter that returns an iterator over all fields declared on some resolved type.
         */
        private static class FieldGetter implements Iterators.Getter<ResolvedType, ResolvedMember> {
+               @Override
                public Iterator<ResolvedMember> get(ResolvedType type) {
                        return Iterators.array(type.getDeclaredFields());
                }
@@ -307,6 +329,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                final Iterators.Getter<ResolvedType, ResolvedType> interfaceGetter = new Iterators.Getter<ResolvedType, ResolvedType>() {
                        List<String> alreadySeen = new ArrayList<String>(); // Strings are signatures (ResolvedType.getSignature())
 
+                       @Override
                        public Iterator<ResolvedType> get(ResolvedType type) {
                                ResolvedType[] interfaces = type.getDeclaredInterfaces();
 
@@ -732,6 +755,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                final Iterators.Filter<ResolvedType> dupFilter = Iterators.dupFilter();
                // same order as fields
                Iterators.Getter<ResolvedType, ResolvedType> typeGetter = new Iterators.Getter<ResolvedType, ResolvedType>() {
+                       @Override
                        public Iterator<ResolvedType> get(ResolvedType o) {
                                return dupFilter.filter(o.getDirectSupertypes());
                        }
@@ -799,6 +823,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        // ret.addAll(getDeclares());
                        final Iterators.Filter<ResolvedType> dupFilter = Iterators.dupFilter();
                        Iterators.Getter<ResolvedType, ResolvedType> typeGetter = new Iterators.Getter<ResolvedType, ResolvedType>() {
+                               @Override
                                public Iterator<ResolvedType> get(ResolvedType o) {
                                        return dupFilter.filter((o).getDirectSupertypes());
                                }
@@ -832,6 +857,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                List<ShadowMunger> acc = new ArrayList<ShadowMunger>();
                final Iterators.Filter<ResolvedType> dupFilter = Iterators.dupFilter();
                Iterators.Getter<ResolvedType, ResolvedType> typeGetter = new Iterators.Getter<ResolvedType, ResolvedType>() {
+                       @Override
                        public Iterator<ResolvedType> get(ResolvedType o) {
                                return dupFilter.filter((o).getDirectSupertypes());
                        }
@@ -1097,7 +1123,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                public final int getSize() {
                        return size;
                }
-
+               
                @Override
                public final int getModifiers() {
                        return Modifier.PUBLIC | Modifier.FINAL;
@@ -1108,6 +1134,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        return true;
                }
 
+               @Override
                public boolean hasAnnotation(UnresolvedType ofType) {
                        return false;
                }
@@ -1236,6 +1263,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        return true;
                }
 
+               @Override
                public boolean hasAnnotation(UnresolvedType ofType) {
                        return false;
                }
@@ -2261,10 +2289,12 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        this.wantGenerics = genericsAware;
                }
 
+               @Override
                public boolean hasNext() {
                        return curr != null;
                }
 
+               @Override
                public ResolvedType next() {
                        ResolvedType ret = curr;
                        if (!wantGenerics && ret.isParameterizedOrGenericType()) {
@@ -2275,6 +2305,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        return ret;
                }
 
+               @Override
                public void remove() {
                        throw new UnsupportedOperationException();
                }
@@ -2296,6 +2327,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        this.delegate = Iterators.one(interfaceType);
                }
 
+               @Override
                public boolean hasNext() {
                        if (delegate == null || !delegate.hasNext()) {
                                // either we set it up or we have run out, is there anything else to look at?
@@ -2315,6 +2347,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        toPersue.add(ret);
                }
 
+               @Override
                public ResolvedType next() {
                        ResolvedType next = delegate.next();
                        // BUG should check for generics and erase?
@@ -2326,6 +2359,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                        return next;
                }
 
+               @Override
                public void remove() {
                        throw new UnsupportedOperationException();
                }
@@ -2883,5 +2917,17 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
                }
                return (bits & IsGroovyObject) != 0;
        }
+       
+       public boolean isPrivilegedAspect() {
+               if ((bits & IsPrivilegedBitInitialized) == 0) {
+                       AnnotationAJ privilegedAnnotation = getAnnotationOfType(UnresolvedType.AJC_PRIVILEGED);
+                       if (privilegedAnnotation != null) {
+                               bits |= IsPrivilegedAspect;
+                       }
+                       // TODO do we need to reset this bit if the annotations are set again ?
+                       bits |= IsPrivilegedBitInitialized;
+               }
+               return (bits & IsPrivilegedAspect) != 0;
+       }
 
 }
index b5d085c3e0f3049577f58063582dfaffc73c5bdf..d8d46176dec376ae79d2918cadbc60a6069748a1 100644 (file)
@@ -61,6 +61,7 @@ public class UnresolvedType implements Traceable, TypeVariableDeclaringElement {
        public static final UnresolvedType[] ARRAY_WITH_JUST_OBJECT = new UnresolvedType[] { OBJECT };
        public static final UnresolvedType JOINPOINT_STATICPART = forSignature("Lorg/aspectj/lang/JoinPoint$StaticPart;");
        public static final UnresolvedType JOINPOINT_ENCLOSINGSTATICPART = forSignature("Lorg/aspectj/lang/JoinPoint$EnclosingStaticPart;");
+       public static final UnresolvedType AJC_PRIVILEGED = forSignature("Lorg/aspectj/internal/lang/annotation/ajcPrivileged;");
 
        public static final UnresolvedType BOOLEAN = forPrimitiveType("Z");
        public static final UnresolvedType BYTE = forPrimitiveType("B");
@@ -868,6 +869,7 @@ public class UnresolvedType implements Traceable, TypeVariableDeclaringElement {
                private final String type;
        }
 
+       @Override
        public TypeVariable getTypeVariableNamed(String name) {
                TypeVariable[] vars = getTypeVariables();
                if (vars == null || vars.length == 0) {
@@ -882,6 +884,7 @@ public class UnresolvedType implements Traceable, TypeVariableDeclaringElement {
                return null;
        }
 
+       @Override
        public String toTraceString() {
                return getClass().getName() + "[" + getName() + "]";
        }
index e8b92ac5a4c99e3fae563da154c30bbd1f8a86d4..d912b52ed4fbf613d188f18be3b0247d238ffa24 100644 (file)
@@ -51,14 +51,17 @@ public class PerTypeWithin extends PerClause {
                typePattern = p;
        }
 
+       @Override
        public Object accept(PatternNodeVisitor visitor, Object data) {
                return visitor.visit(this, data);
        }
 
+       @Override
        public int couldMatchKinds() {
                return kindSet;
        }
 
+       @Override
        public Pointcut parameterizeWith(Map<String,UnresolvedType> typeVariableMap, World w) {
                PerTypeWithin ret = new PerTypeWithin(typePattern.parameterizeWith(typeVariableMap, w));
                ret.copyLocationFrom(this);
@@ -66,6 +69,7 @@ public class PerTypeWithin extends PerClause {
        }
 
        // -----
+       @Override
        public FuzzyBoolean fastMatch(FastMatchInfo info) {
                if (typePattern.annotationPattern instanceof AnyAnnotationTypePattern) {
                        return isWithinType(info.getType());
@@ -73,6 +77,7 @@ public class PerTypeWithin extends PerClause {
                return FuzzyBoolean.MAYBE;
        }
 
+       @Override
        protected FuzzyBoolean matchInternal(Shadow shadow) {
                ResolvedType enclosingType = shadow.getIWorld().resolve(shadow.getEnclosingType(), true);
                if (enclosingType.isMissing()) {
@@ -90,15 +95,20 @@ public class PerTypeWithin extends PerClause {
                if (enclosingType.isInterface()) {
                        return FuzzyBoolean.NO;
                }
+               if (!(enclosingType.canBeSeenBy(inAspect) || inAspect.isPrivilegedAspect())) {
+                       return FuzzyBoolean.NO;
+               }
 
                typePattern.resolve(shadow.getIWorld());
                return isWithinType(enclosingType);
        }
 
+       @Override
        public void resolveBindings(IScope scope, Bindings bindings) {
                typePattern = typePattern.resolveBindings(scope, bindings, false, false);
        }
 
+       @Override
        protected Test findResidueInternal(Shadow shadow, ExposedState state) {
                // Member ptwField =
                // AjcMemberMaker.perTypeWithinField(shadow.getEnclosingType
@@ -135,6 +145,7 @@ public class PerTypeWithin extends PerClause {
                return match(shadow).alwaysTrue() ? Literal.TRUE : Literal.FALSE;
        }
 
+       @Override
        public PerClause concretize(ResolvedType inAspect) {
                PerTypeWithin ret = new PerTypeWithin(typePattern);
                ret.copyLocationFrom(this);
@@ -152,14 +163,12 @@ public class PerTypeWithin extends PerClause {
                Pointcut staticInitStar = new KindedPointcut(Shadow.StaticInitialization, sigpat);
                Pointcut withinTp = new WithinPointcut(typePattern);
                Pointcut andPcut = new AndPointcut(staticInitStar, withinTp);
-               // We want the pointcut to be 'staticinitialization(*) &&
-               // within(<typepattern>' -
+               // We want the pointcut to be:
+               // 'staticinitialization(*) && within(<typepattern>)' -
                // we *cannot* shortcut this to staticinitialization(<typepattern>)
-               // because it
-               // doesnt mean the same thing.
+               // because it doesnt mean the same thing.
 
-               // This munger will initialize the aspect instance field in the matched
-               // type
+               // This munger will initialize the aspect instance field in the matched type
 
                inAspect.crosscuttingMembers.addConcreteShadowMunger(Advice.makePerTypeWithinEntry(world, andPcut, inAspect));
 
@@ -181,6 +190,7 @@ public class PerTypeWithin extends PerClause {
 
        }
 
+       @Override
        public void write(CompressingDataOutputStream s) throws IOException {
                PERTYPEWITHIN.write(s);
                typePattern.write(s);
@@ -193,14 +203,17 @@ public class PerTypeWithin extends PerClause {
                return ret;
        }
 
+       @Override
        public PerClause.Kind getKind() {
                return PERTYPEWITHIN;
        }
 
+       @Override
        public String toString() {
                return "pertypewithin(" + typePattern + ")";
        }
 
+       @Override
        public String toDeclarationString() {
                return toString();
        }
@@ -215,6 +228,7 @@ public class PerTypeWithin extends PerClause {
                return FuzzyBoolean.NO;
        }
 
+       @Override
        public boolean equals(Object other) {
                if (!(other instanceof PerTypeWithin)) {
                        return false;
@@ -224,6 +238,7 @@ public class PerTypeWithin extends PerClause {
                                && ((pc.typePattern == null) ? (typePattern == null) : pc.typePattern.equals(typePattern));
        }
 
+       @Override
        public int hashCode() {
                int result = 17;
                result = 37 * result + ((inAspect == null) ? 0 : inAspect.hashCode());
index 8cb86d4497972dbaf961907fa59579dabb70f11e..d46eeb6d26e4f882bba9e74cd673a8a5007618dd 100644 (file)
@@ -12,6 +12,7 @@ package org.aspectj.systemtest;
 
 import org.aspectj.systemtest.ajc190.AllTestsAspectJ190;
 import org.aspectj.systemtest.ajc191.AllTestsAspectJ191;
+import org.aspectj.systemtest.ajc192.AllTestsAspectJ192;
 
 import junit.framework.Test;
 import junit.framework.TestSuite;
@@ -23,6 +24,7 @@ public class AllTests19 {
                // $JUnit-BEGIN$ 
                suite.addTest(AllTestsAspectJ190.suite()); 
                suite.addTest(AllTestsAspectJ191.suite()); 
+               suite.addTest(AllTestsAspectJ192.suite()); 
                suite.addTest(AllTests18.suite());
                // $JUnit-END$
                return suite;
index 6ba22ed97cca20906a1efa3b877261c7310eb5ce..2e8d3ab6b8865a508872c66faff397df6a9b8a0b 100644 (file)
@@ -1129,6 +1129,7 @@ public class BcelShadow extends Shadow {
                return getThisJoinPointStaticPartBcelVar(false);
        }
 
+       @Override
        public BcelVar getThisAspectInstanceVar(ResolvedType aspectType) {
                return new AspectInstanceVar(aspectType);
        }
@@ -1995,16 +1996,24 @@ public class BcelShadow extends Shadow {
         * Causes the aspect instance to be *set* for later retrievable through localAspectof()/aspectOf()
         */
        public void weavePerTypeWithinAspectInitialization(final BcelAdvice munger, UnresolvedType t) {
-
-               if (t.resolve(world).isInterface()) {
-                       return; // Don't initialize statics in
+               ResolvedType tResolved = t.resolve(world);
+               if (tResolved.isInterface()) {
+                       return; // Don't initialize statics in interfaces
+               }
+               ResolvedType aspectRT = munger.getConcreteAspect();
+               BcelWorld.getBcelObjectType(aspectRT);
+               
+               // Although matched, if the visibility rules prevent the aspect from seeing this type, don't
+               // insert any code (easier to do it here than try to affect the matching logic, unfortunately)
+               if (!(tResolved.canBeSeenBy(aspectRT) || aspectRT.isPrivilegedAspect())) {
+                       return;
                }
+               
                final InstructionFactory fact = getFactory();
 
                InstructionList entryInstructions = new InstructionList();
                InstructionList entrySuccessInstructions = new InstructionList();
 
-               BcelWorld.getBcelObjectType(munger.getConcreteAspect());
                String aspectname = munger.getConcreteAspect().getName();
 
                String ptwField = NameMangler.perTypeWithinFieldForTarget(munger.getConcreteAspect());
index c82c352ae3f93fa64d54b72994b583ace46e6e33..b92760fe97014a72cf2b1873f4bccfd571db545a 100644 (file)
@@ -829,7 +829,9 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
                // e.g ajc$com_blah_SecurityAspect$ptwAspectInstance
                FieldGen fg = makeFieldGen(gen, AjcMemberMaker.perTypeWithinField(gen.getType(), aspectType));
                gen.addField(fg, getSourceLocation());
-
+               if (!gen.getType().canBeSeenBy(aspectType) && aspectType.isPrivilegedAspect()) {
+                       gen.forcePublic();
+               }
                // Add an accessor for this new field, the
                // ajc$<aspectname>$localAspectOf() method
                // e.g.