From 043814b7749e42cf04e55a859c485aae8abbb81c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 21 May 2020 08:11:20 +0200 Subject: [PATCH] WW-5077 Uses better logging to inform user about excluded params --- .../interceptor/ParametersInterceptor.java | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 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 6dc1c19536..549574f8bc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java @@ -25,13 +25,16 @@ import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.security.AcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; -import com.opensymphony.xwork2.util.*; +import com.opensymphony.xwork2.util.ClearableValueStack; +import com.opensymphony.xwork2.util.MemberAccessValueStack; +import com.opensymphony.xwork2.util.ValueStack; +import com.opensymphony.xwork2.util.ValueStackFactory; import com.opensymphony.xwork2.util.reflection.ReflectionContextState; import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -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.Comparator; @@ -100,8 +103,8 @@ static private int countOGNLCharacters(String s) { */ static final Comparator rbCollator = new Comparator() { public int compare(String s1, String s2) { - int l1 = countOGNLCharacters(s1), - l2 = countOGNLCharacters(s2); + int l1 = countOGNLCharacters(s1); + int l2 = countOGNLCharacters(s2); return l1 < l2 ? -1 : (l2 < l1 ? 1 : s1.compareTo(s2)); } @@ -185,7 +188,7 @@ protected void setParameters(final Object action, ValueStack stack, HttpParamete if (clearableStack) { //if the stack's context can be cleared, do that to prevent OGNL //from having access to objects in the stack, see XW-641 - ((ClearableValueStack)newStack).clearContextValues(); + ((ClearableValueStack) newStack).clearContextValues(); Map context = newStack.getContext(); ReflectionContextState.setCreatingNullObjects(context, true); ReflectionContextState.setDenyMethodExecution(context, true); @@ -228,7 +231,7 @@ protected void notifyDeveloperParameterException(Object action, String property, TextProvider tp = (TextProvider) action; developerNotification = tp.getText("devmode.notification", "Developer Notification:\n{0}", - new String[]{ developerNotification } + new String[]{developerNotification} ); } @@ -245,7 +248,7 @@ protected void notifyDeveloperParameterException(Object action, String property, /** * Checks if name of parameter can be accepted or thrown away * - * @param name parameter name + * @param name parameter name * @param action current action * @return true if parameter is accepted */ @@ -289,27 +292,45 @@ protected boolean acceptableName(String name) { return accepted; } - protected boolean isWithinLengthLimit( String name ) { + protected boolean isWithinLengthLimit(String name) { boolean matchLength = name.length() <= paramNameMaxLength; if (!matchLength) { - LOG.debug("Parameter [{}] is too long, allowed length is [{}]", name, String.valueOf(paramNameMaxLength)); + 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); + } else { + LOG.warn("Parameter [{}] is too long, allowed length is [{}]", name, paramNameMaxLength); + } } return matchLength; - } + } protected boolean isAccepted(String paramName) { AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName); if (result.isAccepted()) { 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()); + } else { + LOG.debug("Parameter [{}] didn't match accepted pattern [{}]!", paramName, result.getAcceptedPattern()); } - LOG.debug("Parameter [{}] didn't match accepted pattern [{}]!", paramName, result.getAcceptedPattern()); return false; } protected boolean isExcluded(String paramName) { ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName); if (result.isExcluded()) { - LOG.debug("Parameter [{}] matches excluded pattern [{}]!", paramName, result.getExcludedPattern()); + 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()); + } else { + LOG.debug("Parameter [{}] matches excluded pattern [{}]!", paramName, result.getExcludedPattern()); + } return true; } return false;