From 3d39fba0db128093e04111e00cda96f9d1bcd6eb Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 4 Jan 2022 20:20:28 +0100 Subject: [PATCH 1/7] WW-5117 Restores previous behavior where tag was before action on stack --- .../components/template/FreemarkerTemplateEngine.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java b/core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java index 3bbd2196b6..91e155867e 100644 --- a/core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java +++ b/core/src/main/java/org/apache/struts2/components/template/FreemarkerTemplateEngine.java @@ -148,16 +148,13 @@ public void close() throws IOException { } }; - LOG.debug("Puts action on the top of ValueStack, just before the tag"); - action = stack.pop(); + LOG.debug("Push tag on top of the stack"); stack.push(templateContext.getTag()); - stack.push(action); try { template.process(model, writer); } finally { - stack.pop(); // removes action - stack.pop(); // removes tag - stack.push(action); // puts back action + LOG.debug("Removes tag from top of the stack"); + stack.pop(); } } From 893a8924eb7b9723df3797bb5060847df61f2094 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 4 Jan 2022 20:21:28 +0100 Subject: [PATCH 2/7] WW-5117 Evaluates dynamic attributes when assigning them to tag Reverts https://github.com/apache/struts/pull/447/commits/8bbe1949e17d58e1b5aef9c71e1279ad12ad7ba7#diff-0a39f082871f48bd14037ab2e3a3911b0b1046506c1d93338024d77d412a7075L305-L309 --- .../java/org/apache/struts2/components/UIBean.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 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 24b47fa140..9ec23d5c79 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -21,6 +21,7 @@ import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.util.ValueStack; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -30,7 +31,7 @@ import org.apache.struts2.components.template.TemplateEngine; import org.apache.struts2.components.template.TemplateEngineManager; import org.apache.struts2.components.template.TemplateRenderingContext; -import org.apache.struts2.dispatcher.StaticContentLoader; +import org.apache.struts2.util.ComponentUtils; import org.apache.struts2.util.TextProviderHelper; import org.apache.struts2.views.annotations.StrutsTagAttribute; import org.apache.struts2.views.util.ContextUtil; @@ -1272,10 +1273,15 @@ public void setTooltipIconPath(String tooltipIconPath) { public void setDynamicAttributes(Map tagDynamicAttributes) { for (Map.Entry entry : tagDynamicAttributes.entrySet()) { - String entryKey = entry.getKey(); + String attrName = entry.getKey(); + String attrValue = entry.getValue(); - if (!isValidTagAttribute(entryKey)) { - dynamicAttributes.put(entryKey, entry.getValue()); + if (!isValidTagAttribute(attrName)) { + if (ComponentUtils.altSyntax(getStack()) && ComponentUtils.isExpression(attrValue)) { + dynamicAttributes.put(attrName, String.valueOf(ObjectUtils.defaultIfNull(findString(attrValue), attrValue))); + } else { + dynamicAttributes.put(attrName, attrValue); + } } } } From a4a4e9e2625a9545766b83cd70d2e9f9fc24f21b Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 5 Jan 2022 20:09:44 +0100 Subject: [PATCH 3/7] WW-5117 Does a conditional evaluation depending on the tag Some tags requires to perform a lazy evaluation which can only happen in ftl template as performing it in the component class is not possible --- .../apache/struts2/components/CheckboxList.java | 16 +++++++++++++--- .../apache/struts2/components/ListUIBean.java | 1 - .../org/apache/struts2/components/Radio.java | 17 ++++++++++++++--- .../org/apache/struts2/components/UIBean.java | 14 ++++++++++++-- .../resources/template/simple/checkboxlist.ftl | 8 ++++---- .../template/simple/dynamic-attributes.ftl | 6 +++++- .../main/resources/template/simple/radiomap.ftl | 5 +++-- 7 files changed, 51 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/CheckboxList.java b/core/src/main/java/org/apache/struts2/components/CheckboxList.java index e830918880..601ce5bdf1 100644 --- a/core/src/main/java/org/apache/struts2/components/CheckboxList.java +++ b/core/src/main/java/org/apache/struts2/components/CheckboxList.java @@ -48,7 +48,7 @@ allowDynamicAttributes = true) public class CheckboxList extends ListUIBean { final public static String TEMPLATE = "checkboxlist"; - + public CheckboxList(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); } @@ -56,9 +56,19 @@ public CheckboxList(ValueStack stack, HttpServletRequest request, HttpServletRes protected String getDefaultTemplate() { return TEMPLATE; } - + public void evaluateExtraParams() { super.evaluateExtraParams(); } -} \ No newline at end of file + /** + * Checkboxlist tag requires lazy evaluation as list of tags is dynamically generated using + * + * @return boolean true by default + */ + @Override + protected boolean lazyEvaluation() { + return true; + } + +} diff --git a/core/src/main/java/org/apache/struts2/components/ListUIBean.java b/core/src/main/java/org/apache/struts2/components/ListUIBean.java index 26484f250e..bfaffe6f13 100644 --- a/core/src/main/java/org/apache/struts2/components/ListUIBean.java +++ b/core/src/main/java/org/apache/struts2/components/ListUIBean.java @@ -195,7 +195,6 @@ public void setListTitle(String listTitle) { this.listTitle = listTitle; } - public void setThrowExceptionOnNullValueAttribute(boolean throwExceptionOnNullValueAttribute) { this.throwExceptionOnNullValueAttribute = throwExceptionOnNullValueAttribute; } diff --git a/core/src/main/java/org/apache/struts2/components/Radio.java b/core/src/main/java/org/apache/struts2/components/Radio.java index 0315cb65bb..ba5eb471f7 100644 --- a/core/src/main/java/org/apache/struts2/components/Radio.java +++ b/core/src/main/java/org/apache/struts2/components/Radio.java @@ -57,7 +57,7 @@ allowDynamicAttributes = true) public class Radio extends ListUIBean { final public static String TEMPLATE = "radiomap"; - + public Radio(ValueStack stack, HttpServletRequest request, HttpServletResponse response) { super(stack, request, response); } @@ -65,8 +65,19 @@ public Radio(ValueStack stack, HttpServletRequest request, HttpServletResponse r protected String getDefaultTemplate() { return TEMPLATE; } - + public void evaluateExtraParams() { super.evaluateExtraParams(); } -} \ No newline at end of file + + /** + * Radio tag requires lazy evaluation as list of tags is dynamically generated using + * + * @return boolean true by default + */ + @Override + protected boolean lazyEvaluation() { + return true; + } + +} 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 9ec23d5c79..a9b8f50f96 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -1277,8 +1277,8 @@ public void setDynamicAttributes(Map tagDynamicAttributes) { String attrValue = entry.getValue(); if (!isValidTagAttribute(attrName)) { - if (ComponentUtils.altSyntax(getStack()) && ComponentUtils.isExpression(attrValue)) { - dynamicAttributes.put(attrName, String.valueOf(ObjectUtils.defaultIfNull(findString(attrValue), attrValue))); + if (ComponentUtils.altSyntax(getStack()) && ComponentUtils.containsExpression(attrValue) && !lazyEvaluation()) { + dynamicAttributes.put(attrName, ObjectUtils.defaultIfNull(findString(attrValue), attrValue)); } else { dynamicAttributes.put(attrName, attrValue); } @@ -1302,4 +1302,14 @@ public void copyParams(Map params) { } } + /** + * Used to avoid evaluating attributes in {@link #evaluateParams()} or {@link #evaluateExtraParams()} + * as evaluation will happen in tag's template + * + * @return boolean false if evaluation should be performed in ftl + */ + protected boolean lazyEvaluation() { + return false; + } + } diff --git a/core/src/main/resources/template/simple/checkboxlist.ftl b/core/src/main/resources/template/simple/checkboxlist.ftl index 3fa27f1894..87bef66515 100644 --- a/core/src/main/resources/template/simple/checkboxlist.ftl +++ b/core/src/main/resources/template/simple/checkboxlist.ftl @@ -30,7 +30,7 @@ <#assign itemKeyStr = stack.findString('top')> <#if parameters.listLabelKey??> - <#-- checks the valueStack for the 'valueKey.' The valueKey is then looked-up in the locale + <#-- checks the valueStack for the 'valueKey.' The valueKey is then looked-up in the locale file for it's localized value. This is then used as a label --> <#assign itemValue = struts.getText(stack.findString(parameters.listLabelKey))/> <#elseif parameters.listValue??> @@ -95,9 +95,10 @@ <#include "/${parameters.templateDir}/${parameters.expandTheme}/css.ftl" /> <#include "/${parameters.templateDir}/${parameters.expandTheme}/scripting-events.ftl" /> <#include "/${parameters.templateDir}/${parameters.expandTheme}/common-attributes.ftl" /> + <#global evaluate_dynamic_attributes = true/> <#include "/${parameters.templateDir}/${parameters.expandTheme}/dynamic-attributes.ftl" /> /> - + <#if parameters.id?has_content> for="${parameters.id}-${itemCount}"<#rt/> <#else> @@ -106,11 +107,10 @@ class="checkboxLabel">${itemValue} <#else> -   <#if parameters.disabled!false> disabled="disabled"<#rt/> - /> \ No newline at end of file + /> diff --git a/core/src/main/resources/template/simple/dynamic-attributes.ftl b/core/src/main/resources/template/simple/dynamic-attributes.ftl index 47a91139f2..7f15aa48f0 100644 --- a/core/src/main/resources/template/simple/dynamic-attributes.ftl +++ b/core/src/main/resources/template/simple/dynamic-attributes.ftl @@ -30,7 +30,11 @@ <#list aKeys?filter(acceptKey) as aKey><#rt/> <#assign keyValue = parameters.dynamicAttributes.get(aKey)/> <#if keyValue?is_string> - <#assign value = struts.translateVariables(keyValue)!keyValue/> + <#if evaluate_dynamic_attributes!false == true> + <#assign value = struts.translateVariables(keyValue)!keyValue/><#rt/> + <#else> + <#assign value = keyValue/><#rt/> + <#else> <#assign value = keyValue?string/> diff --git a/core/src/main/resources/template/simple/radiomap.ftl b/core/src/main/resources/template/simple/radiomap.ftl index 5c37a4b4b3..597300e95f 100644 --- a/core/src/main/resources/template/simple/radiomap.ftl +++ b/core/src/main/resources/template/simple/radiomap.ftl @@ -27,7 +27,7 @@ <#assign itemKeyStr = stack.findString('top')> <#if parameters.listValueKey??> - <#-- checks the valueStack for the 'valueKey.' The valueKey is then looked-up in the locale + <#-- checks the valueStack for the 'valueKey.' The valueKey is then looked-up in the locale file for it's localized value. This is then used as a label --> <#assign valueKey = stack.findString(parameters.listValueKey)!''/> <#if valueKey?has_content> @@ -94,9 +94,10 @@ <#include "/${parameters.templateDir}/${parameters.expandTheme}/css.ftl" /> <#include "/${parameters.templateDir}/${parameters.expandTheme}/scripting-events.ftl" /> <#include "/${parameters.templateDir}/${parameters.expandTheme}/common-attributes.ftl" /> +<#global evaluate_dynamic_attributes = true/> <#include "/${parameters.templateDir}/${parameters.expandTheme}/dynamic-attributes.ftl" /> /><#rt/> - \ No newline at end of file + From b0e18e9c577d1f8c6c1417a054a11c52cd624234 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 6 Jan 2022 09:03:07 +0100 Subject: [PATCH 4/7] WW-5117 Uses translateVariables instead of findString to allow join expressions --- core/src/main/java/org/apache/struts2/components/UIBean.java | 4 +++- .../java/org/apache/struts2/views/jsp/ui/TextfieldTest.java | 2 +- 2 files changed, 4 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 a9b8f50f96..e010eadd45 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -20,6 +20,7 @@ import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -1278,7 +1279,8 @@ public void setDynamicAttributes(Map tagDynamicAttributes) { if (!isValidTagAttribute(attrName)) { if (ComponentUtils.altSyntax(getStack()) && ComponentUtils.containsExpression(attrValue) && !lazyEvaluation()) { - dynamicAttributes.put(attrName, ObjectUtils.defaultIfNull(findString(attrValue), attrValue)); + String translated = TextParseUtil.translateVariables('%', attrValue, stack); + dynamicAttributes.put(attrName, ObjectUtils.defaultIfNull(translated, attrValue)); } else { dynamicAttributes.put(attrName, attrValue); } 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 cf300e806d..56d0eaa156 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 @@ -372,7 +372,7 @@ public void testSimple_recursionTest_clearTagStateSet() throws Exception { tag.setName("myname"); tag.setValue("%{foo}"); tag.setSize("10"); - tag.setDynamicAttribute(null, "anotherAttr", "%{foo}"); + tag.setDynamicAttribute(null, "anotherAttr", "another_%{foo}"); tag.doStartTag(); setComponentTagClearTagState(tag, true); // Ensure component tag state clearing is set true (to match tag). From 9c05422bbe23383cd5a39d89ab14f9e26ac0ffb8 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 13 Jan 2022 10:28:30 +0100 Subject: [PATCH 5/7] WW-5117 Adjusts expression checking --- core/src/main/java/org/apache/struts2/components/UIBean.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e010eadd45..85b3d1bb9e 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -32,6 +32,7 @@ import org.apache.struts2.components.template.TemplateEngine; import org.apache.struts2.components.template.TemplateEngineManager; import org.apache.struts2.components.template.TemplateRenderingContext; +import org.apache.struts2.dispatcher.StaticContentLoader; import org.apache.struts2.util.ComponentUtils; import org.apache.struts2.util.TextProviderHelper; import org.apache.struts2.views.annotations.StrutsTagAttribute; @@ -1278,7 +1279,7 @@ public void setDynamicAttributes(Map tagDynamicAttributes) { String attrValue = entry.getValue(); if (!isValidTagAttribute(attrName)) { - if (ComponentUtils.altSyntax(getStack()) && ComponentUtils.containsExpression(attrValue) && !lazyEvaluation()) { + if (ComponentUtils.containsExpression(attrValue) && !lazyEvaluation()) { String translated = TextParseUtil.translateVariables('%', attrValue, stack); dynamicAttributes.put(attrName, ObjectUtils.defaultIfNull(translated, attrValue)); } else { From 41e729205c9ba3e015d55b5f93f2863424d684ee Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 13 Jan 2022 10:32:53 +0100 Subject: [PATCH 6/7] WW-5117 Uses attribute translations in tests --- .../java/org/apache/struts2/views/jsp/ui/TextfieldTest.java | 2 +- .../resources/org/apache/struts2/views/jsp/ui/Textfield-5.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 56d0eaa156..b2d4fdc5d4 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 @@ -346,7 +346,7 @@ public void testSimple_recursionTest() throws Exception { tag.setName("myname"); tag.setValue("%{foo}"); tag.setSize("10"); - tag.setDynamicAttribute(null, "anotherAttr", "%{foo}"); + tag.setDynamicAttribute(null, "anotherAttr", "another_%{foo}"); tag.doStartTag(); tag.doEndTag(); 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 2818476270..6dcdedba53 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 7ed77f5f0493d5d6f4db3479a4e9639c84e096e2 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 14 Jan 2022 09:22:21 +0100 Subject: [PATCH 7/7] WW-5117 Adds a new attribute to test the new behaviour --- .../java/org/apache/struts2/views/jsp/ui/TextfieldTest.java | 6 ++++-- .../org/apache/struts2/views/jsp/ui/Textfield-5.txt | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) 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 b2d4fdc5d4..f42fdbccfa 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 @@ -346,7 +346,8 @@ public void testSimple_recursionTest() throws Exception { tag.setName("myname"); tag.setValue("%{foo}"); tag.setSize("10"); - tag.setDynamicAttribute(null, "anotherAttr", "another_%{foo}"); + tag.setDynamicAttribute(null, "anotherAttr", "%{foo}"); + tag.setDynamicAttribute(null, "secondAttr", "second_%{foo}"); tag.doStartTag(); tag.doEndTag(); @@ -372,7 +373,8 @@ public void testSimple_recursionTest_clearTagStateSet() throws Exception { tag.setName("myname"); tag.setValue("%{foo}"); tag.setSize("10"); - tag.setDynamicAttribute(null, "anotherAttr", "another_%{foo}"); + tag.setDynamicAttribute(null, "anotherAttr", "%{foo}"); + tag.setDynamicAttribute(null, "secondAttr", "second_%{foo}"); tag.doStartTag(); setComponentTagClearTagState(tag, true); // Ensure component tag state clearing is set true (to match tag). 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 6dcdedba53..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 @@ - +