From 298eefe8c8d5922b5a80faa559856adc92446770 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 17 Aug 2023 10:24:09 +0200 Subject: [PATCH 1/6] WW-5331 Uses proper signature of get() --- .../org/apache/struts2/dispatcher/ApplicationMap.java | 9 ++++++--- .../java/org/apache/struts2/dispatcher/RequestMap.java | 7 +++++-- .../main/java/org/apache/struts2/views/jsp/TagUtils.java | 4 +--- .../apache/struts2/portlet/PortletApplicationMap.java | 9 ++++++--- .../org/apache/struts2/portlet/PortletRequestMap.java | 3 +++ .../org/apache/struts2/portlet/PortletSessionMap.java | 2 +- 6 files changed, 22 insertions(+), 12 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 index b7c9977a9..e678d9ec5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ApplicationMap.java @@ -117,12 +117,15 @@ public class ApplicationMap extends AbstractMap implements Seria * @param key the entry key. * @return the servlet context attribute or init parameter or null if the entry is not found. */ - public Object get(final String key) { + 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 index 021a7347f..51d30a2b7 100644 --- 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,11 @@ public class RequestMap extends AbstractMap implements Serializa * @param key the name of the request attribute. * @return the request attribute or null if it doesn't exist. */ - public Object get(final String key) { - return request.getAttribute(key); + 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 index 7bff47dc3..f813e00d1 100644 --- 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 class TagUtils { "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/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java index a5ea0b9ac..73d432390 100644 --- a/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java +++ b/plugins/portlet/src/main/java/org/apache/struts2/portlet/PortletApplicationMap.java @@ -160,12 +160,15 @@ public class PortletApplicationMap extends AbstractMap implement * @return the portlet context attribute or init parameter or null * if the entry is not found. */ - public Object get(String key) { + 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; } /** 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 index ec13f1295..cf4d727db 100644 --- 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 class PortletRequestMap extends AbstractMap { * @return the request attribute or null 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 index 9076640ff..600079a9e 100644 --- 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 class PortletSessionMap extends AbstractMap { */ public Object get(Object key) { synchronized (session) { - return session.getAttribute(key.toString()); + return session.getAttribute(key != null ? key.toString() : null); } } From 5aa1d076b2201b9e049218f344164e5f1cf4f1a6 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 17 Aug 2023 10:52:01 +0200 Subject: [PATCH 2/6] Increases wait time to avoid failing test --- .../struts2/interceptor/exec/StrutsBackgroundProcessTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 4d0dae6c0..331b5a7a9 100644 --- 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 class StrutsBackgroundProcessTest extends StrutsInternalTestCase { executor.execute(bp); } - Thread.sleep(400); + Thread.sleep(500); for (BackgroundProcess bp : bps) { assertTrue("Process is still active: " + bp, bp.isDone()); From e1a80789f19daa9080855ec237120a50b20ff5de Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 17 Aug 2023 12:51:20 +0200 Subject: [PATCH 3/6] WW-5331 Covers new logic with tests --- .../apache/struts2/dispatcher/RequestMap.java | 1 + .../struts2/dispatcher/RequestMapTest.java | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java diff --git a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java index 51d30a2b7..a75dffb75 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/RequestMap.java @@ -94,6 +94,7 @@ public class RequestMap extends AbstractMap implements Serializa * @param key the name of the request attribute. * @return the request attribute or null if it doesn't exist. */ + @Override public Object get(final Object key) { if (key == null) { return null; diff --git a/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java new file mode 100644 index 000000000..1b1873745 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java @@ -0,0 +1,75 @@ +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 shouldAccessRequestAttributes() { + // 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")); + } + +} From e8287ddeeff4d5b4a797dbf9b6dca5854c03c540 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 17 Aug 2023 12:53:55 +0200 Subject: [PATCH 4/6] WW-5331 Adds missing header with licence --- .../struts2/dispatcher/RequestMapTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java index 1b1873745..6ec49ca12 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java @@ -1,3 +1,21 @@ +/* + * 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; From 710ec2edb95c5806a1f3b5b622e6454ed8ffc2ca Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 17 Aug 2023 13:22:06 +0200 Subject: [PATCH 5/6] WW-5331 Adds tests covering ApplicationMap --- .../dispatcher/ApplicationMapTest.java | 93 +++++++++++++++++++ .../struts2/dispatcher/RequestMapTest.java | 2 +- 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/apache/struts2/dispatcher/ApplicationMapTest.java diff --git a/core/src/test/java/org/apache/struts2/dispatcher/ApplicationMapTest.java b/core/src/test/java/org/apache/struts2/dispatcher/ApplicationMapTest.java new file mode 100644 index 000000000..ee44674b9 --- /dev/null +++ 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 index 6ec49ca12..91ccbd842 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/RequestMapTest.java @@ -29,7 +29,7 @@ import static org.junit.Assert.assertNull; public class RequestMapTest { @Test - public void shouldAccessRequestAttributes() { + public void shouldRetrieveRequestAttribute() { // given HttpServletRequest request = new MockHttpServletRequest(); request.setAttribute("attr", "value"); From 4a678f6bbbe240ee6ee4e85161e487163ab7f475 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 21 Aug 2023 15:06:44 +0200 Subject: [PATCH 6/6] WW-5331 Adds missing @Override annotations --- .../java/org/apache/struts2/dispatcher/ApplicationMap.java | 4 ++-- .../org/apache/struts2/portlet/PortletApplicationMap.java | 7 +++++-- 2 files changed, 7 insertions(+), 4 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 index e678d9ec5..f0c5b4bf2 100644 --- 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 implements Seria private static final long serialVersionUID = 9136809763083228202L; - private ServletContext context; + private final ServletContext context; private Set> entries; - /** * Creates a new map object given the servlet context. * @@ -117,6 +116,7 @@ public class ApplicationMap extends AbstractMap implements Seria * @param key the entry key. * @return the servlet context attribute or init parameter or null if the entry is not found. */ + @Override public Object get(final Object key) { if (key == null) { return null; 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 index 73d432390..701deb004 100644 --- 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 implement private static final long serialVersionUID = 2296107511063504414L; - private PortletContext context; - + private final PortletContext context; private Set> entries; /** @@ -52,6 +51,7 @@ public class PortletApplicationMap extends AbstractMap implement * 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 class PortletApplicationMap extends AbstractMap implement * @return a Set of all portlet context attributes as well as context init * parameters. */ + @Override public Set> entrySet() { if (entries == null) { entries = new HashSet>(); @@ -160,6 +161,7 @@ public class PortletApplicationMap extends AbstractMap implement * @return the portlet context attribute or init parameter or null * if the entry is not found. */ + @Override public Object get(Object key) { if (key == null) { return null; @@ -180,6 +182,7 @@ public class PortletApplicationMap extends AbstractMap implement * 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);