diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index f263df383..4a39a335b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -33,6 +33,7 @@ import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker; import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; import com.opensymphony.xwork2.DefaultTextProvider; import com.opensymphony.xwork2.DefaultUnknownHandlerManager; +import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.FileManager; import com.opensymphony.xwork2.FileManagerFactory; @@ -54,6 +55,7 @@ import com.opensymphony.xwork2.conversion.impl.CollectionConverter; import com.opensymphony.xwork2.conversion.impl.DateConverter; import com.opensymphony.xwork2.conversion.impl.DefaultConversionAnnotationProcessor; import com.opensymphony.xwork2.conversion.impl.DefaultConversionFileProcessor; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import org.apache.struts2.conversion.StrutsConversionPropertiesProcessor; import com.opensymphony.xwork2.conversion.impl.DefaultObjectTypeDeterminer; import org.apache.struts2.conversion.StrutsTypeConverterCreator; @@ -212,6 +214,8 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider .factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class, Scope.PROTOTYPE) .factory(AcceptedPatternsChecker.class, DefaultAcceptedPatternsChecker.class, Scope.PROTOTYPE) + .factory(NotExcludedAcceptedPatternsChecker.class, DefaultNotExcludedAcceptedPatternsChecker.class + , Scope.SINGLETON) .factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON) ; diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java index 3a41df722..9edafe3fc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java @@ -22,6 +22,8 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.util.ClearableValueStack; import com.opensymphony.xwork2.util.Evaluated; import com.opensymphony.xwork2.LocalizedTextProvider; @@ -100,6 +102,9 @@ public class AliasInterceptor extends AbstractInterceptor { protected LocalizedTextProvider localizedTextProvider; protected boolean devMode = false; + private ExcludedPatternsChecker excludedPatterns; + private AcceptedPatternsChecker acceptedPatterns; + @Inject(StrutsConstants.STRUTS_DEVMODE) public void setDevMode(String mode) { this.devMode = Boolean.parseBoolean(mode); @@ -115,6 +120,16 @@ public class AliasInterceptor extends AbstractInterceptor { this.localizedTextProvider = localizedTextProvider; } + @Inject + public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) { + this.excludedPatterns = excludedPatterns; + } + + @Inject + public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) { + this.acceptedPatterns = acceptedPatterns; + } + /** *

* Sets the name of the action parameter to look for the alias map. @@ -145,7 +160,7 @@ public class AliasInterceptor extends AbstractInterceptor { ValueStack stack = ac.getValueStack(); Object obj = stack.findValue(aliasExpression); - if (obj != null && obj instanceof Map) { + if (obj instanceof Map) { //get secure stack ValueStack newStack = valueStackFactory.createValueStack(stack); boolean clearableStack = newStack instanceof ClearableValueStack; @@ -167,7 +182,13 @@ public class AliasInterceptor extends AbstractInterceptor { for (Object o : aliases.entrySet()) { Map.Entry entry = (Map.Entry) o; String name = entry.getKey().toString(); + if (isNotAcceptableExpression(name)) { + continue; + } String alias = (String) entry.getValue(); + if (isNotAcceptableExpression(alias)) { + continue; + } Evaluated value = new Evaluated(stack.findValue(name)); if (!value.isDefined()) { // workaround @@ -207,5 +228,65 @@ public class AliasInterceptor extends AbstractInterceptor { return invocation.invoke(); } - + + protected boolean isAccepted(String paramName) { + AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName); + if (result.isAccepted()) { + return true; + } + + 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()); + + return false; + } + + protected boolean isExcluded(String paramName) { + ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName); + if (!result.isExcluded()) { + return false; + } + + LOG.warn("Parameter [{}] matches excluded pattern [{}]! See Accepted / Excluded patterns at\n" + + "https://struts.apache.org/security/#accepted--excluded-patterns", + paramName, result.getExcludedPattern()); + + return true; + } + + /** + * Checks if expression contains vulnerable code + * + * @param expression of interceptor + * @return true|false + */ + protected boolean isNotAcceptableExpression(String expression) { + return isExcluded(expression) || !isAccepted(expression); + } + + /** + * Sets a comma-delimited list of regular expressions to match + * parameters that are allowed in the parameter map (aka whitelist). + *

+ * Don't change the default unless you know what you are doing in terms + * of security implications. + *

+ * + * @param commaDelim A comma-delimited list of regular expressions + */ + public void setAcceptParamNames(String commaDelim) { + acceptedPatterns.setAcceptedPatterns(commaDelim); + } + + /** + * Sets a comma-delimited list of regular expressions to match + * parameters that should be removed from the parameter map. + * + * @param commaDelim A comma-delimited list of regular expressions + */ + public void setExcludeParams(String commaDelim) { + excludedPatterns.setExcludedPatterns(commaDelim); + } + } diff --git a/core/src/main/java/com/opensymphony/xwork2/mock/MockResult.java b/core/src/main/java/com/opensymphony/xwork2/mock/MockResult.java index f85cae4a7..6d3debec3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/mock/MockResult.java +++ b/core/src/main/java/com/opensymphony/xwork2/mock/MockResult.java @@ -31,6 +31,8 @@ public class MockResult implements Result { public static final String DEFAULT_PARAM = "foo"; + private ActionInvocation invocation; + @Override public boolean equals(Object o) { if (this == o) { @@ -41,7 +43,7 @@ public class MockResult implements Result { } public void execute(ActionInvocation invocation) throws Exception { - // no op + this.invocation = invocation; } @Override @@ -53,4 +55,7 @@ public class MockResult implements Result { // no op } + public ActionInvocation getInvocation() { + return invocation; + } } diff --git a/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java index fa92e0b2a..f5a329459 100644 --- a/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java +++ b/core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java @@ -22,7 +22,7 @@ import java.util.Set; import java.util.regex.Pattern; /** - * Used across different interceptors to check if given string matches one of the excluded patterns. + * Used across different interceptors to check if given string matches one of the accepted patterns. */ public interface AcceptedPatternsChecker { diff --git a/core/src/main/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsChecker.java new file mode 100644 index 000000000..b475da1d0 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsChecker.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.inject.Inject; + +import java.util.Set; +import java.util.regex.Pattern; + +public class DefaultNotExcludedAcceptedPatternsChecker implements NotExcludedAcceptedPatternsChecker { + private ExcludedPatternsChecker excludedPatterns; + private AcceptedPatternsChecker acceptedPatterns; + + + @Inject + public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) { + this.excludedPatterns = excludedPatterns; + } + + @Inject + public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) { + this.acceptedPatterns = acceptedPatterns; + } + + @Override + public IsAllowed isAllowed(String value) { + IsExcluded isExcluded = isExcluded(value); + if (isExcluded.isExcluded()) { + return IsAllowed.no(isExcluded.getExcludedPattern()); + } + + IsAccepted isAccepted = isAccepted(value); + if (!isAccepted.isAccepted()) { + return IsAllowed.no(isAccepted.getAcceptedPattern()); + } + + return IsAllowed.yes(isAccepted.getAcceptedPattern()); + } + + @Override + public IsAccepted isAccepted(String value) { + return acceptedPatterns.isAccepted(value); + } + + @Override + public void setAcceptedPatterns(String commaDelimitedPatterns) { + acceptedPatterns.setAcceptedPatterns(commaDelimitedPatterns); + } + + @Override + public void setAcceptedPatterns(String[] patterns) { + acceptedPatterns.setAcceptedPatterns(patterns); + } + + @Override + public void setAcceptedPatterns(Set patterns) { + acceptedPatterns.setAcceptedPatterns(patterns); + } + + @Override + public Set getAcceptedPatterns() { + return acceptedPatterns.getAcceptedPatterns(); + } + + @Override + public IsExcluded isExcluded(String value) { + return excludedPatterns.isExcluded(value); + } + + @Override + public void setExcludedPatterns(String commaDelimitedPatterns) { + excludedPatterns.setExcludedPatterns(commaDelimitedPatterns); + } + + @Override + public void setExcludedPatterns(String[] patterns) { + excludedPatterns.setExcludedPatterns(patterns); + } + + @Override + public void setExcludedPatterns(Set patterns) { + excludedPatterns.setExcludedPatterns(patterns); + } + + @Override + public Set getExcludedPatterns() { + return excludedPatterns.getExcludedPatterns(); + } +} diff --git a/core/src/main/java/com/opensymphony/xwork2/security/NotExcludedAcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/NotExcludedAcceptedPatternsChecker.java new file mode 100644 index 000000000..030b5b67e --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/security/NotExcludedAcceptedPatternsChecker.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.opensymphony.xwork2.security; + +/** + * Used across different places to check if given string is not excluded and is accepted + * @see here + * @since 2.6 + */ +public interface NotExcludedAcceptedPatternsChecker extends ExcludedPatternsChecker, AcceptedPatternsChecker { + + /** + * Checks if value doesn't match excluded pattern and matches accepted pattern + * + * @param value to check + * @return object containing result of matched pattern and pattern itself + */ + IsAllowed isAllowed(String value); + + final class IsAllowed { + + private final boolean allowed; + private final String allowedPattern; + + public static IsAllowed yes(String allowedPattern) { + return new IsAllowed(true, allowedPattern); + } + + public static IsAllowed no(String allowedPattern) { + return new IsAllowed(false, allowedPattern); + } + + private IsAllowed(boolean allowed, String allowedPattern) { + this.allowed = allowed; + this.allowedPattern = allowedPattern; + } + + public boolean isAllowed() { + return allowed; + } + + public String getAllowedPattern() { + return allowedPattern; + } + + @Override + public String toString() { + return "IsAllowed { " + + "allowed=" + allowed + + ", allowedPattern=" + allowedPattern + + " }"; + } + } +} diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index ccba9b101..60007cea7 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -334,6 +334,7 @@ public final class StrutsConstants { /** Dedicated services to check if passed string is excluded/accepted */ public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER = "struts.excludedPatterns.checker"; public static final String STRUTS_ACCEPTED_PATTERNS_CHECKER = "struts.acceptedPatterns.checker"; + public static final String STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER = "struts.notExcludedAcceptedPatterns.checker"; /** Constant is used to override framework's default excluded patterns */ public static final String STRUTS_OVERRIDE_EXCLUDED_PATTERNS = "struts.override.excludedPatterns"; diff --git a/core/src/main/java/org/apache/struts2/components/Component.java b/core/src/main/java/org/apache/struts2/components/Component.java index e20b4f5e8..748285b76 100644 --- a/core/src/main/java/org/apache/struts2/components/Component.java +++ b/core/src/main/java/org/apache/struts2/components/Component.java @@ -19,6 +19,7 @@ package org.apache.struts2.components; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; import org.apache.commons.lang3.BooleanUtils; @@ -73,6 +74,8 @@ public class Component { protected boolean throwExceptionOnELFailure; private UrlHelper urlHelper; + private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns; + /** * Constructor. * @@ -117,6 +120,11 @@ public class Component { this.urlHelper = urlHelper; } + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + /** * Gets the OGNL value stack associated with this component. * @@ -209,7 +217,6 @@ public class Component { if (currPosition >= 0) { int start = componentStack.size() - currPosition - 1; - //for (int i = componentStack.size() - 2; i >= 0; i--) { for (int i = start; i >= 0; i--) { Component component = (Component) componentStack.get(i); if (clazz.isAssignableFrom(component.getClass()) && component != this) { @@ -376,16 +383,6 @@ public class Component { } } - /** - * Detects if expression already contains %{...} - * - * @param expression a string to examined - * @return true if expression contains %{...} - */ - protected boolean recursion(String expression) { - return ComponentUtils.containsExpression(expression); - } - /** * Renders an action URL by consulting the {@link org.apache.struts2.dispatcher.mapper.ActionMapper}. * @@ -403,7 +400,7 @@ public class Component { * @return the action url. */ protected String determineActionURL(String action, String namespace, String method, - HttpServletRequest req, HttpServletResponse res, Map parameters, String scheme, + HttpServletRequest req, HttpServletResponse res, Map parameters, String scheme, boolean includeContext, boolean encodeResult, boolean forceAddSchemeHostAndPort, boolean escapeAmp) { String finalAction = findString(action); @@ -571,4 +568,22 @@ public class Component { return standardAttributes; } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of the component + * @return true|false + * @since 2.6 + */ + protected boolean isAcceptableExpression(String expression) { + NotExcludedAcceptedPatternsChecker.IsAllowed isAllowed = notExcludedAcceptedPatterns.isAllowed(expression); + if (isAllowed.isAllowed()) { + return true; + } + + LOG.warn("Expression [{}] isn't allowed by pattern [{}]! See Accepted / Excluded patterns at\n" + + "https://struts.apache.org/security/", expression, isAllowed.getAllowedPattern()); + + return false; + } } diff --git a/core/src/main/java/org/apache/struts2/components/FormButton.java b/core/src/main/java/org/apache/struts2/components/FormButton.java index 5cf88bc42..d9e75f1e4 100644 --- a/core/src/main/java/org/apache/struts2/components/FormButton.java +++ b/core/src/main/java/org/apache/struts2/components/FormButton.java @@ -122,6 +122,7 @@ public abstract class FormButton extends ClosingUIBean { } } addParameter("id", _tmp_id); + addParameter("escapedId", escape(_tmp_id)); } /** diff --git a/core/src/main/java/org/apache/struts2/components/Param.java b/core/src/main/java/org/apache/struts2/components/Param.java index 023761ad9..c2c299e7e 100644 --- a/core/src/main/java/org/apache/struts2/components/Param.java +++ b/core/src/main/java/org/apache/struts2/components/Param.java @@ -125,23 +125,29 @@ public class Param extends Component { if (component instanceof UnnamedParametric) { ((UnnamedParametric) component).addParameter(findValue(value)); } else { - String name = findString(this.name); + String translatedName = findString(this.name); - if (name == null) { + if (translatedName == null) { throw new StrutsException("No name found for following expression: " + this.name); } - Object value = findValue(this.value); + boolean evaluated = !translatedName.equals(this.name); + boolean reevaluate = !evaluated || isAcceptableExpression(translatedName); + if (!reevaluate) { + throw new StrutsException("Excluded or not accepted name found: " + translatedName); + } + + Object foundValue = findValue(this.value); if (suppressEmptyParameters) { - if (value != null && StringUtils.isNotBlank(value.toString())) { - component.addParameter(name, value); + if (foundValue != null && StringUtils.isNotBlank(foundValue.toString())) { + component.addParameter(translatedName, foundValue); } else { - component.addParameter(name, null); + component.addParameter(translatedName, null); } - } else if (value == null || StringUtils.isBlank(value.toString())) { - component.addParameter(name, ""); + } else if (foundValue == null || StringUtils.isBlank(foundValue.toString())) { + component.addParameter(translatedName, ""); } else { - component.addParameter(name, value); + component.addParameter(translatedName, foundValue); } } } else { @@ -158,7 +164,8 @@ public class Param extends Component { return super.end(writer, ""); } - + + @Override public boolean usesBody() { return true; } @@ -193,7 +200,7 @@ public class Param extends Component { * Adds the given value as a parameter to the outer tag. * @param value the value */ - public void addParameter(Object value); + void addParameter(Object value); } } diff --git a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java index a302e8a77..3fa192584 100644 --- a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java +++ b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java @@ -191,7 +191,9 @@ public class ServletUrlRenderer implements UrlRenderer { // if the id isn't specified, use the action name if (formComponent.getId() == null && actionName != null) { - formComponent.addParameter("id", formComponent.escape(actionName)); + String escapedId = formComponent.escape(actionName); + formComponent.addParameter("id", escapedId); + formComponent.addParameter("escapedId", escapedId); } } else if (action != null) { // Since we can't find an action alias in the configuration, we just @@ -226,7 +228,9 @@ public class ServletUrlRenderer implements UrlRenderer { } else { id = result.substring(slash + 1); } - formComponent.addParameter("id", formComponent.escape(id)); + String escapedId = formComponent.escape(id); + formComponent.addParameter("id", escapedId); + formComponent.addParameter("escapedId", escapedId); } } diff --git a/core/src/main/java/org/apache/struts2/components/UIBean.java b/core/src/main/java/org/apache/struts2/components/UIBean.java index 8ebd50604..24b47fa14 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -563,16 +563,13 @@ public abstract class UIBean extends Component { protected abstract String getDefaultTemplate(); protected Template buildTemplateName(String myTemplate, String myDefaultTemplate) { - String template = myDefaultTemplate; + String templateName = myDefaultTemplate; if (myTemplate != null) { - template = findString(myTemplate); + templateName = findString(myTemplate); } - String templateDir = getTemplateDir(); - String theme = getTheme(); - - return new Template(templateDir, theme, template); + return new Template(getTemplateDir(), getTheme(), templateName); } @@ -589,73 +586,72 @@ public abstract class UIBean extends Component { } public String getTemplateDir() { - String templateDir = null; + String result = null; if (this.templateDir != null) { - templateDir = findString(this.templateDir); + result = findString(this.templateDir); } // If templateDir is not explicitly given, // try to find attribute which states the dir set to use - if (StringUtils.isBlank(templateDir)) { - templateDir = stack.findString("#attr.templateDir"); + if (StringUtils.isBlank(result)) { + result = stack.findString("#attr.templateDir"); } // Default template set - if (StringUtils.isBlank(templateDir)) { - templateDir = defaultTemplateDir; + if (StringUtils.isBlank(result)) { + result = defaultTemplateDir; } // Defaults to 'template' - if (StringUtils.isBlank(templateDir)) { - templateDir = "template"; + if (StringUtils.isBlank(result)) { + result = "template"; } - return templateDir; + return result; } public String getTheme() { - String theme = null; + String result = null; if (this.theme != null) { - theme = findString(this.theme); + result = findString(this.theme); } - if (StringUtils.isBlank(theme)) { + if (StringUtils.isBlank(result)) { Form form = (Form) findAncestor(Form.class); if (form != null) { - theme = form.getTheme(); + result = form.getTheme(); } } // If theme set is not explicitly given, // try to find attribute which states the theme set to use - if (StringUtils.isBlank(theme)) { - theme = stack.findString("#attr.theme"); + if (StringUtils.isBlank(result)) { + result = stack.findString("#attr.theme"); } // Default theme set - if (StringUtils.isBlank(theme)) { - theme = defaultUITheme; + if (StringUtils.isBlank(result)) { + result = defaultUITheme; } - return theme; + return result; } public void evaluateParams() { - String templateDir = getTemplateDir(); - String theme = getTheme(); + String gotTheme = getTheme(); - addParameter("templateDir", templateDir); - addParameter("theme", theme); + addParameter("templateDir", getTemplateDir()); + addParameter("theme", gotTheme); addParameter("template", template != null ? findString(template) : getDefaultTemplate()); addParameter("dynamicAttributes", dynamicAttributes); addParameter("themeExpansionToken", uiThemeExpansionToken); - addParameter("expandTheme", uiThemeExpansionToken + theme); + addParameter("expandTheme", uiThemeExpansionToken + gotTheme); addParameter("staticContentPath", findString(uiStaticContentPath)); - String name = null; + String translatedName = null; String providedLabel = null; if (this.key != null) { @@ -671,8 +667,8 @@ public abstract class UIBean extends Component { } if (this.name != null) { - name = findString(this.name); - addParameter("name", name); + translatedName = findString(this.name); + addParameter("name", translatedName); } if (label != null) { @@ -796,28 +792,31 @@ public abstract class UIBean extends Component { // see if the value was specified as a parameter already + final String NAME_VALUE = "nameValue"; if (parameters.containsKey("value")) { - parameters.put("nameValue", parameters.get("value")); + parameters.put(NAME_VALUE, parameters.get("value")); } else { if (evaluateNameValue()) { final Class valueClazz = getValueClassType(); if (valueClazz != null) { if (value != null) { - addParameter("nameValue", findValue(value, valueClazz)); - } else if (name != null) { - String expr = completeExpression(name); - if (recursion(name)) { - addParameter("nameValue", expr); + addParameter(NAME_VALUE, findValue(value, valueClazz)); + } else if (translatedName != null) { + boolean evaluated = !translatedName.equals(this.name); + boolean reevaluate = !evaluated || isAcceptableExpression(translatedName); + if (!reevaluate) { + addParameter(NAME_VALUE, translatedName); } else { - addParameter("nameValue", findValue(expr, valueClazz)); + String expr = completeExpression(translatedName); + addParameter(NAME_VALUE, findValue(expr, valueClazz)); } } } else { if (value != null) { - addParameter("nameValue", findValue(value)); - } else if (name != null) { - addParameter("nameValue", findValue(name)); + addParameter(NAME_VALUE, findValue(value)); + } else if (translatedName != null) { + addParameter(NAME_VALUE, findValue(translatedName)); } } } @@ -831,10 +830,10 @@ public abstract class UIBean extends Component { if (form != null ) { addParameter("form", form.getParameters()); - if ( name != null ) { + if ( translatedName != null ) { // list should have been created by the form component List tags = (List) form.getParameters().get("tagNames"); - tags.add(name); + tags.add(translatedName); } } @@ -897,11 +896,9 @@ public abstract class UIBean extends Component { // to be used with the CSP interceptor - adds the nonce value as a parameter to be accessed from ftl files Map session = stack.getActionContext().getSession(); - if (session != null) { - if (session.containsKey("nonce")) { - String nonceValue = session.get("nonce").toString(); - addParameter("nonce", nonceValue); - } + Object nonceValue = session != null ? session.get("nonce") : null; + if (nonceValue != null) { + addParameter("nonce", nonceValue.toString()); } evaluateExtraParams(); @@ -910,7 +907,7 @@ public abstract class UIBean extends Component { protected String escape(String name) { // escape any possible values that can make the ID painful to work with in JavaScript if (name != null) { - return name.replaceAll("[/.\\[\\]'\"]", "_"); + return name.replaceAll("[^a-zA-Z0-9_]", "_"); } else { return null; } @@ -961,14 +958,14 @@ public abstract class UIBean extends Component { protected Map getTooltipConfig(UIBean component) { Object tooltipConfigObj = component.getParameters().get("tooltipConfig"); - Map tooltipConfig = new LinkedHashMap<>(); + Map result = new LinkedHashMap<>(); if (tooltipConfigObj instanceof Map) { // we get this if its configured using // 1] UI component's tooltipConfig attribute OR // 2] param tag value attribute - tooltipConfig = new LinkedHashMap<>((Map) tooltipConfigObj); + result = new LinkedHashMap<>((Map) tooltipConfigObj); } else if (tooltipConfigObj instanceof String) { // we get this if its configured using @@ -978,23 +975,23 @@ public abstract class UIBean extends Component { for (String aTooltipConfigArray : tooltipConfigArray) { String[] configEntry = aTooltipConfigArray.trim().split("="); - String key = configEntry[0].trim(); - String value; + String configKey = configEntry[0].trim(); + String configValue; if (configEntry.length > 1) { - value = configEntry[1].trim(); - tooltipConfig.put(key, value); + configValue = configEntry[1].trim(); + result.put(configKey, configValue); } else { - LOG.warn("component {} tooltip config param {} has no value defined, skipped", component, key); + LOG.warn("component {} tooltip config param {} has no value defined, skipped", component, configKey); } } } if (component.javascriptTooltip != null) - tooltipConfig.put("jsTooltipEnabled", component.javascriptTooltip); + result.put("jsTooltipEnabled", component.javascriptTooltip); if (component.tooltipIconPath != null) - tooltipConfig.put("tooltipIcon", component.tooltipIconPath); + result.put("tooltipIcon", component.tooltipIconPath); if (component.tooltipDelay != null) - tooltipConfig.put("tooltipDelay", component.tooltipDelay); - return tooltipConfig; + result.put("tooltipDelay", component.tooltipDelay); + return result; } /** @@ -1275,10 +1272,10 @@ public abstract class UIBean extends Component { public void setDynamicAttributes(Map tagDynamicAttributes) { for (Map.Entry entry : tagDynamicAttributes.entrySet()) { - String key = entry.getKey(); + String entryKey = entry.getKey(); - if (!isValidTagAttribute(key)) { - dynamicAttributes.put(key, entry.getValue()); + if (!isValidTagAttribute(entryKey)) { + dynamicAttributes.put(entryKey, entry.getValue()); } } } @@ -1292,9 +1289,9 @@ public abstract class UIBean extends Component { public void copyParams(Map params) { super.copyParams(params); for (Map.Entryentry : params.entrySet()) { - String key = entry.getKey(); - if (!isValidTagAttribute(key) && !key.equals("dynamicAttributes")) { - dynamicAttributes.put(key, entry.getValue()); + String entryKey = entry.getKey(); + if (!isValidTagAttribute(entryKey) && !entryKey.equals("dynamicAttributes")) { + dynamicAttributes.put(entryKey, entry.getValue()); } } } diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index 08f46fa86..ade6aa0b0 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -49,6 +49,7 @@ import com.opensymphony.xwork2.factory.ResultFactory; import com.opensymphony.xwork2.factory.ValidatorFactory; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.inject.Scope; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.PatternMatcher; import com.opensymphony.xwork2.util.TextParser; import com.opensymphony.xwork2.util.ValueStackFactory; @@ -418,6 +419,8 @@ public class StrutsBeanSelectionProvider extends AbstractBeanSelectionProvider { /** Checker is used mostly in interceptors, so there be one instance of checker per interceptor with Scope.PROTOTYPE **/ alias(ExcludedPatternsChecker.class, StrutsConstants.STRUTS_EXCLUDED_PATTERNS_CHECKER, builder, props, Scope.PROTOTYPE); alias(AcceptedPatternsChecker.class, StrutsConstants.STRUTS_ACCEPTED_PATTERNS_CHECKER, builder, props, Scope.PROTOTYPE); + alias(NotExcludedAcceptedPatternsChecker.class, StrutsConstants.STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER + , builder, props, Scope.SINGLETON); switchDevMode(props); } diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 7729c8d89..df08ac7c1 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -132,6 +132,7 @@ public class ConstantConfig { private Set devModeExcludedPackageNames; private BeanConfig excludedPatternsChecker; private BeanConfig acceptedPatternsChecker; + private BeanConfig notExcludedAcceptedPatternsChecker; private Set overrideExcludedPatterns; private Set overrideAcceptedPatterns; private Set additionalExcludedPatterns; @@ -261,6 +262,7 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, StringUtils.join(devModeExcludedPackageNames, ',')); map.put(StrutsConstants.STRUTS_EXCLUDED_PATTERNS_CHECKER, beanConfToString(excludedPatternsChecker)); map.put(StrutsConstants.STRUTS_ACCEPTED_PATTERNS_CHECKER, beanConfToString(acceptedPatternsChecker)); + map.put(StrutsConstants.STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER, beanConfToString(notExcludedAcceptedPatternsChecker)); map.put(StrutsConstants.STRUTS_OVERRIDE_EXCLUDED_PATTERNS, StringUtils.join(overrideExcludedPatterns, ',')); map.put(StrutsConstants.STRUTS_OVERRIDE_ACCEPTED_PATTERNS, StringUtils.join(overrideAcceptedPatterns, ',')); map.put(StrutsConstants.STRUTS_ADDITIONAL_EXCLUDED_PATTERNS, StringUtils.join(additionalExcludedPatterns, ',')); @@ -1228,6 +1230,18 @@ public class ConstantConfig { this.acceptedPatternsChecker = new BeanConfig(clazz, clazz.getName()); } + public BeanConfig getNotExcludedAcceptedPatternsChecker() { + return notExcludedAcceptedPatternsChecker; + } + + public void setNotExcludedAcceptedPatternsChecker(BeanConfig notExcludedAcceptedPatternsChecker) { + this.notExcludedAcceptedPatternsChecker = notExcludedAcceptedPatternsChecker; + } + + public void setNotExcludedAcceptedPatternsChecker(Class clazz) { + this.notExcludedAcceptedPatternsChecker = new BeanConfig(clazz, clazz.getName()); + } + public Set getOverrideExcludedPatterns() { return overrideExcludedPatterns; } diff --git a/core/src/main/java/org/apache/struts2/result/StreamResult.java b/core/src/main/java/org/apache/struts2/result/StreamResult.java index 711929f4a..131614dd9 100644 --- a/core/src/main/java/org/apache/struts2/result/StreamResult.java +++ b/core/src/main/java/org/apache/struts2/result/StreamResult.java @@ -19,6 +19,8 @@ package org.apache.struts2.result; import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -85,6 +87,8 @@ public class StreamResult extends StrutsResultSupport { protected int bufferSize = 1024; protected boolean allowCaching = true; + private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns; + public StreamResult() { super(); } @@ -93,6 +97,11 @@ public class StreamResult extends StrutsResultSupport { this.inputStream = in; } + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + /** * @return Returns the whether or not the client should be requested to allow caching of the data stream. */ @@ -204,14 +213,17 @@ public class StreamResult extends StrutsResultSupport { OutputStream oOutput = null; try { - if (inputStream == null) { + String parsedInputName = conditionalParse(inputName, invocation); + boolean evaluated = parsedInputName != null && !parsedInputName.equals(inputName); + boolean reevaluate = !evaluated || isAcceptableExpression(parsedInputName); + if (inputStream == null && reevaluate) { LOG.debug("Find the inputstream from the invocation variable stack"); - inputStream = (InputStream) invocation.getStack().findValue(conditionalParse(inputName, invocation)); + inputStream = (InputStream) invocation.getStack().findValue(parsedInputName); } if (inputStream == null) { - String msg = ("Can not find a java.io.InputStream with the name [" + inputName + "] in the invocation stack. " + - "Check the tag specified for this action."); + String msg = ("Can not find a java.io.InputStream with the name [" + parsedInputName + "] in the invocation stack. " + + "Check the tag specified for this action is correct, not excluded and accepted."); LOG.error(msg); throw new IllegalArgumentException(msg); } @@ -228,15 +240,16 @@ public class StreamResult extends StrutsResultSupport { LOG.debug("Set the content length: {}", contentLength); if (contentLength != null) { - String _contentLength = conditionalParse(contentLength, invocation); - int _contentLengthAsInt; + String translatedContentLength = conditionalParse(contentLength, invocation); + int contentLengthAsInt; try { - _contentLengthAsInt = Integer.parseInt(_contentLength); - if (_contentLengthAsInt >= 0) { - oResponse.setContentLength(_contentLengthAsInt); + contentLengthAsInt = Integer.parseInt(translatedContentLength); + if (contentLengthAsInt >= 0) { + oResponse.setContentLength(contentLengthAsInt); } } catch (NumberFormatException e) { - LOG.warn("failed to recognize {} as a number, contentLength header will not be set", _contentLength, e); + LOG.warn("failed to recognize {} as a number, contentLength header will not be set", + translatedContentLength, e); } } @@ -277,4 +290,22 @@ public class StreamResult extends StrutsResultSupport { } } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of result + * @return true|false + * @since 2.6 + */ + protected boolean isAcceptableExpression(String expression) { + NotExcludedAcceptedPatternsChecker.IsAllowed isAllowed = notExcludedAcceptedPatterns.isAllowed(expression); + if (isAllowed.isAllowed()) { + return true; + } + + LOG.warn("Expression [{}] isn't allowed by pattern [{}]! See Accepted / Excluded patterns at\n" + + "https://struts.apache.org/security/", expression, isAllowed.getAllowedPattern()); + + return false; + } } diff --git a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java index b694250b9..7d8379c89 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java @@ -18,9 +18,7 @@ */ package org.apache.struts2.util; -import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ObjectFactory; -import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; @@ -102,7 +100,7 @@ public class StrutsUtil { return responseWrapper.getData(); } catch (Exception e) { - LOG.debug("Cannot include {}", aName.toString(), e); + LOG.debug("Cannot include {}", aName, e); throw e; } } @@ -125,7 +123,7 @@ public class StrutsUtil { } public String getText(String text) { - return (String) stack.findValue("getText('" + text + "')"); + return (String) stack.findValue("getText('" + text.replace('\'', '"') + "')"); } /* diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 22d11ae6b..9dd8fbfa3 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -224,6 +224,7 @@ + diff --git a/core/src/main/resources/template/css_xhtml/form-validate.ftl b/core/src/main/resources/template/css_xhtml/form-validate.ftl index a45aa7189..da7b24080 100644 --- a/core/src/main/resources/template/css_xhtml/form-validate.ftl +++ b/core/src/main/resources/template/css_xhtml/form-validate.ftl @@ -21,8 +21,8 @@ <#if parameters.validate!false == true> <#if parameters.onsubmit??> - ${tag.addParameter('onsubmit', "${parameters.onsubmit}; return validateForm_${parameters.id}();")} + ${tag.addParameter('onsubmit', "${parameters.onsubmit}; return validateForm_${parameters.escapedId}();")} <#else> - ${tag.addParameter('onsubmit', "return validateForm_${parameters.id}();")} + ${tag.addParameter('onsubmit', "return validateForm_${parameters.escapedId}();")} diff --git a/core/src/main/resources/template/simple/combobox.ftl b/core/src/main/resources/template/simple/combobox.ftl index 390e20df2..761b57772 100644 --- a/core/src/main/resources/template/simple/combobox.ftl +++ b/core/src/main/resources/template/simple/combobox.ftl @@ -21,7 +21,7 @@ <#include "/${parameters.templateDir}/simple/text.ftl" /> diff --git a/core/src/main/resources/template/simple/doubleselect.ftl b/core/src/main/resources/template/simple/doubleselect.ftl index 039e1420a..c064f29ae 100644 --- a/core/src/main/resources/template/simple/doubleselect.ftl +++ b/core/src/main/resources/template/simple/doubleselect.ftl @@ -75,9 +75,9 @@ diff --git a/core/src/main/resources/template/simple/form-close.ftl b/core/src/main/resources/template/simple/form-close.ftl index 192932929..c2285b2e6 100644 --- a/core/src/main/resources/template/simple/form-close.ftl +++ b/core/src/main/resources/template/simple/form-close.ftl @@ -27,15 +27,15 @@ submission. --> <#if (parameters.optiontransferselectIds!?size > 0)> - var containingForm = document.getElementById("${parameters.id}"); + var containingForm = document.getElementById("${parameters.id?js_string}"); <#assign selectObjIds = parameters.optiontransferselectIds.keySet() /> <#list selectObjIds as selectObjectId> StrutsUtils.addEventListener(containingForm, "submit", function(evt) { - var selectObj = document.getElementById("${selectObjectId}"); + var selectObj = document.getElementById("${selectObjectId?js_string}"); <#if parameters.optiontransferselectIds.get(selectObjectId)??> <#assign selectTagHeaderKey = parameters.optiontransferselectIds.get(selectObjectId)/> - selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey}"); + selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey?js_string}"); <#else> selectAllOptionsExceptSome(selectObj, "key", ""); @@ -43,15 +43,15 @@ <#if (parameters.inputtransferselectIds!?size > 0)> - var containingForm = document.getElementById("${parameters.id}"); + var containingForm = document.getElementById("${parameters.id?js_string}"); <#assign selectObjIds = parameters.inputtransferselectIds.keySet() /> <#list selectObjIds as selectObjectId> StrutsUtils.addEventListener(containingForm, "submit", function(evt) { - var selectObj = document.getElementById("${selectObjectId}"); + var selectObj = document.getElementById("${selectObjectId?js_string}"); <#if parameters.inputtransferselectIds.get(selectObjectId)??> <#assign selectTagHeaderKey = parameters.inputtransferselectIds.get(selectObjectId)/> - selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey}"); + selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey?js_string}"); <#else> selectAllOptionsExceptSome(selectObj, "key", ""); @@ -59,15 +59,15 @@ <#if (parameters.optiontransferselectDoubleIds!?size > 0)> - var containingForm = document.getElementById("${parameters.id}"); + var containingForm = document.getElementById("${parameters.id?js_string}"); <#assign selectDoubleObjIds = parameters.optiontransferselectDoubleIds.keySet() /> <#list selectDoubleObjIds as selectObjId> StrutsUtils.addEventListener(containingForm, "submit", function(evt) { - var selectObj = document.getElementById("${selectObjId}"); + var selectObj = document.getElementById("${selectObjId?js_string}"); <#if parameters.optiontransferselectDoubleIds.get(selectObjId)??> <#assign selectTagHeaderKey = parameters.optiontransferselectDoubleIds.get(selectObjId)/> - selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey}"); + selectAllOptionsExceptSome(selectObj, "key", "${selectTagHeaderKey?js_string}"); <#else> selectAllOptionsExceptSome(selectObj, "key", ""); @@ -81,15 +81,15 @@ submission --> <#if (parameters.updownselectIds!?size > 0)> - var containingForm = document.getElementById("${parameters.id}"); + var containingForm = document.getElementById("${parameters.id?js_string}"); <#assign tmpIds = parameters.updownselectIds.keySet() /> <#list tmpIds as tmpId> StrutsUtils.addEventListener(containingForm, "submit", function(evt) { - var updownselectObj = document.getElementById("${tmpId}"); + var updownselectObj = document.getElementById("${tmpId?js_string}"); <#if parameters.updownselectIds.get(tmpId)??> <#assign tmpHeaderKey = parameters.updownselectIds.get(tmpId) /> - selectAllOptionsExceptSome(updownselectObj, "key", "${tmpHeaderKey}"); + selectAllOptionsExceptSome(updownselectObj, "key", "${tmpHeaderKey?js_string}"); <#else> selectAllOptionsExceptSome(updownselectObj, "key", ""); diff --git a/core/src/main/resources/template/xhtml/form-close-validate.ftl b/core/src/main/resources/template/xhtml/form-close-validate.ftl index 0a17aac84..afa1c50ae 100644 --- a/core/src/main/resources/template/xhtml/form-close-validate.ftl +++ b/core/src/main/resources/template/xhtml/form-close-validate.ftl @@ -33,7 +33,7 @@ END SNIPPET: supported-validators --> <#if ((parameters.validate!false == true) && (parameters.performValidation!false == true))> <#if parameters.onsubmit??> - ${tag.addParameter('onsubmit', "${parameters.onsubmit}; return validateForm_${parameters.id?replace('[^a-zA-Z0-9_]', '_', 'r')}();")} + ${tag.addParameter('onsubmit', "${parameters.onsubmit}; return validateForm_${parameters.escapedId}();")} <#else> - ${tag.addParameter('onsubmit', "return validateForm_${parameters.id?replace('[^a-zA-Z0-9_]', '_', 'r')}();")} + ${tag.addParameter('onsubmit', "return validateForm_${parameters.escapedId}();")} diff --git a/core/src/test/java/com/opensymphony/xwork2/ChainResultTest.java b/core/src/test/java/com/opensymphony/xwork2/ChainResultTest.java index 84fa0afa1..2806de198 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ChainResultTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ChainResultTest.java @@ -20,6 +20,7 @@ package com.opensymphony.xwork2; import com.mockobjects.dynamic.Mock; import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; +import com.opensymphony.xwork2.mock.MockResult; import com.opensymphony.xwork2.util.ValueStack; import junit.framework.TestCase; import org.apache.struts2.StrutsException; @@ -124,7 +125,18 @@ public class ChainResultTest extends XWorkTestCase { } } - private class NamespaceActionNameTestActionProxyFactory implements ActionProxyFactory { + public void testNamespaceChain() throws Exception { + ActionProxy proxy = actionProxyFactory.createActionProxy(null, "chain_with_namespace", null, null); + ((SimpleAction)proxy.getAction()).setBlah("%{foo}"); + + proxy.execute(); + + assertTrue(proxy.getInvocation().getResult() instanceof MockResult); + MockResult result = (MockResult) proxy.getInvocation().getResult(); + assertEquals("%{foo}", result.getInvocation().getProxy().getNamespace()); + } + + private static class NamespaceActionNameTestActionProxyFactory implements ActionProxyFactory { private final ActionProxy returnVal; private final String expectedActionName; private final String expectedNamespace; diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java index 3671de9b7..00e91411e 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java @@ -34,6 +34,10 @@ import org.apache.struts2.dispatcher.HttpParameters; import java.util.HashMap; import java.util.Map; +import static com.opensymphony.xwork2.security.DefaultAcceptedPatternsCheckerTest.ACCEPT_ALL_PATTERNS_CHECKER; +import static com.opensymphony.xwork2.security.DefaultExcludedPatternsCheckerTest.NO_EXCLUSION_PATTERNS_CHECKER; +import static org.junit.Assert.assertNotEquals; + /** * AliasInterceptorTest @@ -73,6 +77,92 @@ public class AliasInterceptorTest extends XWorkTestCase { assertNull(actionOne.getBlah()); // WW-5087 } + public void testNameNotAccepted() throws Exception { + Map params = new HashMap<>(); + params.put("aliasSource", "source here"); + + Map httpParams = new HashMap<>(); + httpParams.put("name", "getAliasSource()"); + httpParams.put("value", "aliasDest"); + params.put("parameters", HttpParameters.create(httpParams).build()); + + + XmlConfigurationProvider provider = new StrutsXmlConfigurationProvider("xwork-sample.xml"); + container.inject(provider); + loadConfigurationProviders(provider); + ActionProxy proxy = actionProxyFactory.createActionProxy("", "dynamicAliasTest", null, params); + SimpleAction actionOne = (SimpleAction) proxy.getAction(); + actionOne.setAliasSource("name to be copied"); + + // prevent ERROR result + actionOne.setFoo(-1); + actionOne.setBar(1); + + proxy.execute(); + assertEquals("name to be copied", actionOne.getAliasSource()); + assertNotEquals(actionOne.getAliasSource(), actionOne.getAliasDest()); + + proxy = actionProxyFactory.createActionProxy("", "dynamicAliasTest", null, params); + ((AliasInterceptor)proxy.getConfig().getInterceptors().get(1).getInterceptor()) + .setExcludedPatterns(NO_EXCLUSION_PATTERNS_CHECKER); + ((AliasInterceptor)proxy.getConfig().getInterceptors().get(1).getInterceptor()) + .setAcceptedPatterns(ACCEPT_ALL_PATTERNS_CHECKER); + + actionOne = (SimpleAction) proxy.getAction(); + actionOne.setAliasSource("name to be copied"); + + // prevent ERROR result + actionOne.setFoo(-1); + actionOne.setBar(1); + + proxy.execute(); + assertEquals("name to be copied", actionOne.getAliasSource()); + assertEquals(actionOne.getAliasSource(), actionOne.getAliasDest()); + } + + public void testValueNotAccepted() throws Exception { + Map params = new HashMap<>(); + params.put("aliasSource", "source here"); + + Map httpParams = new HashMap<>(); + httpParams.put("name", "aliasSource"); + httpParams.put("value", "[0].aliasDest"); + params.put("parameters", HttpParameters.create(httpParams).build()); + + + XmlConfigurationProvider provider = new StrutsXmlConfigurationProvider("xwork-sample.xml"); + container.inject(provider); + loadConfigurationProviders(provider); + ActionProxy proxy = actionProxyFactory.createActionProxy("", "dynamicAliasTest", null, params); + SimpleAction actionOne = (SimpleAction) proxy.getAction(); + actionOne.setAliasSource("name to be copied"); + + // prevent ERROR result + actionOne.setFoo(-1); + actionOne.setBar(1); + + proxy.execute(); + assertEquals("name to be copied", actionOne.getAliasSource()); + assertNotEquals(actionOne.getAliasSource(), actionOne.getAliasDest()); + + proxy = actionProxyFactory.createActionProxy("", "dynamicAliasTest", null, params); + ((AliasInterceptor) proxy.getConfig().getInterceptors().get(1).getInterceptor()) + .setExcludedPatterns(NO_EXCLUSION_PATTERNS_CHECKER); + ((AliasInterceptor) proxy.getConfig().getInterceptors().get(1).getInterceptor()) + .setAcceptedPatterns(ACCEPT_ALL_PATTERNS_CHECKER); + + actionOne = (SimpleAction) proxy.getAction(); + actionOne.setAliasSource("name to be copied"); + + // prevent ERROR result + actionOne.setFoo(-1); + actionOne.setBar(1); + + proxy.execute(); + assertEquals("name to be copied", actionOne.getAliasSource()); + assertEquals(actionOne.getAliasSource(), actionOne.getAliasDest()); + } + public void testNotExisting() throws Exception { Map params = new HashMap<>(); Map httpParams = new HashMap<>(); diff --git a/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java b/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java index 7100f6cf2..050e56897 100644 --- a/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java @@ -233,4 +233,32 @@ public class DefaultAcceptedPatternsCheckerTest extends XWorkTestCase { // then assertFalse("Dash was accepted", accepted.isAccepted()); } + + + public static final AcceptedPatternsChecker ACCEPT_ALL_PATTERNS_CHECKER = new AcceptedPatternsChecker() { + @Override + public IsAccepted isAccepted(String value) { + return IsAccepted.yes(".*"); + } + + @Override + public void setAcceptedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setAcceptedPatterns(String[] patterns) { + + } + + @Override + public void setAcceptedPatterns(Set patterns) { + + } + + @Override + public Set getAcceptedPatterns() { + return null; + } + }; } diff --git a/core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java b/core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java index 8b88360cb..738def5d6 100644 --- a/core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java @@ -22,6 +22,7 @@ import com.opensymphony.xwork2.XWorkTestCase; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.regex.Pattern; @@ -216,4 +217,32 @@ public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase { // Expected result } } + + + public static final ExcludedPatternsChecker NO_EXCLUSION_PATTERNS_CHECKER = new ExcludedPatternsChecker() { + @Override + public IsExcluded isExcluded(String value) { + return IsExcluded.no(new HashSet<>()); + } + + @Override + public void setExcludedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setExcludedPatterns(String[] patterns) { + + } + + @Override + public void setExcludedPatterns(Set patterns) { + + } + + @Override + public Set getExcludedPatterns() { + return null; + } + }; } diff --git a/core/src/test/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsCheckerTest.java b/core/src/test/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsCheckerTest.java new file mode 100644 index 000000000..85f5b71b9 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsCheckerTest.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.XWorkTestCase; + +import java.util.Set; +import java.util.regex.Pattern; + +import static org.junit.Assert.*; + +public class DefaultNotExcludedAcceptedPatternsCheckerTest extends XWorkTestCase { + + public void testNoExclusionAcceptAllPatternsChecker() { + assertTrue(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER.isAllowed("%{1+1}").isAllowed()); + } + + public static final NotExcludedAcceptedPatternsChecker NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER + = new NotExcludedAcceptedPatternsChecker() { + @Override + public IsAllowed isAllowed(String value) { + return IsAllowed.yes("*"); + } + + @Override + public IsAccepted isAccepted(String value) { + return null; + } + + @Override + public void setAcceptedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setAcceptedPatterns(String[] patterns) { + + } + + @Override + public void setAcceptedPatterns(Set patterns) { + + } + + @Override + public Set getAcceptedPatterns() { + return null; + } + + @Override + public IsExcluded isExcluded(String value) { + return null; + } + + @Override + public void setExcludedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setExcludedPatterns(String[] patterns) { + + } + + @Override + public void setExcludedPatterns(Set patterns) { + + } + + @Override + public Set getExcludedPatterns() { + return null; + } + }; +} \ No newline at end of file diff --git a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java index 9e093f86a..ce6bcc10c 100644 --- a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java +++ b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java @@ -33,7 +33,9 @@ import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.interceptor.ParametersInterceptor; import com.opensymphony.xwork2.mock.MockResult; import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; +import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.validator.ValidationInterceptor; import org.apache.struts2.result.ServletDispatcherResult; @@ -42,6 +44,7 @@ import org.apache.struts2.interceptor.TokenSessionStoreInterceptor; import org.apache.struts2.views.jsp.ui.DoubleValidationAction; import java.util.HashMap; +import java.util.Map; /** @@ -74,7 +77,7 @@ public class TestConfigurationProvider implements ConfigurationProvider { */ public void loadPackages() { - HashMap successParams = new HashMap(); + Map successParams = new HashMap<>(); successParams.put("propertyName", "executionCount"); successParams.put("expectedValue", "1"); @@ -149,9 +152,7 @@ public class TestConfigurationProvider implements ConfigurationProvider { } /** - * Tells whether the ConfigurationProvider should reload its configuration - * - * @return + * @return whether the ConfigurationProvider should reload its configuration */ public boolean needsReload() { return false; @@ -167,5 +168,8 @@ public class TestConfigurationProvider implements ConfigurationProvider { if (!builder.contains(ExcludedPatternsChecker.class)) { builder.factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class); } + if (!builder.contains(NotExcludedAcceptedPatternsChecker.class)) { + builder.factory(NotExcludedAcceptedPatternsChecker.class, DefaultNotExcludedAcceptedPatternsChecker.class); + } } } diff --git a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java index e4b0c6373..90575f341 100644 --- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java +++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java @@ -25,7 +25,6 @@ import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.components.template.Template; import org.apache.struts2.components.template.TemplateEngine; import org.apache.struts2.components.template.TemplateEngineManager; -import org.apache.struts2.dispatcher.DefaultStaticContentLoader; import org.apache.struts2.dispatcher.StaticContentLoader; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -35,9 +34,11 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import static com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsCheckerTest.NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER; + public class UIBeanTest extends StrutsInternalTestCase { - public void testPopulateComponentHtmlId1() throws Exception { + public void testPopulateComponentHtmlId1() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -53,7 +54,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals("txtFldId", txtFld.getParameters().get("id")); } - public void testPopulateComponentHtmlIdWithOgnl() throws Exception { + public void testPopulateComponentHtmlIdWithOgnl() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -69,7 +70,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals("formId_txtFldName1", txtFld.getParameters().get("id")); } - public void testPopulateComponentHtmlId2() throws Exception { + public void testPopulateComponentHtmlId2() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -85,7 +86,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals("formId_txtFldName", txtFld.getParameters().get("id")); } - public void testPopulateComponentHtmlWithoutNameAndId() throws Exception { + public void testPopulateComponentHtmlWithoutNameAndId() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -97,10 +98,10 @@ public class UIBeanTest extends StrutsInternalTestCase { txtFld.populateComponentHtmlId(form); - assertEquals(null, txtFld.getParameters().get("id")); + assertNull(txtFld.getParameters().get("id")); } - public void testEscape() throws Exception { + public void testEscape() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -113,11 +114,11 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals(bean.escape("hello[world"), "hello_world"); assertEquals(bean.escape("hello.world"), "hello_world"); assertEquals(bean.escape("hello]world"), "hello_world"); - assertEquals(bean.escape("hello!world"), "hello!world"); - assertEquals(bean.escape("hello!@#$%^&*()world"), "hello!@#$%^&*()world"); + assertEquals(bean.escape("hello!world"), "hello_world"); + assertEquals(bean.escape("hello!@#$%^&*()world"), "hello__________world"); } - public void testEscapeId() throws Exception { + public void testEscapeId() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -131,7 +132,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals("formId_foo_bar", txtFld.getParameters().get("id")); } - public void testGetThemeFromForm() throws Exception { + public void testGetThemeFromForm() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -143,29 +144,29 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals("foo", txtFld.getTheme()); } - public void testGetThemeFromContext() throws Exception { + public void testGetThemeFromContext() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); - Map context = Collections.singletonMap("theme", "bar"); + Map context = Collections.singletonMap("theme", "bar"); ActionContext.getContext().put("attr", context); TextField txtFld = new TextField(stack, req, res); assertEquals("bar", txtFld.getTheme()); } - public void testGetThemeFromContextNonString() throws Exception { + public void testGetThemeFromContextNonString() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); - Map context = Collections.singletonMap("theme", 12); + Map context = Collections.singletonMap("theme", 12); ActionContext.getContext().put("attr", context); TextField txtFld = new TextField(stack, req, res); assertEquals("12", txtFld.getTheme()); } - public void testMergeTemplateNullEngineException() throws Exception { + public void testMergeTemplateNullEngineException() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -187,7 +188,7 @@ public class UIBeanTest extends StrutsInternalTestCase { } } - public void testBuildTemplate() throws Exception { + public void testBuildTemplate() { String defaultTemplateName = "default"; String customTemplateName = "custom"; ValueStack stack = ActionContext.getContext().getValueStack(); @@ -203,14 +204,14 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals(customTemplateName, customTemplate.getName()); } - public void testGetTemplateDirExplicit() throws Exception { + public void testGetTemplateDirExplicit() { String explicitTemplateDir = "explicitTemplateDirectory"; String attrTemplateDir = "attrTemplateDirectory"; String defaultTemplateDir = "defaultTemplateDirectory"; ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); - Map context = Collections.singletonMap("templateDir", attrTemplateDir); + Map context = Collections.singletonMap("templateDir", attrTemplateDir); ActionContext.getContext().put("attr", context); TextField txtFld = new TextField(stack, req, res); @@ -220,13 +221,13 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals(explicitTemplateDir, txtFld.getTemplateDir()); } - public void testGetTemplateDirAttr() throws Exception { + public void testGetTemplateDirAttr() { String attrTemplateDir = "attrTemplateDirectory"; String defaultTemplateDir = "defaultTemplateDirectory"; ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); - Map context = Collections.singletonMap("templateDir", attrTemplateDir); + Map context = Collections.singletonMap("templateDir", attrTemplateDir); ActionContext.getContext().put("attr", context); TextField txtFld = new TextField(stack, req, res); @@ -235,7 +236,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals(attrTemplateDir, txtFld.getTemplateDir()); } - public void testGetTemplateDirDefault() throws Exception { + public void testGetTemplateDirDefault() { String defaultTemplateDir = "defaultTemplateDirectory"; ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); @@ -247,7 +248,7 @@ public class UIBeanTest extends StrutsInternalTestCase { assertEquals(defaultTemplateDir, txtFld.getTemplateDir()); } - public void testGetTemplateDirNoneSet() throws Exception { + public void testGetTemplateDirNoneSet() { ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); MockHttpServletResponse res = new MockHttpServletResponse(); @@ -298,10 +299,58 @@ public class UIBeanTest extends StrutsInternalTestCase { }); TextField txtFld = new TextField(stack, req, res); + container.inject(txtFld); txtFld.setName("%{myValue}"); txtFld.evaluateParams(); assertEquals("%{myBad}", txtFld.getParameters().get("nameValue")); + assertEquals("%{myBad}", txtFld.getParameters().get("name")); + } + + public void testValueNameParameterNotAccepted() { + ValueStack stack = ActionContext.getContext().getValueStack(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + + stack.push(new Object() { + public String getMyValueName() { + return "getMyValue()"; + } + public String getMyValue() { + return "value"; + } + }); + + TextField txtFld = new TextField(stack, req, res); + container.inject(txtFld); + txtFld.setName("%{myValueName}"); + txtFld.evaluateParams(); + assertEquals("getMyValue()", txtFld.getParameters().get("name")); + assertEquals("getMyValue()", txtFld.getParameters().get("nameValue")); + + txtFld.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + txtFld.evaluateParams(); + assertEquals("getMyValue()", txtFld.getParameters().get("name")); + assertEquals("value", txtFld.getParameters().get("nameValue")); + } + + public void testValueNameParameterGetterAccepted() { + ValueStack stack = ActionContext.getContext().getValueStack(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + + stack.push(new Object() { + public String getMyValue() { + return "value"; + } + }); + + TextField txtFld = new TextField(stack, req, res); + container.inject(txtFld); + txtFld.setName("getMyValue()"); + txtFld.evaluateParams(); + assertEquals("getMyValue()", txtFld.getParameters().get("name")); + assertEquals("value", txtFld.getParameters().get("nameValue")); } public void testSetClass() { diff --git a/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java index 1cc6d24d4..42d9330fa 100644 --- a/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java @@ -93,6 +93,46 @@ public class PostbackResultTest extends StrutsInternalTestCase { control.verify(); } + public void testExpressionNamespace() throws Exception { + + ActionContext context = ActionContext.getContext(); + context.getContextMap().put("namespaceName", "${1-1}"); + context.getContextMap().put("actionName", "${1-1}"); + context.getContextMap().put("methodName", "${1-1}"); + ValueStack stack = context.getValueStack(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + + PostbackResult result = new PostbackResult(); + result.setNamespace("/myNamespace${#namespaceName}"); + result.setActionName("myAction${#actionName}"); + result.setMethod("myMethod${#methodName}"); + result.setPrependServletContext(false); + + IMocksControl control = createControl(); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + expect(mockInvocation.getStack()).andReturn(stack).anyTimes(); + + control.replay(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.execute(mockInvocation); + assertEquals("
" + + "", res.getContentAsString()); + + req = new MockHttpServletRequest(); + res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + result.execute(mockInvocation); + assertEquals("" + + "", res.getContentAsString()); + + control.verify(); + } + public void testPassingNullInvocation() throws Exception{ Result result = new PostbackResult(); try { diff --git a/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java index 365fb945a..19136bf16 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java @@ -104,6 +104,79 @@ public class ServletActionRedirectResultTest extends StrutsInternalTestCase { control.verify(); } + public void testExpressionParameterInResultWithConditionParseOn() throws Exception { + + ResultConfig resultConfig = new ResultConfig.Builder("", "") + .addParam("actionName", "someActionName") + .addParam("namespace", "someNamespace") + .addParam("encode", "true") + .addParam("parse", "true") + .addParam("location", "someLocation") + .addParam("prependServletContext", "true") + .addParam("method", "someMethod") + .addParam("statusCode", "333") + .addParam("param1", "${#value1}") + .addParam("param2", "${#value2}") + .addParam("param3", "${#value3}") + .addParam("anchor", "${#fragment}") + .build(); + + + + ActionContext context = ActionContext.getContext(); + ValueStack stack = context.getValueStack(); + context.getContextMap().put("value1", "value 1"); + context.getContextMap().put("value2", "value 2"); + context.getContextMap().put("value3", "value 3"); + context.getContextMap().put("namespaceName", "${1-1}"); + context.getContextMap().put("actionName", "${1-1}"); + context.getContextMap().put("methodName", "${1-1}"); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + + + Map results= new HashMap<>(); + results.put("myResult", resultConfig); + + ActionConfig actionConfig = new ActionConfig.Builder("", "", "") + .addResultConfigs(results).build(); + + ServletActionRedirectResult result = new ServletActionRedirectResult(); + result.setNamespace("/myNamespace${#namespaceName}"); + result.setActionName("myAction${#actionName}"); + result.setMethod("myMethod${#methodName}"); + result.setParse(true); + result.setEncode(false); + result.setPrependServletContext(false); + result.setAnchor("fragment"); + result.setUrlHelper(new DefaultUrlHelper()); + + IMocksControl control = createControl(); + ActionProxy mockActionProxy = control.createMock(ActionProxy.class); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getProxy()).andReturn(mockActionProxy).anyTimes(); + expect(mockInvocation.getResultCode()).andReturn("myResult").anyTimes(); + expect(mockActionProxy.getConfig()).andReturn(actionConfig).anyTimes(); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + expect(mockInvocation.getStack()).andReturn(stack).anyTimes(); + + control.replay(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.execute(mockInvocation); + assertEquals("/myNamespace${1-1}/myAction${1-1}!myMethod${1-1}.action?param1=value+1¶m2=value+2¶m3=value+3#fragment", res.getRedirectedUrl()); + + req = new MockHttpServletRequest(); + res = new MockHttpServletResponse(); + context.put(ServletActionContext.HTTP_REQUEST, req); + context.put(ServletActionContext.HTTP_RESPONSE, res); + result.execute(mockInvocation); + assertEquals("/myNamespace0/myAction0!myMethod0.action?param1=value+1¶m2=value+2¶m3=value+3#fragment", res.getRedirectedUrl()); + + control.verify(); + } + public void testIncludeParameterInResultWithConditionParseOnWithNoNamespace() throws Exception { ResultConfig resultConfig = new ResultConfig.Builder("", "") diff --git a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java index 1b46d0bd9..5661db5d3 100644 --- a/core/src/test/java/org/apache/struts2/result/StreamResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/StreamResultTest.java @@ -33,6 +33,8 @@ import java.io.InputStream; import java.net.URI; import java.net.URL; +import static com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsCheckerTest.NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER; + /** * Unit test for {@link StreamResult}. * @@ -213,11 +215,41 @@ public class StreamResultTest extends StrutsInternalTestCase { assertEquals("filename=\"logo.png\"", response.getHeader("Content-disposition")); } + public void testStreamResultParseExpression() throws Exception { + result.setParse(true); + result.setInputName("${streamForImageAsExpression}"); + + try { + result.doExecute("helloworld", mai); + fail("double evaluation?!"); + } catch (IllegalArgumentException e) { + assertEquals("Can not find a java.io.InputStream with the name [getStreamForImage()] in the " + + "invocation stack. Check the tag specified for this action is correct, " + + "not excluded and accepted.", e.getMessage()); + } + + // verify that above test has really effect + result.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + assertNull(result.inputStream); + result.doExecute("helloworld", mai); + assertNotNull(result.inputStream); + container.inject(result); // roll back pattern checkers + } + + public void testStreamResultParseGetter() throws Exception { + result.setParse(true); + result.setInputName("getStreamForImage()"); + assertNull(result.inputStream); + result.doExecute("helloworld", mai); + assertNotNull(result.inputStream); + } + protected void setUp() throws Exception { super.setUp(); response = new MockHttpServletResponse(); result = new StreamResult(); + container.inject(result); result.setContentLength("${contentLength}"); stack = ActionContext.getContext().getValueStack(); @@ -244,7 +276,7 @@ public class StreamResultTest extends StrutsInternalTestCase { mai = null; } - public class MyImageAction implements Action { + public static class MyImageAction implements Action { FileInputStream streamForImage; long contentLength; @@ -257,7 +289,7 @@ public class StreamResultTest extends StrutsInternalTestCase { contentLength = file.length(); } - public InputStream getStreamForImage() throws Exception { + public InputStream getStreamForImage() { return streamForImage; } @@ -265,7 +297,7 @@ public class StreamResultTest extends StrutsInternalTestCase { return SUCCESS; } - public long getContentLength() throws Exception { + public long getContentLength() { return contentLength; } @@ -273,6 +305,10 @@ public class StreamResultTest extends StrutsInternalTestCase { return "streamForImage"; } + public String getStreamForImageAsExpression() { + return "getStreamForImage()"; + } + public String getContentCharSetMethod() { return "UTF-8"; } diff --git a/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java b/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java index 02be0f287..4cf0abb82 100644 --- a/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java +++ b/core/src/test/java/org/apache/struts2/util/StrutsUtilTest.java @@ -49,7 +49,7 @@ public class StrutsUtilTest extends StrutsInternalTestCase { assertTrue(o instanceof TestAction); } - public void testIsTrueMethod() throws Exception { + public void testIsTrueMethod() { stack.push(new Object() { public String getMyString() { return "myString"; @@ -64,7 +64,7 @@ public class StrutsUtilTest extends StrutsInternalTestCase { assertFalse(strutsUtil.isTrue("getMyBoolean(false)")); } - public void testFindStringMethod() throws Exception { + public void testFindStringMethod() { stack.push(new Object() { public String getMyString() { return "myString"; @@ -89,13 +89,13 @@ public class StrutsUtilTest extends StrutsInternalTestCase { } - public void testUrlEncodeMethod() throws Exception { + public void testUrlEncodeMethod() { assertEquals( strutsUtil.urlEncode("http://www.opensymphony.com/action2/index.jsp?param1=value1"), "http%3A%2F%2Fwww.opensymphony.com%2Faction2%2Findex.jsp%3Fparam1%3Dvalue1"); } - public void testBuildUrlMethod() throws Exception { + public void testBuildUrlMethod() { request.setContextPath("/myContextPath"); assertEquals(strutsUtil.buildUrl("/someUrl?param1=value1"), "/myContextPath/someUrl?param1=value1"); } @@ -123,21 +123,29 @@ public class StrutsUtilTest extends StrutsInternalTestCase { - public void testGetTextMethod() throws Exception { + public void testGetTextMethod() { // this should be in xwork-messages.properties (included by default // by LocalizedTextUtil - assertNotNull(strutsUtil.getText("xwork.error.action.execution")); - assertEquals(strutsUtil.getText("xwork.error.action.execution"), "Error during Action invocation"); + String expression = "xwork.error.action.execution"; + String text = strutsUtil.getText(expression); + assertNotNull(text); + assertEquals(text, "Error during Action invocation"); + } + + public void testGetTextMethodWithSingleQuote() { + String expression = "xwork.error.action.execution') + getText('xwork.error.action.execution"; + String text = strutsUtil.getText(expression); + assertNull(text); } - public void testGetContextMethod() throws Exception { + public void testGetContextMethod() { request.setContextPath("/myContext"); assertEquals(strutsUtil.getContext(), "/myContext"); } - public void testMakeSelectListMethod() throws Exception { + public void testMakeSelectListMethod() { String[] selectedList = new String[] { "Car", "Airplane", "Bus" }; List list = new ArrayList(); list.add("Lorry"); @@ -152,44 +160,43 @@ public class StrutsUtilTest extends StrutsInternalTestCase { assertEquals(listMade.size(), 3); assertEquals(((ListEntry)listMade.get(0)).getKey(), "Lorry"); assertEquals(((ListEntry)listMade.get(0)).getValue(), "Lorry"); - assertEquals(((ListEntry)listMade.get(0)).getIsSelected(), false); + assertFalse(((ListEntry) listMade.get(0)).getIsSelected()); assertEquals(((ListEntry)listMade.get(1)).getKey(), "Car"); assertEquals(((ListEntry)listMade.get(1)).getValue(), "Car"); - assertEquals(((ListEntry)listMade.get(1)).getIsSelected(), true); + assertTrue(((ListEntry) listMade.get(1)).getIsSelected()); assertEquals(((ListEntry)listMade.get(2)).getKey(), "Helicopter"); assertEquals(((ListEntry)listMade.get(2)).getValue(), "Helicopter"); - assertEquals(((ListEntry)listMade.get(2)).getIsSelected(), false); + assertFalse(((ListEntry) listMade.get(2)).getIsSelected()); } - public void testToInt() throws Exception { - assertEquals(strutsUtil.toInt(11l), 11); + public void testToInt() { + assertEquals(strutsUtil.toInt(11L), 11); } - public void testToLong() throws Exception { - assertEquals(strutsUtil.toLong(11), 11l); + public void testToLong() { + assertEquals(strutsUtil.toLong(11), 11L); } - public void testToString() throws Exception { + public void testToString() { assertEquals(strutsUtil.toString(1), "1"); - assertEquals(strutsUtil.toString(11l), "11"); + assertEquals(strutsUtil.toString(11L), "11"); } - public void testTranslateVariables() throws Exception { + public void testTranslateVariables() { stack.push(new Object() { public String getFoo() { return "bar"; } }); - Object obj1 = strutsUtil.translateVariables("try: %{foo}"); + String obj1 = strutsUtil.translateVariables("try: %{foo}"); assertNotNull(obj1); - assertTrue(obj1 instanceof String); assertEquals(obj1, "try: bar"); } - public void testTranslateVariablesRecursion() throws Exception { + public void testTranslateVariablesRecursion() { stack.push(new Object() { public String getFoo() { return "%{bar}"; @@ -226,7 +233,7 @@ public class StrutsUtilTest extends StrutsInternalTestCase { // === internal class to assist in testing - class InternalMockHttpServletRequest extends MockHttpServletRequest { + static class InternalMockHttpServletRequest extends MockHttpServletRequest { InternalMockRequestDispatcher dispatcher = null; public RequestDispatcher getRequestDispatcher(String path) { dispatcher = new InternalMockRequestDispatcher(path); @@ -238,7 +245,7 @@ public class StrutsUtilTest extends StrutsInternalTestCase { } } - class InternalMockRequestDispatcher extends MockRequestDispatcher { + static class InternalMockRequestDispatcher extends MockRequestDispatcher { private String url; boolean included = false; public InternalMockRequestDispatcher(String url) { diff --git a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java index d05aa6d2f..49e7abeae 100644 --- a/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java +++ b/core/src/test/java/org/apache/struts2/views/freemarker/FreemarkerResultMockedTest.java @@ -36,6 +36,8 @@ import javax.servlet.ServletContext; import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.ArrayList; +import java.util.List; import static org.apache.struts2.views.jsp.AbstractUITagTest.normalize; import static org.hamcrest.CoreMatchers.allOf; @@ -266,6 +268,63 @@ public class FreemarkerResultMockedTest extends StrutsInternalTestCase { assertTrue(stringWriter.toString().contains(" -
-
+ diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt index 08326c776..281847627 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt @@ -1,4 +1,4 @@ - + diff --git a/core/src/test/resources/struts.xml b/core/src/test/resources/struts.xml index ca6609e75..0cf1ab00f 100644 --- a/core/src/test/resources/struts.xml +++ b/core/src/test/resources/struts.xml @@ -92,6 +92,11 @@ + + + org/apache/struts2/views/freemarker/iterator.ftl + + diff --git a/core/src/test/resources/xwork-sample.xml b/core/src/test/resources/xwork-sample.xml index 89eb64554..0a4c0aacd 100644 --- a/core/src/test/resources/xwork-sample.xml +++ b/core/src/test/resources/xwork-sample.xml @@ -99,7 +99,14 @@ - + + + #{ #parameters['name'] : #parameters['value'] } + + + + + @@ -140,6 +147,21 @@ InfiniteRecursionChain + + + chain_without_namespace + %{blah} + + + + + + + Foo + + + + diff --git a/plugins/jasperreports/src/main/java/org/apache/struts2/views/jasperreports/JasperReportsResult.java b/plugins/jasperreports/src/main/java/org/apache/struts2/views/jasperreports/JasperReportsResult.java index b63957014..8a67556c7 100644 --- a/plugins/jasperreports/src/main/java/org/apache/struts2/views/jasperreports/JasperReportsResult.java +++ b/plugins/jasperreports/src/main/java/org/apache/struts2/views/jasperreports/JasperReportsResult.java @@ -19,6 +19,8 @@ package org.apache.struts2.views.jasperreports; import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.ValueStack; import net.sf.jasperreports.engine.JRException; import net.sf.jasperreports.engine.JRExporter; @@ -140,6 +142,7 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe private static final Logger LOG = LogManager.getLogger(JasperReportsResult.class); protected String dataSource; + private String parsedDataSource; protected String format; protected String documentName; protected String contentDisposition; @@ -159,12 +162,16 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe * additional report parameters from the action. */ protected String reportParameters; + private String parsedReportParameters; /** * Names an exporter parameters map stack value, * allowing the use of custom export parameters. */ protected String exportParameters; + private String parsedExportParameters; + + private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns; /** * Default ctor. @@ -182,6 +189,11 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe super(location); } + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + public String getImageServletUrl() { return imageServletUrl; } @@ -273,7 +285,14 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe Connection conn = (Connection) stack.findValue(connection); if (conn == null) { - stackDataSource = new ValueStackDataSource(stack, dataSource, wrapField); + boolean evaluated = parsedDataSource != null && !parsedDataSource.equals(dataSource); + boolean reevaluate = !evaluated || isAcceptableExpression(parsedDataSource); + if (reevaluate) { + stackDataSource = new ValueStackDataSource(stack, parsedDataSource, wrapField); + } else { + throw new ServletException(String.format("Error building dataSource for excluded or not accepted [%s]", + parsedDataSource)); + } } if ("https".equalsIgnoreCase(request.getScheme())) { @@ -305,7 +324,9 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe } // Add any report parameters from action to param map. - Map reportParams = (Map) stack.findValue(reportParameters); + boolean evaluated = parsedReportParameters != null && !parsedReportParameters.equals(reportParameters); + boolean reevaluate = !evaluated || isAcceptableExpression(parsedReportParameters); + Map reportParams = reevaluate ? (Map) stack.findValue(parsedReportParameters) : null; if (reportParams != null) { LOG.debug("Found report parameters; adding to parameters..."); parameters.putAll(reportParams); @@ -387,7 +408,9 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe throw new ServletException("Unknown report format: " + format); } - Map exportParams = (Map) stack.findValue(exportParameters); + evaluated = parsedExportParameters != null && !parsedExportParameters.equals(exportParameters); + reevaluate = !evaluated || isAcceptableExpression(parsedExportParameters); + Map exportParams = reevaluate ? (Map) stack.findValue(parsedExportParameters) : null; if (exportParams != null) { LOG.debug("Found export parameters; adding to exporter parameters..."); exporter.getParameters().putAll(exportParams); @@ -443,7 +466,7 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe throw new RuntimeException(message); } if (dataSource != null) { - dataSource = conditionalParse(dataSource, invocation); + parsedDataSource = conditionalParse(dataSource, invocation); } format = conditionalParse(format, invocation); @@ -459,8 +482,8 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe documentName = conditionalParse(documentName, invocation); } - reportParameters = conditionalParse(reportParameters, invocation); - exportParameters = conditionalParse(exportParameters, invocation); + parsedReportParameters = conditionalParse(reportParameters, invocation); + parsedExportParameters = conditionalParse(exportParameters, invocation); } /** @@ -484,4 +507,22 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe return baos; } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of result + * @return true|false + * @since 2.6 + */ + protected boolean isAcceptableExpression(String expression) { + NotExcludedAcceptedPatternsChecker.IsAllowed isAllowed = notExcludedAcceptedPatterns.isAllowed(expression); + if (isAllowed.isAllowed()) { + return true; + } + + LOG.warn("Expression [{}] isn't allowed by pattern [{}]! See Accepted / Excluded patterns at\n" + + "https://struts.apache.org/security/", expression, isAllowed.getAllowedPattern()); + + return false; + } } diff --git a/plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/JasperReportsResultTest.java b/plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/JasperReportsResultTest.java index 1bf55acf8..02c1eb43a 100644 --- a/plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/JasperReportsResultTest.java +++ b/plugins/jasperreports/src/test/java/org/apache/struts2/views/jasperreports/JasperReportsResultTest.java @@ -20,44 +20,40 @@ package org.apache.struts2.views.jasperreports; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.mock.MockActionInvocation; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.ValueStack; import net.sf.jasperreports.engine.JasperCompileManager; import org.apache.struts2.StrutsStatics; import org.apache.struts2.StrutsTestCase; -import org.easymock.IAnswer; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.mock.web.MockServletContext; +import javax.servlet.ServletException; import java.net.URL; import java.sql.Connection; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; +import static net.sf.jasperreports.engine.JRExporterParameter.OUTPUT_STRING_BUFFER; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertNotEquals; public class JasperReportsResultTest extends StrutsTestCase { private MockActionInvocation invocation; private ValueStack stack; + private JasperReportsResult result; public void testConnClose() throws Exception { - JasperReportsResult result = new JasperReportsResult(); - URL url = ClassLoaderUtil.getResource("org/apache/struts2/views/jasperreports/empty.jrxml", this.getClass()); - JasperCompileManager.compileReportToFile(url.getFile(), url.getFile() + ".jasper"); - result.setLocation("org/apache/struts2/views/jasperreports/empty.jrxml.jasper"); - result.setFormat(JasperReportConstants.FORMAT_XML); - Connection connection = createMock(Connection.class); final Boolean[] closed = {false}; connection.close(); - expectLastCall().andAnswer(new IAnswer() { - @Override - public Object answer() throws Throwable { - closed[0] = true; - return null; - } + expectLastCall().andAnswer(() -> { + closed[0] = true; + return null; }); replay(connection); @@ -70,20 +66,243 @@ public class JasperReportsResultTest extends StrutsTestCase { assertTrue(closed[0]); } + public void testDataSourceNotAccepted() throws Exception { + stack.push(new Object() { + public String getDatasourceName() { + return "getDatasource()"; + } + + public Map[] getDatasource() { + return JR_MAP_ARRAY_DATA_SOURCE; + } + }); + result.setDataSource("${datasourceName}"); + + try { + result.execute(this.invocation); + } catch (ServletException e) { + assertEquals("Error building dataSource for excluded or not accepted [getDatasource()]", + e.getMessage()); + } + + // verify that above test has really effect + result.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Hello Foo Bar!")); + } + + public void testDataSourceAccepted() throws Exception { + stack.push(new Object() { + public String getDatasourceName() { + return "datasource"; + } + + public Map[] getDatasource() { + return JR_MAP_ARRAY_DATA_SOURCE; + } + }); + result.setDataSource("${datasourceName}"); + + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Hello Foo Bar!")); + } + + public void testDataSourceExpressionAccepted() throws Exception { + result.setDataSource("{#{'firstName':'Qux', 'lastName':'Quux'}}"); + + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Hello Qux Quux!")); + } + + public void testReportParametersNotAccepted() throws Exception { + result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}"); + + stack.push(new Object() { + public String getReportParametersName() { + return "getReportParameters()"; + } + + public Map getReportParameters() { + return new HashMap() {{ + put("title", "Baz"); + }}; + } + }); + + result.setReportParameters("${reportParametersName}"); + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("null Report")); + + // verify that above test has really effect + response.setCommitted(false); + response.reset(); + result.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Baz Report")); + } + + public void testReportParametersAccepted() throws Exception { + result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}"); + + stack.push(new Object() { + public String getReportParametersName() { + return "reportParameters"; + } + + public Map getReportParameters() { + return new HashMap() {{ + put("title", "Baz"); + }}; + } + }); + + result.setReportParameters("${reportParametersName}"); + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Baz Report")); + } + + public void testReportParametersExpressionAccepted() throws Exception { + result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}"); + + result.setReportParameters("#{'title':'Qux'}"); + result.execute(this.invocation); + assertTrue(response.getContentAsString().contains("Qux Report")); + } + + public void testExportParametersNotAccepted() throws Exception { + result.setDataSource("{#{'firstName':'ignore', 'lastName':'ignore'}}"); + + final StringBuffer sb = new StringBuffer(); + stack.push(new Object() { + public String getExportParametersName() { + return "getExportParameters()"; + } + + public Map getExportParameters() { + return new HashMap() {{ + put(OUTPUT_STRING_BUFFER, sb); + }}; + } + }); + + result.setExportParameters("${exportParametersName}"); + result.execute(this.invocation); + assertEquals(0, sb.length()); + + // verify that above test has really effect + response.setCommitted(false); + response.reset(); + result.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + result.execute(this.invocation); + assertNotEquals(0, sb.length()); + } + + public void testExportParametersAccepted() throws Exception { + result.setDataSource("{#{'firstName':'Qux', 'lastName':'Quux'}}"); + + final StringBuffer sb = new StringBuffer(); + stack.push(new Object() { + public String getExportParametersName() { + return "exportParameters"; + } + + public Map getExportParameters() { + return new HashMap() {{ + put(OUTPUT_STRING_BUFFER, sb); + }}; + } + }); + + result.setExportParameters("${exportParametersName}"); + result.execute(this.invocation); + assertTrue(sb.toString().contains("Hello Qux Quux!")); + } + @Override protected void setUp() throws Exception { super.setUp(); - MockHttpServletResponse response = new MockHttpServletResponse(); - MockHttpServletRequest request = new MockHttpServletRequest(); - request.setRequestURI("http://sumeruri"); + + request.setRequestURI("http://someuri"); ActionContext context = ActionContext.getContext(); context.put(StrutsStatics.HTTP_RESPONSE, response); context.put(StrutsStatics.HTTP_REQUEST, request); - this.stack = context.getValueStack(); - MockServletContext servletContext = new MockServletContext(); context.put(StrutsStatics.SERVLET_CONTEXT, servletContext); + this.stack = context.getValueStack(); this.invocation = new MockActionInvocation(); this.invocation.setInvocationContext(context); this.invocation.setStack(this.stack); + + result = new JasperReportsResult(); + container.inject(result); + URL url = ClassLoaderUtil.getResource("org/apache/struts2/views/jasperreports/simple.jrxml", this.getClass()); + JasperCompileManager.compileReportToFile(url.getFile(), url.getFile() + ".jasper"); + result.setLocation("org/apache/struts2/views/jasperreports/simple.jrxml.jasper"); + result.setFormat(JasperReportConstants.FORMAT_XML); } + + + private static final Map[] JR_MAP_ARRAY_DATA_SOURCE = new Map[]{ + new HashMap() {{ + put("firstName", "Foo"); + put("lastName", "Bar"); + }} + }; + + private static final NotExcludedAcceptedPatternsChecker NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER + = new NotExcludedAcceptedPatternsChecker() { + @Override + public IsAllowed isAllowed(String value) { + return IsAllowed.yes("*"); + } + + @Override + public IsAccepted isAccepted(String value) { + return null; + } + + @Override + public void setAcceptedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setAcceptedPatterns(String[] patterns) { + + } + + @Override + public void setAcceptedPatterns(Set patterns) { + + } + + @Override + public Set getAcceptedPatterns() { + return null; + } + + @Override + public IsExcluded isExcluded(String value) { + return null; + } + + @Override + public void setExcludedPatterns(String commaDelimitedPatterns) { + + } + + @Override + public void setExcludedPatterns(String[] patterns) { + + } + + @Override + public void setExcludedPatterns(Set patterns) { + + } + + @Override + public Set getExcludedPatterns() { + return null; + } + }; } diff --git a/plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/simple.jrxml b/plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/simple.jrxml new file mode 100644 index 000000000..9de586a9c --- /dev/null +++ b/plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/simple.jrxml @@ -0,0 +1,45 @@ + + + + + + + + <band height="16"> + <textField> + <reportElement x="0" y="0" width="100" height="16" /> + <textFieldExpression><![CDATA[$P{title}]]> + " Report"</textFieldExpression> + </textField> + </band> + + + + + + "Hello " + + " " + + "!" + + + + \ No newline at end of file diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AnchorTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AnchorTest.java index 673d8128f..d727a7440 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AnchorTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/AnchorTest.java @@ -20,6 +20,7 @@ */ package org.apache.struts2.views.java.simple; +import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker; import org.apache.struts2.components.Anchor; import org.apache.struts2.components.UIBean; import org.apache.struts2.components.ServletUrlRenderer; @@ -73,6 +74,7 @@ public class AnchorTest extends AbstractTest { super.setUp(); this.tag = new Anchor(stack, request, response); this.tag.setUrlRenderer(new ServletUrlRenderer()); + this.tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker()); } @Override diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/CheckboxTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/CheckboxTest.java index d29ca88d8..10a8e36c2 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/CheckboxTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/CheckboxTest.java @@ -20,6 +20,7 @@ */ package org.apache.struts2.views.java.simple; +import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker; import org.apache.struts2.components.Checkbox; import org.apache.struts2.components.UIBean; @@ -68,6 +69,7 @@ public class CheckboxTest extends AbstractCommonAttributesTest { protected void setUp() throws Exception { super.setUp(); tag = new Checkbox(stack, request, response); + tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker()); } @Override diff --git a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java index 3536c5e13..832653692 100644 --- a/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java +++ b/plugins/javatemplates/src/test/java/org/apache/struts2/views/java/simple/ScriptTest.java @@ -19,6 +19,7 @@ package org.apache.struts2.views.java.simple; import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker; import org.apache.struts2.components.Script; import org.apache.struts2.components.UIBean; @@ -82,5 +83,6 @@ public class ScriptTest extends AbstractTest { actionContext.withSession(session); this.tag = new Script(stack, request, response); + tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker()); } } diff --git a/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java b/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java index 4b51f7ec0..a286deb3e 100644 --- a/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java +++ b/plugins/portlet/src/main/java/org/apache/struts2/components/PortletUrlRenderer.java @@ -192,7 +192,9 @@ public class PortletUrlRenderer implements UrlRenderer { } else { id = action.substring(slash + 1); } - formComponent.addParameter("id", formComponent.escape(id)); + String escapedId = formComponent.escape(id); + formComponent.addParameter("id", escapedId); + formComponent.addParameter("escapedId", escapedId); } } }