kusalk commented on code in PR #735:
URL: https://github.com/apache/struts/pull/735#discussion_r1300825252
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -124,6 +123,17 @@ public Object findValue(String expression, String
className) throws ClassNotFoun
return stack.findValue(expression, Class.forName(className));
}
+ public Object findValue(String expr, Object context) {
Review Comment:
Copied this here from OgnlTool
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -124,6 +123,17 @@ public Object findValue(String expression, String
className) throws ClassNotFoun
return stack.findValue(expression, Class.forName(className));
}
+ public Object findValue(String expr, Object context) {
+ try {
+ return ognl.getValue(expr,
ActionContext.getContext().getContextMap(), context);
+ } catch (OgnlException e) {
+ if (e.getReason() instanceof SecurityException) {
+ LOG.error(format("Could not evaluate this expression due to
security constraints: [{0}]", expr), e);
Review Comment:
Fixed broken logging
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -50,32 +61,30 @@ public class StrutsUtil {
protected HttpServletRequest request;
protected HttpServletResponse response;
- protected Map<String, Class> classes = new Hashtable<>();
- protected OgnlTool ognl;
+ protected Map<String, Class<?>> classes = new HashMap<>();
Review Comment:
This class is constructed per request thread so I don't believe we need
concurrency handling here
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -50,32 +61,30 @@ public class StrutsUtil {
protected HttpServletRequest request;
protected HttpServletResponse response;
- protected Map<String, Class> classes = new Hashtable<>();
- protected OgnlTool ognl;
+ protected Map<String, Class<?>> classes = new HashMap<>();
+ protected OgnlUtil ognl;
protected ValueStack stack;
- private UrlHelper urlHelper;
- private ObjectFactory objectFactory;
+ private final UrlHelper urlHelper;
+ private final ObjectFactory objectFactory;
public StrutsUtil(ValueStack stack, HttpServletRequest request,
HttpServletResponse response) {
this.stack = stack;
this.request = request;
this.response = response;
- this.ognl =
stack.getActionContext().getContainer().getInstance(OgnlTool.class);
+ this.ognl =
stack.getActionContext().getContainer().getInstance(OgnlUtil.class);
this.urlHelper =
stack.getActionContext().getContainer().getInstance(UrlHelper.class);
this.objectFactory =
stack.getActionContext().getContainer().getInstance(ObjectFactory.class);
}
- public Object bean(Object aName) throws Exception {
- String name = aName.toString();
- Class c = classes.get(name);
-
- if (c == null) {
- c = ClassLoaderUtil.loadClass(name, StrutsUtil.class);
- classes.put(name, c);
+ public Object bean(Object name) throws Exception {
+ String className = name.toString();
+ Class<?> clazz = classes.get(className);
+ if (clazz == null) {
Review Comment:
Using #computeIfAbsent here would change the exception handling semantics so
I've forgone it
##########
core/src/main/java/org/apache/struts2/util/StrutsUtil.java:
##########
@@ -156,71 +166,64 @@ public String translateVariables(String expression) {
* to use as the value of the ListEntry
* @return a List of ListEntry
*/
- public List makeSelectList(String selectedList, String list, String
listKey, String listValue) {
- List selectList = new ArrayList();
-
- Collection selectedItems = null;
-
- Object i = stack.findValue(selectedList);
-
- if (i != null) {
- if (i.getClass().isArray()) {
- selectedItems = Arrays.asList((Object[]) i);
- } else if (i instanceof Collection) {
- selectedItems = (Collection) i;
- } else {
- // treat it is a single item
- selectedItems = new ArrayList();
- selectedItems.add(i);
- }
- }
+ public List<ListEntry> makeSelectList(String selectedList, String list,
String listKey, String listValue) {
Review Comment:
Fixed cognitive complexity and readability
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@struts.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
