Merge pull request apache#727 from apache/fix/WW-5331-proper-get

[WW-5331] Uses proper signature of get()
fp024 · Aug 21, 2023 · f31ba63 · f31ba63
2 parents ea124a4 + 4a678f6
commit f31ba63
Show file tree

Hide file tree

Showing 9 changed files with 217 additions and 17 deletions.
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java b/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java
@@ -36,10 +36,9 @@ public class ApplicationMap extends AbstractMap<String, Object> implements Seria
 
     private static final long serialVersionUID = 9136809763083228202L;
 
-    private ServletContext context;
+    private final ServletContext context;
     private Set<Entry<String, Object>> entries;
 
-
     /**
      * Creates a new map object given the servlet context.
      *
@@ -117,12 +116,16 @@ public Object setValue(final Object obj) {
      * @param key the entry key.
      * @return the servlet context attribute or init parameter or <tt>null</tt> if the entry is not found.
      */
-    public Object get(final String key) {
+    @Override
+    public Object get(final Object key) {
+        if (key == null) {
+            return null;
+        }
         // Try context attributes first, then init params
         // This gives the proper shadowing effects
-        Object value = context.getAttribute(key);
+        Object value = context.getAttribute(key.toString());
 
-        return (value == null) ? context.getInitParameter(key) : value;
+        return (value == null) ? context.getInitParameter(key.toString()) : value;
     }
 
     /**

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java
@@ -94,8 +94,12 @@ public Object setValue(final Object obj) {
      * @param key the name of the request attribute.
      * @return the request attribute or <tt>null</tt> if it doesn't exist.
      */
-    public Object get(final String key) {
-        return request.getAttribute(key);
+    @Override
+    public Object get(final Object key) {
+        if (key == null) {
+            return null;
+        }
+        return request.getAttribute(key.toString());
     }
 
     /**

diff --git a/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java b/core/src/main/java/org/apache/struts2/views/jsp/TagUtils.java
@@ -45,11 +45,9 @@ public static ValueStack getStack(PageContext pageContext) {
                 "Please read https://struts.apache.org/security/#never-expose-jsp-files-directly");
         } else {
             LOG.trace("Adds the current PageContext to ActionContext");
-            AttributeMap attrMap = new AttributeMap(stack.getContext());
-
             stack.getActionContext()
                 .withPageContext(pageContext)
-                .with("attr", attrMap);
+                .with("attr", new AttributeMap(stack.getContext()));
         }
 
         return stack;

diff --git a/core/src/test/java/org/apache/struts2/dispatcher/ApplicationMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/ApplicationMapTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.dispatcher;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockServletContext;
+
+import javax.servlet.ServletContext;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class ApplicationMapTest {
+
+    @Test
+    public void shouldRetrieveAttribute() {
+        // given
+        ServletContext context = new MockServletContext();
+        context.setAttribute("attr", "value");
+
+        // when
+        ApplicationMap am = new ApplicationMap(context);
+        Object value = am.get("attr");
+
+        // then
+        assertEquals("value", value);
+    }
+
+    @Test
+    public void shouldReturnNullIfKeyIsNull() {
+        // given
+        ServletContext context = new MockServletContext();
+
+        // when
+        ApplicationMap am = new ApplicationMap(context);
+        Object value = am.get(null);
+
+        // then
+        assertNull(value);
+    }
+
+    @Test
+    public void shouldRemoveAttributeFromServletContext() {
+        // given
+        ServletContext context = new MockServletContext();
+        context.setAttribute("attr", "value");
+
+        // when
+        ApplicationMap am = new ApplicationMap(context);
+        Object value = am.remove("attr");
+
+        // then
+        assertEquals("value", value);
+        assertNull(context.getAttribute("attr"));
+    }
+
+    @Test
+    public void shouldClearAttributes() {
+        // given
+        ServletContext context = new MockServletContext();
+        context.setAttribute("attr", "value");
+
+        // when
+        ApplicationMap am = new ApplicationMap(context);
+        Object value = am.get("attr");
+
+        // then
+        assertEquals("value", value);
+
+        // when
+        am.clear();
+
+        // then
+        assertNull(context.getAttribute("attr"));
+    }
+
+}
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.dispatcher;
+
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+
+import javax.servlet.http.HttpServletRequest;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class RequestMapTest {
+
+    @Test
+    public void shouldRetrieveRequestAttribute() {
+        // given
+        HttpServletRequest request = new MockHttpServletRequest();
+        request.setAttribute("attr", "value");
+
+        // when
+        RequestMap rm = new RequestMap(request);
+        Object value = rm.get("attr");
+
+        // then
+        assertEquals("value", value);
+    }
+
+    @Test
+    public void shouldReturnNullIfKeyIsNull() {
+        // given
+        HttpServletRequest request = new MockHttpServletRequest();
+
+        // when
+        RequestMap rm = new RequestMap(request);
+        Object value = rm.get(null);
+
+        // then
+        assertNull(value);
+    }
+
+    @Test
+    public void shouldRemoveAttributeFromRequest() {
+        // given
+        HttpServletRequest request = new MockHttpServletRequest();
+        request.setAttribute("attr", "value");
+
+        // when
+        RequestMap rm = new RequestMap(request);
+        Object value = rm.remove("attr");
+
+        // then
+        assertEquals("value", value);
+        assertNull(request.getAttribute("attr"));
+    }
+
+    @Test
+    public void shouldClearAttributes() {
+        // given
+        HttpServletRequest request = new MockHttpServletRequest();
+        request.setAttribute("attr", "value");
+
+        // when
+        RequestMap rm = new RequestMap(request);
+        Object value = rm.get("attr");
+
+        // then
+        assertEquals("value", value);
+
+        // when
+        rm.clear();
+
+        // then
+        assertNull(request.getAttribute("attr"));
+    }
+
+}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
@@ -112,7 +112,7 @@ public void testMultipleProcesses() throws InterruptedException {
             executor.execute(bp);
         }
 
-        Thread.sleep(400);
+        Thread.sleep(500);
 
         for (BackgroundProcess bp : bps) {
             assertTrue("Process is still active: " + bp, bp.isDone());

diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java
@@ -35,8 +35,7 @@ public class PortletApplicationMap extends AbstractMap<String, Object> implement
 
     private static final long serialVersionUID = 2296107511063504414L;
 
-    private PortletContext context;
-
+    private final PortletContext context;
     private Set<Entry<String, Object>> entries;
 
     /**
@@ -52,6 +51,7 @@ public PortletApplicationMap(PortletContext ctx) {
      * Removes all entries from the Map and removes all attributes from the
      * portlet context.
      */
+    @Override
     public void clear() {
         entries = null;
 
@@ -69,6 +69,7 @@ public void clear() {
      * @return a Set of all portlet context attributes as well as context init
      *         parameters.
      */
+    @Override
     public Set<Entry<String, Object>> entrySet() {
         if (entries == null) {
             entries = new HashSet<Entry<String, Object>>();
@@ -160,12 +161,16 @@ public Object setValue(Object obj) {
      * @return the portlet context attribute or init parameter or <tt>null</tt>
      *         if the entry is not found.
      */
-    public Object get(String key) {
+    @Override
+    public Object get(Object key) {
+        if (key == null) {
+            return null;
+        }
         // Try context attributes first, then init params
         // This gives the proper shadowing effects
-        Object value = context.getAttribute(key);
+        Object value = context.getAttribute(key.toString());
 
-        return (value == null) ? context.getInitParameter(key) : value;
+        return (value == null) ? context.getInitParameter(key.toString()) : value;
     }
 
     /**
@@ -177,6 +182,7 @@ public Object get(String key) {
      *            the value to set.
      * @return the attribute that was just set.
      */
+    @Override
     public Object put(String key, Object value) {
         entries = null;
         context.setAttribute(key, value);

diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletRequestMap.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletRequestMap.java
@@ -119,6 +119,9 @@ public Object setValue(Object obj) {
      * @return the request attribute or <tt>null</tt> if it doesn't exist.
      */
     public Object get(Object key) {
+        if (key == null) {
+            return null;
+        }
         return request.getAttribute(key.toString());
     }
 

diff --git a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletSessionMap.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletSessionMap.java
@@ -104,7 +104,7 @@ public Object setValue(Object obj) {
      */
     public Object get(Object key) {
         synchronized (session) {
-            return session.getAttribute(key.toString());
+            return session.getAttribute(key != null ? key.toString() : null);
         }
     }