From ee7fdbd5bd9fab13964c9bef564ece7fc31acf5c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 23 Dec 2025 11:15:21 +0100 Subject: [PATCH] refactor(ognl): use StrutsContext instead of OgnlContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add StrutsContext.create() factory method with default configuration - Update OgnlValueStack to use StrutsContext.create() - Update OgnlUtil to use StrutsContext throughout - Rename ensureOgnlContext() to ensureStrutsContext() - Update XWorkTypeConverterWrapper to use StrutsContext - Update DefaultTypeConverter to check for StrutsContext first - Update OgnlReflectionContextFactory to return StrutsContext This provides a Struts-specific context abstraction layer while maintaining compatibility with OGNL 3.4.8+ API requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../conversion/impl/DefaultTypeConverter.java | 7 +- .../ognl/OgnlReflectionContextFactory.java | 6 +- .../org/apache/struts2/ognl/OgnlUtil.java | 84 +++++++++++-------- .../apache/struts2/ognl/OgnlValueStack.java | 12 +-- .../apache/struts2/ognl/StrutsContext.java | 22 ++++- .../ognl/XWorkTypeConverterWrapper.java | 14 ++-- 6 files changed, 87 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java b/core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java index 66ff2f479..bc80ea73b 100644 --- a/core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java +++ b/core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java @@ -23,6 +23,7 @@ import org.apache.struts2.locale.LocaleProviderFactory; import org.apache.struts2.conversion.TypeConverter; import org.apache.struts2.inject.Container; import org.apache.struts2.inject.Inject; +import org.apache.struts2.ognl.StrutsContext; import org.apache.struts2.ognl.XWorkTypeConverterWrapper; import ognl.OgnlContext; @@ -78,8 +79,10 @@ public abstract class DefaultTypeConverter implements TypeConverter { public TypeConverter getTypeConverter( Map context ) { ognl.TypeConverter converter = null; - if (context instanceof OgnlContext) { - converter = ((OgnlContext) context).getTypeConverter(); + if (context instanceof StrutsContext strutsContext) { + converter = strutsContext.getTypeConverter(); + } else if (context instanceof OgnlContext ognlContext) { + converter = ognlContext.getTypeConverter(); } if (converter != null) { diff --git a/core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java b/core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java index 62dab60a3..22fbf0c9f 100644 --- a/core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java +++ b/core/src/main/java/org/apache/struts2/ognl/OgnlReflectionContextFactory.java @@ -18,9 +18,7 @@ */ package org.apache.struts2.ognl; -import ognl.OgnlContext; import org.apache.struts2.util.reflection.ReflectionContextFactory; -import ognl.Ognl; /** * @deprecated since 6.8.0, to be removed, see {@link ReflectionContextFactory} @@ -29,8 +27,8 @@ import ognl.Ognl; public class OgnlReflectionContextFactory implements ReflectionContextFactory { @Override - public OgnlContext createDefaultContext(Object root) { - return Ognl.createDefaultContext(root); + public StrutsContext createDefaultContext(Object root) { + return StrutsContext.create(root, null, null, null); } } diff --git a/core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java b/core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java index 8169af150..34ddc277f 100644 --- a/core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java +++ b/core/src/main/java/org/apache/struts2/ognl/OgnlUtil.java @@ -204,23 +204,26 @@ public class OgnlUtil { } /** - * Ensures that the given context is an OgnlContext. If it's already an OgnlContext, returns it as-is. - * If it's a plain Map (like HashMap), wraps it in an OgnlContext to ensure compatibility with OGNL 3.4.8+. + * Ensures that the given context is a StrutsContext. If it's already a StrutsContext, returns it as-is. + * If it's an OgnlContext, wraps it. If it's a plain Map (like HashMap), creates a new StrutsContext. * - * @param context the context map that may or may not be an OgnlContext - * @return an OgnlContext instance + * @param context the context map that may or may not be a StrutsContext + * @return a StrutsContext instance * @since 7.2.0 */ - private OgnlContext ensureOgnlContext(Map context) { + private StrutsContext ensureStrutsContext(Map context) { + if (context instanceof StrutsContext strutsContext) { + return strutsContext; + } if (context instanceof OgnlContext ognlContext) { - return ognlContext; + return StrutsContext.wrap(ognlContext); } - // Create a new OgnlContext and copy the Map contents - OgnlContext ognlContext = createDefaultContext(null); + // Create a new StrutsContext and copy the Map contents + StrutsContext strutsContext = createDefaultContext(null); if (context != null) { - ognlContext.putAll(context); + strutsContext.putAll(context); } - return ognlContext; + return strutsContext; } /** @@ -249,16 +252,16 @@ public class OgnlUtil { return; } - OgnlContext ognlContext = ensureOgnlContext(context); - Object oldRoot = Ognl.getRoot(ognlContext); - Ognl.setRoot(ognlContext, o); + StrutsContext strutsContext = ensureStrutsContext(context); + Object oldRoot = Ognl.getRoot(strutsContext); + Ognl.setRoot(strutsContext, o); for (Map.Entry entry : props.entrySet()) { String expression = entry.getKey(); internalSetProperty(expression, entry.getValue(), o, context, throwPropertyExceptions); } - Ognl.setRoot(ognlContext, oldRoot); + Ognl.setRoot(strutsContext, oldRoot); } /** @@ -310,13 +313,13 @@ public class OgnlUtil { */ public void setProperty(String name, Object value, Object o, Map context, boolean throwPropertyExceptions) { - OgnlContext ognlContext = ensureOgnlContext(context); - Object oldRoot = Ognl.getRoot(ognlContext); - Ognl.setRoot(ognlContext, o); + StrutsContext strutsContext = ensureStrutsContext(context); + Object oldRoot = Ognl.getRoot(strutsContext); + Ognl.setRoot(strutsContext, o); internalSetProperty(name, value, o, context, throwPropertyExceptions); - Ognl.setRoot(ognlContext, oldRoot); + Ognl.setRoot(strutsContext, oldRoot); } /** @@ -336,10 +339,11 @@ public class OgnlUtil { } if (root instanceof CompoundRoot compoundRoot) { + StrutsContext strutsContext = ensureStrutsContext(context); try { for (Object target : compoundRoot) { - if (OgnlRuntime.hasSetProperty((OgnlContext) context, target, property) - || OgnlRuntime.hasGetProperty((OgnlContext) context, target, property) + if (OgnlRuntime.hasSetProperty(strutsContext, target, property) + || OgnlRuntime.hasGetProperty(strutsContext, target, property) || OgnlRuntime.getIndexedPropertyType(target.getClass(), property) != OgnlRuntime.INDEXED_PROPERTY_NONE ) { return target; @@ -370,36 +374,42 @@ public class OgnlUtil { private boolean isEvalExpression(Object tree, Map context) throws OgnlException { if (tree instanceof SimpleNode node) { - OgnlContext ognlContext = null; + StrutsContext strutsContext = null; - if (context instanceof OgnlContext oc) { - ognlContext = oc; + if (context instanceof StrutsContext sc) { + strutsContext = sc; + } else if (context instanceof OgnlContext oc) { + strutsContext = StrutsContext.wrap(oc); } - return node.isEvalChain(ognlContext) || node.isSequence(ognlContext); + return node.isEvalChain(strutsContext) || node.isSequence(strutsContext); } return false; } private boolean isArithmeticExpression(Object tree, Map context) throws OgnlException { if (tree instanceof SimpleNode node) { - OgnlContext ognlContext = null; + StrutsContext strutsContext = null; - if (context instanceof OgnlContext oc) { - ognlContext = oc; + if (context instanceof StrutsContext sc) { + strutsContext = sc; + } else if (context instanceof OgnlContext oc) { + strutsContext = StrutsContext.wrap(oc); } - return node.isOperation(ognlContext); + return node.isOperation(strutsContext); } return false; } private boolean isSimpleMethod(Object tree, Map context) throws OgnlException { if (tree instanceof SimpleNode node) { - OgnlContext ognlContext = null; + StrutsContext strutsContext = null; - if (context instanceof OgnlContext oc) { - ognlContext = oc; + if (context instanceof StrutsContext sc) { + strutsContext = sc; + } else if (context instanceof OgnlContext oc) { + strutsContext = StrutsContext.wrap(oc); } - return node.isSimpleMethod(ognlContext) && !node.isChain(ognlContext); + return node.isSimpleMethod(strutsContext) && !node.isChain(strutsContext); } return false; } @@ -425,7 +435,7 @@ public class OgnlUtil { for (TreeValidator validator : treeValidators) { validator.validate(tree, checkContext); } - Ognl.setValue(tree, (OgnlContext) context, root, value); + Ognl.setValue(tree, ensureStrutsContext(context), root, value); } private T ognlGet(String expr, Map context, Object root, Class resultType, Map checkContext, TreeValidator... treeValidators) throws OgnlException { @@ -433,7 +443,7 @@ public class OgnlUtil { for (TreeValidator validator : treeValidators) { validator.validate(tree, checkContext); } - return (T) Ognl.getValue(tree, (OgnlContext) context, root, resultType); + return (T) Ognl.getValue(tree, ensureStrutsContext(context), root, resultType); } private Object toTree(String expr) throws OgnlException { @@ -722,18 +732,18 @@ public class OgnlUtil { } } - protected OgnlContext createDefaultContext(Object root) { + protected StrutsContext createDefaultContext(Object root) { return createDefaultContext(root, null); } - protected OgnlContext createDefaultContext(Object root, ClassResolver resolver) { + protected StrutsContext createDefaultContext(Object root, ClassResolver resolver) { if (resolver == null) { resolver = container.getInstance(RootAccessor.class); if (resolver == null) { throw new IllegalStateException("Cannot find ClassResolver"); } } - return Ognl.createDefaultContext(root, container.getInstance(SecurityMemberAccess.class), resolver, defaultConverter); + return StrutsContext.create(root, container.getInstance(SecurityMemberAccess.class), resolver, defaultConverter); } @FunctionalInterface diff --git a/core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java b/core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java index 2376a418f..082b5277b 100644 --- a/core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java +++ b/core/src/main/java/org/apache/struts2/ognl/OgnlValueStack.java @@ -31,8 +31,6 @@ import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.reflection.ReflectionContextState; import ognl.MethodFailedException; import ognl.NoSuchPropertyException; -import ognl.Ognl; -import ognl.OgnlContext; import ognl.OgnlException; import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; @@ -121,12 +119,9 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS protected void setRoot(XWorkConverter xworkConverter, RootAccessor accessor, CompoundRoot compoundRoot, SecurityMemberAccess securityMemberAccess) { this.root = compoundRoot; this.securityMemberAccess = securityMemberAccess; - OgnlContext ognlContext = Ognl.createDefaultContext(this.root, securityMemberAccess, accessor, new OgnlTypeConverterWrapper(xworkConverter)); - this.context = ognlContext; + this.context = StrutsContext.create(this.root, securityMemberAccess, accessor, new OgnlTypeConverterWrapper(xworkConverter)); this.converter = xworkConverter; context.put(VALUE_STACK, this); - ognlContext.setTraceEvaluations(false); - ognlContext.setKeepLastEvaluation(false); } @Inject(StrutsConstants.STRUTS_DEVMODE) @@ -508,9 +503,8 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS @Override public void clearContextValues() { - //this is an OGNL ValueStack so the context will be an OgnlContext - //it would be better to make context of type OgnlContext - ((OgnlContext) context).getValues().clear(); + //this is an OGNL ValueStack so the context will be a StrutsContext + ((StrutsContext) context).getValues().clear(); } @Override diff --git a/core/src/main/java/org/apache/struts2/ognl/StrutsContext.java b/core/src/main/java/org/apache/struts2/ognl/StrutsContext.java index e35a2954e..9ec2c8c73 100644 --- a/core/src/main/java/org/apache/struts2/ognl/StrutsContext.java +++ b/core/src/main/java/org/apache/struts2/ognl/StrutsContext.java @@ -39,10 +39,30 @@ public class StrutsContext extends OgnlContext { * @param typeConverter the type converter * @param memberAccess the member access policy */ - public StrutsContext(ClassResolver classResolver, TypeConverter typeConverter, MemberAccess memberAccess) { + private StrutsContext(ClassResolver classResolver, TypeConverter typeConverter, MemberAccess memberAccess) { super(classResolver, typeConverter, memberAccess); } + /** + * Creates a new StrutsContext with the specified configuration and root object. + * This is the preferred factory method for creating contexts in Struts. + * + * @param root the root object for OGNL evaluation + * @param memberAccess the member access policy + * @param classResolver the class resolver + * @param typeConverter the type converter + * @return a new StrutsContext instance + * @since 7.2.0 + */ + public static StrutsContext create(Object root, MemberAccess memberAccess, + ClassResolver classResolver, TypeConverter typeConverter) { + StrutsContext context = new StrutsContext(classResolver, typeConverter, memberAccess); + context.setRoot(root); + context.setTraceEvaluations(false); + context.setKeepLastEvaluation(false); + return context; + } + /** * Wraps an existing OgnlContext as a StrutsContext. * diff --git a/core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java b/core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java index 49112ab12..b8b00ce85 100644 --- a/core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java +++ b/core/src/main/java/org/apache/struts2/ognl/XWorkTypeConverterWrapper.java @@ -37,11 +37,15 @@ public class XWorkTypeConverterWrapper implements TypeConverter { @Override public Object convertValue(Map context, Object target, Member member, String propertyName, Object value, Class toType) { - // Cast context to OgnlContext for OGNL 3.4.8+ compatibility - OgnlContext ognlContext = (context instanceof OgnlContext oc) ? oc : null; - if (ognlContext == null) { - throw new IllegalArgumentException("Context must be an OgnlContext for OGNL 3.4.8+"); + // Ensure context is a StrutsContext for OGNL 3.4.+ compatibility + StrutsContext strutsContext; + if (context instanceof StrutsContext sc) { + strutsContext = sc; + } else if (context instanceof OgnlContext oc) { + strutsContext = StrutsContext.wrap(oc); + } else { + throw new IllegalArgumentException("Context must be an OgnlContext for OGNL 3.4.+"); } - return typeConverter.convertValue(ognlContext, target, member, propertyName, value, toType); + return typeConverter.convertValue(strutsContext, target, member, propertyName, value, toType); } }