This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch fix/WW-5589-uibean-field-encapsulation in repository https://gitbox.apache.org/repos/asf/struts.git
commit b6ce38a11b9f9387b425545fce7af8972b49b37a Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Nov 23 17:09:11 2025 +0100 feat(core): WW-5589 convert all UIBean protected fields to private with getters - Convert all remaining protected fields in UIBean to private: - Template-related: templateSuffix, template, templateDir, theme - Style/CSS: cssClass, cssStyle, cssErrorClass, cssErrorStyle - Form attributes: key, disabled, tabindex, title, accesskey - Label attributes: labelPosition, labelSeparator, requiredPosition, errorPosition, requiredLabel - Event handlers: onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onfocus, onblur, onkeypress, onkeydown, onkeyup, onselect, onchange - Tooltip attributes: tooltip, tooltipConfig, javascriptTooltip, tooltipDelay, tooltipCssClass, tooltipIconPath - Dynamic attributes map - Add public getter methods for all converted fields with JavaDoc - Update UIBean subclasses to use getters instead of direct field access: - Anchor.java: use getTemplate() - DoubleSelect.java: use getOnchange() - Link.java: use getDisabled(), getTitle() - Submit.java: use getKey(), getTemplate() - Label.java: use getKey() - Reset.java: use getKey() - Add comprehensive test coverage in UIBeanTest: - testNoOgnlWarningsForAdditionalFields() verifies OGNL can access all newly converted fields via getters - Tests key, title, disabled, cssClass, template, theme, tabindex, and event handler fields This completes WW-5589 by ensuring all UIBean fields follow JavaBean conventions (private fields with public getters), preventing OGNL from attempting direct field access which could trigger false-positive security warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --- .../java/org/apache/struts2/components/Anchor.java | 6 +- .../apache/struts2/components/DoubleSelect.java | 8 +- .../java/org/apache/struts2/components/Label.java | 4 +- .../java/org/apache/struts2/components/Link.java | 38 +- .../java/org/apache/struts2/components/Reset.java | 2 +- .../java/org/apache/struts2/components/Submit.java | 8 +- .../java/org/apache/struts2/components/UIBean.java | 414 +++++++++++++++++++-- .../org/apache/struts2/components/UIBeanTest.java | 75 ++++ 8 files changed, 483 insertions(+), 72 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/Anchor.java b/core/src/main/java/org/apache/struts2/components/Anchor.java index 6bda43f20..c83b90425 100644 --- a/core/src/main/java/org/apache/struts2/components/Anchor.java +++ b/core/src/main/java/org/apache/struts2/components/Anchor.java @@ -117,7 +117,7 @@ public class Anchor extends ClosingUIBean { @Inject(StrutsConstants.STRUTS_URL_INCLUDEPARAMS) public void setUrlIncludeParams(String urlIncludeParams) { - urlProvider.setUrlIncludeParams(urlIncludeParams); + urlProvider.setUrlIncludeParams(urlIncludeParams); } @Inject @@ -126,7 +126,7 @@ public class Anchor extends ClosingUIBean { this.urlRenderer = urlRenderer; } - @Inject(required=false) + @Inject(required = false) public void setExtraParameterProvider(ExtraParameterProvider provider) { urlProvider.setExtraParameterProvider(provider); } @@ -147,7 +147,7 @@ public class Anchor extends ClosingUIBean { evaluateParams(); try { addParameter("body", body); - mergeTemplate(writer, buildTemplateName(template, getDefaultTemplate())); + mergeTemplate(writer, buildTemplateName(getTemplate(), getDefaultTemplate())); } catch (Exception e) { LOG.error("error when rendering", e); } finally { diff --git a/core/src/main/java/org/apache/struts2/components/DoubleSelect.java b/core/src/main/java/org/apache/struts2/components/DoubleSelect.java index 2cb5ecf96..af865cc87 100644 --- a/core/src/main/java/org/apache/struts2/components/DoubleSelect.java +++ b/core/src/main/java/org/apache/struts2/components/DoubleSelect.java @@ -40,8 +40,8 @@ import jakarta.servlet.http.HttpServletResponse; * </pre> * */ -@StrutsTag(name="doubleselect", tldTagClass="org.apache.struts2.views.jsp.ui.DoubleSelectTag", description="Renders two HTML select elements with second one changing displayed values depending on " + - "selected entry of first one.") +@StrutsTag(name = "doubleselect", tldTagClass = "org.apache.struts2.views.jsp.ui.DoubleSelectTag", description = "Renders two HTML select elements with second one changing displayed values depending on " + + "selected entry of first one.") public class DoubleSelect extends DoubleListUIBean { final public static String TEMPLATE = "doubleselect"; @@ -58,8 +58,8 @@ public class DoubleSelect extends DoubleListUIBean { super.evaluateExtraParams(); StringBuilder onchangeParam = new StringBuilder(); onchangeParam.append(getAttributes().get("id")).append("Redirect(this.selectedIndex)"); - if(StringUtils.isNotEmpty(this.onchange)) { - onchangeParam.append(";").append(this.onchange); + if (StringUtils.isNotEmpty(getOnchange())) { + onchangeParam.append(";").append(getOnchange()); } // force the onchange parameter addParameter("onchange", onchangeParam.toString()); diff --git a/core/src/main/java/org/apache/struts2/components/Label.java b/core/src/main/java/org/apache/struts2/components/Label.java index 5f813d886..4ff785de7 100644 --- a/core/src/main/java/org/apache/struts2/components/Label.java +++ b/core/src/main/java/org/apache/struts2/components/Label.java @@ -78,11 +78,11 @@ public class Label extends UIBean { // try value, then key, then name (this overrides the default behavior in the superclass) if (getValue() != null) { addParameter("nameValue", findString(getValue())); - } else if (key != null) { + } else if (getKey() != null) { Object nameValue = attributes.get("nameValue"); if (nameValue == null || nameValue.toString().isEmpty()) { // get the label from a TextProvider (default value is the key) - String providedLabel = TextProviderHelper.getText(key, key, stack); + String providedLabel = TextProviderHelper.getText(getKey(), getKey(), stack); addParameter("nameValue", providedLabel); } } else if (getName() != null) { diff --git a/core/src/main/java/org/apache/struts2/components/Link.java b/core/src/main/java/org/apache/struts2/components/Link.java index 5c29765f7..348777360 100644 --- a/core/src/main/java/org/apache/struts2/components/Link.java +++ b/core/src/main/java/org/apache/struts2/components/Link.java @@ -47,13 +47,13 @@ import jakarta.servlet.http.HttpServletResponse; * </pre> * */ -@StrutsTag(name="link", - tldTagClass="org.apache.struts2.views.jsp.ui.LinkTag", - description="Link tag automatically adds nonces to link elements - should be used in combination with Struts' CSP Interceptor.", - allowDynamicAttributes=true) -public class Link extends UIBean{ +@StrutsTag(name = "link", + tldTagClass = "org.apache.struts2.views.jsp.ui.LinkTag", + description = "Link tag automatically adds nonces to link elements - should be used in combination with Struts' CSP Interceptor.", + allowDynamicAttributes = true) +public class Link extends UIBean { - private static final String TEMPLATE="link"; + private static final String TEMPLATE = "link"; protected String href; protected String hreflang; @@ -69,47 +69,47 @@ public class Link extends UIBean{ super(stack, request, response); } - @StrutsTagAttribute(description="HTML link href attribute") + @StrutsTagAttribute(description = "HTML link href attribute") public void setHref(String href) { this.href = href; } - @StrutsTagAttribute(description="HTML link hreflang attribute") + @StrutsTagAttribute(description = "HTML link hreflang attribute") public void setHreflang(String hreflang) { this.hreflang = hreflang; } - @StrutsTagAttribute(description="HTML link rel attribute") + @StrutsTagAttribute(description = "HTML link rel attribute") public void setRel(String rel) { this.rel = rel; } - @StrutsTagAttribute(description="HTML link sizes attribute") + @StrutsTagAttribute(description = "HTML link sizes attribute") public void setSizes(String sizes) { this.sizes = sizes; } - @StrutsTagAttribute(description="HTML link crossorigin attribute") + @StrutsTagAttribute(description = "HTML link crossorigin attribute") public void setCrossorigin(String crossorigin) { this.crossorigin = crossorigin; } - @StrutsTagAttribute(description="HTML link type attribute") + @StrutsTagAttribute(description = "HTML link type attribute") public void setType(String type) { this.type = type; } - @StrutsTagAttribute(description="HTML link as attribute") + @StrutsTagAttribute(description = "HTML link as attribute") public void setAs(String as) { this.as = as; } - @StrutsTagAttribute(description="HTML link media attribute") + @StrutsTagAttribute(description = "HTML link media attribute") public void setMedia(String media) { this.media = media; } - @StrutsTagAttribute(description="HTML link referrerpolicy attribute") + @StrutsTagAttribute(description = "HTML link referrerpolicy attribute") public void setReferrerpolicy(String referrerpolicy) { this.referrerpolicy = referrerpolicy; } @@ -159,12 +159,12 @@ public class Link extends UIBean{ addParameter("as", findString(as)); } - if (disabled != null) { - addParameter("disabled", findString(disabled)); + if (getDisabled() != null) { + addParameter("disabled", findString(getDisabled())); } - if (title != null) { - addParameter("title", findString(title)); + if (getTitle() != null) { + addParameter("title", findString(getTitle())); } } } diff --git a/core/src/main/java/org/apache/struts2/components/Reset.java b/core/src/main/java/org/apache/struts2/components/Reset.java index dea9c3330..be540a23b 100644 --- a/core/src/main/java/org/apache/struts2/components/Reset.java +++ b/core/src/main/java/org/apache/struts2/components/Reset.java @@ -82,7 +82,7 @@ public class Reset extends FormButton { public void evaluateParams() { if (getValue() == null) { - setValue(key != null ? "%{getText('" + key + "')}" : "Reset"); + setValue(getKey() != null ? "%{getText('" + getKey() + "')}" : "Reset"); } super.evaluateParams(); } diff --git a/core/src/main/java/org/apache/struts2/components/Submit.java b/core/src/main/java/org/apache/struts2/components/Submit.java index 14f607df4..03b6140d5 100644 --- a/core/src/main/java/org/apache/struts2/components/Submit.java +++ b/core/src/main/java/org/apache/struts2/components/Submit.java @@ -66,12 +66,12 @@ public class Submit extends FormButton { } public void evaluateParams() { - if ((key == null) && (getValue() == null)) { + if ((getKey() == null) && (getValue() == null)) { setValue("Submit"); } - if ((key != null) && (getValue() == null)) { - setValue("%{getText('" + key + "')}"); + if ((getKey() != null) && (getValue() == null)) { + setValue("%{getText('" + getKey() + "')}"); } super.evaluateParams(); @@ -119,7 +119,7 @@ public class Submit extends FormButton { try { addParameter("body", body); - mergeTemplate(writer, buildTemplateName(template, getDefaultTemplate())); + mergeTemplate(writer, buildTemplateName(getTemplate(), getDefaultTemplate())); } catch (Exception e) { LOG.error("error when rendering", e); } finally { 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 985103961..fd9f998cf 100644 --- a/core/src/main/java/org/apache/struts2/components/UIBean.java +++ b/core/src/main/java/org/apache/struts2/components/UIBean.java @@ -462,64 +462,64 @@ public abstract class UIBean extends Component { } // The templateSuffic to use, overrides the default one if not null. - protected String templateSuffix; + private String templateSuffix; // The template to use, overrides the default one. - protected String template; + private String template; // templateDir and theme attributes - protected String templateDir; - protected String theme; + private String templateDir; + private String theme; // shortcut, sets label, name, and value - protected String key; + private String key; private String id; - protected String cssClass; - protected String cssStyle; - protected String cssErrorClass; - protected String cssErrorStyle; - protected String disabled; + private String cssClass; + private String cssStyle; + private String cssErrorClass; + private String cssErrorStyle; + private String disabled; private String label; - protected String labelPosition; - protected String labelSeparator; - protected String requiredPosition; - protected String errorPosition; + private String labelPosition; + private String labelSeparator; + private String requiredPosition; + private String errorPosition; private String name; - protected String requiredLabel; - protected String tabindex; + private String requiredLabel; + private String tabindex; private String value; - protected String title; + private String title; // HTML scripting events attributes - protected String onclick; - protected String ondblclick; - protected String onmousedown; - protected String onmouseup; - protected String onmouseover; - protected String onmousemove; - protected String onmouseout; - protected String onfocus; - protected String onblur; - protected String onkeypress; - protected String onkeydown; - protected String onkeyup; - protected String onselect; - protected String onchange; + private String onclick; + private String ondblclick; + private String onmousedown; + private String onmouseup; + private String onmouseover; + private String onmousemove; + private String onmouseout; + private String onfocus; + private String onblur; + private String onkeypress; + private String onkeydown; + private String onkeyup; + private String onselect; + private String onchange; // common html attributes - protected String accesskey; + private String accesskey; // javascript tooltip attribute - protected String tooltip; - protected String tooltipConfig; - protected String javascriptTooltip; - protected String tooltipDelay; - protected String tooltipCssClass; - protected String tooltipIconPath; + private String tooltip; + private String tooltipConfig; + private String javascriptTooltip; + private String tooltipDelay; + private String tooltipCssClass; + private String tooltipIconPath; // dynamic attributes - protected Map<String, Object> dynamicAttributes = new HashMap<>(); + private Map<String, Object> dynamicAttributes = new HashMap<>(); protected String defaultTemplateDir; protected String defaultUITheme; @@ -1111,6 +1111,342 @@ public abstract class UIBean extends Component { return value; } + /** + * Gets the template suffix. + * + * @return the template suffix + */ + public String getTemplateSuffix() { + return templateSuffix; + } + + /** + * Gets the key for this component. + * + * @return the key + */ + public String getKey() { + return key; + } + + /** + * Gets the CSS class. + * + * @return the CSS class + */ + public String getCssClass() { + return cssClass; + } + + /** + * Gets the CSS style. + * + * @return the CSS style + */ + public String getCssStyle() { + return cssStyle; + } + + /** + * Gets the CSS error class. + * + * @return the CSS error class + */ + public String getCssErrorClass() { + return cssErrorClass; + } + + /** + * Gets the CSS error style. + * + * @return the CSS error style + */ + public String getCssErrorStyle() { + return cssErrorStyle; + } + + /** + * Gets the disabled attribute. + * + * @return the disabled attribute + */ + public String getDisabled() { + return disabled; + } + + /** + * Gets the label position. + * + * @return the label position + */ + public String getLabelPosition() { + return labelPosition; + } + + /** + * Gets the label separator. + * + * @return the label separator + */ + public String getLabelSeparator() { + return labelSeparator; + } + + /** + * Gets the required position. + * + * @return the required position + */ + public String getRequiredPosition() { + return requiredPosition; + } + + /** + * Gets the error position. + * + * @return the error position + */ + public String getErrorPosition() { + return errorPosition; + } + + /** + * Gets the required label. + * + * @return the required label + */ + public String getRequiredLabel() { + return requiredLabel; + } + + /** + * Gets the tabindex. + * + * @return the tabindex + */ + public String getTabindex() { + return tabindex; + } + + /** + * Gets the title. + * + * @return the title + */ + public String getTitle() { + return title; + } + + /** + * Gets the onclick event handler. + * + * @return the onclick handler + */ + public String getOnclick() { + return onclick; + } + + /** + * Gets the ondblclick event handler. + * + * @return the ondblclick handler + */ + public String getOndblclick() { + return ondblclick; + } + + /** + * Gets the onmousedown event handler. + * + * @return the onmousedown handler + */ + public String getOnmousedown() { + return onmousedown; + } + + /** + * Gets the onmouseup event handler. + * + * @return the onmouseup handler + */ + public String getOnmouseup() { + return onmouseup; + } + + /** + * Gets the onmouseover event handler. + * + * @return the onmouseover handler + */ + public String getOnmouseover() { + return onmouseover; + } + + /** + * Gets the onmousemove event handler. + * + * @return the onmousemove handler + */ + public String getOnmousemove() { + return onmousemove; + } + + /** + * Gets the onmouseout event handler. + * + * @return the onmouseout handler + */ + public String getOnmouseout() { + return onmouseout; + } + + /** + * Gets the onfocus event handler. + * + * @return the onfocus handler + */ + public String getOnfocus() { + return onfocus; + } + + /** + * Gets the onblur event handler. + * + * @return the onblur handler + */ + public String getOnblur() { + return onblur; + } + + /** + * Gets the onkeypress event handler. + * + * @return the onkeypress handler + */ + public String getOnkeypress() { + return onkeypress; + } + + /** + * Gets the onkeydown event handler. + * + * @return the onkeydown handler + */ + public String getOnkeydown() { + return onkeydown; + } + + /** + * Gets the onkeyup event handler. + * + * @return the onkeyup handler + */ + public String getOnkeyup() { + return onkeyup; + } + + /** + * Gets the onselect event handler. + * + * @return the onselect handler + */ + public String getOnselect() { + return onselect; + } + + /** + * Gets the onchange event handler. + * + * @return the onchange handler + */ + public String getOnchange() { + return onchange; + } + + /** + * Gets the accesskey. + * + * @return the accesskey + */ + public String getAccesskey() { + return accesskey; + } + + /** + * Gets the tooltip. + * + * @return the tooltip + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getTooltip() { + return tooltip; + } + + /** + * Gets the tooltip config. + * + * @return the tooltip config + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getTooltipConfig() { + return tooltipConfig; + } + + /** + * Gets the javascript tooltip setting. + * + * @return the javascript tooltip setting + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getJavascriptTooltip() { + return javascriptTooltip; + } + + /** + * Gets the tooltip delay. + * + * @return the tooltip delay + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getTooltipDelay() { + return tooltipDelay; + } + + /** + * Gets the tooltip CSS class. + * + * @return the tooltip CSS class + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getTooltipCssClass() { + return tooltipCssClass; + } + + /** + * Gets the tooltip icon path. + * + * @return the tooltip icon path + * @deprecated since 7.0.1 + */ + @Deprecated(since = "7.0.1", forRemoval = true) + public String getTooltipIconPath() { + return tooltipIconPath; + } + + /** + * Gets the dynamic attributes map. + * + * @return the dynamic attributes map + */ + public Map<String, Object> getDynamicAttributes() { + return dynamicAttributes; + } + @StrutsTagAttribute(description = "The template directory.") public void setTemplateDir(String templateDir) { this.templateDir = templateDir; 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 811040478..17291aa4f 100644 --- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java +++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java @@ -544,4 +544,79 @@ public class UIBeanTest extends StrutsInternalTestCase { } } + /** + * Test that additional UIBean fields converted from protected to private (WW-5589) + * are accessible via public getters and don't cause OGNL security warnings. + * <p> + * This extends the fix for WW-5368 to cover all protected fields in UIBean that could + * potentially conflict with resource bundle keys or OGNL expressions. Fields like + * "key", "title", "disabled", "template", "theme", etc. are now private with public + * getters to prevent OGNL from attempting direct field access. + */ + public void testNoOgnlWarningsForAdditionalFields() { + ValueStack stack = ActionContext.getContext().getValueStack(); + MockHttpServletRequest req = new MockHttpServletRequest(); + MockHttpServletResponse res = new MockHttpServletResponse(); + ActionContext.getContext().withServletRequest(req); + + // Create a UIBean component with many fields set + TextField txtFld = new TextField(stack, req, res); + txtFld.setKey("testKey"); + txtFld.setTitle("Test Title"); + txtFld.setDisabled("false"); + txtFld.setCssClass("test-class"); + txtFld.setCssStyle("color: red"); + txtFld.setTemplate("custom-template"); + txtFld.setTheme("xhtml"); + txtFld.setTabindex("1"); + txtFld.setOnclick("alert('clicked')"); + txtFld.setOndblclick("alert('double clicked')"); + txtFld.setOnfocus("alert('focused')"); + txtFld.setOnblur("alert('blurred')"); + txtFld.setOnchange("alert('changed')"); + + container.inject(txtFld); + + // Push the component onto the stack + stack.push(txtFld); + + try { + // Test that OGNL can access these fields via getters + Object keyResult = stack.findValue("key"); + Object titleResult = stack.findValue("title"); + Object disabledResult = stack.findValue("disabled"); + Object cssClassResult = stack.findValue("cssClass"); + Object templateResult = stack.findValue("template"); + Object themeResult = stack.findValue("theme"); + Object tabindexResult = stack.findValue("tabindex"); + Object onclickResult = stack.findValue("onclick"); + + // Verify values are accessible + assertEquals("testKey", keyResult); + assertEquals("Test Title", titleResult); + assertEquals("false", disabledResult); + assertEquals("test-class", cssClassResult); + assertEquals("custom-template", templateResult); + assertEquals("xhtml", themeResult); + assertEquals("1", tabindexResult); + assertEquals("alert('clicked')", onclickResult); + + // Verify public getters exist and return correct values + assertEquals("testKey", txtFld.getKey()); + assertEquals("Test Title", txtFld.getTitle()); + assertEquals("false", txtFld.getDisabled()); + assertEquals("test-class", txtFld.getCssClass()); + assertEquals("custom-template", txtFld.getTemplate()); + assertEquals("xhtml", txtFld.getTheme()); + assertEquals("1", txtFld.getTabindex()); + assertEquals("alert('clicked')", txtFld.getOnclick()); + assertEquals("alert('double clicked')", txtFld.getOndblclick()); + assertEquals("alert('focused')", txtFld.getOnfocus()); + assertEquals("alert('blurred')", txtFld.getOnblur()); + assertEquals("alert('changed')", txtFld.getOnchange()); + } finally { + stack.pop(); + } + } + }
