Copilot commented on code in PR #2816:
URL: https://github.com/apache/groovy/pull/2816#discussion_r3809053621
##########
src/test/groovy/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy:
##########
@@ -336,6 +336,42 @@ final class SecureASTCustomizerTest {
}
}
+ // GROOVY-12279: a method pointer's own type is fixed to
groovy.lang.Closure, so the
+ // indirect import check was asking about Closure rather than about the
class the pointer
+ // is taken on. In deny mode that let the pointer through; in allow mode
it rejected every
+ // pointer, since Closure is never in an allow list.
+ @Test
+ void testIndirectImportCheckUsesMethodPointerTarget_denied() {
Review Comment:
Test method names in this file consistently use camelCase; the newly added
name includes an underscore, which is inconsistent and makes grepping for tests
less uniform.
This issue also appears on line 361 of the same file.
##########
src/test/groovy/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy:
##########
@@ -336,6 +336,42 @@ final class SecureASTCustomizerTest {
}
}
+ // GROOVY-12279: a method pointer's own type is fixed to
groovy.lang.Closure, so the
+ // indirect import check was asking about Closure rather than about the
class the pointer
+ // is taken on. In deny mode that let the pointer through; in allow mode
it rejected every
+ // pointer, since Closure is never in an allow list.
+ @Test
+ void testIndirectImportCheckUsesMethodPointerTarget_denied() {
+ customizer.disallowedImports = ['java.util.LinkedList']
+ customizer.indirectImportCheckEnabled = true
+ def shell = new GroovyShell(configuration)
+ assert hasSecurityException {
+ shell.evaluate('return java.util.LinkedList.&size')
+ }
+ assert hasSecurityException {
+ shell.evaluate('return java.util.LinkedList::size')
+ }
+ // The constructor form was already checked, and stays checked.
+ assert hasSecurityException {
+ shell.evaluate('return new java.util.LinkedList()')
+ }
+ }
+
+ @Test
+ void testIndirectImportCheckUsesMethodPointerTarget_allowed() {
+ customizer.allowedImports = ['java.util.ArrayList']
+ customizer.indirectImportCheckEnabled = true
+ def shell = new GroovyShell(configuration)
+ // Permitted because the target is allowed. Previously refused,
because the type being
+ // asked about was Closure, which no allow list names.
+ shell.evaluate('return java.util.ArrayList.&size')
+ shell.evaluate('return java.util.ArrayList::size')
+ // A target which is not allowed is still refused.
+ assert hasSecurityException {
+ shell.evaluate('return java.util.LinkedList.&size')
+ }
Review Comment:
This new allow-list test asserts that a disallowed target is refused for the
method-pointer form (`.&`), but it doesn't assert the same for the
method-reference form (`::`). Since the production change is meant to affect
both syntaxes (and `MethodReferenceExpression` is a distinct AST node), this
test should cover both refusal cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
