[ 
https://issues.apache.org/jira/browse/GROOVY-12416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116513#comment-18116513
 ] 

ASF GitHub Bot commented on GROOVY-12416:
-----------------------------------------

codecov-commenter commented on PR #2941:
URL: https://github.com/apache/groovy/pull/2941#issuecomment-5721689395

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2941?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 67.2996%. Comparing base 
([`ab03e3f`](https://app.codecov.io/gh/apache/groovy/commit/ab03e3f1646da29047350ec28949f3ae778871cf?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`d645863`](https://app.codecov.io/gh/apache/groovy/commit/d645863e23a3741cf02462779774d27783bd1baa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on GROOVY_5_1_X.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2941/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                  Coverage Diff                   @@
   ##             GROOVY_5_1_X      #2941        +/-   ##
   ======================================================
   + Coverage         67.2902%   67.2996%   +0.0094%     
   - Complexity          29649      29652         +3     
   ======================================================
     Files                1384       1384                
     Lines              117118     117118                
     Branches            20602      20602                
   ======================================================
   + Hits                78809      78820        +11     
   + Misses              31756      31746        -10     
   + Partials             6553       6552         -1     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2941?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[...roovy/control/customizers/SecureASTCustomizer.java](https://app.codecov.io/gh/apache/groovy/pull/2941?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2Fcustomizers%2FSecureASTCustomizer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvY3VzdG9taXplcnMvU2VjdXJlQVNUQ3VzdG9taXplci5qYXZh)
 | `51.4580% <100.0000%> (+0.5146%)` | :arrow_up: |
   
   ... and [5 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2941/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   - :package: [JS Bundle 
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save 
yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>




> SecureASTCustomizer: indirect import check does not box primitive receiver 
> types
> --------------------------------------------------------------------------------
>
>                 Key: GROOVY-12416
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12416
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> With {{{}setIndirectImportCheckEnabled(true){}}}, {{SecureASTCustomizer}} 
> checks the receiver type of a method call or method pointer against the 
> import rules. When that receiver has a primitive type, the check uses the 
> primitive type name ({{{}int{}}}, {{{}long{}}}, {{{}boolean{}}}, 
> {{{}double{}}}, ...). Such a name can never match a class in an allow list, 
> so the call is rejected even when the corresponding wrapper type is allowed:
> {code:java}
> SecureASTCustomizer secure = new SecureASTCustomizer();
> secure.setAllowedImports(List.of("java.lang.Object", "java.lang.Integer", 
> "java.lang.Long",
>         "java.lang.Boolean", "java.lang.Double"));
> secure.setAllowedStarImports(List.of());
> secure.setIndirectImportCheckEnabled(true);
> secure.setClosuresAllowed(true);
> CompilerConfiguration config = new CompilerConfiguration();
> config.addCompilationCustomizers(secure);
> new GroovyShell(config).parse("3.times { }");
> // SecurityException: Indirect import checks prevent usage of expression: 
> Importing [int] is not allowed
> {code}
> Results on master (7.0.0-SNAPSHOT), with the wrapper types allowed:
> ||Expression||Result||
> |3.times { }|rejected: {{Importing [int] is not allowed}}|
> |3L.times { }|rejected: {{Importing [long] is not allowed}}|
> |{{true.and(false)}}|rejected: {{Importing [boolean] is not allowed}}|
> |{{1.5d.abs()}}|rejected: {{Importing [double] is not allowed}}|
> |{{int[] a = [1]; a.size()}}|rejected: {{Importing [int] is not allowed}} 
> (array receivers are reduced to their component type)|
> |{{3.&times}}|rejected: {{Importing [int] is not allowed}}|
> |int n = 3; n.times { }|compiles (checked as {{{}java.lang.Integer{}}})|
> |Integer n = 3; n.times { }|compiles|
> The same value is therefore judged under a different name depending on 
> whether it is written as a literal or held in a typed local variable. The 
> behaviour is long-standing: 2.5.6, 2.5.14, 4.0.28 and 5.0.4 behave the same 
> way.
> The {{MethodCallExpression}} and {{MethodPointerExpression}} branches of 
> {{SecuringCodeVisitor}} pass the receiver type name straight to 
> {{{}assertImportIsAllowed{}}}. The construction-coercion cast branch added in 
> GROOVY-12283 already guards against primitives with 
> {{{}ClassHelper.isPrimitiveType{}}}.
> *Proposed behaviour:* check a primitive receiver as its wrapper type 
> ({{{}ClassHelper.getWrapper{}}}), so that {{{}3.times \{ }}} is checked as 
> {{java.lang.Integer{}}}, consistent with a typed local. Simply skipping 
> primitives would be looser: a policy that does not allow 
> {{java.lang.Integer}} would then still accept calls on integer literals.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to