From ddbd02e6bb4c00b647e1a8f89610d5ff3165aeb6 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 28 Sep 2022 11:09:16 +0200 Subject: [PATCH 1/5] WW-5184 Uses debug log level when parameter value was not accepted --- .../interceptor/ParametersInterceptor.java | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java index f81ba15fd..ff962c895 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java @@ -272,7 +272,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { ParameterNameAware parameterNameAware = (action instanceof ParameterNameAware) ? (ParameterNameAware) action : null; return acceptableName(name) && (parameterNameAware == null || parameterNameAware.acceptableParameterName(name)); } - + /** * Checks if parameter value can be accepted or thrown away * @@ -316,13 +316,13 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return logEntry.toString(); } - + /** * Validates the name passed is: * * Within the max length of a parameter name * * Is not excluded * * Is accepted - * + * * @param name - Name to check * @return true if accepted */ @@ -351,19 +351,24 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * * 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); + } } return accepted; } - + protected boolean isWithinLengthLimit(String name) { boolean matchLength = name.length() <= paramNameMaxLength; if (!matchLength) { @@ -407,7 +412,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } return false; } - + public void setAcceptedValuePatterns(String commaDelimitedPatterns) { Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); @@ -427,7 +432,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns); } } - + public void setExcludeValuePatterns(String commaDelimitedPatterns) { Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); if (excludedValuePatterns == null) { @@ -446,7 +451,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns); } } - + protected boolean isParamValueExcluded(String value) { if (hasParamValuesToExclude()) { for (Pattern excludedPattern : excludedValuePatterns) { @@ -478,15 +483,15 @@ public class ParametersInterceptor extends MethodFilterInterceptor { LOG.warn("Parameter value [{}] did not match any acceptedValuePattern pattern and will be dropped.", value); return false; } - + private boolean hasParamValuesToExclude() { return excludedValuePatterns != null && excludedValuePatterns.size() > 0; } - + private boolean hasParamValuesToAccept() { return acceptedValuePatterns != null && acceptedValuePatterns.size() > 0; } - + /** * Whether to order the parameters or not * From cbfd3a7ab94b1fda96536a88eee56a1e9e63fc64 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 28 Sep 2022 11:35:12 +0200 Subject: [PATCH 2/5] WW-5184 Improves logging around excluding/accepting values of incoming parameters --- .../interceptor/ParametersInterceptor.java | 191 ++++++++++-------- .../ParametersInterceptorTest.java | 33 ++- 2 files changed, 121 insertions(+), 103 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java index ff962c895..73f650442 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java @@ -26,6 +26,7 @@ import com.opensymphony.xwork2.security.AcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.util.ClearableValueStack; import com.opensymphony.xwork2.util.MemberAccessValueStack; +import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.ValueStackFactory; import com.opensymphony.xwork2.util.reflection.ReflectionContextState; @@ -33,8 +34,8 @@ import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; -import org.apache.struts2.dispatcher.Parameter; import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.Parameter; import java.util.Collection; import java.util.Collections; @@ -44,7 +45,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; import java.util.regex.Pattern; -import com.opensymphony.xwork2.util.TextParseUtil; /** * This interceptor sets all parameters on the value stack. @@ -69,7 +69,6 @@ public class ParametersInterceptor extends MethodFilterInterceptor { private Set excludedValuePatterns = null; private Set acceptedValuePatterns = null; - @Inject public void setValueStackFactory(ValueStackFactory valueStackFactory) { this.valueStackFactory = valueStackFactory; @@ -246,8 +245,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { if (action instanceof TextProvider) { TextProvider tp = (TextProvider) action; developerNotification = tp.getText("devmode.notification", - "Developer Notification:\n{0}", - new String[]{developerNotification} + "Developer Notification:\n{0}", + new String[]{developerNotification} ); } @@ -281,13 +280,13 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * @return true if parameter is accepted */ protected boolean isAcceptableParameterValue(Parameter param, Object action) { - ParameterValueAware parameterValueAware = (action instanceof ParameterValueAware) ? (ParameterValueAware) action : null; - boolean acceptableParmValue = (parameterValueAware == null || parameterValueAware.acceptableParameterValue(param.getValue())); - if(hasParamValuesToExclude() || hasParamValuesToAccept()) { - // Additional validations to process - acceptableParmValue &= acceptableValue(param.getName(), param.getValue()); - } - return acceptableParmValue; + ParameterValueAware parameterValueAware = (action instanceof ParameterValueAware) ? (ParameterValueAware) action : null; + boolean acceptableParamValue = (parameterValueAware == null || parameterValueAware.acceptableParameterValue(param.getValue())); + if (hasParamValuesToExclude() || hasParamValuesToAccept()) { + // Additional validations to process + acceptableParamValue &= acceptableValue(param.getName(), param.getValue()); + } + return acceptableParamValue; } /** @@ -352,7 +351,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * * Value is not excluded * * Value is accepted * - * @param name - Param name (for logging) + * @param name - Param name (for logging) * @param value - value to check * @return true if accepted */ @@ -374,9 +373,9 @@ public class ParametersInterceptor extends MethodFilterInterceptor { if (!matchLength) { if (devMode) { // warn only when in devMode LOG.warn("Parameter [{}] is too long, allowed length is [{}]. Use Interceptor Parameter Overriding " + - "to override the limit, see more at\n" + - "https://struts.apache.org/core-developers/interceptors.html#interceptor-parameter-overriding", - name, paramNameMaxLength); + "to override the limit, see more at\n" + + "https://struts.apache.org/core-developers/interceptors.html#interceptor-parameter-overriding", + name, paramNameMaxLength); } else { LOG.warn("Parameter [{}] is too long, allowed length is [{}]", name, paramNameMaxLength); } @@ -390,8 +389,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return true; } else if (devMode) { // warn only when in devMode LOG.warn("Parameter [{}] didn't match accepted pattern [{}]! See Accepted / Excluded patterns at\n" + - "https://struts.apache.org/security/#accepted--excluded-patterns", - paramName, result.getAcceptedPattern()); + "https://struts.apache.org/security/#accepted--excluded-patterns", + paramName, result.getAcceptedPattern()); } else { LOG.debug("Parameter [{}] didn't match accepted pattern [{}]!", paramName, result.getAcceptedPattern()); } @@ -403,8 +402,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { if (result.isExcluded()) { if (devMode) { // warn only when in devMode LOG.warn("Parameter [{}] matches excluded pattern [{}]! See Accepted / Excluded patterns at\n" + - "https://struts.apache.org/security/#accepted--excluded-patterns", - paramName, result.getExcludedPattern()); + "https://struts.apache.org/security/#accepted--excluded-patterns", + paramName, result.getExcludedPattern()); } else { LOG.debug("Parameter [{}] matches excluded pattern [{}]!", paramName, result.getExcludedPattern()); } @@ -413,83 +412,55 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return false; } - - public void setAcceptedValuePatterns(String commaDelimitedPatterns) { - Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); - if (acceptedValuePatterns == null) { - // Limit unwanted log entries (for 1st call, acceptedValuePatterns null) - LOG.debug("Sets accepted value patterns to [{}], note this may impact the safety of your application!", patterns); - } else { - LOG.warn("Replacing accepted patterns [{}] with [{}], be aware that this affects all instances and may impact the safety of your application!", - acceptedValuePatterns, patterns); - } - acceptedValuePatterns = new HashSet<>(patterns.size()); - try { - for (String pattern : patterns) { - acceptedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + protected boolean isParamValueExcluded(String value) { + if (hasParamValuesToExclude()) { + for (Pattern excludedPattern : excludedValuePatterns) { + if (value != null) { + if (excludedPattern.matcher(value).matches()) { + if (devMode) { + LOG.warn("Parameter value [{}] matches excluded pattern [{}]! See Accepting/Excluding parameter values at\n" + + "https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values", + value, excludedValuePatterns); + } else { + LOG.debug("Parameter value [{}] matches excluded pattern [{}]", value, excludedPattern); + } + return true; + } + } } - } finally { - acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns); } + return false; } - public void setExcludeValuePatterns(String commaDelimitedPatterns) { - Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); - if (excludedValuePatterns == null) { - // Limit unwanted log entries (for 1st call, excludedValuePatterns null) - LOG.debug("Setting excluded value patterns to [{}]", patterns); - } else { - LOG.warn("Replacing accepted patterns [{}] with [{}], be aware that this affects all instances and may impact safety of your application!", - excludedValuePatterns, patterns); - } - excludedValuePatterns = new HashSet<>(patterns.size()); - try { - for (String pattern : patterns) { - excludedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + protected boolean isParamValueAccepted(String value) { + if (hasParamValuesToAccept()) { + for (Pattern excludedPattern : acceptedValuePatterns) { + if (value != null) { + if (excludedPattern.matcher(value).matches()) { + return true; + } + } } - } finally { - excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns); + } else { + // acceptedValuePatterns not defined so anything is allowed + return true; } + if (devMode) { + LOG.warn("Parameter value [{}] didn't match accepted pattern [{}]! See Accepting/Excluding parameter values at\n" + + "https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values", + value, acceptedValuePatterns); + } else { + LOG.debug("Parameter value [{}] was not accepted!", value); + } + return false; } - protected boolean isParamValueExcluded(String value) { - if (hasParamValuesToExclude()) { - for (Pattern excludedPattern : excludedValuePatterns) { - if (value != null) { - if (excludedPattern.matcher(value).matches()) { - LOG.warn("Parameter value [{}] matches excluded pattern [{}] and will be dropped.", value, - excludedPattern); - return true; - } - } - } - } - return false; - } - - protected boolean isParamValueAccepted(String value) { - if (hasParamValuesToAccept()) { - for (Pattern excludedPattern : acceptedValuePatterns) { - if (value != null) { - if (excludedPattern.matcher(value).matches()) { - return true; - } - } - } - } else { - // acceptedValuePatterns not defined so anything is allowed - return true; - } - LOG.warn("Parameter value [{}] did not match any acceptedValuePattern pattern and will be dropped.", value); - return false; - } - private boolean hasParamValuesToExclude() { - return excludedValuePatterns != null && excludedValuePatterns.size() > 0; + return excludedValuePatterns != null && excludedValuePatterns.size() > 0; } private boolean hasParamValuesToAccept() { - return acceptedValuePatterns != null && acceptedValuePatterns.size() > 0; + return acceptedValuePatterns != null && acceptedValuePatterns.size() > 0; } /** @@ -533,4 +504,54 @@ public class ParametersInterceptor extends MethodFilterInterceptor { public void setExcludeParams(String commaDelim) { excludedPatterns.setExcludedPatterns(commaDelim); } + + /** + * Sets a comma-delimited list of regular expressions to match + * values of parameters that should be accepted and included in the parameter map. + * + * @param commaDelimitedPatterns A comma-delimited set of regular expressions + */ + public void setAcceptedValuePatterns(String commaDelimitedPatterns) { + Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); + if (acceptedValuePatterns == null) { + // Limit unwanted log entries (for 1st call, acceptedValuePatterns null) + LOG.debug("Sets accepted value patterns to [{}], note this may impact the safety of your application!", patterns); + } else { + LOG.warn("Replacing accepted patterns [{}] with [{}], be aware that this may impact safety of your application!", + acceptedValuePatterns, patterns); + } + acceptedValuePatterns = new HashSet<>(patterns.size()); + try { + for (String pattern : patterns) { + acceptedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } finally { + acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns); + } + } + + /** + * Sets a comma-delimited list of regular expressions to match + * values of parameters that should be removed from the parameter map. + * + * @param commaDelimitedPatterns A comma-delimited set of regular expressions + */ + public void setExcludedValuePatterns(String commaDelimitedPatterns) { + Set patterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns); + if (excludedValuePatterns == null) { + // Limit unwanted log entries (for 1st call, excludedValuePatterns null) + LOG.debug("Setting excluded value patterns to [{}]", patterns); + } else { + LOG.warn("Replacing excluded value patterns [{}] with [{}], be aware that this may impact safety of your application!", + excludedValuePatterns, patterns); + } + excludedValuePatterns = new HashSet<>(patterns.size()); + try { + for (String pattern : patterns) { + excludedValuePatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } finally { + excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns); + } + } } diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java index ce8fe8498..245e868ad 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java @@ -34,7 +34,6 @@ import com.opensymphony.xwork2.conversion.impl.XWorkConverter; import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.ognl.OgnlValueStack; import com.opensymphony.xwork2.ognl.OgnlValueStackFactory; -import com.opensymphony.xwork2.ognl.SecurityMemberAccess; import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor; import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.util.ValueStack; @@ -45,11 +44,9 @@ import ognl.PropertyAccessor; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.dispatcher.HttpParameters; import org.junit.Assert; -import org.springframework.ejb.access.SimpleRemoteStatelessSessionProxyFactoryBean; import java.io.File; import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -781,10 +778,10 @@ public class ParametersInterceptorTest extends XWorkTestCase { public void testExcludedParametersValuesAreIgnored() throws Exception { ParametersInterceptor pi = createParametersInterceptor(); // Contains (based on pattern) - pi.setExcludeValuePatterns(".*\\$\\{.*?\\}.*,.*%\\{.*?\\}.*"); - + pi.setExcludedValuePatterns(".*\\$\\{.*?\\}.*,.*%\\{.*?\\}.*"); + assertTrue("${2*2} was excluded by isParamValueExcluded", pi.isParamValueExcluded("${2*2}")); - + final Map actual = injectValueStackFactory(pi); ValueStack stack = injectValueStack(actual); @@ -812,14 +809,14 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(new NoParametersAction(), stack, HttpParameters.create(parameters).build()); assertEquals(expected, actual); } - + public void testAcceptedParametersValuesAreIgnored() throws Exception { ParametersInterceptor pi = createParametersInterceptor(); // Starts with (based on pattern) pi.setAcceptedValuePatterns("^\\$\\{foo\\}.*,^%\\{bar\\}.*,^fooValue"); assertTrue("${foo} was allowed by isParamValueAccepted", pi.isParamValueAccepted("${foo}")); - + final Map actual = injectValueStackFactory(pi); ValueStack stack = injectValueStack(actual); @@ -849,16 +846,16 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(new NoParametersAction(), stack, HttpParameters.create(parameters).build()); assertEquals(expected, actual); } - + public void testAcceptedAndExcludedParametersValuesAreIgnored() throws Exception { ParametersInterceptor pi = createParametersInterceptor(); // Starts with (based on pattern) pi.setAcceptedValuePatterns("^\\$\\{foo\\}.*,^%\\{bar\\}.*,^fooValue"); - pi.setExcludeValuePatterns(".*\\$\\{2.*2\\}.*,.*\\%\\{2.*2\\}.*"); - + pi.setExcludedValuePatterns(".*\\$\\{2.*2\\}.*,.*\\%\\{2.*2\\}.*"); + assertTrue("${foo} was allowed by isParamValueAccepted", pi.isParamValueAccepted("${foo}")); assertTrue("${2*2} was excluded by isParamValueExcluded", pi.isParamValueExcluded("${2*2}")); - + final Map actual = injectValueStackFactory(pi); ValueStack stack = injectValueStack(actual); @@ -878,7 +875,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { put("barKey%", "%{2+2}"); put("barKey2%", "foo%{2+2}"); put("barKey3", "nothing"); - + put("allowedKey", "${foo}"); put("allowedKey2", "%{bar}"); put("fooKey", "fooValue"); @@ -888,14 +885,14 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(new NoParametersAction(), stack, HttpParameters.create(parameters).build()); assertEquals(expected, actual); } - + public void testExcludedParametersValuesAreIgnoredWithParameterValueAware() throws Exception { ParametersInterceptor pi = createParametersInterceptor(); // Contains (based on pattern) - pi.setExcludeValuePatterns(".*\\$\\{.*?\\}.*,.*%\\{.*?\\}.*"); - + pi.setExcludedValuePatterns(".*\\$\\{.*?\\}.*,.*%\\{.*?\\}.*"); + assertTrue("${2*2} was excluded by isParamValueExcluded", pi.isParamValueExcluded("${2*2}")); - + final Map actual = injectValueStackFactory(pi); ValueStack stack = injectValueStack(actual); @@ -905,7 +902,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { put("fooKey", "fooValue"); } }; - + Object a = new ParameterValueAware() { @Override public boolean acceptableParameterValue(String parameterValue) { From 2217e2c1ca96760cba59950efe06194f05a10604 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 30 Sep 2022 08:19:09 +0200 Subject: [PATCH 3/5] Disables Code quality step in Jenkins pipeline to avoid overriding GH Actions results --- Jenkinsfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index 233ade32d..762d88240 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -88,6 +88,7 @@ pipeline { } } } +/* stage('Code Quality') { when { branch 'master' @@ -98,6 +99,7 @@ pipeline { } } } +*/ } post { always { From 5338b44c233dbc7a26a5b0c6b0993d770fa5340e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 30 Sep 2022 08:25:44 +0200 Subject: [PATCH 4/5] WW-5184 Reduces code complexity when handling excluded/accepted values patterns --- .../interceptor/ParametersInterceptor.java | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java index 73f650442..3fa04220a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java @@ -413,38 +413,35 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } protected boolean isParamValueExcluded(String value) { - if (hasParamValuesToExclude()) { - for (Pattern excludedPattern : excludedValuePatterns) { - if (value != null) { - if (excludedPattern.matcher(value).matches()) { - if (devMode) { - LOG.warn("Parameter value [{}] matches excluded pattern [{}]! See Accepting/Excluding parameter values at\n" + - "https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values", - value, excludedValuePatterns); - } else { - LOG.debug("Parameter value [{}] matches excluded pattern [{}]", value, excludedPattern); - } - return true; - } + if (!hasParamValuesToExclude()) { + LOG.debug("'excludedValuePatterns' not defined so anything is allowed"); + return false; + } + for (Pattern excludedValuePattern : excludedValuePatterns) { + if (excludedValuePattern.matcher(value).matches()) { + if (devMode) { + LOG.warn("Parameter value [{}] matches excluded pattern [{}]! See Accepting/Excluding parameter values at\n" + + "https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values", + value, excludedValuePatterns); + } else { + LOG.debug("Parameter value [{}] matches excluded pattern [{}]", value, excludedValuePattern); } + return true; } } return false; } protected boolean isParamValueAccepted(String value) { - if (hasParamValuesToAccept()) { - for (Pattern excludedPattern : acceptedValuePatterns) { - if (value != null) { - if (excludedPattern.matcher(value).matches()) { - return true; - } - } - } - } else { - // acceptedValuePatterns not defined so anything is allowed + if (!hasParamValuesToAccept()) { + LOG.debug("'acceptedValuePatterns' not defined so anything is allowed"); return true; } + for (Pattern acceptedValuePattern : acceptedValuePatterns) { + if (acceptedValuePattern.matcher(value).matches()) { + return true; + } + } if (devMode) { LOG.warn("Parameter value [{}] didn't match accepted pattern [{}]! See Accepting/Excluding parameter values at\n" + "https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values", From e0da03c2627530c0e77eca94ee5351c733be795c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 30 Sep 2022 10:38:15 +0200 Subject: [PATCH 5/5] Sets proper Sonar options in Jenkins pipeline and removes duplicated sonar properties --- Jenkinsfile | 6 ++---- pom.xml | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 762d88240..2adf56ac9 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -79,7 +79,7 @@ pipeline { } stage('Test') { steps { - sh './mvnw -B test' + sh './mvnw -B verify -Pcoverage -DskipAssembly' } post { always { @@ -88,18 +88,16 @@ pipeline { } } } -/* stage('Code Quality') { when { branch 'master' } steps { withCredentials([string(credentialsId: 'asf-struts-sonarcloud', variable: 'SONARCLOUD_TOKEN')]) { - sh './mvnw sonar:sonar -DskipAssembly -Dsonar.login=${SONARCLOUD_TOKEN}' + sh './mvnw -B -Pcoverage -DskipAssembly -Dsonar.login=${SONARCLOUD_TOKEN} sonar:sonar' } } } -*/ } post { always { diff --git a/pom.xml b/pom.xml index ff48f95f1..b80a4b2f6 100644 --- a/pom.xml +++ b/pom.xml @@ -125,9 +125,9 @@ 1.9 + apache apache_struts ${project.artifactId} - apache https://sonarcloud.io apps/** @@ -213,8 +213,6 @@ coverage https://sonarcloud.io - apache - apache_struts