From 8d6e26e0feb8cb1669f45a66e458860534b94571 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Fri, 9 Jul 2021 10:45:22 +0430 Subject: [PATCH 1/6] fix double evaluations address known issues reported at https://securitylab.github.com/research/apache-struts-double-evaluation/ --- .../providers/XWorkConfigurationProvider.java | 4 + .../xwork2/interceptor/AliasInterceptor.java | 85 +++++- .../opensymphony/xwork2/mock/MockResult.java | 7 +- .../security/AcceptedPatternsChecker.java | 2 +- ...ultNotExcludedAcceptedPatternsChecker.java | 105 ++++++++ .../NotExcludedAcceptedPatternsChecker.java | 70 +++++ .../org/apache/struts2/StrutsConstants.java | 1 + .../apache/struts2/components/Component.java | 42 ++- .../apache/struts2/components/FormButton.java | 1 + .../org/apache/struts2/components/Param.java | 29 +- .../components/ServletUrlRenderer.java | 8 +- .../org/apache/struts2/components/UIBean.java | 123 +++++---- .../config/DefaultBeanSelectionProvider.java | 3 + .../apache/struts2/result/StreamResult.java | 51 +++- .../org/apache/struts2/util/StrutsUtil.java | 4 +- core/src/main/resources/struts-default.xml | 1 + .../template/css_xhtml/form-validate.ftl | 4 +- .../resources/template/simple/combobox.ftl | 4 +- .../template/simple/doubleselect.ftl | 44 +-- .../resources/template/simple/form-close.ftl | 24 +- .../template/xhtml/form-close-validate.ftl | 10 +- .../resources/template/xhtml/form-close.ftl | 2 +- .../template/xhtml/form-validate.ftl | 4 +- .../opensymphony/xwork2/ChainResultTest.java | 14 +- .../interceptor/AliasInterceptorTest.java | 90 +++++++ .../DefaultAcceptedPatternsCheckerTest.java | 28 ++ .../DefaultExcludedPatternsCheckerTest.java | 29 ++ ...otExcludedAcceptedPatternsCheckerTest.java | 91 +++++++ .../struts2/TestConfigurationProvider.java | 12 +- .../apache/struts2/components/UIBeanTest.java | 94 +++++-- .../struts2/result/PostbackResultTest.java | 135 ++++++++++ .../ServletActionRedirectResultTest.java | 135 ++++++++++ .../struts2/result/StreamResultTest.java | 42 ++- .../apache/struts2/util/StrutsUtilTest.java | 55 ++-- .../FreemarkerResultMockedTest.java | 65 ++++- .../apache/struts2/views/jsp/BeanTagTest.java | 61 +++++ .../struts2/views/jsp/ui/ComboBoxTest.java | 2 +- .../struts2/views/jsp/ui/TextfieldTest.java | 3 +- .../struts2/views/freemarker/iterator.ftl | 8 +- .../struts2/views/jsp/ui/ComboBox-4.txt | 8 +- .../apache/struts2/views/jsp/ui/Radio-6.txt | 12 +- .../struts2/views/jsp/ui/Textfield-5.txt | 2 +- core/src/test/resources/struts.xml | 5 + core/src/test/resources/xwork-sample.xml | 24 +- .../jasperreports/JasperReportsResult.java | 59 +++- .../JasperReportsResultTest.java | 251 +++++++++++++++++- .../struts2/views/jasperreports/simple.jrxml | 45 ++++ .../struts2/views/java/simple/AnchorTest.java | 2 + .../views/java/simple/CheckboxTest.java | 2 + .../components/PortletUrlRenderer.java | 4 +- 50 files changed, 1657 insertions(+), 249 deletions(-) create mode 100644 core/src/main/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsChecker.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/security/NotExcludedAcceptedPatternsChecker.java create mode 100644 core/src/test/java/com/opensymphony/xwork2/security/DefaultNotExcludedAcceptedPatternsCheckerTest.java create mode 100644 core/src/test/java/org/apache/struts2/result/PostbackResultTest.java rename plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/empty.jrxml => core/src/test/resources/org/apache/struts2/views/freemarker/iterator.ftl (61%) create mode 100644 plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/simple.jrxml diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java index f1d4f11fb2..f677a3e970 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java @@ -33,6 +33,7 @@ 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; @@ -88,6 +89,7 @@ import com.opensymphony.xwork2.ognl.accessor.XWorkListPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor; +import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider; @@ -215,6 +217,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) .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 d5135bf68d..2cb1a4e323 100644 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java +++ b/core/src/main/java/com/opensymphony/xwork2/interceptor/AliasInterceptor.java @@ -23,6 +23,8 @@ import com.opensymphony.xwork2.XWorkConstants; 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(XWorkConstants.DEV_MODE) public void setDevMode(String mode) { this.devMode = Boolean.parseBoolean(mode); @@ -115,6 +120,16 @@ public void setLocalizedTextProvider(LocalizedTextProvider localizedTextProvider 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 void setAliasesKey(String aliasesKey) { 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 void setAliasesKey(String aliasesKey) { 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 @@ -206,5 +227,65 @@ public void setAliasesKey(String aliasesKey) { 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 f85cae4a76..6d3debec37 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 boolean equals(Object o) { } public void execute(ActionInvocation invocation) throws Exception { - // no op + this.invocation = invocation; } @Override @@ -53,4 +55,7 @@ public void setFoo(String foo) { // 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 fa92e0b2ae..f5a329459f 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.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 0000000000..b475da1d06 --- /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 0000000000..3a1f2d86ba --- /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.5.27 + */ +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 ae4c278f5e..8c076d24e5 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -322,6 +322,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 2612ea0054..3e25a528fd 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; @@ -70,6 +71,8 @@ public class Component { protected boolean throwExceptionOnELFailure; private UrlHelper urlHelper; + private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns; + /** * Constructor. * @@ -112,6 +115,12 @@ public void setThrowExceptionsOnELFailure(String throwException) { public void setUrlHelper(UrlHelper urlHelper) { this.urlHelper = urlHelper; } + + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + /** * Gets the OGNL value stack associated with this component. * @return the OGNL value stack associated with this component. @@ -276,7 +285,7 @@ protected Object findValue(String expr) { } /** - * If altsyntax (%{...}) is applied, simply strip the "%{" and "}" off. + * If altsyntax (%{...}) is applied, simply strip the "%{" and "}" off. * @param expr the expression (must be not null) * @return the stripped expression if altSyntax is enabled. Otherwise * the parameter expression is returned as is. @@ -296,7 +305,7 @@ public boolean altSyntax() { /** * Adds the surrounding %{ } to the expression for proper processing. * @param expr the expression. - * @return the modified expression if altSyntax is enabled, or the parameter + * @return the modified expression if altSyntax is enabled, or the parameter * expression otherwise. */ protected String completeExpressionIfAltSyntax(String expr) { @@ -382,15 +391,6 @@ protected Object findValue(String expr, Class toType) { } } - /** - * Detects if altSyntax is enabled and then checks if expression contains %{...} - * @param expr a string to examined - * @return true if altSyntax is enabled and expr contains %{...} - */ - protected boolean recursion(String expr) { - return ComponentUtils.altSyntax(stack) && ComponentUtils.containsExpression(expr); - } - /** * Renders an action URL by consulting the {@link org.apache.struts2.dispatcher.mapper.ActionMapper}. * @param action the action @@ -407,7 +407,7 @@ protected boolean recursion(String expr) { * @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); @@ -560,4 +560,22 @@ protected Collection getStandardAttributes() { return standardAttributes; } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of the component + * @return true|false + * @since 2.5.27 + */ + 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 7dcc7ae686..e61ed5d71b 100644 --- a/core/src/main/java/org/apache/struts2/components/FormButton.java +++ b/core/src/main/java/org/apache/struts2/components/FormButton.java @@ -125,6 +125,7 @@ protected void populateComponentHtmlId(Form form) { } } 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 023761ad92..c2c299e7ec 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 boolean end(Writer writer, String body) { 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 boolean end(Writer writer, String body) { return super.end(writer, ""); } - + + @Override public boolean usesBody() { return true; } @@ -193,7 +200,7 @@ public interface UnnamedParametric { * 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 b41b4d60ee..b2704bc06e 100644 --- a/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java +++ b/core/src/main/java/org/apache/struts2/components/ServletUrlRenderer.java @@ -188,7 +188,9 @@ public void renderFormUrl(Form formComponent) { // 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 @@ -223,7 +225,9 @@ public void renderFormUrl(Form formComponent) { } 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 21ae1d5880..9a8ae45742 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -555,16 +555,13 @@ public boolean end(Writer writer, String body) { 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); } @@ -581,71 +578,70 @@ protected void mergeTemplate(Writer writer, Template template) throws Exception } 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); - String name = null; + String translatedName = null; String providedLabel = null; if (this.key != null) { @@ -661,8 +657,8 @@ public void evaluateParams() { } if (this.name != null) { - name = findString(this.name); - addParameter("name", name); + translatedName = findString(this.name); + addParameter("name", translatedName); } if (label != null) { @@ -786,28 +782,31 @@ public void evaluateParams() { // 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 = completeExpressionIfAltSyntax(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 = completeExpressionIfAltSyntax(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)); } } } @@ -821,10 +820,10 @@ public void evaluateParams() { 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); } } @@ -892,7 +891,7 @@ public void evaluateParams() { 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; } @@ -943,14 +942,14 @@ protected void enableAncestorFormCustomOnsubmit() { 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 @@ -960,23 +959,23 @@ protected Map getTooltipConfig(UIBean 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; } /** @@ -1257,10 +1256,10 @@ public void setTooltipIconPath(String tooltipIconPath) { 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()); } } } @@ -1275,9 +1274,9 @@ public void copyParams(Map params) { super.copyParams(params); for (Object o : params.entrySet()) { Map.Entry entry = (Map.Entry) o; - String key = (String) entry.getKey(); - if (!isValidTagAttribute(key) && !key.equals("dynamicAttributes")) { - dynamicAttributes.put(key, entry.getValue()); + String entryKey = (String) entry.getKey(); + if (!isValidTagAttribute(entryKey) && !entryKey.equals("dynamicAttributes")) { + dynamicAttributes.put(entryKey, entry.getValue()); } } } diff --git a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java index fb780037bd..cb39ac741c 100644 --- a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java @@ -50,6 +50,7 @@ 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; @@ -424,6 +425,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) { /** 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/result/StreamResult.java b/core/src/main/java/org/apache/struts2/result/StreamResult.java index 554b1a9618..3b5340882e 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; @@ -100,6 +102,8 @@ public class StreamResult extends StrutsResultSupport { protected int bufferSize = 1024; protected boolean allowCaching = true; + private NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns; + public StreamResult() { super(); } @@ -115,6 +119,11 @@ public boolean getAllowCaching() { return allowCaching; } + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + /** * Set allowCaching to false to indicate that the client should be requested not to cache the data stream. * This is set to false by default @@ -219,14 +228,17 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro 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); } @@ -243,15 +255,16 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro 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); } } @@ -292,4 +305,22 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro } } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of result + * @return true|false + * @since 2.5.27 + */ + 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 647a743520..499652ea39 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java @@ -102,7 +102,7 @@ public String include(Object aName) throws Exception { return responseWrapper.getData(); } catch (Exception e) { - LOG.debug("Cannot include {}", aName.toString(), e); + LOG.debug("Cannot include {}", aName, e); throw e; } } @@ -125,7 +125,7 @@ public Object findValue(String expression, String className) throws ClassNotFoun } 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 2b305d101e..f05109d856 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -202,6 +202,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 f1b2053821..04f1a450e7 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 3d953c5e32..87e5b6a8df 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 f1adf8f6eb..40e700d1fd 100644 --- a/core/src/main/resources/template/simple/doubleselect.ftl +++ b/core/src/main/resources/template/simple/doubleselect.ftl @@ -72,9 +72,9 @@ \ No newline at end of file diff --git a/core/src/main/resources/template/simple/form-close.ftl b/core/src/main/resources/template/simple/form-close.ftl index 0efea0c599..c454f07a85 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 2cbb174f3e..d6033f7642 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 68def20c52..c091f0f32e 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 @@ 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; @@ -83,7 +84,18 @@ public void testRecursiveChain() throws Exception { } } - 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 ActionProxy returnVal; private String expectedActionName; private 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 90141252e1..12ffd86aaa 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/AliasInterceptorTest.java @@ -28,6 +28,10 @@ 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 @@ -66,6 +70,92 @@ public void testUsingDefaultInterceptorThatAliasPropertiesAreCopied() throws Exc 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 XmlConfigurationProvider("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 XmlConfigurationProvider("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 1dc8d8a7cd..d69ac8fbea 100644 --- a/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java @@ -174,4 +174,32 @@ public void testDmiIsEnabled() { assertTrue("dmi isn't 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; + } + }; } \ No newline at end of file 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 cec0c1fc41..d6e37583b4 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 java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.regex.Pattern; @@ -218,4 +219,32 @@ public void testExcludedPatternsImmutable() throws Exception { // 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 0000000000..85f5b71b9c --- /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 9e093f86ac..ce6bcc10c7 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.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.views.jsp.ui.DoubleValidationAction; import java.util.HashMap; +import java.util.Map; /** @@ -74,7 +77,7 @@ public void init(Configuration config) { */ public void loadPackages() { - HashMap successParams = new HashMap(); + Map successParams = new HashMap<>(); successParams.put("propertyName", "executionCount"); successParams.put("expectedValue", "1"); @@ -149,9 +152,7 @@ public void loadPackages() { } /** - * 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 void register(ContainerBuilder builder, LocatableProperties props) throws 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 ca2ffa28af..3b4dca5124 100644 --- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java +++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java @@ -32,9 +32,11 @@ import java.util.Collections; 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(); @@ -50,7 +52,7 @@ public void testPopulateComponentHtmlId1() throws Exception { 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(); @@ -66,7 +68,7 @@ public void testPopulateComponentHtmlIdWithOgnl() throws Exception { 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(); @@ -82,7 +84,7 @@ public void testPopulateComponentHtmlId2() throws Exception { 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(); @@ -94,10 +96,10 @@ public void testPopulateComponentHtmlWithoutNameAndId() throws Exception { 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(); @@ -110,11 +112,11 @@ protected String getDefaultTemplate() { 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(); @@ -128,7 +130,7 @@ public void testEscapeId() throws Exception { 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(); @@ -140,29 +142,29 @@ public void testGetThemeFromForm() throws Exception { 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(); @@ -184,7 +186,7 @@ public TemplateEngine getTemplateEngine(Template template, String templateTypeOv } } - public void testBuildTemplate() throws Exception { + public void testBuildTemplate() { String defaultTemplateName = "default"; String customTemplateName = "custom"; ValueStack stack = ActionContext.getContext().getValueStack(); @@ -200,14 +202,14 @@ public void testBuildTemplate() throws Exception { 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); @@ -217,13 +219,13 @@ public void testGetTemplateDirExplicit() throws Exception { 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); @@ -232,7 +234,7 @@ public void testGetTemplateDirAttr() throws Exception { assertEquals(attrTemplateDir, txtFld.getTemplateDir()); } - public void testGetTemplateDirDefault() throws Exception { + public void testGetTemplateDirDefault() { String defaultTemplateDir = "defaultTemplateDirectory"; ValueStack stack = ActionContext.getContext().getValueStack(); MockHttpServletRequest req = new MockHttpServletRequest(); @@ -244,7 +246,7 @@ public void testGetTemplateDirDefault() throws Exception { 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(); @@ -295,10 +297,58 @@ public String getMyBad() { }); 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 new file mode 100644 index 0000000000..8d64340b77 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/result/PostbackResultTest.java @@ -0,0 +1,135 @@ +/* + * 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 org.apache.struts2.result; + +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.ActionProxy; +import com.opensymphony.xwork2.Result; +import com.opensymphony.xwork2.util.ValueStack; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.dispatcher.mapper.ActionMapper; +import org.easymock.IMocksControl; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import static org.easymock.EasyMock.createControl; +import static org.easymock.EasyMock.expect; + +public class PostbackResultTest extends StrutsInternalTestCase { + + public void testWithNoNamespace() throws Exception { + + ActionContext context = ActionContext.getContext(); + 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.setActionName("myAction${1-1}"); + result.setPrependServletContext(false); + + IMocksControl control = createControl(); + ActionProxy mockActionProxy = control.createMock(ActionProxy.class); + ActionInvocation mockInvocation = control.createMock(ActionInvocation.class); + expect(mockInvocation.getInvocationContext()).andReturn(context).anyTimes(); + expect(mockInvocation.getStack()).andReturn(stack).anyTimes(); + expect(mockInvocation.getProxy()).andReturn(mockActionProxy); + expect(mockActionProxy.getNamespace()).andReturn("${1-1}"); + + control.replay(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.execute(mockInvocation); + assertEquals("
" + + "", res.getContentAsString()); + + control.verify(); + } + + public void testWithNamespace() throws Exception { + + ActionContext context = ActionContext.getContext(); + 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.setActionName("myAction${1-1}"); + result.setNamespace("myNamespace${1-1}"); + 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()); + + 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(); + } +} 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 b7ed61c41e..acd79c1dce 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletActionRedirectResultTest.java @@ -108,6 +108,141 @@ public void testIncludeParameterInResultWithConditionParseOn() throws Exception 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("", "") + .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"); + 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.setActionName("myAction${1-1}"); + 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).times(2); + expect(mockInvocation.getResultCode()).andReturn("myResult"); + expect(mockActionProxy.getConfig()).andReturn(actionConfig); + expect(mockActionProxy.getNamespace()).andReturn("${1-1}"); + expect(mockInvocation.getInvocationContext()).andReturn(context); + expect(mockInvocation.getStack()).andReturn(stack).anyTimes(); + + control.replay(); + result.setActionMapper(container.getInstance(ActionMapper.class)); + result.execute(mockInvocation); + assertEquals("/${1-1}/myAction0.action?param1=value+1¶m2=value+2¶m3=value+3#fragment", res.getRedirectedUrl()); + + control.verify(); + } + public void testIncludeParameterInResult() 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 1b46d0bd9e..5661db5d3d 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.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 void testStreamResultParse2() throws Exception { 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 @@ protected void tearDown() throws Exception { mai = null; } - public class MyImageAction implements Action { + public static class MyImageAction implements Action { FileInputStream streamForImage; long contentLength; @@ -257,7 +289,7 @@ public MyImageAction() throws Exception { contentLength = file.length(); } - public InputStream getStreamForImage() throws Exception { + public InputStream getStreamForImage() { return streamForImage; } @@ -265,7 +297,7 @@ public String execute() throws Exception { return SUCCESS; } - public long getContentLength() throws Exception { + public long getContentLength() { return contentLength; } @@ -273,6 +305,10 @@ public String getStreamForImageAsString() { 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 02be0f2879..4cf0abb827 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 void testBeanMethod() throws Exception { 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 boolean getMyBoolean(boolean bool) { 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 void testIncludeMethod() throws Exception { } - 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 boolean getMyBoolean(boolean bool) { - 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 void testMakeSelectListMethod() throws Exception { 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 @@ protected void tearDown() throws Exception { // === 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 InternalMockRequestDispatcher getDispatcher() { } } - 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 1067ddd164..7fd439ab1f 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 @@ -22,9 +22,7 @@ import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.ValueStack; -import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory; import freemarker.template.Configuration; -import freemarker.template.TemplateException; import freemarker.template.TemplateExceptionHandler; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsInternalTestCase; @@ -39,8 +37,8 @@ import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; -import java.net.MalformedURLException; -import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.List; import static org.apache.struts2.views.jsp.AbstractUITagTest.normalize; @@ -194,7 +192,64 @@ public void testDynamicAttributesInTheme() throws Exception { assertEquals(expected, stringWriter.toString()); } - private void init() throws MalformedURLException, URISyntaxException { + public void testIterator() throws Exception { + File file = new File(FreeMarkerResultTest.class.getResource("iterator.ftl").toURI()); + EasyMock.expect(servletContext.getRealPath("/tutorial/org/apache/struts2/views/freemarker/iterator.ftl")).andReturn(file.getAbsolutePath()); + + file = new File(ClassLoaderUtil.getResource("template/simple/text.ftl", getClass()).toURI()); + EasyMock.expect(servletContext.getRealPath("/template/simple/text.ftl")).andReturn(file.getAbsolutePath()); + + file = new File(ClassLoaderUtil.getResource("template/simple/css.ftl", getClass()).toURI()); + EasyMock.expect(servletContext.getRealPath("/template/simple/css.ftl")).andReturn(file.getAbsolutePath()); + EasyMock.expect(servletContext.getRealPath("/template/~~~simple/css.ftl")).andReturn(file.getAbsolutePath()); + + file = new File(ClassLoaderUtil.getResource("template/simple/scripting-events.ftl", getClass()).toURI()); + EasyMock.expect(servletContext.getRealPath("/template/simple/scripting-events.ftl")).andReturn(file.getAbsolutePath()); + EasyMock.expect(servletContext.getRealPath("/template/~~~simple/scripting-events.ftl")).andReturn(file.getAbsolutePath()); + + file = new File(ClassLoaderUtil.getResource("template/simple/common-attributes.ftl", getClass()).toURI()); + EasyMock.expect(servletContext.getRealPath("/template/simple/common-attributes.ftl")).andReturn(file.getAbsolutePath()); + EasyMock.expect(servletContext.getRealPath("/template/~~~simple/common-attributes.ftl")).andReturn(file.getAbsolutePath()); + + file = new File(ClassLoaderUtil.getResource("template/simple/dynamic-attributes.ftl", getClass()).toURI()); + EasyMock.expect(servletContext.getRealPath("/template/simple/dynamic-attributes.ftl")).andReturn(file.getAbsolutePath()); + EasyMock.expect(servletContext.getRealPath("/template/~~~simple/dynamic-attributes.ftl")).andReturn(file.getAbsolutePath()); + + EasyMock.replay(servletContext); + + init(); + + stack.push(new Object() { + List items = null; + + public List getItems() { + if (items == null) { + items = new ArrayList<>(3); + for (int i = 0; i < 3; i++) { + final int finalI = i; + items.add(new Object() { + public String getName() { + return "value" + finalI; + } + }); + } + } + return items; + } + }); + + request.setRequestURI("/tutorial/test11.action"); + ActionMapping mapping = container.getInstance(ActionMapper.class).getMapping(request, configurationManager); + dispatcher.serviceAction(request, response, mapping); + String result = stringWriter.toString(); + for (int i = 0; i < 3; i++) { + assertTrue(result.contains("id=\"itemId" + i + "\"")); + assertTrue(result.contains("name=\"items[" + i + "].name\"")); + assertTrue(result.contains("value=\"value" + i + "\"")); + } + } + + private void init() { stringWriter = new StringWriter(); writer = new PrintWriter(stringWriter); response = new StrutsMockHttpServletResponse(); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java index 168ffe563d..4b72ee03f8 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/BeanTagTest.java @@ -18,7 +18,14 @@ */ package org.apache.struts2.views.jsp; +import org.apache.struts2.StrutsException; +import org.apache.struts2.dispatcher.HttpParameters; + import javax.servlet.jsp.JspException; +import java.util.HashMap; +import java.util.Map; + +import static com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsCheckerTest.NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER; /** @@ -47,4 +54,58 @@ public void testSimple() { request.verify(); pageContext.verify(); } + + public void testNotAccepted() throws Exception { + BeanTag tag = new BeanTag(); + tag.setPageContext(pageContext); + tag.setName("org.apache.struts2.TestAction"); + + Map tmp = new HashMap<>(); + tmp.put("paramName", "getArray()[0]"); + context.put("parameters", HttpParameters.create(tmp).build()); + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("%{#parameters['paramName']}"); + param1.setValue("'success'"); + + tag.doStartTag(); + param1.doStartTag(); + + try { + param1.doEndTag(); + fail("an excluded or not accepted is evaluated?!"); + } catch (StrutsException e) { + assertEquals("Excluded or not accepted name found: getArray()[0]", e.getMessage()); + assertNull(stack.findValue("result")); + } + + param1.component.setNotExcludedAcceptedPatterns(NO_EXCLUSION_ACCEPT_ALL_PATTERNS_CHECKER); + tag.component.addParameter("array", "just to instantiate array to avoid null for getArray()"); + + param1.doEndTag(); + assertEquals("success", stack.findValue("array[0]")); + + tag.doEndTag(); + } + + public void testGetterAccepted() throws Exception { + BeanTag tag = new BeanTag(); + tag.setPageContext(pageContext); + tag.setName("org.apache.struts2.TestAction"); + + ParamTag param1 = new ParamTag(); + param1.setPageContext(pageContext); + param1.setName("getArray()[0]"); + param1.setValue("'success'"); + + tag.doStartTag(); + param1.doStartTag(); + + tag.component.addParameter("array", "just to instantiate array to avoid null for getArray()"); + + param1.doEndTag(); + assertEquals("success", stack.findValue("array[0]")); + + tag.doEndTag(); + } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java index f7ef72a42c..18c8e972f7 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/ComboBoxTest.java @@ -145,7 +145,7 @@ public void testJsCallNamingUsesEscapedId() throws Exception { tag.setPageContext(pageContext); tag.setLabel("mylabel"); tag.setName("foo"); - tag.setId("cb.bc"); + tag.setId("cb['\".\"'] = bc(){};//"); tag.setList("collection"); tag.doStartTag(); diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java index bd52b8ad94..f1701a86da 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java @@ -117,7 +117,7 @@ public void testNoLabelFtl() throws Exception { testAction.setFoo("bar"); TextFieldModel model = new TextFieldModel(stack, request, response); - Map params = new HashMap(); + Map params = new HashMap<>(); params.put("name", "myname"); params.put("value", "%{foo}"); params.put("size", "10"); @@ -172,6 +172,7 @@ public void testSimple_recursionTest() throws Exception { tag.setName("myname"); tag.setValue("%{foo}"); tag.setSize("10"); + tag.setDynamicAttribute(null, "anotherAttr", "%{foo}"); tag.doStartTag(); tag.doEndTag(); diff --git a/plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/empty.jrxml b/core/src/test/resources/org/apache/struts2/views/freemarker/iterator.ftl similarity index 61% rename from plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/empty.jrxml rename to core/src/test/resources/org/apache/struts2/views/freemarker/iterator.ftl index 816d8609f4..5114f40380 100644 --- a/plugins/jasperreports/src/test/resources/org/apache/struts2/views/jasperreports/empty.jrxml +++ b/core/src/test/resources/org/apache/struts2/views/freemarker/iterator.ftl @@ -1,5 +1,4 @@ - - - - \ No newline at end of file +<@s.iterator value="{'a','b','c'}" status="stat"> + <@s.textfield id="itemId%{#stat.index}" name="items[%{#stat.index}].name" theme="simple"/> + diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt index cea746551d..5fdcaf814f 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/ComboBox-4.txt @@ -1,13 +1,13 @@ - + -
-
+ diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-6.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-6.txt index 6bc0381dcf..146cb0a997 100644 --- a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-6.txt +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Radio-6.txt @@ -1,11 +1,11 @@ - - - - - - + + + + + + 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 08326c7765..2818476270 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 5b29c1e87a..6415ba394c 100644 --- a/core/src/test/resources/struts.xml +++ b/core/src/test/resources/struts.xml @@ -80,6 +80,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 5ff1e39e37..63c32b169d 100644 --- a/core/src/test/resources/xwork-sample.xml +++ b/core/src/test/resources/xwork-sample.xml @@ -102,7 +102,14 @@ - + + + #{ #parameters['name'] : #parameters['value'] } + + + + + @@ -143,6 +150,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 c79e1b6c77..05fff2ecee 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.*; @@ -131,6 +133,7 @@ public class JasperReportsResult extends StrutsResultSupport implements JasperRe private final static Logger LOG = LogManager.getLogger(JasperReportsResult.class); protected String dataSource; + private String parsedDataSource; protected String format; protected String documentName; protected String contentDisposition; @@ -150,12 +153,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. @@ -173,6 +180,11 @@ public JasperReportsResult(String location) { super(location); } + @Inject + public void setNotExcludedAcceptedPatterns(NotExcludedAcceptedPatternsChecker notExcludedAcceptedPatterns) { + this.notExcludedAcceptedPatterns = notExcludedAcceptedPatterns; + } + public String getImageServletUrl() { return imageServletUrl; } @@ -265,8 +277,16 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro ValueStackDataSource stackDataSource = null; Connection conn = (Connection) stack.findValue(connection); - if (conn == null) - stackDataSource = new ValueStackDataSource(stack, dataSource, wrapField); + if (conn == null) { + 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())) { // set the the HTTP Header to work around IE SSL weirdness @@ -297,7 +317,9 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro } // 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); @@ -372,7 +394,9 @@ protected void doExecute(String finalLocation, ActionInvocation invocation) thro 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); @@ -427,8 +451,9 @@ private void initializeProperties(ActionInvocation invocation) throws Exception LOG.error(message); throw new RuntimeException(message); } - if (dataSource != null) - dataSource = conditionalParse(dataSource, invocation); + if (dataSource != null) { + parsedDataSource = conditionalParse(dataSource, invocation); + } format = conditionalParse(format, invocation); if (StringUtils.isEmpty(format)) { @@ -443,8 +468,8 @@ private void initializeProperties(ActionInvocation invocation) throws Exception documentName = conditionalParse(documentName, invocation); } - reportParameters = conditionalParse(reportParameters, invocation); - exportParameters = conditionalParse(exportParameters, invocation); + parsedReportParameters = conditionalParse(reportParameters, invocation); + parsedExportParameters = conditionalParse(exportParameters, invocation); } /** @@ -469,4 +494,22 @@ private ByteArrayOutputStream exportReportToBytes(JasperPrint jasperPrint, JRExp return baos; } + /** + * Checks if expression doesn't contain vulnerable code + * + * @param expression of result + * @return true|false + * @since 2.5.27 + */ + 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 1bf55acf88..0d87be5040 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,35 +20,35 @@ 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(); @@ -70,20 +70,243 @@ public Object answer() throws Throwable { 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 0000000000..9de586a9c4 --- /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 673d8128f4..d727a74407 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 @@ protected void setUp() throws Exception { 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 d29ca88d86..10a8e36c2f 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 @@ protected void setUpStack() { protected void setUp() throws Exception { super.setUp(); tag = new Checkbox(stack, request, response); + tag.setNotExcludedAcceptedPatterns(new DefaultNotExcludedAcceptedPatternsChecker()); } @Override 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 c31a3ff0a4..62eed8f29c 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 void renderFormUrl(Form formComponent) { } 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); } } } From 4e05233cc26612f6ed2be49d7da39d74647ae057 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Tue, 18 Jan 2022 10:18:06 +0330 Subject: [PATCH 2/6] reorder to pass tests after merge --- .../resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5dc125c18a..f0bcb78ee0 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 @@ - + From 8a46e9313f296772b30f135912425a0e8b2e9844 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 19 Jan 2022 23:10:46 +0330 Subject: [PATCH 3/6] keep dynamic attributes order --- core/src/main/java/org/apache/struts2/components/UIBean.java | 3 +-- .../java/org/apache/struts2/views/jsp/ui/AbstractUITag.java | 4 ++-- .../resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) 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 afd20a2a38..00526572c4 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -40,7 +40,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.Writer; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -504,7 +503,7 @@ public UIBean(ValueStack stack, HttpServletRequest request, HttpServletResponse protected String tooltipIconPath; // dynamic attributes - protected Map dynamicAttributes = new HashMap<>(); + protected Map dynamicAttributes = new LinkedHashMap<>(); protected String defaultTemplateDir; protected String defaultUITheme; diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java index c5548f70b5..c2a90b47a4 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java @@ -23,7 +23,7 @@ import javax.servlet.jsp.JspException; import javax.servlet.jsp.tagext.DynamicAttributes; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; /** @@ -79,7 +79,7 @@ public abstract class AbstractUITag extends ComponentTagSupport implements Dynam protected String tooltipIconPath; // dynamic attributes. - protected Map dynamicAttributes = new HashMap<>(); + protected Map dynamicAttributes = new LinkedHashMap<>(); protected void populateParams() { super.populateParams(); 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 f0bcb78ee0..5dc125c18a 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 @@ - + From 88f04fa3376d998097d8eead267027e287123806 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Thu, 20 Jan 2022 01:30:00 +0330 Subject: [PATCH 4/6] Revert "keep dynamic attributes order" This reverts commit 8a46e931 --- core/src/main/java/org/apache/struts2/components/UIBean.java | 3 ++- .../resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) 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 00526572c4..afd20a2a38 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -40,6 +40,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.Writer; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -503,7 +504,7 @@ public UIBean(ValueStack stack, HttpServletRequest request, HttpServletResponse protected String tooltipIconPath; // dynamic attributes - protected Map dynamicAttributes = new LinkedHashMap<>(); + protected Map dynamicAttributes = new HashMap<>(); protected String defaultTemplateDir; protected String defaultUITheme; 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 5dc125c18a..f0bcb78ee0 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 @@ - + From bb6c18607882ea38cb9636e003d96eb61be28601 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Thu, 20 Jan 2022 02:17:05 +0330 Subject: [PATCH 5/6] fix test for jdk8+ --- .../struts2/views/jsp/AbstractUITagTest.java | 42 +++++++++++-------- .../struts2/views/jsp/ui/TextfieldTest.java | 3 +- .../struts2/views/jsp/ui/Textfield-5jdk8.txt | 4 ++ 3 files changed, 31 insertions(+), 18 deletions(-) create mode 100644 core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5jdk8.txt diff --git a/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java index 2cde3d7f6b..66112577bc 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java @@ -24,6 +24,7 @@ import org.apache.commons.beanutils.BeanUtils; import org.apache.struts2.ServletActionContext; import org.apache.struts2.views.jsp.ui.AbstractUITag; +import org.fest.assertions.Assertions; import java.io.InputStream; import java.lang.reflect.InvocationTargetException; @@ -213,24 +214,28 @@ public void verifyResource(String resource) throws Exception { * Attempt to verify the contents of this.writer against the contents of the URL specified. verify() performs a * trim on both ends * - * @param url the HTML snippet that we want to validate against + * @param urls the HTML snippets that we want to validate against any of them * @throws Exception if the validation failed */ - public void verify(URL url) throws Exception { - if (url == null) { - fail("unable to verify a null URL"); - } else if (this.writer == null) { - fail("AbstractJspWriter.writer not initialized. Unable to verify"); - } + public void verify(URL... urls) throws Exception { + List bufferStrings = new ArrayList<>(); + for(URL url : urls) { + if (url == null) { + fail("unable to verify a null URL"); + } else if (this.writer == null) { + fail("AbstractJspWriter.writer not initialized. Unable to verify"); + } - StringBuilder buffer = new StringBuilder(128); - try (InputStream in = url.openStream()) { - byte[] buf = new byte[4096]; - int nbytes; - - while ((nbytes = in.read(buf)) > 0) { - buffer.append(new String(buf, 0, nbytes)); - } + StringBuilder buffer = new StringBuilder(128); + try (InputStream in = url.openStream()) { + byte[] buf = new byte[4096]; + int nbytes; + + while ((nbytes = in.read(buf)) > 0) { + buffer.append(new String(buf, 0, nbytes)); + } + } + bufferStrings.add(normalize(buffer.toString(), true)); } /** @@ -238,9 +243,12 @@ public void verify(URL url) throws Exception { * normalize the strings first to account for line termination differences between platforms. */ String writerString = normalize(writer.toString(), true); - String bufferString = normalize(buffer.toString(), true); - assertEquals(bufferString, writerString); + if (bufferStrings.size() == 1) { + assertEquals(bufferStrings.get(0), writerString); + } else { + Assertions.assertThat(writerString).isIn(bufferStrings); + } } protected void setUp() throws Exception { diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java index af447612c7..d921156d1a 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/TextfieldTest.java @@ -178,7 +178,8 @@ public void testSimple_recursionTest() throws Exception { tag.doStartTag(); tag.doEndTag(); - verify(TextFieldTag.class.getResource("Textfield-5.txt")); + verify(TextFieldTag.class.getResource("Textfield-5.txt"), + TextFieldTag.class.getResource("Textfield-5jdk8.txt")); } public void testSimple_recursionTestNoValue() throws Exception { diff --git a/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5jdk8.txt b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5jdk8.txt new file mode 100644 index 0000000000..5dc125c18a --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/views/jsp/ui/Textfield-5jdk8.txt @@ -0,0 +1,4 @@ + + + + From e7834d4345c73dedfd582e9a616dd7f37db6e0f2 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Thu, 20 Jan 2022 02:22:13 +0330 Subject: [PATCH 6/6] Revert "keep dynamic attributes order" This reverts commit 8a46e931 --- .../java/org/apache/struts2/views/jsp/ui/AbstractUITag.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java index c2a90b47a4..c5548f70b5 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/AbstractUITag.java @@ -23,7 +23,7 @@ import javax.servlet.jsp.JspException; import javax.servlet.jsp.tagext.DynamicAttributes; -import java.util.LinkedHashMap; +import java.util.HashMap; import java.util.Map; /** @@ -79,7 +79,7 @@ public abstract class AbstractUITag extends ComponentTagSupport implements Dynam protected String tooltipIconPath; // dynamic attributes. - protected Map dynamicAttributes = new LinkedHashMap<>(); + protected Map dynamicAttributes = new HashMap<>(); protected void populateParams() { super.populateParams();