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/06/06 17:36:12 UTC
[GitHub] [struts] yasserzamani commented on a diff in pull request #559: WW-5184 - Add optional parameter value check to ParametersInterceptor
yasserzamani commented on code in PR #559:
URL: https://github.com/apache/struts/pull/559#discussion_r890375109
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -354,7 +407,86 @@ protected boolean isExcluded(String paramName) {
}
return false;
}
+
+ public void setAcceptedValuePatterns(String commaDelimitedPatterns) {
Review Comment:
BTW another methods annotated with `@Inject` as well as some other changes are required enabling user to configure these patterns via XML configuration.
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -354,7 +407,86 @@ protected boolean isExcluded(String paramName) {
}
return false;
}
+
+ public void setAcceptedValuePatterns(String commaDelimitedPatterns) {
+ Set<String> 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));
+ }
+ } finally {
+ acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns);
+ }
+ }
+
+ public void setExcludeValuePatterns(String commaDelimitedPatterns) {
+ Set<String> 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));
+ }
+ } finally {
+ excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns);
+ }
+ }
+
+ protected boolean isParamValueExcluded(String value) {
+ if (hasParamValuesToExclude()) {
+ for (Pattern excludedPattern : excludedValuePatterns) {
+ if (value != null) {
+ if (excludedPattern.matcher(value).matches()) {
+ LOG.info("Parameter value [{}] matches excluded pattern [{}] and will be dropped.", value,
Review Comment:
same here
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -311,6 +346,24 @@ private boolean isIgnoredDMI(String name) {
}
}
+ /**
+ * Validates:
+ * * 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)));
+ if (!accepted) {
+ LOG.info("Parameter [{}] was not accepted with value [{}] and will be dropped!", name, value);
Review Comment:
Sorry my failure in previous PR. Could you please log such thing with WARN level? as well as other similar places.
##########
core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java:
##########
@@ -354,7 +407,86 @@ protected boolean isExcluded(String paramName) {
}
return false;
}
+
+ public void setAcceptedValuePatterns(String commaDelimitedPatterns) {
+ Set<String> 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));
+ }
+ } finally {
+ acceptedValuePatterns = Collections.unmodifiableSet(acceptedValuePatterns);
+ }
+ }
+
+ public void setExcludeValuePatterns(String commaDelimitedPatterns) {
+ Set<String> 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));
+ }
+ } finally {
+ excludedValuePatterns = Collections.unmodifiableSet(excludedValuePatterns);
+ }
+ }
+
+ protected boolean isParamValueExcluded(String value) {
+ if (hasParamValuesToExclude()) {
+ for (Pattern excludedPattern : excludedValuePatterns) {
+ if (value != null) {
+ if (excludedPattern.matcher(value).matches()) {
+ LOG.info("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.info("Parameter value [{}] did not match any acceptedValuePattern pattern and will be dropped.", value);
Review Comment:
and here
--
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