[
https://issues.apache.org/jira/browse/GROOVY-12279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105728#comment-18105728
]
ASF GitHub Bot commented on GROOVY-12279:
-----------------------------------------
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.
> SecureASTCustomizer: apply import rules to a method pointer's target type
> -------------------------------------------------------------------------
>
> Key: GROOVY-12279
> URL: https://issues.apache.org/jira/browse/GROOVY-12279
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
