Skip to content

Commit

Permalink
GROOVY-11373: SC: cross-package direct target of package-private method
Browse files Browse the repository at this point in the history 
4_0_X backport
  • Loading branch information
@eric-milles
eric-milles committed May 9, 2024
1 parent 4c8533b commit 0456e0e
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 50 deletions.
44 changes: 24 additions & 20 deletions src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

import static org.apache.groovy.ast.tools.ClassNodeUtils.formatTypeName;
import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
Expand Down Expand Up @@ -346,16 +348,17 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
return true;
}

Expression fixedReceiver = receiver;
boolean fixedImplicitThis = implicitThis;
if (target.isProtected()) {
ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
Expression fixedReceiver = receiver;
boolean fixedImplicitThis = implicitThis;
if (target.isPackageScope()) { // GROOVY-11373
if (!samePackageName(target.getDeclaringClass(), classNode)) {
writeMethodAccessError(target, receiver != null ? receiver : args);
}
} else if (target.isProtected()) {
ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, classNode);
if (!implicitThis && !isThisOrSuper(receiver) && !samePackageName(node, classNode)
&& StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass())) {
controller.getSourceUnit().addError(new SyntaxException(
"Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
receiver != null ? receiver : args
));
&& StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node, target.getDeclaringClass())) {
writeMethodAccessError(target, receiver != null ? receiver : args);
} else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
}
Expand Down Expand Up @@ -388,28 +391,29 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
return super.writeDirectMethodCall(target, implicitThis, receiver, args);
}

private void writeMethodAccessError(final MethodNode target, final Expression origin) {
StringJoiner descriptor = new StringJoiner(", ", target.getName() + "(", ")");
for (Parameter parameter : target.getParameters()) {
descriptor.add(formatTypeName(parameter.getOriginType()));
}
String message = "Cannot access method: " + descriptor + " of class: " + formatTypeName(target.getDeclaringClass());

controller.getSourceUnit().addError(new SyntaxException(message, origin));
}

private boolean tryPrivateMethod(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args, final ClassNode classNode) {
ClassNode declaringClass = target.getDeclaringClass();
if ((isPrivateBridgeMethodsCallAllowed(declaringClass, classNode) || isPrivateBridgeMethodsCallAllowed(classNode, declaringClass))
&& declaringClass.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS) != null
&& !declaringClass.equals(classNode)) {
if (tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
} else {
checkAndAddCannotCallPrivateMethodError(target, receiver, classNode, declaringClass);
}
}
checkAndAddCannotCallPrivateMethodError(target, receiver, classNode, declaringClass);
return false;
}

private void checkAndAddCannotCallPrivateMethodError(final MethodNode target, final Expression receiver, final ClassNode classNode, final ClassNode declaringClass) {
if (declaringClass != classNode) {
controller.getSourceUnit().addError(new SyntaxException(
"Cannot call private method " + (target.isStatic() ? "static " : "") + declaringClass.toString(false) + "#" + target.getName() + " from class " + classNode.toString(false),
receiver
));
writeMethodAccessError(target, receiver);
}
return false;
}

protected static boolean isPrivateBridgeMethodsCallAllowed(final ClassNode receiver, final ClassNode caller) {
Expand Down
9 changes: 4 additions & 5 deletions src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
super.testPublicFieldVersusPrivateGetter()
}

// GROOVY-11223
// GROOVY-11223, GROOVY-11373
@Override
void testMapPropertyAccess10() {
assertScript """
Expand All @@ -879,13 +879,12 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
def map = new ${MapType.name}()
map.bar = 22 // protected setter
""",
"Method setBar is protected in ${MapType.name}"
/*
"Cannot access method: setBar(java.lang.Object) of class: ${MapType.name}"

shouldFailWithMessages """
def map = new ${MapType.name}()
map.baz = 33 // package-private setter
""",
"Method setBaz is package-private in ${MapType.name}"
*/
"Cannot access method: setBaz(java.lang.Object) of class: ${MapType.name}"
}
}
47 changes: 22 additions & 25 deletions src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,40 @@
* specific language governing permissions and limitations
* under the License.
*/


package org.codehaus.groovy.classgen.asm.sc.bugs

import groovy.transform.stc.StaticTypeCheckingTestCase
import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport

class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
final class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {

void testGenericIdentityWithClosure() {
assertScript '''
public static <T> T identity(T self) { self }
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
})
Integer i = identity(2)
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
})
Closure c = identity {'foo'}
'''
static <T> T itself(T self) { self }
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
})
Integer i = itself(2)
@ASTTest(phase=INSTRUCTION_SELECTION,value={
assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
})
Closure c = itself {'foo'}
'''
}

void testShouldNotThrowIllegalAccessToProtectedData() {
shouldFailWithMessages('''
class Test {
final Set<String> HISTORY = [] as HashSet
Set<String> getHistory() {
return HISTORY.clone() as HashSet<String>
}
shouldFailWithMessages '''
class C {
private final Set<String> history = []
Set<String> getHistory() {
(Set<String>) history.clone()
}
}
Test test = new Test()
println test.history
''', 'Method clone is protected in java.lang.Object')
def set = new C().history
''',
'Cannot access method: clone() of class: java.lang.Object'
}
}

0 comments on commit 0456e0e

Please sign in to comment.