You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by "lukaszlenart (via GitHub)" <gi...@apache.org> on 2023/10/04 07:47:36 UTC

[PR] [WW-5338] Removes deprecated OgnTool [struts]

lukaszlenart opened a new pull request, #758:
URL: https://github.com/apache/struts/pull/758

   Closes [WW-5338](https://issues.apache.org/jira/browse/WW-5338)


-- 
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: commits-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on code in PR #758:
URL: https://github.com/apache/struts/pull/758#discussion_r1346818913


##########
plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java:
##########
@@ -56,8 +56,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
-import static org.apache.struts2.views.util.ContextUtil.OGNL;
+import static java.util.stream.Collectors.toList;;

Review Comment:
   😵‍💫 



##########
plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java:
##########
@@ -56,8 +56,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
-import static org.apache.struts2.views.util.ContextUtil.OGNL;
+import static java.util.stream.Collectors.toList;;

Review Comment:
   😵‍💫 



-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #758:
URL: https://github.com/apache/struts/pull/758#issuecomment-1748115876

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=758)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL)
   
   [![83.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '83.3%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnTool [struts]

Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #758:
URL: https://github.com/apache/struts/pull/758#discussion_r1345567133


##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -125,12 +120,10 @@ public Object findValue(String expression, String className) throws ClassNotFoun
 
     public Object findValue(String expr, Object context) {
         try {
-            return ognl.getValue(expr, ActionContext.getContext().getContextMap(), context);
-        } catch (OgnlException e) {
-            if (e.getReason() instanceof SecurityException) {
-                LOG.error(format("Could not evaluate this expression due to security constraints: [{0}]", expr), e);
-            }
-            return null;
+            stack.push(context);

Review Comment:
   Should be outside the try block so that we don't pop unless successfully pushed



-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #758:
URL: https://github.com/apache/struts/pull/758#discussion_r1345831172


##########
plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java:
##########
@@ -56,8 +56,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
-import static org.apache.struts2.views.util.ContextUtil.OGNL;
+import static java.util.stream.Collectors.toList;;

Review Comment:
   Double semicolon 😁



##########
plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java:
##########
@@ -56,8 +56,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
-import static org.apache.struts2.views.util.ContextUtil.OGNL;
+import static java.util.stream.Collectors.toList;;

Review Comment:
   Double semicolon 😁



-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart merged PR #758:
URL: https://github.com/apache/struts/pull/758


-- 
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: commits-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnTool [struts]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #758:
URL: https://github.com/apache/struts/pull/758#issuecomment-1746333297

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=758)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL)
   
   [![83.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '83.3%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #758:
URL: https://github.com/apache/struts/pull/758#issuecomment-1746749538

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=758)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=758&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=758&resolved=false&types=CODE_SMELL)
   
   [![83.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '83.3%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=758&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [WW-5338] Removes deprecated OgnlTool [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on code in PR #758:
URL: https://github.com/apache/struts/pull/758#discussion_r1345633947


##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -125,12 +120,10 @@ public Object findValue(String expression, String className) throws ClassNotFoun
 
     public Object findValue(String expr, Object context) {
         try {
-            return ognl.getValue(expr, ActionContext.getContext().getContextMap(), context);
-        } catch (OgnlException e) {
-            if (e.getReason() instanceof SecurityException) {
-                LOG.error(format("Could not evaluate this expression due to security constraints: [{0}]", expr), e);
-            }
-            return null;
+            stack.push(context);

Review Comment:
   fixed



-- 
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: issues-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org