From a024df9675409344ecdf232d6ac3a283602323d5 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Fri, 25 May 2018 12:49:56 -0700 Subject: [PATCH] Initial cut at bug 535086 - pertypewithin and non vis types 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. --- ...MissingResolvedTypeWithKnownSignature.java | 7 ++- .../weaver/PerTypeWithinTargetTypeMunger.java | 8 +++- .../src/org/aspectj/weaver/ResolvedType.java | 48 ++++++++++++++++++- .../org/aspectj/weaver/UnresolvedType.java | 3 ++ .../weaver/patterns/PerTypeWithin.java | 27 ++++++++--- .../org/aspectj/systemtest/AllTests19.java | 2 + .../org/aspectj/weaver/bcel/BcelShadow.java | 17 +++++-- .../aspectj/weaver/bcel/BcelTypeMunger.java | 4 +- 8 files changed, 101 insertions(+), 15 deletions(-) diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/MissingResolvedTypeWithKnownSignature.java b/org.aspectj.matcher/src/org/aspectj/weaver/MissingResolvedTypeWithKnownSignature.java index 18b2cdff2..da61ffd8b 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/MissingResolvedTypeWithKnownSignature.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/MissingResolvedTypeWithKnownSignature.java @@ -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; diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/PerTypeWithinTargetTypeMunger.java b/org.aspectj.matcher/src/org/aspectj/weaver/PerTypeWithinTargetTypeMunger.java index 036ab9b7c..ad549417a 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/PerTypeWithinTargetTypeMunger.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/PerTypeWithinTargetTypeMunger.java @@ -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; } - + } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java b/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java index daccec105..98400ebdc 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java @@ -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 getFields() { final Iterators.Filter dupFilter = Iterators.dupFilter(); Iterators.Getter typeGetter = new Iterators.Getter() { + @Override public Iterator 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 { + @Override public Iterator 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 { + @Override public Iterator 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 { + @Override public Iterator 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 { + @Override public Iterator get(ResolvedType type) { return Iterators.array(type.getDeclaredFields()); } @@ -307,6 +329,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl final Iterators.Getter interfaceGetter = new Iterators.Getter() { List alreadySeen = new ArrayList(); // Strings are signatures (ResolvedType.getSignature()) + @Override public Iterator get(ResolvedType type) { ResolvedType[] interfaces = type.getDeclaredInterfaces(); @@ -732,6 +755,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl final Iterators.Filter dupFilter = Iterators.dupFilter(); // same order as fields Iterators.Getter typeGetter = new Iterators.Getter() { + @Override public Iterator 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 dupFilter = Iterators.dupFilter(); Iterators.Getter typeGetter = new Iterators.Getter() { + @Override public Iterator get(ResolvedType o) { return dupFilter.filter((o).getDirectSupertypes()); } @@ -832,6 +857,7 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl List acc = new ArrayList(); final Iterators.Filter dupFilter = Iterators.dupFilter(); Iterators.Getter typeGetter = new Iterators.Getter() { + @Override public Iterator 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; + } } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/UnresolvedType.java b/org.aspectj.matcher/src/org/aspectj/weaver/UnresolvedType.java index b5d085c3e..d8d46176d 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/UnresolvedType.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/UnresolvedType.java @@ -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() + "]"; } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PerTypeWithin.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PerTypeWithin.java index e8b92ac5a..d912b52ed 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PerTypeWithin.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PerTypeWithin.java @@ -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 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(' - + // We want the pointcut to be: + // 'staticinitialization(*) && within()' - // we *cannot* shortcut this to staticinitialization() - // 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()); diff --git a/tests/src/org/aspectj/systemtest/AllTests19.java b/tests/src/org/aspectj/systemtest/AllTests19.java index 8cb86d449..d46eeb6d2 100644 --- a/tests/src/org/aspectj/systemtest/AllTests19.java +++ b/tests/src/org/aspectj/systemtest/AllTests19.java @@ -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; diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index 6ba22ed97..2e8d3ab6b 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -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()); diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java index c82c352ae..b92760fe9 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java @@ -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$$localAspectOf() method // e.g. -- 2.39.5