You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by GitBox <gi...@apache.org> on 2022/09/28 09:18:45 UTC
[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
github-code-scanning[bot] commented on code in PR #607:
URL: https://github.com/apache/struts/pull/607#discussion_r982159941
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -351,19 +351,24 @@ private boolean isIgnoredDMI(String name) {
* * 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, value, name);
+ } else {
+ LOG.debug(message, value, 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/123)
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -351,19 +351,24 @@ private boolean isIgnoredDMI(String name) {
* * 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, value, 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/122)
--
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