diff --git a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java index 36cfabcc9..727d67a26 100644 --- a/core/src/main/java/org/apache/struts2/util/StrutsUtil.java +++ b/core/src/main/java/org/apache/struts2/util/StrutsUtil.java @@ -18,14 +18,16 @@ */ package org.apache.struts2.util; +import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ObjectFactory; +import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; +import ognl.OgnlException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsException; -import org.apache.struts2.views.jsp.ui.OgnlTool; import org.apache.struts2.views.util.UrlHelper; import javax.servlet.RequestDispatcher; @@ -39,7 +41,16 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static java.text.MessageFormat.format; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; /** * Struts base utility class, for use in Velocity and Freemarker templates @@ -50,32 +61,30 @@ public class StrutsUtil { protected HttpServletRequest request; protected HttpServletResponse response; - protected Map classes = new Hashtable<>(); - protected OgnlTool ognl; + protected Map> 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) { + clazz = ClassLoaderUtil.loadClass(className, StrutsUtil.class); + classes.put(className, clazz); } - - return objectFactory.buildBean(c, stack.getContext()); + return objectFactory.buildBean(clazz, stack.getContext()); } public boolean isTrue(String expression) { @@ -88,30 +97,20 @@ public class StrutsUtil { } public String include(Object aName) throws Exception { - try { - RequestDispatcher dispatcher = request.getRequestDispatcher(aName.toString()); - - if (dispatcher == null) { - throw new IllegalArgumentException("Cannot find included file " + aName); - } - - ResponseWrapper responseWrapper = new ResponseWrapper(response); - - dispatcher.include(request, responseWrapper); - - return responseWrapper.getData(); - } - catch (Exception e) { - LOG.debug("Cannot include {}", aName, e); - throw e; + RequestDispatcher dispatcher = request.getRequestDispatcher(aName.toString()); + if (dispatcher == null) { + throw new IllegalArgumentException("Cannot find included file " + aName); } + ResponseWrapper responseWrapper = new ResponseWrapper(response); + dispatcher.include(request, responseWrapper); + return responseWrapper.getData(); } public String urlEncode(String s) { try { return URLEncoder.encode(s, "UTF-8"); } catch (UnsupportedEncodingException e) { - LOG.debug("Cannot encode URL [{}]", s, e); + LOG.debug(format("Cannot encode URL [{0}]", s), e); return s; } } @@ -124,6 +123,17 @@ public class StrutsUtil { 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); + } + return null; + } + } + public String getText(String text) { return (String) stack.findValue("getText('" + text.replace('\'', '"') + "')"); } @@ -132,7 +142,7 @@ public class StrutsUtil { * @return the url ContextPath. An empty string if one does not exist. */ public String getContext() { - return (request == null)? "" : request.getContextPath(); + return request == null ? "" : request.getContextPath(); } public String translateVariables(String expression) { @@ -156,71 +166,64 @@ public class StrutsUtil { * 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 makeSelectList(String selectedList, String list, String listKey, String listValue) { + List selectList = new ArrayList<>(); Collection items = (Collection) stack.findValue(list); + if (items == null) { + return selectList; + } - if (items != null) { - for (Object element : items) { - Object key; - - if ((listKey == null) || (listKey.length() == 0)) { - key = element; - } else { - key = ognl.findValue(listKey, element); - } - - Object value = null; - - if ((listValue == null) || (listValue.length() == 0)) { - value = element; - } else { - value = ognl.findValue(listValue, element); - } - - boolean isSelected = false; - - if ((value != null) && (selectedItems != null) && selectedItems.contains(value)) { - isSelected = true; - } - - selectList.add(new ListEntry(key, value, isSelected)); - } + Collection selectedItems = getSelectedItems(selectedList); + for (Object element : items) { + Object key = computeKey(listKey, element); + Object value = computeValue(listValue, element); + boolean isSelected = value != null && selectedItems.contains(value); + selectList.add(new ListEntry(key, value, isSelected)); } return selectList; } + private Collection getSelectedItems(String selectedListName) { + Object i = stack.findValue(selectedListName); + if (i == null) { + return emptyList(); + } + if (i.getClass().isArray()) { + return Arrays.asList((Object[]) i); + } else if (i instanceof Collection) { + return (Collection) i; + } + return singletonList(i); + } + + private Object computeKey(String listKey, Object element) { + if (listKey == null || listKey.isEmpty()) { + return element; + } + return findValue(listKey, element); + } + + private Object computeValue(String listValue, Object element) { + if (listValue == null || listValue.isEmpty()) { + return element; + } + return findValue(listValue, element); + } + public int toInt(long aLong) { return (int) aLong; } public long toLong(int anInt) { - return (long) anInt; + return anInt; } public long toLong(String aLong) { - if (aLong == null) { + if (aLong == null || aLong.isEmpty()) { return 0; } - return Long.parseLong(aLong); } @@ -233,14 +236,7 @@ public class StrutsUtil { } public String toStringSafe(Object obj) { - try { - if (obj != null) { - return String.valueOf(obj); - } - return ""; - } catch (Exception e) { - return "Exception thrown: " + e; - } + return obj == null ? "" : obj.toString(); } static class ResponseWrapper extends HttpServletResponseWrapper { @@ -257,7 +253,6 @@ public class StrutsUtil { public String getData() { writer.flush(); - return strout.toString(); } diff --git a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java index 7f72d0b1f..62c7cc9ae 100644 --- a/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java +++ b/core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerManager.java @@ -24,16 +24,24 @@ import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.util.ClassLoaderUtil; import com.opensymphony.xwork2.util.ValueStack; -import freemarker.cache.*; +import freemarker.cache.ClassTemplateLoader; +import freemarker.cache.FileTemplateLoader; +import freemarker.cache.MultiTemplateLoader; +import freemarker.cache.TemplateLoader; +import freemarker.cache.WebappTemplateLoader; import freemarker.core.HTMLOutputFormat; -import freemarker.core.OutputFormat; import freemarker.core.TemplateClassResolver; import freemarker.ext.jsp.TaglibFactory; import freemarker.ext.servlet.HttpRequestHashModel; import freemarker.ext.servlet.HttpRequestParametersHashModel; import freemarker.ext.servlet.HttpSessionHashModel; import freemarker.ext.servlet.ServletContextHashModel; -import freemarker.template.*; +import freemarker.template.Configuration; +import freemarker.template.ObjectWrapper; +import freemarker.template.TemplateException; +import freemarker.template.TemplateExceptionHandler; +import freemarker.template.TemplateModel; +import freemarker.template.Version; import freemarker.template.utility.StringUtil; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -51,7 +59,13 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.text.SimpleDateFormat; -import java.util.*; +import java.util.Calendar; +import java.util.Collections; +import java.util.GregorianCalendar; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; +import java.util.Set; /** *

@@ -176,27 +190,27 @@ public class FreemarkerManager { public void setEncoding(String encoding) { this.encoding = encoding; } - + @Inject(StrutsConstants.STRUTS_FREEMARKER_WRAPPER_ALT_MAP) public void setWrapperAltMap(String val) { altMapWrapper = "true".equals(val); } - + @Inject(StrutsConstants.STRUTS_FREEMARKER_BEANWRAPPER_CACHE) public void setCacheBeanWrapper(String val) { cacheBeanWrapper = "true".equals(val); } - + @Inject(StrutsConstants.STRUTS_FREEMARKER_MRU_MAX_STRONG_SIZE) public void setMruMaxStrongSize(String size) { mruMaxStrongSize = Integer.parseInt(size); } - + @Inject(value = StrutsConstants.STRUTS_FREEMARKER_TEMPLATES_CACHE_UPDATE_DELAY, required = false) public void setTemplateUpdateDelay(String delay) { templateUpdateDelay = delay; } - + @Inject public void setContainer(Container container) { Map map = new HashMap<>(); @@ -281,8 +295,8 @@ public class FreemarkerManager { loadSettings(servletContext); } - /** - * Sets the Freemarker Configuration's template loader with the FreemarkerThemeTemplateLoader + /** + * Sets the Freemarker Configuration's template loader with the FreemarkerThemeTemplateLoader * at the top. * * @param templateLoader the template loader @@ -293,7 +307,7 @@ public class FreemarkerManager { themeTemplateLoader.init(templateLoader); config.setTemplateLoader(themeTemplateLoader); } - + /** * Create the instance of the freemarker Configuration object. *

@@ -543,7 +557,7 @@ public class FreemarkerManager { protected void populateContext(ScopesHashModel model, ValueStack stack, Object action, HttpServletRequest request, HttpServletResponse response) { // put the same objects into the context that the velocity result uses - Map standard = ContextUtil.getStandardContext(stack, request, response); + Map standard = ContextUtil.getStandardContext(stack, request, response); model.putAll(standard); // support for JSP exception pages, exposing the servlet or JSP exception diff --git a/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java b/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java index e77665210..bb9d2b5b8 100644 --- a/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java +++ b/core/src/main/java/org/apache/struts2/views/jsp/ui/OgnlTool.java @@ -19,16 +19,16 @@ package org.apache.struts2.views.jsp.ui; import com.opensymphony.xwork2.ActionContext; -import ognl.OgnlException; - import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.ognl.OgnlUtil; +import ognl.OgnlException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; /** - * FIXME: remove? + * @deprecated since 6.3.0. Use {@link org.apache.struts2.util.StrutsUtil} instead. */ +@Deprecated public class OgnlTool { private static final Logger LOG = LogManager.getLogger(OgnlTool.class); @@ -43,6 +43,10 @@ public class OgnlTool { this.ognlUtil = ognlUtil; } + /** + * @deprecated since 6.3.0. Use {@link org.apache.struts2.util.StrutsUtil#findValue(String, Object)} instead. + */ + @Deprecated public Object findValue(String expr, Object context) { try { return ognlUtil.getValue(expr, ActionContext.getContext().getContextMap(), context); diff --git a/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java b/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java index 941321c4d..97c148f37 100644 --- a/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java +++ b/core/src/main/java/org/apache/struts2/views/util/ContextUtil.java @@ -21,7 +21,6 @@ package org.apache.struts2.views.util; import com.opensymphony.xwork2.ActionInvocation; import com.opensymphony.xwork2.util.ValueStack; import org.apache.struts2.util.StrutsUtil; -import org.apache.struts2.views.jsp.ui.OgnlTool; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -48,8 +47,9 @@ public class ContextUtil { map.put(SESSION, req.getSession(false)); map.put(BASE, req.getContextPath()); map.put(STACK, stack); - map.put(OGNL, stack.getActionContext().getContainer().getInstance(OgnlTool.class)); - map.put(STRUTS, new StrutsUtil(stack, req, res)); + StrutsUtil util = new StrutsUtil(stack, req, res); + map.put(STRUTS, util); + map.put(OGNL, util); // Deprecated since 6.3.0 ActionInvocation invocation = stack.getActionContext().getActionInvocation(); if (invocation != null) { diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 7f98e4db5..fc1fb2ee7 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -198,6 +198,7 @@ + list = Arrays.asList("Lorry", "Car", "Helicopter"); stack.getContext().put("mySelectedList", selectedList); stack.getContext().put("myList", list); - List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); + List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); - assertEquals(listMade.size(), 3); - assertEquals(((ListEntry)listMade.get(0)).getKey(), "Lorry"); - assertEquals(((ListEntry)listMade.get(0)).getValue(), "Lorry"); - assertFalse(((ListEntry) listMade.get(0)).getIsSelected()); - assertEquals(((ListEntry)listMade.get(1)).getKey(), "Car"); - assertEquals(((ListEntry)listMade.get(1)).getValue(), "Car"); - assertTrue(((ListEntry) listMade.get(1)).getIsSelected()); - assertEquals(((ListEntry)listMade.get(2)).getKey(), "Helicopter"); - assertEquals(((ListEntry)listMade.get(2)).getValue(), "Helicopter"); - assertFalse(((ListEntry) listMade.get(2)).getIsSelected()); + LinkedHashMap expectedItems = new LinkedHashMap<>(); + expectedItems.put("Lorry", false); + expectedItems.put("Car", true); + expectedItems.put("Helicopter", false); + makeSelectListCommonAssertions(listMade, expectedItems); + } + + public void testMakeSelectListCollection() { + List selectedList = Arrays.asList("Airplane", "Helicopter", "Bus"); // Collection + List list = Arrays.asList("Lorry", "Car", "Helicopter"); + + stack.getContext().put("mySelectedList", selectedList); + stack.getContext().put("myList", list); + + List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); + + LinkedHashMap expectedItems = new LinkedHashMap<>(); + expectedItems.put("Lorry", false); + expectedItems.put("Car", false); + expectedItems.put("Helicopter", true); + makeSelectListCommonAssertions(listMade, expectedItems); + } + + public void testMakeSelectListSingleton() { + String selectedItem = "Lorry"; // Singleton + List list = Arrays.asList("Lorry", "Car", "Helicopter"); + + stack.getContext().put("mySelectedList", selectedItem); + stack.getContext().put("myList", list); + + List listMade = strutsUtil.makeSelectList("#mySelectedList", "#myList", null, null); + + LinkedHashMap expectedItems = new LinkedHashMap<>(); + expectedItems.put("Lorry", true); + expectedItems.put("Car", false); + expectedItems.put("Helicopter", false); + makeSelectListCommonAssertions(listMade, expectedItems); + } + + private void makeSelectListCommonAssertions(List listMade, LinkedHashMap expectedItems) { + assertThat(listMade).extracting("key").containsExactly(expectedItems.keySet().toArray()); + assertThat(listMade).extracting("value").containsExactly(expectedItems.keySet().toArray()); + assertThat(listMade).extracting("isSelected").containsExactly(expectedItems.values().toArray()); + } + + public void testMakeSelectListNonExistent() { + List listMade = strutsUtil.makeSelectList("#mySelectedList", "#nonexistent", null, null); + assertThat(listMade).isEmpty(); } public void testToInt() { @@ -178,12 +216,22 @@ public class StrutsUtilTest extends StrutsInternalTestCase { assertEquals(strutsUtil.toLong(11), 11L); } + public void testStringToLong() { + assertEquals(11L, strutsUtil.toLong("11")); + assertEquals(0L, strutsUtil.toLong(null)); + assertEquals(0L, strutsUtil.toLong("")); + } public void testToString() { assertEquals(strutsUtil.toString(1), "1"); assertEquals(strutsUtil.toString(11L), "11"); } + public void testToStringSafe() { + assertEquals("1", strutsUtil.toStringSafe(1)); + assertEquals("", strutsUtil.toStringSafe(null)); + } + public void testTranslateVariables() { stack.push(new Object() { public String getFoo() { @@ -233,7 +281,7 @@ public class StrutsUtilTest extends StrutsInternalTestCase { // === internal class to assist in testing - static class InternalMockHttpServletRequest extends MockHttpServletRequest { + protected static class InternalMockHttpServletRequest extends MockHttpServletRequest { InternalMockRequestDispatcher dispatcher = null; public RequestDispatcher getRequestDispatcher(String path) { dispatcher = new InternalMockRequestDispatcher(path); @@ -245,8 +293,8 @@ public class StrutsUtilTest extends StrutsInternalTestCase { } } - static class InternalMockRequestDispatcher extends MockRequestDispatcher { - private String url; + protected static class InternalMockRequestDispatcher extends MockRequestDispatcher { + private final String url; boolean included = false; public InternalMockRequestDispatcher(String url) { super(url); diff --git a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java index 8c8ee17bf..b702d2249 100644 --- a/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java +++ b/plugins/velocity/src/main/java/org/apache/struts2/views/velocity/VelocityManager.java @@ -29,7 +29,6 @@ import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsConstants; import org.apache.struts2.StrutsException; import org.apache.struts2.views.TagLibraryDirectiveProvider; -import org.apache.struts2.views.jsp.ui.OgnlTool; import org.apache.struts2.views.util.ContextUtil; import org.apache.velocity.VelocityContext; import org.apache.velocity.app.Velocity; @@ -56,6 +55,7 @@ import java.util.Properties; import static java.lang.String.format; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; +import static org.apache.struts2.views.util.ContextUtil.OGNL; import static org.apache.struts2.views.util.ContextUtil.STRUTS; /** @@ -101,27 +101,15 @@ public class VelocityManager { } /** - * @return a reference to the VelocityEngine used by all struts velocity thingies with the exception of - * directly accessed *.vm pages + * @return a reference to the VelocityEngine used by all Struts Velocity results except directly + * accessed *.vm pages (unless otherwise configured) */ public VelocityEngine getVelocityEngine() { return velocityEngine; } /** - *

- * This method is responsible for creating the standard VelocityContext used by all WW2 velocity views. The - * following context parameters are defined: - *

- * - *
    - *
  • request - the current HttpServletRequest
    • - *
  • response - the current HttpServletResponse
    • - *
  • stack - the current {@link ValueStack}
    • - *
  • ognl - an {@link OgnlTool}
    • - *
  • struts - an instance of {@link org.apache.struts2.util.StrutsUtil}
    • - *
  • action - the current Struts action
    • - *
+ * This method is responsible for creating the standard VelocityContext used by all Struts Velocity views. * * @param stack the current {@link ValueStack} * @param req the current HttpServletRequest @@ -141,7 +129,9 @@ public class VelocityManager { List chainedContexts = prepareChainedContexts(req, res, stack.getContext()); Context context = new StrutsVelocityContext(chainedContexts, stack); ContextUtil.getStandardContext(stack, req, res).forEach(context::put); - context.put(STRUTS, new VelocityStrutsUtil(velocityEngine, context, stack, req, res)); + VelocityStrutsUtil util = new VelocityStrutsUtil(velocityEngine, context, stack, req, res); + context.put(STRUTS, util); + context.put(OGNL, util); // Deprecated since 6.3.0 return context; } diff --git a/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/VelocityManagerTest.java b/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/VelocityManagerTest.java index d550e9efb..b86ff2eec 100644 --- a/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/VelocityManagerTest.java +++ b/plugins/velocity/src/test/java/org/apache/struts2/views/velocity/VelocityManagerTest.java @@ -22,7 +22,6 @@ import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.util.ValueStack; import org.apache.struts2.ServletActionContext; import org.apache.struts2.junit.StrutsJUnit4TestCase; -import org.apache.struts2.views.jsp.ui.OgnlTool; import org.apache.velocity.context.Context; import org.apache.velocity.tools.ToolContext; import org.junit.After; @@ -95,7 +94,7 @@ public class VelocityManagerTest extends StrutsJUnit4TestCase { assertNotNull(context); assertThat(context.get("struts")).isInstanceOf(VelocityStrutsUtil.class); - assertThat(context.get("ognl")).isInstanceOf(OgnlTool.class); + assertThat(context.get("ognl")).isInstanceOf(VelocityStrutsUtil.class); // Deprecated since 6.3.0 assertThat(context.get("stack")).isInstanceOf(ValueStack.class); assertThat(context.get("request")).isInstanceOf(HttpServletRequest.class); assertThat(context.get("response")).isInstanceOf(HttpServletResponse.class);