Skip to content

WW-5368 Eliminate OGNL warnings for component field access in resource bundle expressions #1420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

WW-5368 Eliminate OGNL warnings for component field access in resource bundle expressions #1420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bab769
docs(research): WW-5368 add OGNL label field access warning analysis
lukaszlenart Nov 23, 2025
55930d2
fix(core): WW-5368 eliminate OGNL warnings for component field access
lukaszlenart Nov 23, 2025
b23091e
Removes additional <p> tag
lukaszlenart Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions core/src/main/java/org/apache/struts2/components/Bean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
*/
package org.apache.struts2.components;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.ObjectFactory;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.util.ValueStack;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.struts2.util.reflection.ReflectionProvider;
import org.apache.struts2.views.annotations.StrutsTag;
import org.apache.struts2.views.annotations.StrutsTagAttribute;
Expand All @@ -36,10 +36,9 @@
*
* <p>If the var attribute is set on the BeanTag, it will place the instantiated bean into the
* stack's Context.</p>
*
* <p>
* <!-- END SNIPPET: javadoc -->
*
*
* <p>
* <!-- START SNIPPET: params -->
* <ul>
* <li>var - the stack's context name (if supplied) that the created bean will be store under</li>
Expand All @@ -65,8 +64,8 @@
* &lt;/s:bean&gt;
* <!-- END SNIPPET: examples -->
* </pre>
*
*
* <p>
* <p>
* <!-- START SNIPPET: examplesdescription -->
* <p>This example instantiates a bean called SimpleCounter and sets the foo property (setFoo('BAR')). The
* SimpleCounter object is then pushed onto the Valuestack, which means that we can call its accessor methods (getFoo())
Expand Down Expand Up @@ -95,7 +94,7 @@ public class Bean extends ContextBean {
protected static final Logger LOG = LogManager.getLogger(Bean.class);

protected Object bean;
protected String name;
private String name;
Copy link
Member

@kusalk kusalk Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a public getter should already suppress the warning, this could stay as protected but I guess it doesn't really matter either way

protected ObjectFactory objectFactory;
protected ReflectionProvider reflectionProvider;

Expand Down Expand Up @@ -146,6 +145,15 @@ public void addParameter(String key, Object value) {
reflectionProvider.setProperty(key, value, bean, getStack().getContext());
}

/**
* Gets the class name of the bean to be instantiated.
*
* @return the class name
*/
public String getName() {
return name;
}

@StrutsTagAttribute(description = "The class name of the bean to be instantiated (must respect JavaBean specification)", required = true)
public void setName(String name) {
this.name = name;
Expand Down
22 changes: 11 additions & 11 deletions core/src/main/java/org/apache/struts2/components/Form.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@
*/
package org.apache.struts2.components;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils;
import org.apache.struts2.ObjectFactory;
import org.apache.struts2.config.Configuration;
import org.apache.struts2.config.RuntimeConfiguration;
import org.apache.struts2.config.entities.ActionConfig;
import org.apache.struts2.config.entities.InterceptorMapping;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.interceptor.MethodFilterInterceptorUtil;
import org.apache.struts2.util.ValueStack;
Expand All @@ -33,10 +37,6 @@
import org.apache.struts2.validator.Validator;
import org.apache.struts2.validator.ValidatorContext;
import org.apache.struts2.validator.validators.VisitorFieldValidator;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.views.annotations.StrutsTag;
import org.apache.struts2.views.annotations.StrutsTagAttribute;

Expand Down Expand Up @@ -90,10 +90,10 @@
* </pre>
*/
@StrutsTag(
name = "form",
tldTagClass = "org.apache.struts2.views.jsp.ui.FormTag",
description = "Renders an input form",
allowDynamicAttributes = true)
name = "form",
tldTagClass = "org.apache.struts2.views.jsp.ui.FormTag",
description = "Renders an input form",
allowDynamicAttributes = true)
public class Form extends ClosingUIBean {
public static final String OPEN_TEMPLATE = "form";
public static final String TEMPLATE = "form-close";
Expand Down Expand Up @@ -170,7 +170,7 @@ protected void evaluateExtraParams() {
addParameter("validate", findValue(validate, Boolean.class));
}

if (name == null) {
if (getName() == null) {
//make the name the same as the id
String id = (String) getAttributes().get("id");
if (StringUtils.isNotEmpty(id)) {
Expand Down Expand Up @@ -223,7 +223,7 @@ protected void evaluateExtraParams() {
*/
@Override
protected void populateComponentHtmlId(Form form) {
if (id != null) {
if (getId() != null) {
super.populateComponentHtmlId(null);
}

Expand Down Expand Up @@ -508,7 +508,7 @@ public void setNamespace(String namespace) {
}

@StrutsTagAttribute(description = "Whether client side/remote validation should be performed. Only" +
" useful with theme xhtml/ajax", type = "Boolean", defaultValue = "false")
" useful with theme xhtml/ajax", type = "Boolean", defaultValue = "false")
public void setValidate(String validate) {
this.validate = validate;
}
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/org/apache/struts2/components/FormButton.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
*/
package org.apache.struts2.components;

import org.apache.struts2.util.ValueStack;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.struts2.dispatcher.mapper.ActionMapping;
import org.apache.struts2.util.ValueStack;
import org.apache.struts2.views.annotations.StrutsTagAttribute;

/**
Expand Down Expand Up @@ -54,7 +54,7 @@ public void evaluateExtraParams() {

addParameter("type", submitType);

if (!BUTTON_TYPE_INPUT.equals(submitType) && (label == null)) {
if (!BUTTON_TYPE_INPUT.equals(submitType) && (getLabel() == null)) {
addParameter("label", getAttributes().get("nameValue"));
}

Expand Down Expand Up @@ -94,15 +94,15 @@ public void evaluateExtraParams() {
*/
protected void populateComponentHtmlId(Form form) {
String tmpId = "";
if (id != null) {
if (getId() != null) {
// this check is needed for backwards compatibility with 2.1.x
tmpId = findString(id);
tmpId = findString(getId());
} else {
if (form != null && form.getAttributes().get("id") != null) {
tmpId = tmpId + form.getAttributes().get("id").toString() + "_";
}
if (name != null) {
tmpId = tmpId + escape(findString(name));
if (getName() != null) {
tmpId = tmpId + escape(findString(getName()));
} else if (action != null || method != null) {
if (action != null) {
tmpId = tmpId + escape(findString(action));
Expand Down Expand Up @@ -141,7 +141,7 @@ public void setMethod(String method) {


@StrutsTagAttribute(description = "The type of submit to use. Valid values are <i>input</i>, " +
"<i>button</i> and <i>image</i>.", defaultValue = "input")
"<i>button</i> and <i>image</i>.", defaultValue = "input")
public void setType(String type) {
this.type = type;
}
Expand Down
52 changes: 30 additions & 22 deletions core/src/main/java/org/apache/struts2/components/I18n.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,38 +18,37 @@
*/
package org.apache.struts2.components;

import java.io.Writer;
import java.util.ResourceBundle;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsException;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.locale.LocaleProvider;
import org.apache.struts2.locale.LocaleProviderFactory;
import org.apache.struts2.text.LocalizedTextProvider;
import org.apache.struts2.text.TextProvider;
import org.apache.struts2.text.TextProviderFactory;
import org.apache.struts2.util.ValueStack;
import org.apache.struts2.views.annotations.StrutsTag;
import org.apache.struts2.views.annotations.StrutsTagAttribute;
import org.apache.struts2.StrutsException;

import org.apache.struts2.locale.LocaleProvider;
import org.apache.struts2.text.TextProvider;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.util.ValueStack;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.io.Writer;
import java.util.ResourceBundle;

/**
* <!-- START SNIPPET: javadoc -->
*
* <p>
* Gets a resource bundle and place it on the value stack. This allows
* the text tag to access messages from any bundle, and not just the bundle
* associated with the current action.
*
* <p>
* <!-- END SNIPPET: javadoc -->
*
* <p>
* <!-- START SNIPPET: params-->
*
* <ul>
* <li>name* - the resource bundle's name (eg foo/bar/customBundle)</li>
* </ul>
*
* <p>
* <!-- END SNIPPET: params -->
*
* <p>
Expand Down Expand Up @@ -78,14 +77,14 @@
* </pre>
*
*/
@StrutsTag(name="i18n", tldTagClass="org.apache.struts2.views.jsp.I18nTag", description="Get a resource bundle" +
" and place it on the value stack")
@StrutsTag(name = "i18n", tldTagClass = "org.apache.struts2.views.jsp.I18nTag", description = "Get a resource bundle" +
" and place it on the value stack")
public class I18n extends Component {

private static final Logger LOG = LogManager.getLogger(I18n.class);

protected boolean pushed;
protected String name;
private String name;

private LocalizedTextProvider localizedTextProvider;
private TextProvider textProvider;
Expand Down Expand Up @@ -145,17 +144,26 @@ public boolean end(Writer writer, String body) throws StrutsException {
if (pushed) {
Object o = getStack().pop();
if ((o == null) || (!o.equals(textProvider))) {
LOG.error("A closing i18n tag attempted to pop its own TextProvider from the top of the ValueStack but popped an unexpected object ("+(o != null ? o.getClass() : "null")+"). " +
"Refactor the page within the i18n tags to ensure no objects are pushed onto the ValueStack without popping them prior to the closing tag. " +
"If you see this message it's likely that the i18n's TextProvider is still on the stack and will continue to provide message resources after the closing tag.");
throw new StrutsException("A closing i18n tag attempted to pop its TextProvider from the top of the ValueStack but popped an unexpected object ("+(o != null ? o.getClass() : "null")+")");
LOG.error("A closing i18n tag attempted to pop its own TextProvider from the top of the ValueStack but popped an unexpected object (" + (o != null ? o.getClass() : "null") + "). " +
"Refactor the page within the i18n tags to ensure no objects are pushed onto the ValueStack without popping them prior to the closing tag. " +
"If you see this message it's likely that the i18n's TextProvider is still on the stack and will continue to provide message resources after the closing tag.");
throw new StrutsException("A closing i18n tag attempted to pop its TextProvider from the top of the ValueStack but popped an unexpected object (" + (o != null ? o.getClass() : "null") + ")");
}
}

return super.end(writer, body);
}

@StrutsTagAttribute(description="Name of resource bundle to use (eg foo/bar/customBundle)", required=true, defaultValue="String")
/**
* Gets the name of the resource bundle to use.
*
* @return the resource bundle name
*/
public String getName() {
return name;
}

@StrutsTagAttribute(description = "Name of resource bundle to use (eg foo/bar/customBundle)", required = true, defaultValue = "String")
public void setName(String name) {
this.name = name;
}
Expand Down
22 changes: 11 additions & 11 deletions core/src/main/java/org/apache/struts2/components/Label.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
*/
package org.apache.struts2.components;

import org.apache.struts2.util.ValueStack;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.struts2.util.TextProviderHelper;
import org.apache.struts2.util.ValueStack;
import org.apache.struts2.views.annotations.StrutsTag;
import org.apache.struts2.views.annotations.StrutsTagAttribute;

Expand All @@ -32,7 +32,7 @@
* <!-- END SNIPPET: javadoc -->
*
* <p><b>Examples</b></p>
*
* <p>
* <!-- START SNIPPET: exdescription -->
* <p>In this example, a label is rendered. The label is retrieved from a ResourceBundle via the key attribute
* giving you an output of 'User Name: Ford.Prefect'. Assuming that i18n message userName corresponds
Expand All @@ -51,10 +51,10 @@
*
*/
@StrutsTag(
name="label",
tldTagClass="org.apache.struts2.views.jsp.ui.LabelTag",
description="Render a label that displays read-only information",
allowDynamicAttributes=true)
name = "label",
tldTagClass = "org.apache.struts2.views.jsp.ui.LabelTag",
description = "Render a label that displays read-only information",
allowDynamicAttributes = true)
public class Label extends UIBean {
final public static String TEMPLATE = "label";

Expand All @@ -76,22 +76,22 @@ protected void evaluateExtraParams() {
}

// try value, then key, then name (this overrides the default behavior in the superclass)
if (value != null) {
addParameter("nameValue", findString(value));
if (getValue() != null) {
addParameter("nameValue", findString(getValue()));
} else if (key != 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);
addParameter("nameValue", providedLabel);
}
} else if (name != null) {
String expr = completeExpression(name);
} else if (getName() != null) {
String expr = completeExpression(getName());
addParameter("nameValue", findString(expr));
}
}

@StrutsTagAttribute(description=" HTML for attribute")
@StrutsTagAttribute(description = " HTML for attribute")
public void setFor(String forAttr) {
this.forAttr = forAttr;
}
Expand Down
Loading