[GitHub] [struts] github-code-scanning[bot] commented on a diff in pull request #607: [WW-5184] Uses debug log level when parameter value was not accepted

[GitBox]

github-code-scanning[bot] commented on code in PR #607:
URL: https://github.com/apache/struts/pull/607#discussion_r982186582


##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -351,19 +351,24 @@
      * * Value is null/blank
      * * Value is not excluded
      * * Value is accepted
-     * 
+     *
      * @param name - Param name (for logging)
      * @param value - value to check
      * @return true if accepted
      */
     protected boolean acceptableValue(String name, String value) {
-       boolean accepted = (value == null || value.isEmpty() || 
(!isParamValueExcluded(value) && isParamValueAccepted(value)));
+        boolean accepted = (value == null || value.isEmpty() || 
(!isParamValueExcluded(value) && isParamValueAccepted(value)));
         if (!accepted) {
-            LOG.warn("Parameter [{}] was not accepted with value [{}] and will 
be dropped!", name, value);
+            String message = "Value of parameter [{}] was not accepted and 
will be dropped!";
+            if (devMode) {
+                LOG.warn(message, name);
+            } else {
+                LOG.debug(message, name);

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AYODY9tvQiP6RP8X9SmM-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AYODY9tvQiP6RP8X9SmM&open=AYODY9tvQiP6RP8X9SmM&pullRequest=607";>SonarCloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/124)



##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -351,19 +351,24 @@
      * * Value is null/blank
      * * Value is not excluded
      * * Value is accepted
-     * 
+     *
      * @param name - Param name (for logging)
      * @param value - value to check
      * @return true if accepted
      */
     protected boolean acceptableValue(String name, String value) {
-       boolean accepted = (value == null || value.isEmpty() || 
(!isParamValueExcluded(value) && isParamValueAccepted(value)));
+        boolean accepted = (value == null || value.isEmpty() || 
(!isParamValueExcluded(value) && isParamValueAccepted(value)));
         if (!accepted) {
-            LOG.warn("Parameter [{}] was not accepted with value [{}] and will 
be dropped!", name, value);
+            String message = "Value of parameter [{}] was not accepted and 
will be dropped!";
+            if (devMode) {
+                LOG.warn(message, name);

Review Comment:
   ## Logging should not be vulnerable to injection attacks
   
   <!--SONAR_ISSUE_KEY:AYODY9tvQiP6RP8X9SmN-->Change this code to not log 
user-controlled data. <p>See more on <a 
href="https://sonarcloud.io/project/issues?id=apache_struts&issues=AYODY9tvQiP6RP8X9SmN&open=AYODY9tvQiP6RP8X9SmN&pullRequest=607";>SonarCloud</a></p>
   
   [Show more 
details](https://github.com/apache/struts/security/code-scanning/125)



-- 
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-unsubscr...@struts.apache.org

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